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

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

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

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

Super nit: I would typically do

'panend', ]);

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

packages/flutter/test/material/text_field_test.dart

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

…7341)

Roll Flutter from d33bb8f to bf7191f (34 revisions) flutter/flutter@d33bb8f...bf7191f 2024-05-02 [email protected] Roll Flutter Engine from b989d239e281 to 1fb36ac9d718 (2 revisions) (flutter/flutter#147713) 2024-05-02 [email protected] Roll Flutter Engine from 3c9c2ce8369e to b989d239e281 (1 revision) (flutter/flutter#147711) 2024-05-02 [email protected] Roll Flutter Engine from 58b031c096ea to 3c9c2ce8369e (2 revisions) (flutter/flutter#147703) 2024-05-02 [email protected] Roll Flutter Engine from fc28057dbd4d to 58b031c096ea (1 revision) (flutter/flutter#147701) 2024-05-02 [email protected] Roll Flutter Engine from bfc6787eedc3 to fc28057dbd4d (1 revision) (flutter/flutter#147700) 2024-05-02 [email protected] Roll Flutter Engine from 7cbef71f4f54 to bfc6787eedc3 (1 revision) (flutter/flutter#147699) 2024-05-02 [email protected] Roll Flutter Engine from 90ce9e5959fc to 7cbef71f4f54 (1 revision) (flutter/flutter#147696) 2024-05-02 [email protected] Roll Flutter Engine from 78dced50c467 to 90ce9e5959fc (1 revision) (flutter/flutter#147695) 2024-05-02 [email protected] add verbose logging to select hot reload/hot restart tests (flutter/flutter#147673) 2024-05-02 [email protected] 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 [email protected] Roll Flutter Engine from f56c20c6ac67 to 3087ec1adddd (2 revisions) (flutter/flutter#147688) 2024-05-02 [email protected] Allow explicit exclusion of packages from pinned packages in `flutter update-packages --force-update` (flutter/flutter#147679) 2024-05-02 [email protected] Implement getDryBaseline for Stack and Overlay (flutter/flutter#146253) 2024-05-02 [email protected] Roll Flutter Engine from 2d73fa207927 to f56c20c6ac67 (2 revisions) (flutter/flutter#147681) 2024-05-02 [email protected] Update selectable_text_test.dart (flutter/flutter#147677) 2024-05-01 [email protected] Roll Flutter Engine from c536a14052e5 to 2d73fa207927 (2 revisions) (flutter/flutter#147678) 2024-05-01 [email protected] Implement computeDryBaseline for `RenderWrap` (flutter/flutter#146260) 2024-05-01 [email protected] 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 [email protected] Remove obsolete performance analysis tools. (flutter/flutter#147663) 2024-05-01 [email protected] Roll Flutter Engine from 5129b4919434 to 842cf254ec58 (3 revisions) (flutter/flutter#147670) 2024-05-01 [email protected] fix DropdownMenu overflow (flutter/flutter#147233) 2024-05-01 [email protected] [web] remove platform_messages_integration test (flutter/flutter#147654) 2024-05-01 [email protected] Fix `TextField` horizontal drag conflicts (flutter/flutter#147341) 2024-05-01 [email protected] `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 [email protected] Roll Flutter Engine from 0014e0353aa9 to 5129b4919434 (1 revision) (flutter/flutter#147655) 2024-05-01 [email protected] add lang attribute to the a11y_assessments app (flutter/flutter#147586) 2024-05-01 [email protected] Draggable feedback positioning (flutter/flutter#145647) 2024-05-01 [email protected] Roll Flutter Engine from 0ce67714ce4c to 0014e0353aa9 (13 revisions) (flutter/flutter#147649) 2024-05-01 [email protected] Add test for animated_fractionally_sized_box.0.dart API example. (flutter/flutter#146721) 2024-05-01 [email protected] Roll Packages from cc47b06 to aea93d2 (5 revisions) (flutter/flutter#147647) 2024-05-01 [email protected] 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 [email protected],[email protected],[email protected] 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 ...

…7341)

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

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

Closed

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

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

cc3df78

…7341)

Piinks mentioned this pull request May 3, 2024

Flutter Framework Team: Notable Commits #121415

Closed

polina-c mentioned this pull request May 4, 2024

Fix test. #147813

Merged

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

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

fdb6700

…7341)

Fix TextField horizontal drag conflicts #147341

Fix TextField horizontal drag conflicts #147341

Uh oh!

Conversation

Renzo-Olivares commented Apr 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

justinmc left a comment

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!

Renzo-Olivares commented Apr 25, 2024

Uh oh!

justinmc left a comment

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Fix `TextField` horizontal drag conflicts #147341

Fix `TextField` horizontal drag conflicts #147341

Renzo-Olivares commented Apr 24, 2024 •

edited

Loading