Skip to content

Commit 246d6c6

Browse files
authored
Make pressing and moving on CupertinoButton closer to native behavior. (#161731)
Fixes: #91581 This PR adds an `onTapMove` event to `TapGestureRecognizer`, and then `CupertinoButton` uses this event to implement the behavior of the native iOS `UIButton` (as shown in the issue). It is worth noting that this PR slightly changes the way `CupertinoButton`'s `onPressed` is triggered. Specifically, it changes from being triggered by `TapGestureRecognizer`'s `onTap` to checking if the event position is still above the button in `TapGestureRecognizer`'s `onTapUp`. Additionally, it removes the previous behavior where the gesture was canceled if moved beyond `kScaleSlop` (18 logical pixels). Overall, previously, `onPressed` could not be triggered if the button was pressed and then moved more than 18 pixels. This PR adjusts it so that `onPressed` cannot be triggered if the button is pressed and then moved outside the button. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
1 parent 7f23dc7 commit 246d6c6

File tree

6 files changed

+254
-11
lines changed

6 files changed

+254
-11
lines changed

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

Lines changed: 53 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
library;
77

88
import 'package:flutter/foundation.dart';
9+
import 'package:flutter/gestures.dart';
910
import 'package:flutter/semantics.dart';
1011
import 'package:flutter/widgets.dart';
1112

@@ -263,6 +264,18 @@ class CupertinoButton extends StatefulWidget {
263264
/// enable a button, set [onPressed] or [onLongPress] to a non-null value.
264265
bool get enabled => onPressed != null || onLongPress != null;
265266

267+
/// The distance a button needs to be moved after being pressed for its opacity to change.
268+
///
269+
/// The opacity changes when the position moved is this distance away from the button.
270+
static double tapMoveSlop() {
271+
return switch (defaultTargetPlatform) {
272+
TargetPlatform.iOS ||
273+
TargetPlatform.android ||
274+
TargetPlatform.fuchsia => kCupertinoButtonTapMoveSlop,
275+
TargetPlatform.macOS || TargetPlatform.linux || TargetPlatform.windows => 0.0,
276+
};
277+
}
278+
266279
@override
267280
State<CupertinoButton> createState() => _CupertinoButtonState();
268281

@@ -329,6 +342,11 @@ class _CupertinoButtonState extends State<CupertinoButton> with SingleTickerProv
329342
_buttonHeldDown = false;
330343
_animate();
331344
}
345+
final RenderBox renderObject = context.findRenderObject()! as RenderBox;
346+
final Offset localPosition = renderObject.globalToLocal(event.globalPosition);
347+
if (renderObject.paintBounds.inflate(CupertinoButton.tapMoveSlop()).contains(localPosition)) {
348+
_handleTap();
349+
}
332350
}
333351

334352
void _handleTapCancel() {
@@ -338,6 +356,18 @@ class _CupertinoButtonState extends State<CupertinoButton> with SingleTickerProv
338356
}
339357
}
340358

