Unverified Commit 777463c2 authored by Greg Spencer's avatar Greg Spencer Committed by GitHub

Adjust size of delete button to take up at most less than half of chip. (#90845)

This adjusts the size of the delete button so that it takes up just slightly less than half of the chip, so that legacy tests that tap on the center of the chip still hit the chip, and not the delete button.

A follow-on change for #90531
parent 885b2f56
......@@ -2960,22 +2960,22 @@ bool _hitIsOnDeleteIcon({
final EdgeInsets resolvedPadding = padding.resolve(textDirection);
final Size deflatedSize = resolvedPadding.deflateSize(chipSize);
final Offset adjustedPosition = tapPosition - Offset(resolvedPadding.left, resolvedPadding.top);
// The delete button hit area should be at least the width of the delete button,
// but, if there's room, up to 24 pixels from the center of the delete icon
// (corresponding to part of a 48x48 square that Material would prefer for touch
// targets), but no more than half of the overall size of the chip when the chip is
// small.
// The delete button hit area should be at least the width of the delete
// button, but, if there's room, up to 24 pixels from the center of the delete
// icon (corresponding to part of a 48x48 square that Material would prefer
// for touch targets), but no more than approximately half of the overall size
// of the chip when the chip is small.
//
// This isn't affected by materialTapTargetSize because it only applies to the
// width of the tappable region within the chip, not outside of the chip, which is
// handled elsewhere. Also because delete buttons aren't specified to be used on
// touch devices, only desktop devices.
final double accessibleDeleteButtonWidth = math.max(
deleteButtonSize.width,
math.min(
deflatedSize.width * 0.5,
24.0 + deleteButtonSize.width / 2.0,
),
// width of the tappable region within the chip, not outside of the chip,
// which is handled elsewhere. Also because delete buttons aren't specified to
// be used on touch devices, only desktop devices.
// Max out at not quite half, so that tests that tap on the center of a small
// chip will still hit the chip, not the delete button.
final double accessibleDeleteButtonWidth = math.min(
deflatedSize.width * 0.499,
math.max(deleteButtonSize.width, 24.0 + deleteButtonSize.width / 2.0),
);
switch (textDirection) {
case TextDirection.ltr:
......
......@@ -565,7 +565,7 @@ void main() {
children: <Widget>[
Chip(
label: const SizedBox(), // Short label
deleteIcon: Icon(Icons.delete, key: deleteKey),
deleteIcon: Icon(Icons.cancel, key: deleteKey),
onDeleted: () {
calledDelete = true;
},
......@@ -574,12 +574,18 @@ void main() {
),
),
);
await tester.tapAt(tester.getCenter(find.byKey(deleteKey)) - const Offset(12.0, 0.0));
// Chip width is 48 with padding, 40 without padding, so halfway is at 20. Cancel
// icon is 24x24, so since 24 > 20 the split location should be halfway across the
// chip, which is at 12 + 8 = 20 from the right side. Since the split is just
// slightly less than 50%, 8 from the center of the delete button should hit the
// chip, not the delete button.
await tester.tapAt(tester.getCenter(find.byKey(deleteKey)) - const Offset(7.0, 0.0));
await tester.pump();
expect(calledDelete, isTrue);
calledDelete = false;
await tester.tapAt(tester.getCenter(find.byKey(deleteKey)) - const Offset(13.0, 0.0));
await tester.tapAt(tester.getCenter(find.byKey(deleteKey)) - const Offset(8.0, 0.0));
await tester.pump();
expect(calledDelete, isFalse);
});
......@@ -1079,6 +1085,42 @@ void main() {
expect(find.byKey(deleteButtonKey), findsNothing);
});
testWidgets('Delete button takes up at most half of the chip', (WidgetTester tester) async {
final UniqueKey chipKey = UniqueKey();
bool chipPressed = false;
bool deletePressed = false;
await tester.pumpWidget(
_wrapForChip(
child: Wrap(
children: <Widget>[
RawChip(
key: chipKey,
onPressed: () {
chipPressed = true;
},
onDeleted: () {
deletePressed = true;
},
label: const Text(''),
),
],
),
),
);
await tester.tapAt(tester.getCenter(find.byKey(chipKey)));
await tester.pump();
expect(chipPressed, isTrue);
expect(deletePressed, isFalse);
chipPressed = false;
await tester.tapAt(tester.getCenter(find.byKey(chipKey)) + const Offset(1.0, 0.0));
await tester.pump();
expect(chipPressed, isFalse);
expect(deletePressed, isTrue);
});
testWidgets('Chip creates centered, unique ripple when label is tapped', (WidgetTester tester) async {
final UniqueKey labelKey = UniqueKey();
final UniqueKey deleteButtonKey = UniqueKey();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment