Fix chip ripple bug — No longer two ripples (#42779)

* Fixed chip ripple.

* Fixed chip ripple. All tests passed.

* Fix one minor detail.

* Fixed reference box.

* Playing around 2.

* Added tests for chip ripple.

* Reverting print-debugging statements

* Remove extra blank line.

* Fixed chip ripple.

* Remove commented code.

* Reconciles with upstream/master.

* Remove print-debugging statement.

* Remove empty line.

* Edit comments.

* Edit style and comments.

* Edit style.

* Fix style and capitalization.

* Return bool.

* Edit style.

* Use getMaterialBox instead of Material.of(...).

* Experiment 3.

* Revert.

* Using tester.pump instead of pumpFrames

* Delete pumpFrames.

* Edit comments.

packages/flutter/lib/src/material/chip.dart

@@ -13,6 +13,7 @@ import 'chip_theme.dart';
 import 'colors.dart';
 import 'constants.dart';
 import 'debug.dart';
+import 'feedback.dart';
 import 'icons.dart';
 import 'ink_well.dart';
 import 'material.dart';
@@ -1525,6 +1526,8 @@ class _RawChipState extends State<RawChip> with TickerProviderStateMixin<RawChip
 
   final Set<MaterialState> _states = <MaterialState>{};
 
+  final GlobalKey deleteIconKey = GlobalKey();
+
   bool get hasDeleteButton => widget.onDeleted != null;
   bool get hasAvatar => widget.avatar != null;
 
@@ -1729,15 +1732,27 @@ class _RawChipState extends State<RawChip> with TickerProviderStateMixin<RawChip
     );
   }
 
