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
This commit is contained in:
parent
7f23dc7a0d
commit
246d6c6834
@ -6,6 +6,7 @@
|
||||
library;
|
||||
|
||||
import 'package:flutter/foundation.dart';
|
||||
import 'package:flutter/gestures.dart';
|
||||
import 'package:flutter/semantics.dart';
|
||||
import 'package:flutter/widgets.dart';
|
||||
|
||||
@ -263,6 +264,18 @@ class CupertinoButton extends StatefulWidget {
|
||||
/// enable a button, set [onPressed] or [onLongPress] to a non-null value.
|
||||
bool get enabled => onPressed != null || onLongPress != null;
|
||||
|
||||
/// The distance a button needs to be moved after being pressed for its opacity to change.
|
||||
///
|
||||
/// The opacity changes when the position moved is this distance away from the button.
|
||||
static double tapMoveSlop() {
|
||||
return switch (defaultTargetPlatform) {
|
||||
TargetPlatform.iOS ||
|
||||
TargetPlatform.android ||
|
||||
TargetPlatform.fuchsia => kCupertinoButtonTapMoveSlop,
|
||||
TargetPlatform.macOS || TargetPlatform.linux || TargetPlatform.windows => 0.0,
|
||||
};
|
||||
}
|
||||
|
||||
@override
|
||||
State<CupertinoButton> createState() => _CupertinoButtonState();
|
||||
|
||||
@ -329,6 +342,11 @@ class _CupertinoButtonState extends State<CupertinoButton> with SingleTickerProv
|
||||
_buttonHeldDown = false;
|
||||
_animate();
|
||||
}
|
||||
final RenderBox renderObject = context.findRenderObject()! as RenderBox;
|
||||
final Offset localPosition = renderObject.globalToLocal(event.globalPosition);
|
||||
if (renderObject.paintBounds.inflate(CupertinoButton.tapMoveSlop()).contains(localPosition)) {
|
||||
_handleTap();
|
||||
}
|
||||
}
|
||||
|
||||
void _handleTapCancel() {
|
||||
@ -338,6 +356,18 @@ class _CupertinoButtonState extends State<CupertinoButton> with SingleTickerProv
|
||||
}
|
||||
}
|
||||
|
||||
void _handTapMove(TapMoveDetails event) {
|
||||
final RenderBox renderObject = context.findRenderObject()! as RenderBox;
|
||||
final Offset localPosition = renderObject.globalToLocal(event.globalPosition);
|
||||
final bool buttonShouldHeldDown = renderObject.paintBounds
|
||||
.inflate(CupertinoButton.tapMoveSlop())
|
||||
.contains(localPosition);
|
||||
if (buttonShouldHeldDown != _buttonHeldDown) {
|
||||
_buttonHeldDown = buttonShouldHeldDown;
|
||||
_animate();
|
||||
}
|
||||
}
|
||||
|
||||
void _handleTap([Intent? _]) {
|
||||
if (widget.onPressed != null) {
|
||||
widget.onPressed!();
|
||||
@ -429,7 +459,7 @@ class _CupertinoButtonState extends State<CupertinoButton> with SingleTickerProv
|
||||
size:
|
||||
textStyle.fontSize != null ? textStyle.fontSize! * 1.2 : kCupertinoButtonDefaultIconSize,
|
||||
);
|
||||
|
||||
final DeviceGestureSettings? gestureSettings = MediaQuery.maybeGestureSettingsOf(context);
|
||||
return MouseRegion(
|
||||
cursor: enabled && kIsWeb ? SystemMouseCursors.click : MouseCursor.defer,
|
||||
child: FocusableActionDetector(
|
||||
@ -439,13 +469,29 @@ class _CupertinoButtonState extends State<CupertinoButton> with SingleTickerProv
|
||||
onFocusChange: widget.onFocusChange,
|
||||
onShowFocusHighlight: _onShowFocusHighlight,
|
||||
enabled: enabled,
|
||||
child: GestureDetector(
|
||||
child: RawGestureDetector(
|
||||
behavior: HitTestBehavior.opaque,
|
||||
onTapDown: enabled ? _handleTapDown : null,
|
||||
onTapUp: enabled ? _handleTapUp : null,
|
||||
onTapCancel: enabled ? _handleTapCancel : null,
|
||||
onTap: widget.onPressed,
|
||||
onLongPress: widget.onLongPress,
|
||||
gestures: <Type, GestureRecognizerFactory>{
|
||||
TapGestureRecognizer: GestureRecognizerFactoryWithHandlers<TapGestureRecognizer>(
|
||||
() => TapGestureRecognizer(postAcceptSlopTolerance: null),
|
||||
(TapGestureRecognizer instance) {
|
||||
instance.onTapDown = enabled ? _handleTapDown : null;
|
||||
instance.onTapUp = enabled ? _handleTapUp : null;
|
||||
instance.onTapCancel = enabled ? _handleTapCancel : null;
|
||||
instance.onTapMove = enabled ? _handTapMove : null;
|
||||
instance.gestureSettings = gestureSettings;
|
||||
},
|
||||
),
|
||||
if (widget.onLongPress != null)
|
||||
LongPressGestureRecognizer:
|
||||
GestureRecognizerFactoryWithHandlers<LongPressGestureRecognizer>(
|
||||
() => LongPressGestureRecognizer(),
|
||||
(LongPressGestureRecognizer instance) {
|
||||
instance.onLongPress = widget.onLongPress;
|
||||
instance.gestureSettings = gestureSettings;
|
||||
},
|
||||
),
|
||||
},
|
||||
child: Semantics(
|
||||
button: true,
|
||||
child: ConstrainedBox(
|
||||
|
@ -84,3 +84,11 @@ const Map<CupertinoButtonSize, double> kCupertinoButtonMinSize = <CupertinoButto
|
||||
CupertinoButtonSize.medium: 32,
|
||||
CupertinoButtonSize.large: 44,
|
||||
};
|
||||
|
||||
/// The distance a button needs to be moved after being pressed for its opacity to change.
|
||||
///
|
||||
/// The opacity changes when the position moved is this distance away from the button.
|
||||
/// This variable is effective on mobile platforms. For desktop platforms, a distance of 0 is used.
|
||||
///
|
||||
/// This value was obtained through actual testing on an iOS 18.1 simulator.
|
||||
const double kCupertinoButtonTapMoveSlop = 70.0;
|
||||
|
@ -78,6 +78,35 @@ class TapUpDetails {
|
||||
final PointerDeviceKind kind;
|
||||
}
|
||||
|
||||
/// Details object for callbacks that use [GestureTapMoveCallback].
|
||||
///
|
||||
/// See also:
|
||||
///
|
||||
/// * [GestureDetector.onTapMove], which receives this information.
|
||||
/// * [TapGestureRecognizer], which passes this information to one of its callbacks.
|
||||
class TapMoveDetails {
|
||||
/// Creates a [TapMoveDetails] data object.
|
||||
TapMoveDetails({
|
||||
required this.kind,
|
||||
this.globalPosition = Offset.zero,
|
||||
this.delta = Offset.zero,
|
||||
Offset? localPosition,
|
||||
}) : localPosition = localPosition ?? globalPosition;
|
||||
|
||||
/// The global position at which the pointer contacted the screen.
|
||||
final Offset globalPosition;
|
||||
|
||||
/// The local position at which the pointer contacted the screen.
|
||||
final Offset localPosition;
|
||||
|
||||
/// The kind of the device that initiated the event.
|
||||
final PointerDeviceKind kind;
|
||||
|
||||
/// The amount the pointer has moved in the coordinate space of the
|
||||
/// event receiver since the previous update.
|
||||
final Offset delta;
|
||||
}
|
||||
|
||||
/// {@template flutter.gestures.tap.GestureTapUpCallback}
|
||||
/// Signature for when a pointer that will trigger a tap has stopped contacting
|
||||
/// the screen.
|
||||
@ -100,6 +129,16 @@ typedef GestureTapUpCallback = void Function(TapUpDetails details);
|
||||
/// * [TapGestureRecognizer], which uses this signature in one of its callbacks.
|
||||
typedef GestureTapCallback = void Function();
|
||||
|
||||
/// Signature for when a pointer that triggered a tap has moved.
|
||||
///
|
||||
/// The position at which the pointer moved is available in the `details`.
|
||||
///
|
||||
/// See also:
|
||||
///
|
||||
/// * [GestureDetector.onTapMove], which matches this signature.
|
||||
/// * [TapGestureRecognizer], which uses this signature in one of its callbacks.
|
||||
typedef GestureTapMoveCallback = void Function(TapMoveDetails details);
|
||||
|
||||
/// Signature for when the pointer that previously triggered a
|
||||
/// [GestureTapDownCallback] will not end up causing a tap.
|
||||
///
|
||||
@ -143,8 +182,13 @@ abstract class BaseTapGestureRecognizer extends PrimaryPointerGestureRecognizer
|
||||
/// Creates a tap gesture recognizer.
|
||||
///
|
||||
/// {@macro flutter.gestures.GestureRecognizer.supportedDevices}
|
||||
BaseTapGestureRecognizer({super.debugOwner, super.supportedDevices, super.allowedButtonsFilter})
|
||||
: super(deadline: kPressTimeout);
|
||||
BaseTapGestureRecognizer({
|
||||
super.debugOwner,
|
||||
super.supportedDevices,
|
||||
super.allowedButtonsFilter,
|
||||
super.preAcceptSlopTolerance,
|
||||
super.postAcceptSlopTolerance,
|
||||
}) : super(deadline: kPressTimeout);
|
||||
|
||||
bool _sentTapDown = false;
|
||||
bool _wonArenaForPrimaryPointer = false;
|
||||
@ -178,6 +222,15 @@ abstract class BaseTapGestureRecognizer extends PrimaryPointerGestureRecognizer
|
||||
@protected
|
||||
void handleTapUp({required PointerDownEvent down, required PointerUpEvent up});
|
||||
|
||||
/// A pointer that triggered a tap has moved.
|
||||
///
|
||||
/// This triggers on the move event if the recognizer has recognized the tap gesture.
|
||||
///
|
||||
/// The parameter `move` is the move event of the primary pointer that started
|
||||
/// the tap sequence.
|
||||
@protected
|
||||
void handleTapMove({required PointerMoveEvent move}) {}
|
||||
|
||||
/// A pointer that previously triggered [handleTapDown] will not end up
|
||||
/// causing a tap.
|
||||
///
|
||||
@ -247,6 +300,8 @@ abstract class BaseTapGestureRecognizer extends PrimaryPointerGestureRecognizer
|
||||
} else if (event.buttons != _down!.buttons) {
|
||||
resolve(GestureDisposition.rejected);
|
||||
stopTrackingPointer(primaryPointer!);
|
||||
} else if (event is PointerMoveEvent) {
|
||||
_checkMove(event);
|
||||
}
|
||||
}
|
||||
|
||||
@ -312,6 +367,11 @@ abstract class BaseTapGestureRecognizer extends PrimaryPointerGestureRecognizer
|
||||
handleTapCancel(down: _down!, cancel: event, reason: note);
|
||||
}
|
||||
|
||||
void _checkMove(PointerMoveEvent event) {
|
||||
assert(event.pointer == _down!.pointer);
|
||||
handleTapMove(move: event);
|
||||
}
|
||||
|
||||
void _reset() {
|
||||
_sentTapDown = false;
|
||||
_wonArenaForPrimaryPointer = false;
|
||||
@ -381,7 +441,13 @@ class TapGestureRecognizer extends BaseTapGestureRecognizer {
|
||||
/// Creates a tap gesture recognizer.
|
||||
///
|
||||
/// {@macro flutter.gestures.GestureRecognizer.supportedDevices}
|
||||
TapGestureRecognizer({super.debugOwner, super.supportedDevices, super.allowedButtonsFilter});
|
||||
TapGestureRecognizer({
|
||||
super.debugOwner,
|
||||
super.supportedDevices,
|
||||
super.allowedButtonsFilter,
|
||||
super.preAcceptSlopTolerance,
|
||||
super.postAcceptSlopTolerance,
|
||||
});
|
||||
|
||||
/// {@template flutter.gestures.tap.TapGestureRecognizer.onTapDown}
|
||||
/// A pointer has contacted the screen at a particular location with a primary
|
||||
@ -438,6 +504,20 @@ class TapGestureRecognizer extends BaseTapGestureRecognizer {
|
||||
/// * [GestureDetector.onTap], which exposes this callback.
|
||||
GestureTapCallback? onTap;
|
||||
|
||||
/// A pointer that triggered a tap has moved.
|
||||
///
|
||||
/// This callback is triggered after the tap gesture has been recognized and the pointer starts to move.
|
||||
///
|
||||
/// If the pointer moves beyond the `postAcceptSlopTolerance` distance, the tap will be canceled.
|
||||
/// To make `onTapMove` more useful, consider setting `postAcceptSlopTolerance` to a larger value,
|
||||
/// or to `null` for no limit on movement.
|
||||
///
|
||||
/// See also:
|
||||
///
|
||||
/// * [kPrimaryButton], the button this callback responds to.
|
||||
/// * [GestureDetector.onTapMove], which exposes this callback.
|
||||
GestureTapMoveCallback? onTapMove;
|
||||
|
||||
/// {@template flutter.gestures.tap.TapGestureRecognizer.onTapCancel}
|
||||
/// A pointer that previously triggered [onTapDown] will not end up causing
|
||||
/// a tap.
|
||||
@ -591,7 +671,11 @@ class TapGestureRecognizer extends BaseTapGestureRecognizer {
|
||||
bool isPointerAllowed(PointerDownEvent event) {
|
||||
switch (event.buttons) {
|
||||
case kPrimaryButton:
|
||||
if (onTapDown == null && onTap == null && onTapUp == null && onTapCancel == null) {
|
||||
if (onTapDown == null &&
|
||||
onTap == null &&
|
||||
onTapUp == null &&
|
||||
onTapCancel == null &&
|
||||
onTapMove == null) {
|
||||
return false;
|
||||
}
|
||||
case kSecondaryButton:
|
||||
@ -667,6 +751,20 @@ class TapGestureRecognizer extends BaseTapGestureRecognizer {
|
||||
}
|
||||
}
|
||||
|
||||
@protected
|
||||
@override
|
||||
void handleTapMove({required PointerMoveEvent move}) {
|
||||
if (onTapMove != null && move.buttons == kPrimaryButton) {
|
||||
final TapMoveDetails details = TapMoveDetails(
|
||||
globalPosition: move.position,
|
||||
localPosition: move.localPosition,
|
||||
kind: getKindForPointer(move.pointer),
|
||||
delta: move.delta,
|
||||
);
|
||||
invokeCallback<void>('onTapMove', () => onTapMove!(details));
|
||||
}
|
||||
}
|
||||
|
||||
@protected
|
||||
@override
|
||||
void handleTapCancel({
|
||||
|
@ -240,6 +240,7 @@ class GestureDetector extends StatelessWidget {
|
||||
this.onTapDown,
|
||||
this.onTapUp,
|
||||
this.onTap,
|
||||
this.onTapMove,
|
||||
this.onTapCancel,
|
||||
this.onSecondaryTap,
|
||||
this.onSecondaryTapDown,
|
||||
@ -373,6 +374,15 @@ class GestureDetector extends StatelessWidget {
|
||||
/// regarding the pointer position.
|
||||
final GestureTapCallback? onTap;
|
||||
|
||||
/// A pointer that triggered a tap has moved.
|
||||
///
|
||||
/// This triggers when the pointer moves after the tap gesture has been recognized.
|
||||
///
|
||||
/// See also:
|
||||
///
|
||||
/// * [kPrimaryButton], the button this callback responds to.
|
||||
final GestureTapMoveCallback? onTapMove;
|
||||
|
||||
/// The pointer that previously triggered [onTapDown] will not end up causing
|
||||
/// a tap.
|
||||
///
|
||||
|
@ -885,6 +885,62 @@ void main() {
|
||||
await tester.pump();
|
||||
expect(value, isTrue);
|
||||
});
|
||||
|
||||
testWidgets('Press and move on button and animation works', (WidgetTester tester) async {
|
||||
await tester.pumpWidget(
|
||||
boilerplate(child: CupertinoButton(onPressed: () {}, child: const Text('Tap me'))),
|
||||
);
|
||||
final TestGesture gesture = await tester.startGesture(
|
||||
tester.getTopLeft(find.byType(CupertinoButton)),
|
||||
);
|
||||
addTearDown(gesture.removePointer);
|
||||
// Check opacity.
|
||||
final FadeTransition opacity = tester.widget(
|
||||
find.descendant(of: find.byType(CupertinoButton), matching: find.byType(FadeTransition)),
|
||||
);
|
||||
await tester.pumpAndSettle();
|
||||
expect(opacity.opacity.value, 0.4);
|
||||
final double moveDistance = CupertinoButton.tapMoveSlop();
|
||||
await gesture.moveBy(Offset(0, -moveDistance + 1));
|
||||
await tester.pumpAndSettle();
|
||||
expect(opacity.opacity.value, 0.4);
|
||||
await gesture.moveBy(const Offset(0, -2));
|
||||
await tester.pumpAndSettle();
|
||||
expect(opacity.opacity.value, 1.0);
|
||||
await gesture.moveBy(const Offset(0, 1));
|
||||
await tester.pumpAndSettle();
|
||||
expect(opacity.opacity.value, 0.4);
|
||||
}, variant: TargetPlatformVariant.all());
|
||||
|
||||
testWidgets('onPressed trigger takes into account MoveSlop.', (WidgetTester tester) async {
|
||||
bool value = false;
|
||||
await tester.pumpWidget(
|
||||
boilerplate(
|
||||
child: CupertinoButton(
|
||||
onPressed: () {
|
||||
value = true;
|
||||
},
|
||||
child: const Text('Tap me'),
|
||||
),
|
||||
),
|
||||
);
|
||||
TestGesture gesture = await tester.startGesture(tester.getCenter(find.byType(CupertinoButton)));
|
||||
await gesture.moveTo(
|
||||
tester.getBottomRight(find.byType(CupertinoButton)) +
|
||||
Offset(0, CupertinoButton.tapMoveSlop()),
|
||||
);
|
||||
await gesture.up();
|
||||
expect(value, isFalse);
|
||||
|
||||
gesture = await tester.startGesture(tester.getTopLeft(find.byType(CupertinoButton)));
|
||||
await gesture.moveTo(
|
||||
tester.getBottomRight(find.byType(CupertinoButton)) +
|
||||
Offset(0, CupertinoButton.tapMoveSlop()),
|
||||
);
|
||||
await gesture.moveBy(const Offset(0, -1));
|
||||
await gesture.up();
|
||||
expect(value, isTrue);
|
||||
});
|
||||
}
|
||||
|
||||
Widget boilerplate({required Widget child}) {
|
||||
|
@ -1110,4 +1110,29 @@ void main() {
|
||||
|
||||
expect(didTap, isFalse);
|
||||
});
|
||||
|
||||
testGesture('onTapMove works', (GestureTester tester) {
|
||||
TapMoveDetails? tapMoveDetails;
|
||||
final TapGestureRecognizer tap = TapGestureRecognizer(postAcceptSlopTolerance: null)
|
||||
..onTapMove = (TapMoveDetails detail) {
|
||||
tapMoveDetails = detail;
|
||||
};
|
||||
addTearDown(tap.dispose);
|
||||
|
||||
final TestPointer pointer1 = TestPointer();
|
||||
final PointerDownEvent down = pointer1.down(Offset.zero);
|
||||
tap.addPointer(down);
|
||||
tester.closeArena(1);
|
||||
tester.route(down);
|
||||
tester.route(pointer1.move(const Offset(50.0, 0)));
|
||||
expect(tapMoveDetails, isNotNull);
|
||||
expect(tapMoveDetails!.globalPosition, const Offset(50.0, 0));
|
||||
expect(tapMoveDetails!.delta, const Offset(50.0, 0));
|
||||
tapMoveDetails = null;
|
||||
|
||||
tester.route(pointer1.move(const Offset(60.0, 10)));
|
||||
expect(tapMoveDetails, isNotNull);
|
||||
expect(tapMoveDetails!.globalPosition, const Offset(60.0, 10));
|
||||
expect(tapMoveDetails!.delta, const Offset(10.0, 10));
|
||||
});
|
||||
}
|
||||
|
Loading…
x
Reference in New Issue
Block a user