Skip to content

Commit 5181086

Browse files
Fix TextField horizontal drag conflicts (#147341)
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 `Scrollable`s. 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
1 parent 586e1fc commit 5181086

File tree

8 files changed

+343
-84
lines changed

8 files changed

+343
-84
lines changed

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,9 +143,10 @@ class CupertinoTextSelectionControls extends TextSelectionControls {
143143
..translate(-desiredSize.width / 2, -desiredSize.height / 2),
144144
child: handle,
145145
);
146-
// iOS doesn't draw anything for collapsed selections.
146+
// iOS should draw an invisible box so the handle can still receive gestures
147+
// on collapsed selections.
147148
case TextSelectionHandleType.collapsed:
148-
return const SizedBox.shrink();
149+
return SizedBox.fromSize(size: getHandleSize(textLineHeight));
149150
}
150151
}
151152

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

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -652,9 +652,12 @@ mixin _TapStatusTrackerMixin on OneSequenceGestureRecognizer {
652652
/// ### When competing with `TapGestureRecognizer` and `DragGestureRecognizer`
653653
///
654654
/// Similar to [TapGestureRecognizer] and [DragGestureRecognizer],
655-
/// [BaseTapAndDragGestureRecognizer] will not aggressively declare victory when it detects
656-
/// a tap, so when it is competing with those gesture recognizers and others it has a chance
657-
/// of losing.
655+
/// [BaseTapAndDragGestureRecognizer] will not aggressively declare victory when
656+
/// it detects a tap, so when it is competing with those gesture recognizers and
657+
/// others it has a chance of losing. Similarly, when `eagerVictoryOnDrag` is set
658+
/// to `false`, this recognizer will not aggressively declare victory when it
659+
/// detects a drag. By default, `eagerVictoryOnDrag` is set to `true`, so this
660+
/// recognizer will aggressively declare victory when it detects a drag.
658661
///
659662
/// When competing against [TapGestureRecognizer], if the pointer does not move past the tap
660663
/// tolerance, then the recognizer that entered the arena first will win. In this case the
@@ -748,6 +751,7 @@ sealed class BaseTapAndDragGestureRecognizer extends OneSequenceGestureRecognize
748751
super.debugOwner,
749752
super.supportedDevices,
750753
super.allowedButtonsFilter,
754+
this.eagerVictoryOnDrag = true,
751755
}) : _deadline = kPressTimeout,
752756
dragStartBehavior = DragStartBehavior.start;
753757

@@ -782,6 +786,15 @@ sealed class BaseTapAndDragGestureRecognizer extends OneSequenceGestureRecognize
782786
@override
783787
int? maxConsecutiveTap;
784788

789+
/// Whether this recognizer eagerly declares victory when it has detected
790+
/// a drag.
791+
///
792+
/// When this value is `false`, this recognizer will wait until it is the last
793+
/// recognizer in the gesture arena before declaring victory on a drag.
794+
///
795+
/// Defaults to `true`.
796+
bool eagerVictoryOnDrag;
797+
785798
/// {@macro flutter.gestures.tap.TapGestureRecognizer.onTapDown}
786799
///
787800
/// This triggers after the down event, once a short timeout ([kPressTimeout]) has
@@ -984,14 +997,24 @@ sealed class BaseTapAndDragGestureRecognizer extends OneSequenceGestureRecognize
984997

985998
_wonArenaForPrimaryPointer = true;
986999

987-
// resolve(GestureDisposition.accepted) will be called when the [PointerMoveEvent] has
988-
// moved a sufficient global distance.
989-
if (_start != null) {
1000+
// resolve(GestureDisposition.accepted) will be called when the [PointerMoveEvent]
1001+
// has moved a sufficient global distance to be considered a drag and
1002+
// `eagerVictoryOnDrag` is set to `true`.
1003+
if (_start != null && eagerVictoryOnDrag) {
9901004
assert(_dragState == _DragState.accepted);
9911005
assert(currentUp == null);
9921006
_acceptDrag(_start!);
9931007
}
9941008

1009+
// This recognizer will wait until it is the last one in the gesture arena
1010+
// before accepting a drag when `eagerVictoryOnDrag` is set to `false`.
1011+
if (_start != null && !eagerVictoryOnDrag) {
1012+
assert(_dragState == _DragState.possible);
1013+
assert(currentUp == null);
1014+
_dragState = _DragState.accepted;
1015+
_acceptDrag(_start!);
1016+
}
1017+
9951018
if (currentUp != null) {
9961019
_checkTapUp(currentUp!);
9971020
}
@@ -1076,7 +1099,8 @@ sealed class BaseTapAndDragGestureRecognizer extends OneSequenceGestureRecognize
10761099

10771100
// This can occur when the recognizer is accepted before a [PointerMoveEvent] has been
10781101
// received that moves the pointer a sufficient global distance to be considered a drag.
1079-
if (_start != null) {
1102+
if (_start != null && _wonArenaForPrimaryPointer) {
1103+
_dragState = _DragState.accepted;
10801104
_acceptDrag(_start!);
10811105
}
10821106
}
@@ -1156,9 +1180,11 @@ sealed class BaseTapAndDragGestureRecognizer extends OneSequenceGestureRecognize
11561180
if (_hasSufficientGlobalDistanceToAccept(event.kind)
11571181
|| (_wonArenaForPrimaryPointer && _globalDistanceMovedAllAxes.abs() > computePanSlop(event.kind, gestureSettings))) {
11581182
_start = event;
1159-
_dragState = _DragState.accepted;
1160-
if (!_wonArenaForPrimaryPointer) {
1161-
resolve(GestureDisposition.accepted);
1183+
if (eagerVictoryOnDrag) {
1184+
_dragState = _DragState.accepted;
1185+
if (!_wonArenaForPrimaryPointer) {
1186+
resolve(GestureDisposition.accepted);
1187+
}
11621188
}
11631189
}
11641190
}

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

Lines changed: 9 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1898,6 +1898,11 @@ class _SelectionHandleOverlayState extends State<_SelectionHandleOverlay> with S
18981898
math.max((interactiveRect.height - handleRect.height) / 2, 0),
18991899
);
19001900

1901+
// Make sure a drag is eagerly accepted. This is used on iOS to match the
1902+
// behavior where a drag directly on a collapse handle will always win against
1903+
// other drag gestures.
1904+
final bool eagerlyAcceptDragWhenCollapsed = widget.type == TextSelectionHandleType.collapsed && defaultTargetPlatform == TargetPlatform.iOS;
1905+
19011906
return CompositedTransformFollower(
19021907
link: widget.handleLayerLink,
19031908
offset: interactiveRect.topLeft,
@@ -1924,6 +1929,7 @@ class _SelectionHandleOverlayState extends State<_SelectionHandleOverlay> with S
19241929
(PanGestureRecognizer instance) {
19251930
instance
19261931
..dragStartBehavior = widget.dragStartBehavior
1932+
..gestureSettings = eagerlyAcceptDragWhenCollapsed ? const DeviceGestureSettings(touchSlop: 1.0) : null
19271933
..onStart = widget.onSelectionHandleDragStart
19281934
..onUpdate = widget.onSelectionHandleDragUpdate
19291935
..onEnd = widget.onSelectionHandleDragEnd;
@@ -2081,18 +2087,6 @@ class TextSelectionGestureDetectorBuilder {
20812087
&& selection.end >= textPosition.offset;
20822088
}
20832089

2084-
/// Returns true if position was on selection.
2085-
bool _positionOnSelection(Offset position, TextSelection? targetSelection) {
2086-
if (targetSelection == null) {
2087-
return false;
2088-
}
2089-
2090-
final TextPosition textPosition = renderEditable.getPositionForPoint(position);
2091-
2092-
return targetSelection.start <= textPosition.offset
2093-
&& targetSelection.end >= textPosition.offset;
2094-
}
2095-
20962090
// Expand the selection to the given global position.
20972091
//
20982092
// Either base or extent will be moved to the last tapped position, whichever
@@ -2203,15 +2197,6 @@ class TextSelectionGestureDetectorBuilder {
22032197
// inversion of the base and offset happens.
22042198
TextSelection? _dragStartSelection;
22052199

2206-
// For tap + drag gesture on iOS, whether the position where the drag started
2207-
// was on the previous TextSelection. iOS uses this value to determine if
2208-
// the cursor should move on drag update.
2209-
//
2210-
// If the drag started on the previous selection then the cursor will move on
2211-
// drag update. If the drag did not start on the previous selection then the
2212-
// cursor will not move on drag update.
2213-
bool? _dragBeganOnPreviousSelection;
2214-
22152200
// For iOS long press behavior when the field is not focused. iOS uses this value
22162201
// to determine if a long press began on a field that was not focused.
22172202
//
@@ -2807,7 +2792,6 @@ class TextSelectionGestureDetectorBuilder {
28072792
_dragStartSelection = renderEditable.selection;
28082793
_dragStartScrollOffset = _scrollPosition;
28092794
_dragStartViewportOffset = renderEditable.offset.pixels;
2810-
_dragBeganOnPreviousSelection = _positionOnSelection(details.globalPosition, _dragStartSelection);
28112795

28122796
if (_TextSelectionGestureDetectorState._getEffectiveConsecutiveTapCount(details.consecutiveTapCount) > 1) {
28132797
// Do not set the selection on a consecutive tap and drag.
@@ -2839,16 +2823,6 @@ class TextSelectionGestureDetectorBuilder {
28392823
case PointerDeviceKind.invertedStylus:
28402824
case PointerDeviceKind.touch:
28412825
case PointerDeviceKind.unknown:
2842-
// For iOS platforms, a touch drag does not initiate unless the
2843-
// editable has focus and the drag began on the previous selection.
2844-
assert(_dragBeganOnPreviousSelection != null);
2845-
if (renderEditable.hasFocus && _dragBeganOnPreviousSelection!) {
2846-
renderEditable.selectPositionAt(
2847-
from: details.globalPosition,
2848-
cause: SelectionChangedCause.drag,
2849-
);
2850-
_showMagnifierIfSupportedByPlatform(details.globalPosition);
2851-
}
28522826
case null:
28532827
}
28542828
case TargetPlatform.android:
@@ -2975,12 +2949,10 @@ class TextSelectionGestureDetectorBuilder {
29752949

29762950
switch (defaultTargetPlatform) {
29772951
case TargetPlatform.iOS:
2978-
// With a touch device, nothing should happen, unless there was a double tap, or
2979-
// there was a collapsed selection, and the tap/drag position is at the collapsed selection.
2980-
// In that case the caret should move with the drag position.
2981-
//
29822952
// With a mouse device, a drag should select the range from the origin of the drag
29832953
// to the current position of the drag.
2954+
//
2955+
// With a touch device, nothing should happen.
29842956
switch (details.kind) {
29852957
case PointerDeviceKind.mouse:
29862958
case PointerDeviceKind.trackpad:
@@ -2993,17 +2965,6 @@ class TextSelectionGestureDetectorBuilder {
29932965
case PointerDeviceKind.invertedStylus:
29942966
case PointerDeviceKind.touch:
29952967
case PointerDeviceKind.unknown:
2996-
assert(_dragBeganOnPreviousSelection != null);
2997-
if (renderEditable.hasFocus
2998-
&& _dragStartSelection!.isCollapsed
2999-
&& _dragBeganOnPreviousSelection!
3000-
) {
3001-
renderEditable.selectPositionAt(
3002-
from: details.globalPosition,
3003-
cause: SelectionChangedCause.drag,
3004-
);
3005-
return _showMagnifierIfSupportedByPlatform(details.globalPosition);
3006-
}
30072968
case null:
30082969
break;
30092970
}
@@ -3100,8 +3061,6 @@ class TextSelectionGestureDetectorBuilder {
31003061
/// callback.
31013062
@protected
31023063
void onDragSelectionEnd(TapDragEndDetails details) {
3103-
_dragBeganOnPreviousSelection = null;
3104-
31053064
if (_shouldShowSelectionToolbar && _TextSelectionGestureDetectorState._getEffectiveConsecutiveTapCount(details.consecutiveTapCount) == 2) {
31063065
editableText.showToolbar();
31073066
}
@@ -3437,6 +3396,7 @@ class _TextSelectionGestureDetectorState extends State<TextSelectionGestureDetec
34373396
// Text selection should start from the position of the first pointer
34383397
// down event.
34393398
..dragStartBehavior = DragStartBehavior.down
3399+
..eagerVictoryOnDrag = defaultTargetPlatform != TargetPlatform.iOS
34403400
..onTapTrackStart = _handleTapTrackStart
34413401
..onTapTrackReset = _handleTapTrackReset
34423402
..onTapDown = _handleTapDown

packages/flutter/test/cupertino/text_field_test.dart

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5923,29 +5923,23 @@ void main() {
59235923
// If the position we tap during a drag start is on the collapsed selection, then
59245924
// we can move the cursor with a drag.
59255925
// Here we tap on '|a', where our selection was previously, and attempt move
5926-
// to '|g'. The cursor will not move because the `VerticalDragGestureRecognizer`
5927-
// in the scrollable will beat the `TapAndHorizontalDragGestureRecognizer`
5928-
// in the TextField. This is because moving from `|a` to `|g` is a completely
5929-
// vertical movement.
5926+
// to '|g'.
59305927
await gesture.down(aPos);
59315928
await tester.pump();
59325929
await gesture.moveTo(gPos);
59335930
await tester.pumpAndSettle();
59345931

59355932
expect(controller.selection.isCollapsed, true);
5936-
expect(controller.selection.baseOffset, 0);
5933+
expect(controller.selection.baseOffset, testValue.indexOf('g'));
59375934

59385935
// Release the pointer.
59395936
await gesture.up();
59405937
await tester.pumpAndSettle();
59415938

59425939
// If the position we tap during a drag start is on the collapsed selection, then
59435940
// we can move the cursor with a drag.
5944-
// Here we tap on '|a', where our selection was previously, and move to '|i'.
5945-
// Unlike our previous attempt to drag to `|g`, this works because moving
5946-
// to `|i` includes a horizontal movement so the `TapAndHorizontalDragGestureRecognizer`
5947-
// in TextField can beat the `VerticalDragGestureRecognizer` in the scrollable.
5948-
await gesture.down(aPos);
5941+
// Here we tap on '|g', where our selection was previously, and move to '|i'.
5942+
await gesture.down(gPos);
59495943
await tester.pump();
59505944
await gesture.moveTo(iPos);
59515945
await tester.pumpAndSettle();

packages/flutter/test/gestures/tap_and_drag_test.dart

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,12 @@ void main() {
1717
late List<String> events;
1818
late BaseTapAndDragGestureRecognizer tapAndDrag;
1919

20-
void setUpTapAndPanGestureRecognizer() {
20+
void setUpTapAndPanGestureRecognizer({
21+
bool eagerVictoryOnDrag = true, // This is the default for [BaseTapAndDragGestureRecognizer].
22+
}) {
2123
tapAndDrag = TapAndPanGestureRecognizer()
2224
..dragStartBehavior = DragStartBehavior.down
25+
..eagerVictoryOnDrag = eagerVictoryOnDrag
2326
..maxConsecutiveTap = 3
2427
..onTapDown = (TapDragDownDetails details) {
2528
events.add('down#${details.consecutiveTapCount}');
@@ -653,6 +656,41 @@ void main() {
653656
'panend#1']);
654657
});
655658

659+
testGesture('Recognizer loses when competing against a DragGestureRecognizer for a drag when eagerVictoryOnDrag is disabled', (GestureTester tester) {
660+
setUpTapAndPanGestureRecognizer(eagerVictoryOnDrag: false);
661+
final PanGestureRecognizer pans = PanGestureRecognizer()
662+
..onStart = (DragStartDetails details) {
663+
events.add('panstart');
664+
}
665+
..onUpdate = (DragUpdateDetails details) {
666+
events.add('panupdate');
667+
}
668+
..onEnd = (DragEndDetails details) {
669+
events.add('panend');
670+
}
671+
..onCancel = () {
672+
events.add('pancancel');
673+
};
674+
675+
final TestPointer pointer = TestPointer(5);
676+
final PointerDownEvent downB = pointer.down(const Offset(10.0, 10.0));
677+
// When competing against another [DragGestureRecognizer], the [TapAndPanGestureRecognizer]
678+
// will only win when it is the last recognizer in the arena.
679+
tapAndDrag.addPointer(downB);
680+
pans.addPointer(downB);
681+
tester.closeArena(5);
682+
tester.route(downB);
683+
tester.route(pointer.move(const Offset(40.0, 45.0)));
684+
tester.route(pointer.up());
685+
expect(
686+
events,
687+
<String>[
688+
'panstart',
689+
'panend',
690+
],
691+
);
692+
});
693+
656694
testGesture('Beats LongPressGestureRecognizer on a consecutive tap greater than one', (GestureTester tester) {
657695
setUpTapAndPanGestureRecognizer();
658696

0 commit comments

Comments
 (0)