-  Widget _buildDeleteIcon(BuildContext context, ThemeData theme, ChipThemeData chipTheme) {
+  Widget _buildDeleteIcon(
+    BuildContext context,
+    ThemeData theme,
+    ChipThemeData chipTheme,
+    GlobalKey deleteIconKey,
+  ) {
     if (!hasDeleteButton) {
       return null;
     }
     return _wrapWithTooltip(
       widget.deleteButtonTooltipMessage ?? MaterialLocalizations.of(context)?.deleteButtonTooltip,
       widget.onDeleted,
-      InkResponse(
-        onTap: widget.isEnabled ? widget.onDeleted : null,
+      GestureDetector(
+        key: deleteIconKey,
+        behavior: HitTestBehavior.opaque,
+        onTap: widget.isEnabled
+            ? () {
+                Feedback.forTap(context);
+                widget.onDeleted();
+            }
+            : null,
         child: IconTheme(
           data: theme.iconTheme.copyWith(
             color: widget.deleteIconColor ?? chipTheme.deleteIconColor,
@@ -1789,6 +1804,11 @@ class _RawChipState extends State<RawChip> with TickerProviderStateMixin<RawChip
         onTapDown: canTap ? _handleTapDown : null,
         onTapCancel: canTap ? _handleTapCancel : null,
         onHover: canTap ? _handleHover : null,
+        splashFactory: _LocationAwareInkRippleFactory(
+            hasDeleteButton,
+            context,
+            deleteIconKey,
+        ),
         customBorder: shape,
         child: AnimatedBuilder(
           animation: Listenable.merge(<Listenable>[selectController, enableController]),
@@ -1820,7 +1840,7 @@ class _RawChipState extends State<RawChip> with TickerProviderStateMixin<RawChip
                   switchInCurve: Curves.fastOutSlowIn,
                 ),
                 deleteIcon: AnimatedSwitcher(
-                  child: _buildDeleteIcon(context, theme, chipTheme),
+                  child: _buildDeleteIcon(context, theme, chipTheme, deleteIconKey),
                   duration: _kDrawerDuration,
                   switchInCurve: Curves.fastOutSlowIn,
                 ),
@@ -2441,23 +2461,19 @@ class _RenderChip extends RenderBox {
 
   @override
   bool hitTest(BoxHitTestResult result, { Offset position }) {
-    if (!size.contains(position))
+    if (!size.contains(position)) {
       return false;
-    RenderBox hitTestChild;
-    switch (textDirection) {
-      case TextDirection.ltr:
-        if (position.dx / size.width > 0.66)
-          hitTestChild = deleteIcon ?? label ?? avatar;
-        else
-          hitTestChild = label ?? avatar;
-        break;
-      case TextDirection.rtl:
-        if (position.dx / size.width < 0.33)
-          hitTestChild = deleteIcon ?? label ?? avatar;
-        else
-          hitTestChild = label ?? avatar;
-        break;
     }
+    final bool tapIsOnDeleteIcon = _tapIsOnDeleteIcon(
+      hasDeleteButton: deleteIcon != null,
+      tapPosition: position,
+      chipSize: size,
+      textDirection: textDirection,
+    );
+    final RenderBox hitTestChild = tapIsOnDeleteIcon
+        ? (deleteIcon ?? label ?? avatar)
+        : (label ?? avatar);
+
     if (hitTestChild != null) {
       final Offset center = hitTestChild.size.center(Offset.zero);
       return result.addWithRawTransform(
@@ -2795,3 +2811,88 @@ class _RenderChip extends RenderBox {
   @override
   bool hitTestSelf(Offset position) => deleteButtonRect.contains(position) || pressRect.contains(position);
 }
+
+class _LocationAwareInkRippleFactory extends InteractiveInkFeatureFactory {
+  const _LocationAwareInkRippleFactory(
+    this.hasDeleteButton,
+    this.chipContext,
+    this.deleteIconKey,
+  );
+
+  final bool hasDeleteButton;
+  final BuildContext chipContext;
+  final GlobalKey deleteIconKey;
+
+  @override
+  InteractiveInkFeature create({
+    MaterialInkController controller,
+    RenderBox referenceBox,
+    Offset position,
+    Color color,
+    TextDirection textDirection,
+    bool containedInkWell = false,
+    RectCallback rectCallback,
+    BorderRadius borderRadius,
+    ShapeBorder customBorder,
+    double radius,
+    VoidCallback onRemoved,
+  }) {
+
+    final bool tapIsOnDeleteIcon = _tapIsOnDeleteIcon(
+      hasDeleteButton: hasDeleteButton,
+      tapPosition: position,
+      chipSize: chipContext.size,
+      textDirection: textDirection,
+    );
+
+    final BuildContext splashContext = tapIsOnDeleteIcon
+        ? deleteIconKey.currentContext
+        : chipContext;
+
+    final InteractiveInkFeatureFactory splashFactory = Theme.of(splashContext).splashFactory;
+
+    if (tapIsOnDeleteIcon) {
+      final RenderBox currentBox = referenceBox;
+      referenceBox = deleteIconKey.currentContext.findRenderObject();
+      position = referenceBox.globalToLocal(currentBox.localToGlobal(position));
+      containedInkWell = false;
+    }
+
+    return splashFactory.create(
+      controller: controller,
+      referenceBox: referenceBox,
+      position: position,
+      color: color,
+      textDirection: textDirection,
+      containedInkWell: containedInkWell,
+      rectCallback: rectCallback,
+      borderRadius: borderRadius,
+      customBorder: customBorder,
+      radius: radius,
+      onRemoved: onRemoved,
+    );
+  }
+}
+
+bool _tapIsOnDeleteIcon({
+  bool hasDeleteButton,
+  Offset tapPosition,
+  Size chipSize,
+  TextDirection textDirection,
+}) {
+  bool tapIsOnDeleteIcon;
+  if (!hasDeleteButton) {
+    tapIsOnDeleteIcon = false;
+  } else {
+    switch (textDirection) {
+      case TextDirection.ltr:
+        tapIsOnDeleteIcon = tapPosition.dx / chipSize.width > 0.66;
+        break;
+      case TextDirection.rtl:
+        tapIsOnDeleteIcon = tapPosition.dx / chipSize.width < 0.33;
+        break;
+    }
+  }
+  return tapIsOnDeleteIcon;
+}
+

packages/flutter/test/material/chip_test.dart

@@ -188,6 +188,77 @@ void _expectCheckmarkColor(Finder finder, Color color) {
   );
 }
 
+Widget _chipWithOptionalDeleteButton({
+  UniqueKey deleteButtonKey,
+  UniqueKey labelKey,
+  bool deletable,
+  TextDirection textDirection = TextDirection.ltr,
+}){
+  return _wrapForChip(
+    textDirection: textDirection,
+    child: Wrap(
+      children: <Widget>[
+        RawChip(
+          onPressed: () {},
+          onDeleted: deletable ? () {} : null,
+          deleteIcon: Icon(Icons.close, key: deleteButtonKey),
+          label: Text(
+            deletable
+              ? 'Chip with Delete Button'
+              : 'Chip without Delete Button',
+            key: labelKey,
+          ),
+        ),
+      ],
+    ),
+  );
+}
+
+bool offsetsAreClose(Offset a, Offset b) => (a - b).distance < 1.0;
+bool radiiAreClose(double a, double b) => (a - b).abs() < 1.0;
+
+// Ripple pattern matches if there exists at least one ripple
+// with the [expectedCenter] and [expectedRadius].
+// This ensures the existence of a ripple.
+PaintPattern ripplePattern(Offset expectedCenter, double expectedRadius) {
+  return paints
+    ..something((Symbol method, List<dynamic> arguments) {
+        if (method != #drawCircle)
+          return false;
+        final Offset center = arguments[0];
+        final double radius = arguments[1];
+        return offsetsAreClose(center, expectedCenter) && radiiAreClose(radius, expectedRadius);
+      }
+    );
+}
+
+// Unique ripple pattern matches if there does not exist ripples
+// other than ones with the [expectedCenter] and [expectedRadius].
+// This ensures the nonexistence of two different ripples.
+PaintPattern uniqueRipplePattern(Offset expectedCenter, double expectedRadius) {
+  return paints
+    ..everything((Symbol method, List<dynamic> arguments) {
+        if (method != #drawCircle)
+          return true;
+        final Offset center = arguments[0];
+        final double radius = arguments[1];
+        if (offsetsAreClose(center, expectedCenter) && radiiAreClose(radius, expectedRadius))
+          return true;
+        throw '''
+              Expected: center == $expectedCenter, radius == $expectedRadius
+              Found: center == $center radius == $radius''';
+      }
+    );
+}
+
+// Finds any container of a tooltip.
+Finder findTooltipContainer(String tooltipText) {
+  return find.ancestor(
+    of: find.text(tooltipText),
+    matching: find.byType(Container),
+  );
+}
+
 void main() {
   testWidgets('Chip control test', (WidgetTester tester) async {
     final FeedbackTester feedback = FeedbackTester();
@@ -923,6 +994,202 @@ void main() {
     expect(find.byKey(deleteButtonKey), findsNothing);
   }, skip: isBrowser);
 
+  testWidgets('Chip creates centered, unique ripple when label is tapped', (WidgetTester tester) async {
+    // Creates a chip with a delete button.
+    final UniqueKey labelKey = UniqueKey();
+    final UniqueKey deleteButtonKey = UniqueKey();
+
+    await tester.pumpWidget(
+      _chipWithOptionalDeleteButton(
+        labelKey: labelKey,
+        deleteButtonKey: deleteButtonKey,
+        deletable: true,
+      ),
+    );
+
+    final RenderBox box = getMaterialBox(tester);
+
+    // Taps at a location close to the center of the label.
+    final Offset centerOfLabel = tester.getCenter(find.byKey(labelKey));
+    final Offset tapLocationOfLabel = centerOfLabel + const Offset(-10, -10);
+    final TestGesture gesture = await tester.startGesture(tapLocationOfLabel);
+    await tester.pump();
+
+    // Waits for 100 ms.
+    await tester.pump(const Duration(milliseconds: 100));
+
+    // There should be exactly one ink-creating widget.
+    expect(find.byType(InkWell), findsOneWidget);
+    expect(find.byType(InkResponse), findsNothing);
+
+    // There should be one unique, centered ink ripple.
+    expect(box, ripplePattern(const Offset(163.0, 6.0), 20.9));
+    expect(box, uniqueRipplePattern(const Offset(163.0, 6.0), 20.9));
+
+    // There should be no tooltip.
+    expect(findTooltipContainer('Delete'), findsNothing);
+
+    // Waits for 100 ms again.
+    await tester.pump(const Duration(milliseconds: 100));
+
+    // The ripple should grow, with the same center.
+    expect(box, ripplePattern(const Offset(163.0, 6.0), 41.8));
+    expect(box, uniqueRipplePattern(const Offset(163.0, 6.0), 41.8));
+
+    // There should be no tooltip.
+    expect(findTooltipContainer('Delete'), findsNothing);
+
+    // Waits for a very long time.
+    await tester.pumpAndSettle();
+
+    // There should still be no tooltip.
+    expect(findTooltipContainer('Delete'), findsNothing);
+
+    await gesture.up();
+  }, skip: isBrowser);
+
+  testWidgets('Delete button creates non-centered, unique ripple when tapped', (WidgetTester tester) async {
+    // Creates a chip with a delete button.
+    final UniqueKey labelKey = UniqueKey();
+    final UniqueKey deleteButtonKey = UniqueKey();
+
+    await tester.pumpWidget(
+      _chipWithOptionalDeleteButton(
+        labelKey: labelKey,
+        deleteButtonKey: deleteButtonKey,
+        deletable: true,
+      ),
+    );
+
+    final RenderBox box = getMaterialBox(tester);
+
+    // Taps at a location close to the center of the delete icon.
+    final Offset centerOfDeleteButton = tester.getCenter(find.byKey(deleteButtonKey));
+    final Offset tapLocationOfDeleteButton = centerOfDeleteButton + const Offset(-10, -10);
+    final TestGesture gesture = await tester.startGesture(tapLocationOfDeleteButton);
+    await tester.pump();
+
+    // Waits for 200 ms.
+    await tester.pump(const Duration(milliseconds: 100));
+    await tester.pump(const Duration(milliseconds: 100));
+
+    // There should be exactly one ink-creating widget.
+    expect(find.byType(InkWell), findsOneWidget);
+    expect(find.byType(InkResponse), findsNothing);
+
+    // There should be one unique ink ripple.
+    expect(box, ripplePattern(const Offset(3.0, 3.0), 3.5));
+    expect(box, uniqueRipplePattern(const Offset(3.0, 3.0), 3.5));
+
+    // There should be no tooltip.
+    expect(findTooltipContainer('Delete'), findsNothing);
+
+    // Waits for 200 ms again.
+    await tester.pump(const Duration(milliseconds: 100));
+    await tester.pump(const Duration(milliseconds: 100));
+
+    // The ripple should grow, but the center should move,
+    // Towards the center of the delete icon.
+    expect(box, ripplePattern(const Offset(5.0, 5.0), 10.5));
+    expect(box, uniqueRipplePattern(const Offset(5.0, 5.0), 10.5));
+
+    // There should be no tooltip.
+    expect(findTooltipContainer('Delete'), findsNothing);
+
+    // Waits for a very long time.
+    // This is pressing and holding the delete button.
+    await tester.pumpAndSettle();
+
+    // There should be a tooltip.
+    expect(findTooltipContainer('Delete'), findsOneWidget);
+
+    await gesture.up();
+  }, skip: isBrowser);
+
+  testWidgets('RTL delete button responds to tap on the left of the chip', (WidgetTester tester) async {
+    // Creates an RTL chip with a delete button.
+    final UniqueKey labelKey = UniqueKey();
+    final UniqueKey deleteButtonKey = UniqueKey();
+
+    await tester.pumpWidget(
+      _chipWithOptionalDeleteButton(
+        labelKey: labelKey,
+        deleteButtonKey: deleteButtonKey,
+        deletable: true,
+        textDirection: TextDirection.rtl,
+      ),
+    );
+
+    // Taps at a location close to the center of the delete icon,
+    // Which is on the left side of the chip.
+    final Offset topLeftOfInkWell = tester.getTopLeft(find.byType(InkWell));
+    final Offset tapLocation = topLeftOfInkWell + const Offset(8, 8);
+    final TestGesture gesture = await tester.startGesture(tapLocation);
+    await tester.pump();
+
+    await tester.pumpAndSettle();
+
+    // The existence of a 'Delete' tooltip indicates the delete icon is tapped,
+    // Instead of the label.
+    expect(findTooltipContainer('Delete'), findsOneWidget);
+
+    await gesture.up();
+  }, skip: isBrowser);
+
+  testWidgets('Chip without delete button creates correct ripple', (WidgetTester tester) async {
+    // Creates a chip with a delete button.
+    final UniqueKey labelKey = UniqueKey();
+
+    await tester.pumpWidget(
+      _chipWithOptionalDeleteButton(
+        labelKey: labelKey,
+        deletable: false,
+      ),
+    );
+
+    final RenderBox box = getMaterialBox(tester);
+
+    // Taps at a location close to the bottom-right corner of the chip.
+    final Offset bottomRightOfInkWell = tester.getBottomRight(find.byType(InkWell));
+    final Offset tapLocation = bottomRightOfInkWell + const Offset(-10, -10);
+    final TestGesture gesture = await tester.startGesture(tapLocation);
+    await tester.pump();
+
+    // Waits for 100 ms.
+    await tester.pump(const Duration(milliseconds: 100));
+
+    // There should be exactly one ink-creating widget.
+    expect(find.byType(InkWell), findsOneWidget);
+    expect(find.byType(InkResponse), findsNothing);
+
+    // There should be one unique, centered ink ripple.
+    expect(box, ripplePattern(const Offset(378.0, 22.0), 37.9));
+    expect(box, uniqueRipplePattern(const Offset(378.0, 22.0), 37.9));
+
+    // There should be no tooltip.
+    expect(findTooltipContainer('Delete'), findsNothing);
+
+    // Waits for 100 ms again.
+    await tester.pump(const Duration(milliseconds: 100));
+
+    // The ripple should grow, with the same center.
+    // This indicates that the tap is not on a delete icon.
+    expect(box, ripplePattern(const Offset(378.0, 22.0), 75.8));
+    expect(box, uniqueRipplePattern(const Offset(378.0, 22.0), 75.8));
+
+    // There should be no tooltip.
+    expect(findTooltipContainer('Delete'), findsNothing);
+
+    // Waits for a very long time.
+    await tester.pumpAndSettle();
+
+    // There should still be no tooltip.
+    // This indicates that the tap is not on a delete icon.
+    expect(findTooltipContainer('Delete'), findsNothing);
+
+    await gesture.up();
+  }, skip: isBrowser);
+
   testWidgets('Selection with avatar works as expected on RawChip', (WidgetTester tester) async {
     bool selected = false;
     final UniqueKey labelKey = UniqueKey();