Fix `TextField` horizontal drag conflicts #147341

Renzo-Olivares · 2024-04-24T23:10:51Z

Currently on iOS TextField horizontal drag gestures will have precedence over parent horizontal drag gestures when competing with each other because the default touchSlop value used by TextField to calculate whether the threshold for a drag has been met is the same as the one used by parent horizontal drag gestures like PageView, and other Scrollables. The default value is 18.0, and because the TextField receives the PointerEvent first in this scenario, it always declares victory before any parent horizontal drag gestures has the chance to.

Native iOS behavior: The parent horizontal drag gestures will always win unless the drag originated on the cursor (collapsed selection), in that case the TextField cursor drag gestures will win.

This change:

Introduces BaseTapAndDragGestureRecognizer.eagerVictoryOnDrag which can be used to configure the recognizer to declare victory immediately when it detects a drag, or when disabled it will wait until it is the last recognizer in the arena before declaring victory. The default behavior is that it declares victory immediately when the drag is detected.
Eliminates the iOS cursor drag logic from TextSelectionGestureDetector, this logic is now delegated to the selection handle overlay which already had this logic in place.
Enables iOS cursor to always beat other drag gestures by setting the touch slop lower.
Disables eagerVictoryOnDrag for iOS to allow for parent drag gestures to win and match native behavior.

Fixes #124421, Fixes #130198, Fixes #142624, Fixes #142447, Fixes #127017

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.

justinmc

Here's my understanding, tell me if anything is wrong:

For the case where a TextField is inside of a horizontal scrollable on iOS, dragging the selection handle should move the selection handle and not scroll. On other platforms, it should scroll horizontally.

The current behavior is that it always scrolls horizontally. The reason is that the touchSlop on the scrollable is less than the touchSlop on the selection handle. When the drag exceeds the lower touchSlop, that recognizer immediately wins.

What would happen if the touchSlops were identical? By having different touchSlops, are we sidestepping the part of the gesture arena where two competing recognizers decide who receives the gesture?

Are the different touchSlops intentional, like we know that native recognizes these drags after different distances?

I want to understand whether this is a canonical Flutter solution to competing gesture detectors, or if not, make sure that this is a compromise that we're willing to make. If so I'm totally on board, this looks like a pretty straightforward change.

justinmc · 2024-04-25T17:07:09Z

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

