Skip to content

Commit 5a6d0cf

Browse files
dkwingsmtpull[bot]
authored andcommitted
[CupertinoActionSheet] Support legacy buttons (flutter#151136)
Fixes flutter#150980 This PR allows buttons implemented with `GestureDetector.onTap` to be selected in the action sheet.
1 parent 0b33898 commit 5a6d0cf

File tree

2 files changed

+97
-8
lines changed

2 files changed

+97
-8
lines changed

packages/flutter/lib/src/cupertino/dialog.dart

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -546,21 +546,35 @@ class _SlidingTapGestureRecognizer extends VerticalDragGestureRecognizer {
546546
if (event is PointerMoveEvent) {
547547
onResponsiveUpdate?.call(event.position);
548548
}
549-
// If this gesture has a competing gesture (such as scrolling), and the
550-
// pointer has not moved far enough to get this panning accepted, a
551-
// pointer up event should still be considered as an accepted tap up.
552-
// Manually accept this gesture here, which triggers onDragEnd.
549+
// Sliding tap needs to handle 'up' events differently compared to typical
550+
// drag gestures. If there's another gesture recognizer (like scrolling)
551+
// competing and the pointer hasn't moved beyond the tolerance limit
552+
// (slop), this gesture must still be accepted.
553+
//
554+
// Simply calling `accept()` here to handle this won't work because it
555+
// would break backward compatibility with legacy buttons (see
556+
// https://github.com/flutter/flutter/issues/150980 for more details).
557+
// Legacy buttons recognize taps using `GestureDetector.onTap`, which
558+
// neither accepts nor rejects for short taps. Instead, they wait for the
559+
// default resolution as the last contender in the gesture arena.
560+
//
561+
// Therefore, this gesture should also follow the same strategy of not
562+
// immediately accepting or rejecting. This allows tap gestures to take
563+
// precedence for being inner, while sliding taps can take precedence over
564+
// scroll gestures when the latter give up.
553565
if (event is PointerUpEvent) {
554-
resolve(GestureDisposition.accepted);
555566
stopTrackingPointer(_primaryPointer!);
556567
onResponsiveEnd?.call(event.position);
557-
} else {
558-
super.handleEvent(event);
568+
_primaryPointer = null;
569+
// Do not call `super.handleEvent`, which gives up the pointer and thus
570+
// rejects the gesture.
571+
return;
559572
}
560-
if (event is PointerUpEvent || event is PointerCancelEvent) {
573+
if (event is PointerCancelEvent) {
561574
_primaryPointer = null;
562575
}
563576
}
577+
super.handleEvent(event);
564578
}
565579

566580
@override

packages/flutter/test/cupertino/action_sheet_test.dart

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1155,6 +1155,52 @@ void main() {
11551155
expect(pressed, null);
11561156
});
11571157

1158+
testWidgets('Taps on legacy button calls onPressed and renders correctly', (WidgetTester tester) async {
1159+
// Legacy buttons are implemented with [GestureDetector.onTap]. Apps that
1160+
// use customized legacy buttons should continue to work.
1161+
//
1162+
// Regression test for https://github.com/flutter/flutter/issues/150980 .
1163+
bool wasPressed = false;
1164+
await tester.pumpWidget(
1165+
createAppWithButtonThatLaunchesActionSheet(
1166+
Builder(builder: (BuildContext context) {
1167+
return CupertinoActionSheet(
1168+
actions: <Widget>[
1169+
LegacyAction(
1170+
child: const Text('Legacy'),
1171+
onPressed: () {
1172+
expect(wasPressed, false);
1173+
wasPressed = true;
1174+
Navigator.pop(context);
1175+
},
1176+
),
1177+
CupertinoActionSheetAction(child: const Text('One'), onPressed: () {}),
1178+
CupertinoActionSheetAction(child: const Text('Two'), onPressed: () {}),
1179+
],
1180+
);
1181+
}),
1182+
),
1183+
);
1184+
1185+
await tester.tap(find.text('Go'));
1186+
await tester.pumpAndSettle();
1187+
expect(wasPressed, isFalse);
1188+
1189+
// Push the legacy button and hold for a while to activate the pressing effect.
1190+
final TestGesture gesture = await tester.startGesture(tester.getCenter(find.text('Legacy')));
1191+
await tester.pump(const Duration(seconds: 1));
1192+
expect(wasPressed, isFalse);
1193+
await expectLater(
1194+
find.byType(CupertinoActionSheet),
1195+
matchesGoldenFile('cupertinoActionSheet.legacyButton.png'),
1196+
);
1197+
1198+
await gesture.up();
1199+
await tester.pumpAndSettle();
1200+
expect(wasPressed, isTrue);
1201+
expect(find.text('Legacy'), findsNothing);
1202+
});
1203+
11581204
testWidgets('Action sheet width is correct when given infinite horizontal space', (WidgetTester tester) async {
11591205
await tester.pumpWidget(
11601206
createAppWithButtonThatLaunchesActionSheet(
@@ -2054,3 +2100,32 @@ class OverrideMediaQuery extends StatelessWidget {
20542100
);
20552101
}
20562102
}
2103+
2104+
// Old-style action sheet buttons, which are implemented with
2105+
// `GestureDetector.onTap`.
2106+
class LegacyAction extends StatelessWidget {
2107+
const LegacyAction({
2108+
super.key,
2109+
required this.onPressed,
2110+
required this.child,
2111+
});
2112+
2113+
final VoidCallback onPressed;
2114+
final Widget child;
2115+
2116+
@override
2117+
Widget build(BuildContext context) {
2118+
return GestureDetector(
2119+
onTap: onPressed,
2120+
behavior: HitTestBehavior.opaque,
2121+
child: ConstrainedBox(
2122+
constraints: const BoxConstraints(minHeight: 57),
2123+
child: Container(
2124+
alignment: AlignmentDirectional.center,
2125+
padding: const EdgeInsets.symmetric(vertical: 16.0, horizontal: 10.0),
2126+
child: child,
2127+
),
2128+
),
2129+
);
2130+
}
2131+
}

0 commit comments

Comments
 (0)