Fix SelectionArea scrolling conflicts #151138

Renzo-Olivares · 2024-07-01T21:39:10Z

Pre-launch Checklist

I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
I read the [Tree Hygiene] wiki page, which explains my responsibilities.
I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
I signed the [CLA].
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is [test-exempt].
I followed the [breaking change policy] and added [Data Driven Fixes] where supported.
All existing and new tests are passing.

Renzo-Olivares · 2024-07-11T18:08:50Z

Friendly bump for review.

chunhtai · 2024-07-16T16:24:09Z

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


  void _initMouseGestureRecognizer() {
-    switch (defaultTargetPlatform) {
-      case TargetPlatform.android:


nice clean up. I wonder why we have the switch case before

Yeah I think I just forgot to remove it.

chunhtai · 2024-07-16T16:28:03Z

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

+    // A [TapAndHorizontalDragGestureRecognizer] is used on non-precise pointer devices
+    // like PointerDeviceKind.touch so [SelectableRegion] gestures do not conflict with
+    // ancestor Scrollable gestures in common scenarios like a vertically scrolling list view.
+    _gestureRecognizers[TapAndHorizontalDragGestureRecognizer] = GestureRecognizerFactoryWithHandlers<TapAndHorizontalDragGestureRecognizer>(


does the touch gesture support drag selection without tapping the selection gesture?

Do you mean a single tap followed by a drag? Touch devices do not support this only mouse. There is an existing check that looks for this

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

Lines 632 to 636 in b9e4416

case 1:

if (details.kind != null && !_isPrecisePointerDevice(details.kind!)) {

// Drag to select is only enabled with a precise pointer device.

return;

}

and

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

Lines 644 to 648 in b9e4416

case 1:

if (details.kind != null && !_isPrecisePointerDevice(details.kind!)) {

// Drag to select is only enabled with a precise pointer device.

return;

}

.

In that case, why do we need this TapAndHorizontalDragGestureRecognizer for non precision device?

also if the reason we use tap and horizontal drag is to not conflict with vertical scroll. wouldn't that still cause issues with horizontal scroll?

We still need to use TapAndHorizontalDragGestureRecognizer for touch devices to support consecutive tap + drag just not single tap + drag.

It would still cause an issue with horizontal scroll, but in most cases this is not an issue. Take Android as an example, the HorizontalScrollable will have a touchSlop of 8.0 derived from https://developer.android.com/reference/android/view/ViewConfiguration#getScaledTouchSlop(). In my observations I've seen this value mostly set to ~8.0 on Android devices. The SelectableRegions TapAndHorizontalDragGestureRecognizer uses the default 18.0 for touch slop, so the HorizontalScrollable will still be able to win.

In the case of iOS it is a little more tricky since there is no getScaledTouchSlop equivalent provided by the iOS platform, so the HorizontalScrollable will always default to 18.0 and that will match the default of SelectableRegions TapAndHorizontalDragGestureRecognizer. That is why on iOS we set eagerVictoryOnDrag to false, which means the recognizer will wait until it is the last one in the arena before declaring victory, allowing horizontal drags to go through.

will using this gesture recognizer causes weirdness when user tap and vertical select? Base on reading the doc it looks like it would only accept horizontal drag first before winning the arena.

If the issue is with the slop, could we fix the vertical conflict by increase or decrease the tapandPanGestureRecognizer as well?

It shouldn't cause weirdness with tap and vertical select, for example in the case of a double tap + vertical drag on Android or iOS, the TapAndHorizontalDragGestureRecognizer will declare victory on a consecutive tap

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

Lines 1322 to 1327 in 22a5c6c

if (consecutiveTapCount > 1) {

// If our consecutive tap count is greater than 1, i.e. is a double tap or greater,

// then this recognizer declares victory to prevent the [LongPressGestureRecognizer]

// from declaring itself the winner if a double tap is held for too long.

resolve(GestureDisposition.accepted);

}

, so when the drag happens the recognizer has already won and will accept the drag in any direction.

I think the more we increase the slop the less responsive the selection gesture will feel. I don't think this is ideal since selection gestures will not always be used in a Scrollable. I explored this method with TextFields when fixing a similar issue and found it to be fragile.

sounds good to me then.

chunhtai

LGTM

chunhtai · 2024-07-16T20:10:03Z

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

+    // A [TapAndHorizontalDragGestureRecognizer] is used on non-precise pointer devices
+    // like PointerDeviceKind.touch so [SelectableRegion] gestures do not conflict with
+    // ancestor Scrollable gestures in common scenarios like a vertically scrolling list view.
+    _gestureRecognizers[TapAndHorizontalDragGestureRecognizer] = GestureRecognizerFactoryWithHandlers<TapAndHorizontalDragGestureRecognizer>(


sounds good to me then.

mark8044 · 2024-07-18T04:52:42Z

Itching to get this onto my app for testing, how does it get past the Google testing?

auto-submit · 2024-07-18T21:17:24Z

auto label is removed for flutter/flutter/151138, due to - The status or check suite Linux build_tests_2_3 has failed. Please fix the issues identified (or deflake) before re-applying this label.

Roll Flutter from 58068d8 to 7d5c1c0 (104 revisions) flutter/flutter@58068d8...7d5c1c0 2024-07-19 [email protected] Enhances intuitiveness of RawMagnifier's example (flutter/flutter#150308) 2024-07-19 [email protected] Roll pub packages (flutter/flutter#151992) 2024-07-19 [email protected] Add test for scrollbar.1.dart (flutter/flutter#151463) 2024-07-19 [email protected] Roll Flutter Engine from ea1e53a4e810 to 969fb7abc449 (3 revisions) (flutter/flutter#152018) 2024-07-19 [email protected] docimports for rendering library (flutter/flutter#151958) 2024-07-19 [email protected] Roll Flutter Engine from b65c93ea948e to ea1e53a4e810 (2 revisions) (flutter/flutter#152012) 2024-07-19 [email protected] painting: drop deprecated (exported) hashList and hashValues functions (flutter/flutter#151677) 2024-07-18 [email protected] Roll Flutter Engine from 766f7bed7185 to b65c93ea948e (2 revisions) (flutter/flutter#152004) 2024-07-18 [email protected] Update TESTOWNERS (flutter/flutter#151907) 2024-07-18 [email protected] chore: fix test name & add description of tests (flutter/flutter#151959) 2024-07-18 [email protected] Roll Flutter Engine from 564ded4c4742 to 766f7bed7185 (2 revisions) (flutter/flutter#151998) 2024-07-18 [email protected] Fix SelectionArea scrolling conflicts (flutter/flutter#151138) 2024-07-18 [email protected] Fix: BaseTapAndDragGestureRecognizer should reset drag state after losing gesture arena (flutter/flutter#151989) 2024-07-18 [email protected] Roll pub packages (flutter/flutter#151975) 2024-07-18 [email protected] Roll Flutter Engine from 8bcf638eb893 to 564ded4c4742 (2 revisions) (flutter/flutter#151986) 2024-07-18 [email protected] Fix WidgetStateTextStyle's doc (flutter/flutter#151935) 2024-07-18 [email protected] Roll Flutter Engine from d58ba74250ce to 8bcf638eb893 (2 revisions) (flutter/flutter#151977) 2024-07-18 [email protected] Adds 3.22.3 changelog (flutter/flutter#151974) 2024-07-18 [email protected] Roll Packages from d03b1b4 to c7f0526 (8 revisions) (flutter/flutter#151971) 2024-07-18 [email protected] `WidgetState` mapping (flutter/flutter#146043) 2024-07-18 [email protected] Fix AppBar doc to keep diagram next to its description (flutter/flutter#151937) 2024-07-18 [email protected] Small fixes to Image docs: NNBD, and add a cross-reference (flutter/flutter#151938) 2024-07-18 [email protected] Roll Flutter Engine from b043fe447bb3 to d58ba74250ce (1 revision) (flutter/flutter#151964) 2024-07-18 [email protected] Roll pub packages (flutter/flutter#151946) 2024-07-18 [email protected] Roll pub packages (flutter/flutter#151904) 2024-07-18 [email protected] Roll Flutter Engine from e3abca2d8105 to b043fe447bb3 (1 revision) (flutter/flutter#151942) 2024-07-18 [email protected] Roll Flutter Engine from 8073523b4623 to e3abca2d8105 (1 revision) (flutter/flutter#151936) 2024-07-18 [email protected] Roll Flutter Engine from dfe22e3acc19 to 8073523b4623 (1 revision) (flutter/flutter#151934) 2024-07-18 [email protected] Roll Flutter Engine from 184c3f0de6b3 to dfe22e3acc19 (1 revision) (flutter/flutter#151930) 2024-07-18 [email protected] Roll Flutter Engine from 00f0f6b74da7 to 184c3f0de6b3 (1 revision) (flutter/flutter#151928) 2024-07-18 [email protected] Roll Flutter Engine from d194a2f0e5da to 00f0f6b74da7 (1 revision) (flutter/flutter#151927) 2024-07-18 [email protected] Roll Flutter Engine from a5a93bb80bd1 to d194a2f0e5da (3 revisions) (flutter/flutter#151925) 2024-07-17 [email protected] Roll Flutter Engine from e9dc62074c2b to a5a93bb80bd1 (1 revision) (flutter/flutter#151918) 2024-07-17 [email protected] [web] use the new backlog Github project in triage links (flutter/flutter#151920) 2024-07-17 [email protected] Update Flutter-Web-Triage.md (flutter/flutter#151607) 2024-07-17 [email protected] Reland fix InputDecorator hint default text style on M3 (flutter/flutter#150835) 2024-07-17 [email protected] Roll Flutter Engine from 39ee1a549581 to e9dc62074c2b (3 revisions) (flutter/flutter#151915) 2024-07-17 [email protected] Constrain `CupertinoContextMenu` animation to safe area (flutter/flutter#151860) 2024-07-17 [email protected] Create `CarouselView` widget - Part 2 (flutter/flutter#149775) 2024-07-17 [email protected] Roll Flutter Engine from 45b722b661f0 to 39ee1a549581 (3 revisions) (flutter/flutter#151905) 2024-07-17 [email protected] docs: Fix typo in data driven fixes test folder section (flutter/flutter#151836) 2024-07-17 [email protected] Stop running flaky mac tests in presubmit (flutter/flutter#151870) 2024-07-17 [email protected] Roll Flutter Engine from 7e2579634027 to 45b722b661f0 (1 revision) (flutter/flutter#151895) 2024-07-17 [email protected] fix(Flutter Web App): fixes html lang typo (flutter/flutter#151866) 2024-07-17 [email protected] Delete `docs/engine` directory (flutter/flutter#151616) 2024-07-17 [email protected] Make `CupertinoSlidingSegmentedControl` type parameter non-null (flutter/flutter#151803) ...

Fixes flutter#150897

Fixes `SelectionArea`/`SelectableRegion` scrolling jank when a `SelectionArea`/`SelectableRegion` is used as a child of a `Scrollable` like `ListView` or `PageView`. Fixes #152420 Cherry-pick of: #151138 Related issue: #150897

Fixes flutter#150897

github-actions bot added the framework flutter/packages/flutter repository. See also f: labels. label Jul 1, 2024

Renzo-Olivares changed the title ~~fix scrolling conflicts~~ Fix SelectionArea scrolling conflicts Jul 1, 2024

Renzo-Olivares requested a review from chunhtai July 1, 2024 21:42

chunhtai reviewed Jul 16, 2024

View reviewed changes

chunhtai approved these changes Jul 16, 2024

View reviewed changes

Renzo Olivares added 4 commits July 18, 2024 14:01

fix scrolling conflicts

3bfbef6

updates

502f13b

fix analyzer

354e91d

really fix analyzer

d22dee3

Renzo-Olivares force-pushed the selection-area-scrolling-fixes branch from 8d731a0 to d22dee3 Compare July 18, 2024 21:01

Renzo-Olivares added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 18, 2024

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jul 18, 2024

Renzo-Olivares added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 18, 2024

auto-submit bot merged commit 93fd294 into flutter:master Jul 18, 2024

engine-flutter-autoroll mentioned this pull request Jul 19, 2024

Manual roll Flutter from 58068d8ea4ec to 73321aa95009 (97 revisions) flutter/packages#7173

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 19, 2024

93fd294 Fix SelectionArea scrolling conflicts (flutter/flutter#151138)

68bd39f

engine-flutter-autoroll mentioned this pull request Jul 19, 2024

Roll Flutter from 58068d8ea4ec to 7d5c1c04fb56 (104 revisions) flutter/packages#7176

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 19, 2024

93fd294 Fix SelectionArea scrolling conflicts (flutter/flutter#151138)

08a4191

engine-flutter-autoroll mentioned this pull request Jul 19, 2024

Roll Flutter from 58068d8ea4ec to 7d5c1c04fb56 (104 revisions) flutter/packages#7177

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 19, 2024

93fd294 Fix SelectionArea scrolling conflicts (flutter/flutter#151138)

4568db3

engine-flutter-autoroll mentioned this pull request Jul 19, 2024

Roll Flutter from 58068d8ea4ec to 49a315284d11 (105 revisions) flutter/packages#7178

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 19, 2024

93fd294 Fix SelectionArea scrolling conflicts (flutter/flutter#151138)

0d64375

engine-flutter-autoroll mentioned this pull request Jul 19, 2024

Roll Flutter from 58068d8ea4ec to 20242d0c4142 (107 revisions) flutter/packages#7179

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 19, 2024

93fd294 Fix SelectionArea scrolling conflicts (flutter/flutter#151138)

b1d1fcf

Piinks mentioned this pull request Jul 26, 2024

Flutter Framework Team: Notable Commits #121415

Closed

Renzo-Olivares added a commit to Renzo-Olivares/flutter that referenced this pull request Jul 27, 2024

Fix SelectionArea scrolling conflicts (flutter#151138)

56f24c1

Fixes flutter#150897

This was referenced Jul 27, 2024

[CP-beta] Fix SelectionArea scrolling conflicts (#151138) #152421

Closed

[CP-beta] Fix SelectionArea scrolling conflicts (#151138) #152423

Merged

darshankawar mentioned this pull request Aug 7, 2024

Selection Area sometimes need to double click on certain widget #153004

Open

TytaniumDev pushed a commit to TytaniumDev/flutter that referenced this pull request Aug 7, 2024

Fix SelectionArea scrolling conflicts (flutter#151138)

8497f6a

Fixes flutter#150897

Buchimi pushed a commit to Buchimi/flutter that referenced this pull request Sep 2, 2024

Fix SelectionArea scrolling conflicts (flutter#151138)

d047b9a

Fixes flutter#150897

	case 1:
	if (details.kind != null && !_isPrecisePointerDevice(details.kind!)) {
	// Drag to select is only enabled with a precise pointer device.
	return;
	}

	if (consecutiveTapCount > 1) {
	// If our consecutive tap count is greater than 1, i.e. is a double tap or greater,
	// then this recognizer declares victory to prevent the [LongPressGestureRecognizer]
	// from declaring itself the winner if a double tap is held for too long.
	resolve(GestureDisposition.accepted);
	}

Fix SelectionArea scrolling conflicts #151138

Fix SelectionArea scrolling conflicts #151138

Uh oh!

Conversation

Renzo-Olivares commented Jul 1, 2024

Pre-launch Checklist

Uh oh!

Renzo-Olivares commented Jul 11, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chunhtai left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mark8044 commented Jul 18, 2024

Uh oh!

auto-submit bot commented Jul 18, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants