Use Device specific gesture configuration for scroll views #87604
Conversation
…ter into gesture_settings_impl
|
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
left a comment
There was a problem hiding this comment.
In every place where null is passed over the real device value a comment would be good explaining why we want the framework fallback rather than the device-provided value.
| case PointerDeviceKind.unknown: | ||
| case PointerDeviceKind.touch: | ||
| return kPanSlop; | ||
| return deviceTouchSlop != null ? (2 * deviceTouchSlop) : 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.
| _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?
| @override | ||
| MultiDragPointerState createNewPointerState(PointerDownEvent event) { | ||
| return _VerticalPointerState(event.position, event.kind); | ||
| return _VerticalPointerState(event.position, event.kind, null); |
|
|
||
| /// 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?
| event: event, | ||
| entry: GestureBinding.instance!.gestureArena.add(event.pointer, this), | ||
| doubleTapMinTime: kDoubleTapMinTime, | ||
| touchSlop: null, |
| invokeCallback<void>('onSerialTapDown', () => onSerialTapDown!(details)); | ||
| } | ||
| final _TapTracker tracker = _TapTracker( | ||
| touchSlop: null, |
| 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.
| 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?
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.