Skip to content

Commit

Permalink
Fix TextField horizontal drag conflicts (#147341)
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@Renzo-Olivares
Renzo-Olivares committed May 1, 2024
1 parent 586e1fc commit 5181086
Show file tree
Hide file tree
Showing 8 changed files with 343 additions and 84 deletions.
5 changes: 3 additions & 2 deletions packages/flutter/lib/src/cupertino/text_selection.dart
View file
Expand Up @@ -143,9 +143,10 @@ class CupertinoTextSelectionControls extends TextSelectionControls {
..translate(-desiredSize.width / 2, -desiredSize.height / 2),
child: handle,
);
// iOS doesn't draw anything for collapsed selections.
// iOS should draw an invisible box so the handle can still receive gestures
// on collapsed selections.
case TextSelectionHandleType.collapsed:
return const SizedBox.shrink();
return SizedBox.fromSize(size: getHandleSize(textLineHeight));
}
}

Expand Down
46 changes: 36 additions & 10 deletions packages/flutter/lib/src/gestures/tap_and_drag.dart
View file
Expand Up @@ -652,9 +652,12 @@ mixin _TapStatusTrackerMixin on OneSequenceGestureRecognizer {
/// ### When competing with `TapGestureRecognizer` and `DragGestureRecognizer`
///
/// Similar to [TapGestureRecognizer] and [DragGestureRecognizer],
/// [BaseTapAndDragGestureRecognizer] will not aggressively declare victory when it detects
/// a tap, so when it is competing with those gesture recognizers and others it has a chance
/// of losing.
/// [BaseTapAndDragGestureRecognizer] will not aggressively declare victory when
/// it detects a tap, so when it is competing with those gesture recognizers and
/// others it has a chance of losing. Similarly, when `eagerVictoryOnDrag` is set
/// to `false`, this recognizer will not aggressively declare victory when it
/// detects a drag. By default, `eagerVictoryOnDrag` is set to `true`, so this
/// recognizer will aggressively declare victory when it detects a drag.
///
/// When competing against [TapGestureRecognizer], if the pointer does not move past the tap
/// tolerance, then the recognizer that entered the arena first will win. In this case the
Expand Down Expand Up @@ -748,6 +751,7 @@ sealed class BaseTapAndDragGestureRecognizer extends OneSequenceGestureRecognize
super.debugOwner,
super.supportedDevices,
super.allowedButtonsFilter,
this.eagerVictoryOnDrag = true,
}) : _deadline = kPressTimeout,
dragStartBehavior = DragStartBehavior.start;

Expand Down Expand Up @@ -782,6 +786,15 @@ sealed class BaseTapAndDragGestureRecognizer extends OneSequenceGestureRecognize
@override
int? maxConsecutiveTap;

/// Whether this recognizer eagerly declares victory when it has detected
/// a drag.
///
/// When this value is `false`, this recognizer will wait until it is the last
/// recognizer in the gesture arena before declaring victory on a drag.
///
/// Defaults to `true`.
bool eagerVictoryOnDrag;

/// {@macro flutter.gestures.tap.TapGestureRecognizer.onTapDown}
///
/// This triggers after the down event, once a short timeout ([kPressTimeout]) has
Expand Down Expand Up @@ -984,14 +997,24 @@ sealed class BaseTapAndDragGestureRecognizer extends OneSequenceGestureRecognize

_wonArenaForPrimaryPointer = true;

// resolve(GestureDisposition.accepted) will be called when the [PointerMoveEvent] has
// moved a sufficient global distance.
if (_start != null) {
// resolve(GestureDisposition.accepted) will be called when the [PointerMoveEvent]
// has moved a sufficient global distance to be considered a drag and
// `eagerVictoryOnDrag` is set to `true`.
if (_start != null && eagerVictoryOnDrag) {
assert(_dragState == _DragState.accepted);
assert(currentUp == null);
_acceptDrag(_start!);
}

// This recognizer will wait until it is the last one in the gesture arena
// before accepting a drag when `eagerVictoryOnDrag` is set to `false`.
if (_start != null && !eagerVictoryOnDrag) {
assert(_dragState == _DragState.possible);
assert(currentUp == null);
_dragState = _DragState.accepted;
_acceptDrag(_start!);
}

if (currentUp != null) {
_checkTapUp(currentUp!);
}
Expand Down Expand Up @@ -1076,7 +1099,8 @@ sealed class BaseTapAndDragGestureRecognizer extends OneSequenceGestureRecognize

// This can occur when the recognizer is accepted before a [PointerMoveEvent] has been
// received that moves the pointer a sufficient global distance to be considered a drag.
if (_start != null) {
if (_start != null && _wonArenaForPrimaryPointer) {
_dragState = _DragState.accepted;
_acceptDrag(_start!);
}
}
Expand Down Expand Up @@ -1156,9 +1180,11 @@ sealed class BaseTapAndDragGestureRecognizer extends OneSequenceGestureRecognize
if (_hasSufficientGlobalDistanceToAccept(event.kind)
|| (_wonArenaForPrimaryPointer && _globalDistanceMovedAllAxes.abs() > computePanSlop(event.kind, gestureSettings))) {
_start = event;
_dragState = _DragState.accepted;
if (!_wonArenaForPrimaryPointer) {
resolve(GestureDisposition.accepted);
if (eagerVictoryOnDrag) {
_dragState = _DragState.accepted;
if (!_wonArenaForPrimaryPointer) {
resolve(GestureDisposition.accepted);
}
}
}
}
Expand Down
58 changes: 9 additions & 49 deletions packages/flutter/lib/src/widgets/text_selection.dart
View file
Expand Up @@ -1898,6 +1898,11 @@ class _SelectionHandleOverlayState extends State<_SelectionHandleOverlay> with S
math.max((interactiveRect.height - handleRect.height) / 2, 0),
);

// Make sure a drag is eagerly accepted. This is used on iOS to match the
// behavior where a drag directly on a collapse handle will always win against
// other drag gestures.
final bool eagerlyAcceptDragWhenCollapsed = widget.type == TextSelectionHandleType.collapsed && defaultTargetPlatform == TargetPlatform.iOS;

return CompositedTransformFollower(
link: widget.handleLayerLink,
offset: interactiveRect.topLeft,
Expand All @@ -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
..onStart = widget.onSelectionHandleDragStart
..onUpdate = widget.onSelectionHandleDragUpdate
..onEnd = widget.onSelectionHandleDragEnd;
Expand Down Expand Up @@ -2081,18 +2087,6 @@ class TextSelectionGestureDetectorBuilder {
&& selection.end >= textPosition.offset;
}

/// Returns true if position was on selection.
bool _positionOnSelection(Offset position, TextSelection? targetSelection) {
if (targetSelection == null) {
return false;
}

final TextPosition textPosition = renderEditable.getPositionForPoint(position);

return targetSelection.start <= textPosition.offset
&& targetSelection.end >= textPosition.offset;
}

// Expand the selection to the given global position.
//
// Either base or extent will be moved to the last tapped position, whichever
Expand Down Expand Up @@ -2203,15 +2197,6 @@ class TextSelectionGestureDetectorBuilder {
// inversion of the base and offset happens.
TextSelection? _dragStartSelection;

// For tap + drag gesture on iOS, whether the position where the drag started
// was on the previous TextSelection. iOS uses this value to determine if
// the cursor should move on drag update.
//
// If the drag started on the previous selection then the cursor will move on
// drag update. If the drag did not start on the previous selection then the
// cursor will not move on drag update.
bool? _dragBeganOnPreviousSelection;

// For iOS long press behavior when the field is not focused. iOS uses this value
// to determine if a long press began on a field that was not focused.
//
Expand Down Expand Up @@ -2807,7 +2792,6 @@ class TextSelectionGestureDetectorBuilder {
_dragStartSelection = renderEditable.selection;
_dragStartScrollOffset = _scrollPosition;
_dragStartViewportOffset = renderEditable.offset.pixels;
_dragBeganOnPreviousSelection = _positionOnSelection(details.globalPosition, _dragStartSelection);

if (_TextSelectionGestureDetectorState._getEffectiveConsecutiveTapCount(details.consecutiveTapCount) > 1) {
// Do not set the selection on a consecutive tap and drag.
Expand Down Expand Up @@ -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
// editable has focus and the drag began on the previous selection.
assert(_dragBeganOnPreviousSelection != null);
if (renderEditable.hasFocus && _dragBeganOnPreviousSelection!) {
renderEditable.selectPositionAt(
from: details.globalPosition,
cause: SelectionChangedCause.drag,
);
_showMagnifierIfSupportedByPlatform(details.globalPosition);
}
case null:
}
case TargetPlatform.android:
Expand Down Expand Up @@ -2975,12 +2949,10 @@ class TextSelectionGestureDetectorBuilder {

switch (defaultTargetPlatform) {
case TargetPlatform.iOS:
// With a touch device, nothing should happen, unless there was a double tap, or
// there was a collapsed selection, and the tap/drag position is at the collapsed selection.
// In that case the caret should move with the drag position.
//
// With a mouse device, a drag should select the range from the origin of the drag
// to the current position of the drag.
//
// With a touch device, nothing should happen.
switch (details.kind) {
case PointerDeviceKind.mouse:
case PointerDeviceKind.trackpad:
Expand All @@ -2993,17 +2965,6 @@ class TextSelectionGestureDetectorBuilder {
case PointerDeviceKind.invertedStylus:
case PointerDeviceKind.touch:
case PointerDeviceKind.unknown:
assert(_dragBeganOnPreviousSelection != null);
if (renderEditable.hasFocus
&& _dragStartSelection!.isCollapsed
&& _dragBeganOnPreviousSelection!
) {
renderEditable.selectPositionAt(
from: details.globalPosition,
cause: SelectionChangedCause.drag,
);
return _showMagnifierIfSupportedByPlatform(details.globalPosition);
}
case null:
break;
}
Expand Down Expand Up @@ -3100,8 +3061,6 @@ class TextSelectionGestureDetectorBuilder {
/// callback.
@protected
void onDragSelectionEnd(TapDragEndDetails details) {
_dragBeganOnPreviousSelection = null;

if (_shouldShowSelectionToolbar && _TextSelectionGestureDetectorState._getEffectiveConsecutiveTapCount(details.consecutiveTapCount) == 2) {
editableText.showToolbar();
}
Expand Down Expand Up @@ -3437,6 +3396,7 @@ class _TextSelectionGestureDetectorState extends State<TextSelectionGestureDetec
// Text selection should start from the position of the first pointer
// down event.
..dragStartBehavior = DragStartBehavior.down
..eagerVictoryOnDrag = defaultTargetPlatform != TargetPlatform.iOS
..onTapTrackStart = _handleTapTrackStart
..onTapTrackReset = _handleTapTrackReset
..onTapDown = _handleTapDown
Expand Down
14 changes: 4 additions & 10 deletions packages/flutter/test/cupertino/text_field_test.dart
View file
Expand Up @@ -5923,29 +5923,23 @@ void main() {
// If the position we tap during a drag start is on the collapsed selection, then
// we can move the cursor with a drag.
// Here we tap on '|a', where our selection was previously, and attempt move
// to '|g'. The cursor will not move because the `VerticalDragGestureRecognizer`
// in the scrollable will beat the `TapAndHorizontalDragGestureRecognizer`
// in the TextField. This is because moving from `|a` to `|g` is a completely
// vertical movement.
// to '|g'.
await gesture.down(aPos);
await tester.pump();
await gesture.moveTo(gPos);
await tester.pumpAndSettle();

expect(controller.selection.isCollapsed, true);
expect(controller.selection.baseOffset, 0);
expect(controller.selection.baseOffset, testValue.indexOf('g'));

// Release the pointer.
await gesture.up();
await tester.pumpAndSettle();

// If the position we tap during a drag start is on the collapsed selection, then
// we can move the cursor with a drag.
// Here we tap on '|a', where our selection was previously, and move to '|i'.
// Unlike our previous attempt to drag to `|g`, this works because moving
// to `|i` includes a horizontal movement so the `TapAndHorizontalDragGestureRecognizer`
// in TextField can beat the `VerticalDragGestureRecognizer` in the scrollable.
await gesture.down(aPos);
// Here we tap on '|g', where our selection was previously, and move to '|i'.
await gesture.down(gPos);
await tester.pump();
await gesture.moveTo(iPos);
await tester.pumpAndSettle();
Expand Down
40 changes: 39 additions & 1 deletion packages/flutter/test/gestures/tap_and_drag_test.dart
View file
Expand Up @@ -17,9 +17,12 @@ void main() {
late List<String> events;
late BaseTapAndDragGestureRecognizer tapAndDrag;

void setUpTapAndPanGestureRecognizer() {
void setUpTapAndPanGestureRecognizer({
bool eagerVictoryOnDrag = true, // This is the default for [BaseTapAndDragGestureRecognizer].
}) {
tapAndDrag = TapAndPanGestureRecognizer()
..dragStartBehavior = DragStartBehavior.down
..eagerVictoryOnDrag = eagerVictoryOnDrag
..maxConsecutiveTap = 3
..onTapDown = (TapDragDownDetails details) {
events.add('down#${details.consecutiveTapCount}');
Expand Down Expand Up @@ -653,6 +656,41 @@ void main() {
'panend#1']);
});

testGesture('Recognizer loses when competing against a DragGestureRecognizer for a drag when eagerVictoryOnDrag is disabled', (GestureTester tester) {
setUpTapAndPanGestureRecognizer(eagerVictoryOnDrag: false);
final PanGestureRecognizer pans = PanGestureRecognizer()
..onStart = (DragStartDetails details) {
events.add('panstart');
}
..onUpdate = (DragUpdateDetails details) {
events.add('panupdate');
}
..onEnd = (DragEndDetails details) {
events.add('panend');
}
..onCancel = () {
events.add('pancancel');
};

final TestPointer pointer = TestPointer(5);
final PointerDownEvent downB = pointer.down(const Offset(10.0, 10.0));
// When competing against another [DragGestureRecognizer], the [TapAndPanGestureRecognizer]
// will only win when it is the last recognizer in the arena.
tapAndDrag.addPointer(downB);
pans.addPointer(downB);
tester.closeArena(5);
tester.route(downB);
tester.route(pointer.move(const Offset(40.0, 45.0)));
tester.route(pointer.up());
expect(
events,
<String>[
'panstart',
'panend',
],
);
});

testGesture('Beats LongPressGestureRecognizer on a consecutive tap greater than one', (GestureTester tester) {
setUpTapAndPanGestureRecognizer();

Expand Down

0 comments on commit 5181086

Please sign in to comment.