359+
void _handTapMove(TapMoveDetails event) {
360+
final RenderBox renderObject = context.findRenderObject()! as RenderBox;
361+
final Offset localPosition = renderObject.globalToLocal(event.globalPosition);
362+
final bool buttonShouldHeldDown = renderObject.paintBounds
363+
.inflate(CupertinoButton.tapMoveSlop())
364+
.contains(localPosition);
365+
if (buttonShouldHeldDown != _buttonHeldDown) {
366+
_buttonHeldDown = buttonShouldHeldDown;
367+
_animate();
368+
}
369+
}
370+
341371
void _handleTap([Intent? _]) {
342372
if (widget.onPressed != null) {
343373
widget.onPressed!();
@@ -429,7 +459,7 @@ class _CupertinoButtonState extends State<CupertinoButton> with SingleTickerProv
429459
size:
430460
textStyle.fontSize != null ? textStyle.fontSize! * 1.2 : kCupertinoButtonDefaultIconSize,
431461
);
432-
462+
final DeviceGestureSettings? gestureSettings = MediaQuery.maybeGestureSettingsOf(context);
433463
return MouseRegion(
434464
cursor: enabled && kIsWeb ? SystemMouseCursors.click : MouseCursor.defer,
435465
child: FocusableActionDetector(
@@ -439,13 +469,29 @@ class _CupertinoButtonState extends State<CupertinoButton> with SingleTickerProv
439469
onFocusChange: widget.onFocusChange,
440470
onShowFocusHighlight: _onShowFocusHighlight,
441471
enabled: enabled,
442-
child: GestureDetector(
472+
child: RawGestureDetector(
443473
behavior: HitTestBehavior.opaque,
444-
onTapDown: enabled ? _handleTapDown : null,
445-
onTapUp: enabled ? _handleTapUp : null,
446-
onTapCancel: enabled ? _handleTapCancel : null,
447-
onTap: widget.onPressed,
448-
onLongPress: widget.onLongPress,
474+
gestures: <Type, GestureRecognizerFactory>{
475+
TapGestureRecognizer: GestureRecognizerFactoryWithHandlers<TapGestureRecognizer>(
476+
() => TapGestureRecognizer(postAcceptSlopTolerance: null),
477+
(TapGestureRecognizer instance) {
478+
instance.onTapDown = enabled ? _handleTapDown : null;
479+
instance.onTapUp = enabled ? _handleTapUp : null;
480+
instance.onTapCancel = enabled ? _handleTapCancel : null;
481+
instance.onTapMove = enabled ? _handTapMove : null;
482+
instance.gestureSettings = gestureSettings;
483+
},
484+
),
485+
if (widget.onLongPress != null)
486+
LongPressGestureRecognizer:
487+
GestureRecognizerFactoryWithHandlers<LongPressGestureRecognizer>(
488+
() => LongPressGestureRecognizer(),
489+
(LongPressGestureRecognizer instance) {
490+
instance.onLongPress = widget.onLongPress;
491+
instance.gestureSettings = gestureSettings;
492+
},
493+
),
494+
},
449495
child: Semantics(
450496
button: true,
451497
child: ConstrainedBox(

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,3 +84,11 @@ const Map<CupertinoButtonSize, double> kCupertinoButtonMinSize = <CupertinoButto
8484
CupertinoButtonSize.medium: 32,
8585
CupertinoButtonSize.large: 44,
8686
};
87+
88+
/// The distance a button needs to be moved after being pressed for its opacity to change.
89+
///
90+
/// The opacity changes when the position moved is this distance away from the button.
91+
/// This variable is effective on mobile platforms. For desktop platforms, a distance of 0 is used.
92+
///
93+
/// This value was obtained through actual testing on an iOS 18.1 simulator.
94+
const double kCupertinoButtonTapMoveSlop = 70.0;

packages/flutter/lib/src/gestures/tap.dart

Lines changed: 102 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,35 @@ class TapUpDetails {
7878
final PointerDeviceKind kind;
7979
}
8080

81+
/// Details object for callbacks that use [GestureTapMoveCallback].
82+
///
83+
/// See also:
84+
///
85+
/// * [GestureDetector.onTapMove], which receives this information.
86+
/// * [TapGestureRecognizer], which passes this information to one of its callbacks.
87+
class TapMoveDetails {
88+
/// Creates a [TapMoveDetails] data object.
89+
TapMoveDetails({
90+
required this.kind,
91+
this.globalPosition = Offset.zero,
92+
this.delta = Offset.zero,
93+
Offset? localPosition,
94+
}) : localPosition = localPosition ?? globalPosition;
95+
96+
/// The global position at which the pointer contacted the screen.
97+
final Offset globalPosition;
98+
99+
/// The local position at which the pointer contacted the screen.
100+
final Offset localPosition;
101+
102+
/// The kind of the device that initiated the event.
103+
final PointerDeviceKind kind;
104+
105+
/// The amount the pointer has moved in the coordinate space of the
106+
/// event receiver since the previous update.
107+
final Offset delta;
108+
}
109+
81110
/// {@template flutter.gestures.tap.GestureTapUpCallback}
82111
/// Signature for when a pointer that will trigger a tap has stopped contacting
83112
/// the screen.
@@ -100,6 +129,16 @@ typedef GestureTapUpCallback = void Function(TapUpDetails details);
100129
/// * [TapGestureRecognizer], which uses this signature in one of its callbacks.
101130
typedef GestureTapCallback = void Function();
102131

132+
/// Signature for when a pointer that triggered a tap has moved.
133+
///
134+
/// The position at which the pointer moved is available in the `details`.
135+
///
136+
/// See also:
137+
///
138+
/// * [GestureDetector.onTapMove], which matches this signature.
139+
/// * [TapGestureRecognizer], which uses this signature in one of its callbacks.
140+
typedef GestureTapMoveCallback = void Function(TapMoveDetails details);
141+
103142
/// Signature for when the pointer that previously triggered a
104143
/// [GestureTapDownCallback] will not end up causing a tap.
105144
///
@@ -143,8 +182,13 @@ abstract class BaseTapGestureRecognizer extends PrimaryPointerGestureRecognizer
143182
/// Creates a tap gesture recognizer.
144183
///
145184
/// {@macro flutter.gestures.GestureRecognizer.supportedDevices}
146-
BaseTapGestureRecognizer({super.debugOwner, super.supportedDevices, super.allowedButtonsFilter})
147-
: super(deadline: kPressTimeout);
185+
BaseTapGestureRecognizer({
186+
super.debugOwner,
187+
super.supportedDevices,
188+
super.allowedButtonsFilter,
189+
super.preAcceptSlopTolerance,
190+
super.postAcceptSlopTolerance,
191+
}) : super(deadline: kPressTimeout);
148192

149193
bool _sentTapDown = false;
150194
bool _wonArenaForPrimaryPointer = false;
@@ -178,6 +222,15 @@ abstract class BaseTapGestureRecognizer extends PrimaryPointerGestureRecognizer
178222
@protected
179223
void handleTapUp({required PointerDownEvent down, required PointerUpEvent up});
180224

225+
/// A pointer that triggered a tap has moved.
226+
///
227+
/// This triggers on the move event if the recognizer has recognized the tap gesture.
228+
///
229+
/// The parameter `move` is the move event of the primary pointer that started
230+
/// the tap sequence.
231+
@protected
232+
void handleTapMove({required PointerMoveEvent move}) {}
233+
181234
/// A pointer that previously triggered [handleTapDown] will not end up
182235
/// causing a tap.
183236
///
@@ -247,6 +300,8 @@ abstract class BaseTapGestureRecognizer extends PrimaryPointerGestureRecognizer
247300
} else if (event.buttons != _down!.buttons) {
248301
resolve(GestureDisposition.rejected);
249302
stopTrackingPointer(primaryPointer!);
303+
} else if (event is PointerMoveEvent) {
304+
_checkMove(event);
250305
}
251306
}
252307

@@ -312,6 +367,11 @@ abstract class BaseTapGestureRecognizer extends PrimaryPointerGestureRecognizer
312367
handleTapCancel(down: _down!, cancel: event, reason: note);
313368
}
314369

370+
void _checkMove(PointerMoveEvent event) {
371+
assert(event.pointer == _down!.pointer);
372+
handleTapMove(move: event);
373+
}
374+
315375
void _reset() {
316376
_sentTapDown = false;
317377
_wonArenaForPrimaryPointer = false;
@@ -381,7 +441,13 @@ class TapGestureRecognizer extends BaseTapGestureRecognizer {
381441
/// Creates a tap gesture recognizer.
382442
///
383443
/// {@macro flutter.gestures.GestureRecognizer.supportedDevices}
384-
TapGestureRecognizer({super.debugOwner, super.supportedDevices, super.allowedButtonsFilter});
444+
TapGestureRecognizer({
445+
super.debugOwner,
446+
super.supportedDevices,
447+
super.allowedButtonsFilter,
448+
super.preAcceptSlopTolerance,
449+
super.postAcceptSlopTolerance,
450+
});
385451

386452
/// {@template flutter.gestures.tap.TapGestureRecognizer.onTapDown}
387453
/// A pointer has contacted the screen at a particular location with a primary
@@ -438,6 +504,20 @@ class TapGestureRecognizer extends BaseTapGestureRecognizer {
438504
/// * [GestureDetector.onTap], which exposes this callback.
439505
GestureTapCallback? onTap;
440506

507+
/// A pointer that triggered a tap has moved.
508+
///
509+
/// This callback is triggered after the tap gesture has been recognized and the pointer starts to move.
510+
///
511+
/// If the pointer moves beyond the `postAcceptSlopTolerance` distance, the tap will be canceled.
512+
/// To make `onTapMove` more useful, consider setting `postAcceptSlopTolerance` to a larger value,
513+
/// or to `null` for no limit on movement.
514+
///
515+
/// See also:
516+
///
517+
/// * [kPrimaryButton], the button this callback responds to.
518+
/// * [GestureDetector.onTapMove], which exposes this callback.
519+
GestureTapMoveCallback? onTapMove;
520+
441521
/// {@template flutter.gestures.tap.TapGestureRecognizer.onTapCancel}
442522
/// A pointer that previously triggered [onTapDown] will not end up causing
443523
/// a tap.
@@ -591,7 +671,11 @@ class TapGestureRecognizer extends BaseTapGestureRecognizer {
591671
bool isPointerAllowed(PointerDownEvent event) {
592672
switch (event.buttons) {
593673
case kPrimaryButton:
594-
if (onTapDown == null && onTap == null && onTapUp == null && onTapCancel == null) {
674+
if (onTapDown == null &&
675+
onTap == null &&
676+
onTapUp == null &&
677+
onTapCancel == null &&
678+
onTapMove == null) {
595679
return false;
596680
}
597681
case kSecondaryButton:
@@ -667,6 +751,20 @@ class TapGestureRecognizer extends BaseTapGestureRecognizer {
667751
}
668752
}
669753

754+
@protected
755+
@override
756+
void handleTapMove({required PointerMoveEvent move}) {
757+
if (onTapMove != null && move.buttons == kPrimaryButton) {
758+
final TapMoveDetails details = TapMoveDetails(
759+
globalPosition: move.position,
760+
localPosition: move.localPosition,
761+
kind: getKindForPointer(move.pointer),
762+
delta: move.delta,
763+
);
764+
invokeCallback<void>('onTapMove', () => onTapMove!(details));
765+
}
766+
}
767+
670768
@protected
671769
@override
672770
void handleTapCancel({

packages/flutter/lib/src/widgets/gesture_detector.dart

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,7 @@ class GestureDetector extends StatelessWidget {
240240
this.onTapDown,
241241
this.onTapUp,
242242
this.onTap,
243+
this.onTapMove,
243244
this.onTapCancel,
244245
this.onSecondaryTap,
245246
this.onSecondaryTapDown,
@@ -373,6 +374,15 @@ class GestureDetector extends StatelessWidget {
373374
/// regarding the pointer position.
374375
final GestureTapCallback? onTap;
375376

377+
/// A pointer that triggered a tap has moved.
378+
///
379+
/// This triggers when the pointer moves after the tap gesture has been recognized.
380+
///
381+
/// See also:
382+
///
383+
/// * [kPrimaryButton], the button this callback responds to.
384+
final GestureTapMoveCallback? onTapMove;
385+
376386
/// The pointer that previously triggered [onTapDown] will not end up causing
377387
/// a tap.
378388
///

packages/flutter/test/cupertino/button_test.dart

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -885,6 +885,62 @@ void main() {
885885
await tester.pump();
886886
expect(value, isTrue);
887887
});
888+
889+
testWidgets('Press and move on button and animation works', (WidgetTester tester) async {
890+
await tester.pumpWidget(
891+
boilerplate(child: CupertinoButton(onPressed: () {}, child: const Text('Tap me'))),
892+
);
893+
final TestGesture gesture = await tester.startGesture(
894+
tester.getTopLeft(find.byType(CupertinoButton)),
895+
);
896+
addTearDown(gesture.removePointer);
897+
// Check opacity.
898+
final FadeTransition opacity = tester.widget(
899+
find.descendant(of: find.byType(CupertinoButton), matching: find.byType(FadeTransition)),
900+
);
901+
await tester.pumpAndSettle();
902+
expect(opacity.opacity.value, 0.4);
903+
final double moveDistance = CupertinoButton.tapMoveSlop();
904+
await gesture.moveBy(Offset(0, -moveDistance + 1));
905+
await tester.pumpAndSettle();
906+
expect(opacity.opacity.value, 0.4);
907+
await gesture.moveBy(const Offset(0, -2));
908+
await tester.pumpAndSettle();
909+
expect(opacity.opacity.value, 1.0);
910+
await gesture.moveBy(const Offset(0, 1));
911+
await tester.pumpAndSettle();
912+
expect(opacity.opacity.value, 0.4);
913+
}, variant: TargetPlatformVariant.all());
914+
915+
testWidgets('onPressed trigger takes into account MoveSlop.', (WidgetTester tester) async {
916+
bool value = false;
917+
await tester.pumpWidget(
918+
boilerplate(
919+
child: CupertinoButton(
920+
onPressed: () {
921+
value = true;
922+
},
923+
child: const Text('Tap me'),
924+
),
925+
),
926+
);
927+
TestGesture gesture = await tester.startGesture(tester.getCenter(find.byType(CupertinoButton)));
928+
await gesture.moveTo(
929+
tester.getBottomRight(find.byType(CupertinoButton)) +
930+
Offset(0, CupertinoButton.tapMoveSlop()),
931+
);
932+
await gesture.up();
933+
expect(value, isFalse);
934+
935+
gesture = await tester.startGesture(tester.getTopLeft(find.byType(CupertinoButton)));
936+
await gesture.moveTo(
937+
tester.getBottomRight(find.byType(CupertinoButton)) +
938+
Offset(0, CupertinoButton.tapMoveSlop()),
939+
);
940+
await gesture.moveBy(const Offset(0, -1));
941+
await gesture.up();
942+
expect(value, isTrue);
943+
});
888944
}
889945

890946
Widget boilerplate({required Widget child}) {

0 commit comments

Comments
 (0)