Unverified Commit eba38c4b authored by Renzo Olivares's avatar Renzo Olivares Committed by GitHub

Fix text selection edge scrolling when inside a horizontal scrollable (#140250)

Fixes #129590

* Consider `AxisDirection` when calculating scroll offset used in determining TextSelection during a drag/long press drag. Previously it seems that we were assuming the direction was always vertical https://github.com/flutter/flutter/blob/30cc83198544582b858e48c7bb9d761ecdb3d944/packages/flutter/lib/src/widgets/text_selection.dart#L2842-L2844 .
* SelectableText now considers RenderEditable offset changes and Scrollable offset changes when calculating the TextSelection during a long press drag.
parent d83eff44
...@@ -59,6 +59,31 @@ class _SelectableTextSelectionGestureDetectorBuilder extends TextSelectionGestur ...@@ -59,6 +59,31 @@ class _SelectableTextSelectionGestureDetectorBuilder extends TextSelectionGestur
final _SelectableTextState _state; final _SelectableTextState _state;
/// The viewport offset pixels of any [Scrollable] containing the
/// [RenderEditable] at the last drag start.
double _dragStartScrollOffset = 0.0;
/// The viewport offset pixels of the [RenderEditable] at the last drag start.
double _dragStartViewportOffset = 0.0;
double get _scrollPosition {
final ScrollableState? scrollableState =
delegate.editableTextKey.currentContext == null
? null
: Scrollable.maybeOf(delegate.editableTextKey.currentContext!);
return scrollableState == null
? 0.0
: scrollableState.position.pixels;
}
AxisDirection? get _scrollDirection {
final ScrollableState? scrollableState =
delegate.editableTextKey.currentContext == null
? null
: Scrollable.maybeOf(delegate.editableTextKey.currentContext!);
return scrollableState?.axisDirection;
}
@override @override
void onForcePressStart(ForcePressDetails details) { void onForcePressStart(ForcePressDetails details) {
super.onForcePressStart(details); super.onForcePressStart(details);
...@@ -72,15 +97,37 @@ class _SelectableTextSelectionGestureDetectorBuilder extends TextSelectionGestur ...@@ -72,15 +97,37 @@ class _SelectableTextSelectionGestureDetectorBuilder extends TextSelectionGestur
// Not required. // Not required.
} }
@override
void onSingleLongTapStart(LongPressStartDetails details) {
if (!delegate.selectionEnabled) {
return;
}
renderEditable.selectWord(cause: SelectionChangedCause.longPress);
Feedback.forLongPress(_state.context);
_dragStartViewportOffset = renderEditable.offset.pixels;
_dragStartScrollOffset = _scrollPosition;
}
@override @override
void onSingleLongTapMoveUpdate(LongPressMoveUpdateDetails details) { void onSingleLongTapMoveUpdate(LongPressMoveUpdateDetails details) {
if (delegate.selectionEnabled) { if (!delegate.selectionEnabled) {
renderEditable.selectWordsInRange( return;
from: details.globalPosition - details.offsetFromOrigin,
to: details.globalPosition,
cause: SelectionChangedCause.longPress,
);
} }
// Adjust the drag start offset for possible viewport offset changes.
final Offset editableOffset = renderEditable.maxLines == 1
? Offset(renderEditable.offset.pixels - _dragStartViewportOffset, 0.0)
: Offset(0.0, renderEditable.offset.pixels - _dragStartViewportOffset);
final double effectiveScrollPosition = _scrollPosition - _dragStartScrollOffset;
final bool scrollingOnVerticalAxis = _scrollDirection == AxisDirection.up || _scrollDirection == AxisDirection.down;
final Offset scrollableOffset = Offset(
!scrollingOnVerticalAxis ? effectiveScrollPosition : 0.0,
scrollingOnVerticalAxis ? effectiveScrollPosition : 0.0,
);
renderEditable.selectWordsInRange(
from: details.globalPosition - details.offsetFromOrigin - editableOffset - scrollableOffset,
to: details.globalPosition,
cause: SelectionChangedCause.longPress,
);
} }
@override @override
...@@ -100,14 +147,6 @@ class _SelectableTextSelectionGestureDetectorBuilder extends TextSelectionGestur ...@@ -100,14 +147,6 @@ class _SelectableTextSelectionGestureDetectorBuilder extends TextSelectionGestur
} }
_state.widget.onTap?.call(); _state.widget.onTap?.call();
} }
@override
void onSingleLongTapStart(LongPressStartDetails details) {
if (delegate.selectionEnabled) {
renderEditable.selectWord(cause: SelectionChangedCause.longPress);
Feedback.forLongPress(_state.context);
}
}
} }
/// A run of selectable text with a single style. /// A run of selectable text with a single style.
......
...@@ -2131,6 +2131,14 @@ class TextSelectionGestureDetectorBuilder { ...@@ -2131,6 +2131,14 @@ class TextSelectionGestureDetectorBuilder {
: scrollableState.position.pixels; : scrollableState.position.pixels;
} }
AxisDirection? get _scrollDirection {
final ScrollableState? scrollableState =
delegate.editableTextKey.currentContext == null
? null
: Scrollable.maybeOf(delegate.editableTextKey.currentContext!);
return scrollableState?.axisDirection;
}
// For a shift + tap + drag gesture, the TextSelection at the point of the // For a shift + tap + drag gesture, the TextSelection at the point of the
// tap. Mac uses this value to reset to the original selection when an // tap. Mac uses this value to reset to the original selection when an
// inversion of the base and offset happens. // inversion of the base and offset happens.
...@@ -2498,9 +2506,11 @@ class TextSelectionGestureDetectorBuilder { ...@@ -2498,9 +2506,11 @@ class TextSelectionGestureDetectorBuilder {
final Offset editableOffset = renderEditable.maxLines == 1 final Offset editableOffset = renderEditable.maxLines == 1
? Offset(renderEditable.offset.pixels - _dragStartViewportOffset, 0.0) ? Offset(renderEditable.offset.pixels - _dragStartViewportOffset, 0.0)
: Offset(0.0, renderEditable.offset.pixels - _dragStartViewportOffset); : Offset(0.0, renderEditable.offset.pixels - _dragStartViewportOffset);
final double effectiveScrollPosition = _scrollPosition - _dragStartScrollOffset;
final bool scrollingOnVerticalAxis = _scrollDirection == AxisDirection.up || _scrollDirection == AxisDirection.down;
final Offset scrollableOffset = Offset( final Offset scrollableOffset = Offset(
0.0, !scrollingOnVerticalAxis ? effectiveScrollPosition : 0.0,
_scrollPosition - _dragStartScrollOffset, scrollingOnVerticalAxis ? effectiveScrollPosition : 0.0,
); );
switch (defaultTargetPlatform) { switch (defaultTargetPlatform) {
case TargetPlatform.iOS: case TargetPlatform.iOS:
...@@ -2839,9 +2849,11 @@ class TextSelectionGestureDetectorBuilder { ...@@ -2839,9 +2849,11 @@ class TextSelectionGestureDetectorBuilder {
final Offset editableOffset = renderEditable.maxLines == 1 final Offset editableOffset = renderEditable.maxLines == 1
? Offset(renderEditable.offset.pixels - _dragStartViewportOffset, 0.0) ? Offset(renderEditable.offset.pixels - _dragStartViewportOffset, 0.0)
: Offset(0.0, renderEditable.offset.pixels - _dragStartViewportOffset); : Offset(0.0, renderEditable.offset.pixels - _dragStartViewportOffset);
final double effectiveScrollPosition = _scrollPosition - _dragStartScrollOffset;
final bool scrollingOnVerticalAxis = _scrollDirection == AxisDirection.up || _scrollDirection == AxisDirection.down;
final Offset scrollableOffset = Offset( final Offset scrollableOffset = Offset(
0.0, !scrollingOnVerticalAxis ? effectiveScrollPosition : 0.0,
_scrollPosition - _dragStartScrollOffset, scrollingOnVerticalAxis ? effectiveScrollPosition : 0.0,
); );
final Offset dragStartGlobalPosition = details.globalPosition - details.offsetFromOrigin; final Offset dragStartGlobalPosition = details.globalPosition - details.offsetFromOrigin;
......
...@@ -3689,36 +3689,208 @@ void main() { ...@@ -3689,36 +3689,208 @@ void main() {
variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.macOS }), variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.macOS }),
); );
testWidgets('long press drag can edge scroll', (WidgetTester tester) async { testWidgets('long press drag can edge scroll when inside a scrollable', (WidgetTester tester) async {
// This is a regression test for https://github.com/flutter/flutter/issues/129590.
await tester.pumpWidget( await tester.pumpWidget(
const MaterialApp( MaterialApp(
home: Material( home: Material(
child: Center( child: Center(
child: SelectableText( child: SizedBox(
'Atwater Peel Sherbrooke Bonaventure Angrignon Peel Côte-des-Neiges', width: 300.0,
maxLines: 1, child: SingleChildScrollView(
scrollDirection: Axis.horizontal,
child: SelectableText(
'Atwater Peel Sherbrooke Bonaventure Angrignon Peel Côte-des-Neiges ' * 2,
maxLines: 1,
),
),
), ),
), ),
), ),
), ),
); );
final Offset selectableTextStart = tester.getTopLeft(find.byType(SelectableText));
final TestGesture gesture =
await tester.startGesture(selectableTextStart + const Offset(200.0, 0.0));
await tester.pump(kLongPressTimeout);
final EditableText editableTextWidget = tester.widget(find.byType(EditableText).first);
final TextEditingController controller = editableTextWidget.controller;
expect(
controller.selection,
const TextSelection(baseOffset: 13, extentOffset: 23),
);
await gesture.moveBy(const Offset(100, 0));
// To the edge of the screen basically.
await tester.pump();
expect(
controller.selection,
const TextSelection(
baseOffset: 13,
extentOffset: 23,
),
);
// Keep moving out.
await gesture.moveBy(const Offset(100, 0));
await tester.pump();
expect(
controller.selection,
const TextSelection(
baseOffset: 13,
extentOffset: 35,
),
);
await gesture.moveBy(const Offset(1600, 0));
await tester.pump();
expect(
controller.selection,
const TextSelection(
baseOffset: 13,
extentOffset: 134,
),
);
await gesture.up();
await tester.pumpAndSettle();
// The selection isn't affected by the gesture lift.
expect(
controller.selection,
const TextSelection(
baseOffset: 13,
extentOffset: 134,
),
);
// The toolbar shows up.
if (defaultTargetPlatform == TargetPlatform.iOS) {
expectCupertinoSelectionToolbar();
} else {
expectMaterialSelectionToolbar();
}
final RenderEditable renderEditable = findRenderEditable(tester); final RenderEditable renderEditable = findRenderEditable(tester);
final List<TextSelectionPoint> endpoints = globalize(
renderEditable.getEndpointsForSelection(controller.selection),
renderEditable,
);
expect(endpoints.isNotEmpty, isTrue);
expect(endpoints.length, 2);
expect(endpoints[0].point.dx, isNegative);
expect(endpoints[1].point.dx, isPositive);
},
// TODO(Renzo-Olivares): Add in TargetPlatform.android in the line below when
// we fix edge scrolling in a Scrollable https://github.com/flutter/flutter/issues/64059.
variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.iOS }),
);
List<TextSelectionPoint> lastCharEndpoint = renderEditable.getEndpointsForSelection( testWidgets('Desktop mouse drag can edge scroll when inside a horizontal scrollable', (WidgetTester tester) async {
const TextSelection.collapsed(offset: 66), // Last character's position. // This is a regression test for https://github.com/flutter/flutter/issues/129590.
await tester.pumpWidget(
MaterialApp(
home: Material(
child: Center(
child: SizedBox(
width: 300.0,
child: SingleChildScrollView(
scrollDirection: Axis.horizontal,
child: SelectableText(
'Atwater Peel Sherbrooke Bonaventure Angrignon Peel Côte-des-Neiges ' * 2,
maxLines: 1,
),
),
),
),
),
),
); );
expect(lastCharEndpoint.length, 1); final Offset selectableTextStart = tester.getTopLeft(find.byType(SelectableText));
// Just testing the test and making sure that the last character is off
// the right side of the screen. final TestGesture gesture =
expect(lastCharEndpoint[0].point.dx, 924.0); await tester.startGesture(selectableTextStart + const Offset(200.0, 0.0));
await tester.pump();
final EditableText editableTextWidget = tester.widget(find.byType(EditableText).first);
final TextEditingController controller = editableTextWidget.controller;
await gesture.moveBy(const Offset(100, 0));
// To the edge of the screen basically.
await tester.pump();
expect(
controller.selection,
const TextSelection(
baseOffset: 14,
extentOffset: 21,
),
);
// Keep moving out.
await gesture.moveBy(const Offset(100, 0));
await tester.pump();
expect(
controller.selection,
const TextSelection(
baseOffset: 14,
extentOffset: 28,
),
);
await gesture.moveBy(const Offset(1600, 0));
await tester.pump();
expect(
controller.selection,
const TextSelection(
baseOffset: 14,
extentOffset: 134,
),
);
await gesture.up();
await tester.pumpAndSettle();
// The selection isn't affected by the gesture lift.
expect(
controller.selection,
const TextSelection(
baseOffset: 14,
extentOffset: 134,
),
);
final RenderEditable renderEditable = findRenderEditable(tester);
final List<TextSelectionPoint> endpoints = globalize(
renderEditable.getEndpointsForSelection(controller.selection),
renderEditable,
);
expect(endpoints.isNotEmpty, isTrue);
expect(endpoints.length, 2);
expect(endpoints[0].point.dx, isNegative);
expect(endpoints[1].point.dx, isPositive);
},
variant: TargetPlatformVariant.desktop(),
);
testWidgets('long press drag can edge scroll', (WidgetTester tester) async {
await tester.pumpWidget(
MaterialApp(
home: Material(
child: Center(
child: SelectableText(
'Atwater Peel Sherbrooke Bonaventure Angrignon Peel Côte-des-Neiges ' * 2,
maxLines: 1,
),
),
),
),
);
final Offset selectableTextStart = tester.getTopLeft(find.byType(SelectableText)); final Offset selectableTextStart = tester.getTopLeft(find.byType(SelectableText));
final TestGesture gesture = final TestGesture gesture =
await tester.startGesture(selectableTextStart + const Offset(300, 5)); await tester.startGesture(selectableTextStart + const Offset(300, 5));
await tester.pump(const Duration(milliseconds: 500)); await tester.pump(kLongPressTimeout);
final EditableText editableTextWidget = tester.widget(find.byType(EditableText).first); final EditableText editableTextWidget = tester.widget(find.byType(EditableText).first);
final TextEditingController controller = editableTextWidget.controller; final TextEditingController controller = editableTextWidget.controller;
...@@ -3727,20 +3899,19 @@ void main() { ...@@ -3727,20 +3899,19 @@ void main() {
controller.selection, controller.selection,
const TextSelection(baseOffset: 13, extentOffset: 23), const TextSelection(baseOffset: 13, extentOffset: 23),
); );
expect(find.byType(CupertinoButton), findsNothing);
await gesture.moveBy(const Offset(600, 0)); await gesture.moveBy(const Offset(300, 0));
// To the edge of the screen basically. // To the edge of the screen basically.
await tester.pump(); await tester.pump();
expect( expect(
controller.selection, controller.selection,
const TextSelection( const TextSelection(
baseOffset: 13, baseOffset: 13,
extentOffset: 66, extentOffset: 45,
), ),
); );
// Keep moving out. // Keep moving out.
await gesture.moveBy(const Offset(1, 0)); await gesture.moveBy(const Offset(300, 0));
await tester.pump(); await tester.pump();
expect( expect(
controller.selection, controller.selection,
...@@ -3749,48 +3920,60 @@ void main() { ...@@ -3749,48 +3920,60 @@ void main() {
extentOffset: 66, extentOffset: 66,
), ),
); );
await gesture.moveBy(const Offset(1, 0)); await gesture.moveBy(const Offset(400, 0));
await tester.pump(); await tester.pump();
expect( expect(
controller.selection, controller.selection,
const TextSelection( const TextSelection(
baseOffset: 13, baseOffset: 13,
extentOffset: 66, extentOffset: 102,
), ),
); );
expect(find.byType(CupertinoButton), findsNothing);
await gesture.up(); await gesture.moveBy(const Offset(700, 0));
await tester.pump(); await tester.pump();
expect(
controller.selection,
const TextSelection(
baseOffset: 13,
extentOffset: 134,
),
);
await gesture.up();
await tester.pumpAndSettle();
// The selection isn't affected by the gesture lift. // The selection isn't affected by the gesture lift.
expect( expect(
controller.selection, controller.selection,
const TextSelection( const TextSelection(
baseOffset: 13, baseOffset: 13,
extentOffset: 66, extentOffset: 134,
), ),
); );
// The toolbar shows up with one button (copy). // The toolbar shows up.
expect(find.byType(CupertinoButton), findsNWidgets(1)); if (defaultTargetPlatform == TargetPlatform.iOS) {
expectCupertinoSelectionToolbar();
} else {
expectMaterialSelectionToolbar();
}
lastCharEndpoint = renderEditable.getEndpointsForSelection( // Find the selection handle fade transition after the start handle has been
const TextSelection.collapsed(offset: 66), // Last character's position. // hidden because it is out of view.
); final List<FadeTransition> transitionsAfter = find.descendant(
of: find.byWidgetPredicate((Widget w) => '${w.runtimeType}' == '_SelectionHandleOverlay'),
matching: find.byType(FadeTransition),
).evaluate().map((Element e) => e.widget).cast<FadeTransition>().toList();
expect(lastCharEndpoint.length, 1); expect(transitionsAfter.length, 2);
// The last character is now on screen near the right edge.
expect(lastCharEndpoint[0].point.dx, moreOrLessEquals(798, epsilon: 1));
final List<TextSelectionPoint> firstCharEndpoint = renderEditable.getEndpointsForSelection( final FadeTransition startHandleAfter = transitionsAfter[0];
const TextSelection.collapsed(offset: 0), // First character's position. final FadeTransition endHandleAfter = transitionsAfter[1];
);
expect(firstCharEndpoint.length, 1); expect(startHandleAfter.opacity.value, 0.0);
// The first character is now offscreen to the left. expect(endHandleAfter.opacity.value, 1.0);
expect(firstCharEndpoint[0].point.dx, moreOrLessEquals(-125, epsilon: 1));
}, },
variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.iOS, TargetPlatform.macOS }), variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.iOS, TargetPlatform.android }),
skip: true, // https://github.com/flutter/flutter/issues/64059
); );
testWidgets( testWidgets(
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment