Use Device specific gesture configuration for scroll views #87604
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
flutter-dashboard
bot
added
a: tests
…ter into gesture_settings_impl
jonahwilliams
changed the title
|
I don't really want to hit everything at once, but making this optional for the sake of backwards compat - maybe we can migrate to this over time. @goderbauer for thoughts? |
…ter into gesture_settings_impl
goderbauer
reviewed
Aug 6, 2021
| switch (kind) { | ||
| case PointerDeviceKind.mouse: | ||
| return kPrecisePointerPanSlop; | ||
| case PointerDeviceKind.stylus: | ||
| case PointerDeviceKind.invertedStylus: | ||
| case PointerDeviceKind.unknown: | ||
| case PointerDeviceKind.touch: | ||
| return kPanSlop; | ||
| return deviceTouchSlop != null ? (2 * deviceTouchSlop) : kPanSlop; |
| switch (kind) { | ||
| case PointerDeviceKind.mouse: | ||
| return kPrecisePointerPanSlop; | ||
| case PointerDeviceKind.stylus: | ||
| case PointerDeviceKind.invertedStylus: | ||
| case PointerDeviceKind.unknown: | ||
| case PointerDeviceKind.touch: | ||
| return kPanSlop; | ||
| return deviceTouchSlop != null ? (2 * deviceTouchSlop) : kPanSlop; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
What about the scaleSlop below?
There was a problem hiding this comment.
I'm not quite sure what to do with the scale slop right now.
| @@ -249,7 +249,7 @@ class ForcePressGestureRecognizer extends OneSequenceGestureRecognizer { | |||
| if (pressure > startPressure) { | |||
| _state = _ForceState.started; | |||
| resolve(GestureDisposition.accepted); | |||
| } else if (event.delta.distanceSquared > computeHitSlop(event.kind)) { | |||
| } else if (event.delta.distanceSquared > computeHitSlop(event.kind, null)) { | |||
There was a problem hiding this comment.
Why null over gestureSettings.touchSlop?
| @@ -497,17 +504,17 @@ class VerticalMultiDragGestureRecognizer extends MultiDragGestureRecognizer { | |||
|
|
|||
| @override | |||
| MultiDragPointerState createNewPointerState(PointerDownEvent event) { | |||
| return _VerticalPointerState(event.position, event.kind); | |||
| return _VerticalPointerState(event.position, event.kind, null); | |||
| @@ -2099,28 +2099,28 @@ class PointerCancelEvent extends PointerEvent with _PointerEventDescription, _Co | |||
| } | |||
|
|
|||
| /// Determine the appropriate hit slop pixels based on the [kind] of pointer. | |||
| double computeHitSlop(PointerDeviceKind kind) { | |||
| double computeHitSlop(PointerDeviceKind kind, double? deviceTouchSlop) { | |||
There was a problem hiding this comment.
should these just take a DeviceGestureSettings obj and the method can pick the appropriate value from it?
| @@ -244,6 +246,7 @@ class DoubleTapGestureRecognizer extends GestureRecognizer { | |||
| event: event, | |||
| entry: GestureBinding.instance!.gestureArena.add(event.pointer, this), | |||
| doubleTapMinTime: kDoubleTapMinTime, | |||
| touchSlop: null, | |||
| @@ -879,6 +885,7 @@ class SerialTapGestureRecognizer extends GestureRecognizer { | |||
| invokeCallback<void>('onSerialTapDown', () => onSerialTapDown!(details)); | |||
| } | |||
| final _TapTracker tracker = _TapTracker( | |||
| touchSlop: null, | |||
goderbauer
approved these changes
Aug 9, 2021
| final double? touchSlop; | ||
|
|
||
| /// The touch slop value for pan gestures, in logical pixels, or `null` if it | ||
| /// was not set. |
There was a problem hiding this comment.
Looks like there is no way to actually set it independently? Maybe document that it's either null or 2* touchSlop if touchSlop is set?
There was a problem hiding this comment.
Will do. In the future we could support setting it separately if need be.
| @@ -104,6 +105,7 @@ class MediaQueryData { | |||
| this.disableAnimations = false, | |||
| this.boldText = false, | |||
| this.navigationMode = NavigationMode.traditional, | |||
| this.gestureSettings = const DeviceGestureSettings(touchSlop: 16.0) | |||
There was a problem hiding this comment.
Can this use kTouchSlop instead of hard-coding 16?
added 2 commits
August 9, 2021 09:39
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #87322
Wires up the window specific GestureSettings into a dpr transformed DeviceGestureSettings. This can be configured on the base GestureDetector, and then each gesture detector can choose which (if any) configuration it wants to use.