+    if (_start != null && !eagerVictoryOnDrag) {
+      assert(_dragState == _DragState.possible);
+      assert(currentUp == null);
+      _dragState = _DragState.accepted;


Why is this not set in the previous if too?

Because in the case of eagerVictoryOnDrag == true, it is set immediately when the drag is detected.

justinmc · 2024-04-25T17:19:15Z

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

@@ -1924,6 +1929,7 @@ class _SelectionHandleOverlayState extends State<_SelectionHandleOverlay> with S
                (PanGestureRecognizer instance) {
                  instance
                    ..dragStartBehavior = widget.dragStartBehavior
+                    ..gestureSettings = eagerlyAcceptDragWhenCollapsed ? const DeviceGestureSettings(touchSlop: 1.0) : null


Why 1.0? Could it be competing-slop-value minus one, or is 1.0 better? (I know you said you're still thinking about cleaning this up.)

I like the idea of competing-slop-value minus one, or maybe minus something higher than 1 to be sure that it always wins.

justinmc · 2024-04-25T17:31:20Z

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

@@ -2839,16 +2823,6 @@ class TextSelectionGestureDetectorBuilder {
            case PointerDeviceKind.invertedStylus:
            case PointerDeviceKind.touch:
            case PointerDeviceKind.unknown:
-              // For iOS platforms, a touch drag does not initiate unless the


Where is this removed logic covered now? I guess the drag listener on the selection handle?

Yeah the drag listener on the selection handle already has this logic by default.

Renzo-Olivares · 2024-04-25T18:48:40Z

For the case where a TextField is inside of a horizontal scrollable on iOS, dragging the selection handle should move the selection handle and not scroll. On other platforms, it should scroll horizontally.

The current behavior is that it always scrolls horizontally. The reason is that the touchSlop on the scrollable is less than the touchSlop on the selection handle. When the drag exceeds the lower touchSlop, that recognizer immediately wins.

On iOS the current behavior is that the TextField will always win horizontal drags. This is because the touchSlop of a Scrollable defaults to 18.0, and the touchSlop that the TextField is using is also 18.0. Because they are identical the first recognizer to receive the drag will win, and that will be the TextField. The desired behavior here is that the parent Scrollable will always win drag gestures, unless the gesture began on the cursor.

On Android the current behavior is that the Scrollable always wins horizontal drags. This is because the touchSlop of a Scrollable on Android provided by the native platform seems to be around 8.0, and the touchSlop on the TextField is 18.0. Because the Scrollable has a lower touch slop it will detect the drag first even if the drag is on the TextField. This is the expected behavior on Android, but if anyone changes their touchSlops i.e. making the Scrollables touchSlop higher or identical to the touchSlop of the TextField (18.0) then they will also experience the unexpected behavior that is being observed on iOS.

The changes in this PR only affect iOS at the moment.

What would happen if the touchSlops were identical? By having different touchSlops, are we sidestepping the part of the gesture arena where two competing recognizers decide who receives the gesture?

Are the different touchSlops intentional, like we know that native recognizes these drags after different distances?

If touchSlops are identical then the recognizer that first receives the PointerEvent will declare victory first. I'm not too sure if different touchSlops are intentional, Android provides getScaledTouchSlop which we utilize to set the default on Android https://developer.android.com/reference/android/view/ViewConfiguration#getScaledTouchSlop() , which is why Android devices aren't experiencing this issue. iOS does not provide such a constant besides some related documentation in the UILongPressGestureRecognizer https://developer.apple.com/documentation/uikit/uilongpressgesturerecognizer/1616427-allowablemovement?language=objc (states 10.0). I think changing the touchSlop is sidestepping the arena, but I think it is reasonable in the scenario that it is being used in this PR (to allow the cursor to always win). An alternative would be to add some type of flag to PanGestureRecognizer so that it can achieve this behavior, but i'm leaning towards overriding the touchSlop versus expanding the API surface.

I want to understand whether this is a canonical Flutter solution to competing gesture detectors, or if not, make sure that this is a compromise that we're willing to make. If so I'm totally on board, this looks like a pretty straightforward change.

Delaying the drag acceptance until all other recognizers have lost is similar to what we do with TapGestureRecognizer so that other recognizers can beat it.

justinmc

LGTM with nits 👍

Thanks for talking this over with me yesterday and for the clear explanation of the changes in the PR description, it saved me some time in understanding everything.

justinmc · 2024-05-01T17:25:44Z

packages/flutter/test/gestures/tap_and_drag_test.dart

+    tester.route(pointer.up());
+    expect(events, <String>[
+      'panstart',
+      'panend']);


Super nit: I would typically do

'panend', ]);

justinmc · 2024-05-01T17:26:46Z

packages/flutter/test/material/text_field_test.dart

+    expect(dismissed, true);
+  },
+    variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.iOS }),
+  );


Nit: Consider also testing the behavior for this on the other platforms (cursor is not dragged).

I'll add these tests in a follow up PR that changes the Android/Fuchsia behavior.

…t can always beat other gestures

Fix leak introduced by #147341

…147341)

Roll Flutter from d33bb8fa5eb8 to bf7191fd3884 (34 revisions) flutter/flutter@d33bb8f...bf7191f 2024-05-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from b989d239e281 to 1fb36ac9d718 (2 revisions) (flutter/flutter#147713) 2024-05-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from 3c9c2ce8369e to b989d239e281 (1 revision) (flutter/flutter#147711) 2024-05-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from 58b031c096ea to 3c9c2ce8369e (2 revisions) (flutter/flutter#147703) 2024-05-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from fc28057dbd4d to 58b031c096ea (1 revision) (flutter/flutter#147701) 2024-05-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from bfc6787eedc3 to fc28057dbd4d (1 revision) (flutter/flutter#147700) 2024-05-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from 7cbef71f4f54 to bfc6787eedc3 (1 revision) (flutter/flutter#147699) 2024-05-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from 90ce9e5959fc to 7cbef71f4f54 (1 revision) (flutter/flutter#147696) 2024-05-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from 78dced50c467 to 90ce9e5959fc (1 revision) (flutter/flutter#147695) 2024-05-02 andrewrkolos@gmail.com add verbose logging to select hot reload/hot restart tests (flutter/flutter#147673) 2024-05-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from 3087ec1adddd to 78dced50c467 (3 revisions) (flutter/flutter#147693) 2024-05-02 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Implement computeDryBaseline for `RenderWrap` (#146260)" (flutter/flutter#147692) 2024-05-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from f56c20c6ac67 to 3087ec1adddd (2 revisions) (flutter/flutter#147688) 2024-05-02 gspencergoog@users.noreply.github.com Allow explicit exclusion of packages from pinned packages in `flutter update-packages --force-update` (flutter/flutter#147679) 2024-05-02 31859944+LongCatIsLooong@users.noreply.github.com Implement getDryBaseline for Stack and Overlay (flutter/flutter#146253) 2024-05-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from 2d73fa207927 to f56c20c6ac67 (2 revisions) (flutter/flutter#147681) 2024-05-02 polinach@google.com Update selectable_text_test.dart (flutter/flutter#147677) 2024-05-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from c536a14052e5 to 2d73fa207927 (2 revisions) (flutter/flutter#147678) 2024-05-01 31859944+LongCatIsLooong@users.noreply.github.com Implement computeDryBaseline for `RenderWrap` (flutter/flutter#146260) 2024-05-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from 842cf254ec58 to c536a14052e5 (1 revision) (flutter/flutter#147675) 2024-05-01 49699333+dependabot[bot]@users.noreply.github.com Bump codecov/codecov-action from 4.3.0 to 4.3.1 (flutter/flutter#147674) 2024-05-01 ian@hixie.ch Remove obsolete performance analysis tools. (flutter/flutter#147663) 2024-05-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from 5129b4919434 to 842cf254ec58 (3 revisions) (flutter/flutter#147670) 2024-05-01 120297255+PurplePolyhedron@users.noreply.github.com fix DropdownMenu overflow (flutter/flutter#147233) 2024-05-01 yjbanov@google.com [web] remove platform_messages_integration test (flutter/flutter#147654) 2024-05-01 rmolivares@renzo-olivares.dev Fix `TextField` horizontal drag conflicts (flutter/flutter#147341) 2024-05-01 nate.w5687@gmail.com `flutter/lib/src/`: refactoring if-chains into switch expressions (flutter/flutter#147472) 2024-05-01 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Draggable feedback positioning (#145647)" (flutter/flutter#147658) 2024-05-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from 0014e0353aa9 to 5129b4919434 (1 revision) (flutter/flutter#147655) 2024-05-01 yjbanov@google.com add lang attribute to the a11y_assessments app (flutter/flutter#147586) 2024-05-01 hi@timcreated.it Draggable feedback positioning (flutter/flutter#145647) 2024-05-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from 0ce67714ce4c to 0014e0353aa9 (13 revisions) (flutter/flutter#147649) 2024-05-01 sokolovskyi.konstantin@gmail.com Add test for animated_fractionally_sized_box.0.dart API example. (flutter/flutter#146721) 2024-05-01 engine-flutter-autoroll@skia.org Roll Packages from cc47b06 to aea93d2 (5 revisions) (flutter/flutter#147647) 2024-05-01 73608287+ellet0@users.noreply.github.com Update reorderable_list.dart to use Dart 3 return switch statement for consistency (flutter/flutter#147505) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC dit@google.com,rmistry@google.com,stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 ...

Renzo-Olivares changed the title ~~[WIP] Fix TextField Drag Conflicts~~ [WIP] Fix TextField drag conflicts Apr 24, 2024

Renzo-Olivares force-pushed the delay-drag branch 2 times, most recently from 08935d9 to d5bd704 Compare April 25, 2024 07:18

justinmc reviewed Apr 25, 2024

View reviewed changes

Renzo-Olivares force-pushed the delay-drag branch from c96862d to 69ec079 Compare April 30, 2024 18:32

Renzo-Olivares changed the title ~~[WIP] Fix TextField drag conflicts~~ Fix TextField horizontal drag conflicts Apr 30, 2024

Renzo-Olivares requested a review from justinmc April 30, 2024 20:59

Renzo-Olivares marked this pull request as ready for review April 30, 2024 21:16

Renzo-Olivares force-pushed the delay-drag branch from ab83353 to 8db991f Compare April 30, 2024 21:26

This was linked to issues Apr 30, 2024

The behavior of Dismissible with TextFormFields is different for Android and iOS #142447

Closed

[iOS] Cannot scroll horizontally if swipe over TextField child #142624

Closed

justinmc approved these changes May 1, 2024

View reviewed changes

Renzo-Olivares added 12 commits May 1, 2024 12:38

iOS should handle cursor drag gestures in SelectionHandleOverlay so i…

1342996

…t can always beat other gestures

Delay drag and remove unused code

e94fa28

Fix trailing whitespace

9bffd82

updates

918b061

Add eagerVictoryOnDrag to BaseTapAndDragGestureRecognizer

5b6f1e8

fix test and analyzer

1fe71f9

fix analyzer

821a809

updates

2f85589

fix broken tests

3b71a60

update docs

d253a1f

add eagerVictoryOnDrag tests

4d363af

Add tests

8895ffa

Address reviewer comments

d49da90

Renzo-Olivares force-pushed the delay-drag branch from 8db991f to d49da90 Compare May 1, 2024 19:38

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

auto-submit bot merged commit 5181086 into flutter:master May 1, 2024
72 checks passed

polina-c mentioned this pull request May 1, 2024

Update selectable_text_test.dart #147677

Merged

auto-submit bot pushed a commit that referenced this pull request May 2, 2024

Update selectable_text_test.dart (#147677)

5eb1807

Fix leak introduced by #147341

Renzo-Olivares mentioned this pull request May 2, 2024

Give the option to disable TapAndHorizontalDragGestureRecognizer and LongPressGestureRecognizer for TextFormFields and TextFields #130224

Open

2 tasks

engine-flutter-autoroll mentioned this pull request May 2, 2024

Roll Flutter from d33bb8fa5eb8 to bf7191fd3884 (34 revisions) flutter/packages#6637

Merged

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

5181086fa Fix TextField horizontal drag conflicts (flutter/flutter#…

cc3df78

…147341)

Piinks mentioned this pull request May 3, 2024

Flutter Framework Team: Notable Commits #121415

Open

polina-c mentioned this pull request May 4, 2024

Fix test. #147813

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix `TextField` horizontal drag conflicts #147341

Fix `TextField` horizontal drag conflicts #147341

Renzo-Olivares commented Apr 24, 2024 •

edited

justinmc left a comment

justinmc Apr 25, 2024

Renzo-Olivares Apr 25, 2024

justinmc Apr 25, 2024

Renzo-Olivares Apr 25, 2024

justinmc Apr 25, 2024

Renzo-Olivares Apr 25, 2024

Renzo-Olivares commented Apr 25, 2024

justinmc left a comment

justinmc May 1, 2024

justinmc May 1, 2024

Renzo-Olivares May 1, 2024

Fix TextField horizontal drag conflicts #147341

Fix TextField horizontal drag conflicts #147341

Conversation

Renzo-Olivares commented Apr 24, 2024 • edited

Pre-launch Checklist

justinmc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Renzo-Olivares commented Apr 25, 2024

justinmc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Fix `TextField` horizontal drag conflicts #147341

Fix `TextField` horizontal drag conflicts #147341

Renzo-Olivares commented Apr 24, 2024 •

edited