From 909e29edc587392ec533e98ec85869125ff982d7 Mon Sep 17 00:00:00 2001 From: takashi kasai <52235899+takassh@users.noreply.github.com> Date: Thu, 2 Mar 2023 04:52:27 +0900 Subject: [PATCH] moving the left handle automatically scrolls EditableText to the right handle, which doesn't happen on native (#105836) Fixes a scrolling issue when dragging selection handles. --- .../flutter/lib/src/cupertino/text_field.dart | 3 +- .../flutter/lib/src/material/text_field.dart | 3 +- .../lib/src/widgets/editable_text.dart | 27 +++ .../lib/src/widgets/text_selection.dart | 12 +- .../test/cupertino/text_field_test.dart | 157 +++++++++++++++++ .../test/material/text_field_test.dart | 165 ++++++++++++++++++ 6 files changed, 356 insertions(+), 11 deletions(-) diff --git a/packages/flutter/lib/src/cupertino/text_field.dart b/packages/flutter/lib/src/cupertino/text_field.dart index 8215d38953..f3eb800243 100644 --- a/packages/flutter/lib/src/cupertino/text_field.dart +++ b/packages/flutter/lib/src/cupertino/text_field.dart @@ -996,8 +996,7 @@ class _CupertinoTextFieldState extends State with Restoratio case TargetPlatform.windows: case TargetPlatform.fuchsia: case TargetPlatform.android: - if (cause == SelectionChangedCause.longPress - || cause == SelectionChangedCause.drag) { + if (cause == SelectionChangedCause.longPress) { _editableText.bringIntoView(selection.extent); } break; diff --git a/packages/flutter/lib/src/material/text_field.dart b/packages/flutter/lib/src/material/text_field.dart index 7fe2d0e16e..dc261592fa 100644 --- a/packages/flutter/lib/src/material/text_field.dart +++ b/packages/flutter/lib/src/material/text_field.dart @@ -1125,8 +1125,7 @@ class _TextFieldState extends State with RestorationMixin implements case TargetPlatform.windows: case TargetPlatform.fuchsia: case TargetPlatform.android: - if (cause == SelectionChangedCause.longPress - || cause == SelectionChangedCause.drag) { + if (cause == SelectionChangedCause.longPress) { _editableText?.bringIntoView(selection.extent); } break; diff --git a/packages/flutter/lib/src/widgets/editable_text.dart b/packages/flutter/lib/src/widgets/editable_text.dart index 7edebee32b..ebe4680709 100644 --- a/packages/flutter/lib/src/widgets/editable_text.dart +++ b/packages/flutter/lib/src/widgets/editable_text.dart @@ -3557,6 +3557,8 @@ class EditableTextState extends State with AutomaticKeepAliveClien } } + final TextSelection oldTextSelection = textEditingValue.selection; + // Put all optional user callback invocations in a batch edit to prevent // sending multiple `TextInput.updateEditingValue` messages. beginBatchEdit(); @@ -3570,6 +3572,7 @@ class EditableTextState extends State with AutomaticKeepAliveClien (cause == SelectionChangedCause.longPress || cause == SelectionChangedCause.keyboard))) { _handleSelectionChanged(_value.selection, cause); + _bringIntoViewBySelectionState(oldTextSelection, value.selection, cause); } final String currentText = _value.text; if (oldValue.text != currentText) { @@ -3587,6 +3590,30 @@ class EditableTextState extends State with AutomaticKeepAliveClien endBatchEdit(); } + void _bringIntoViewBySelectionState(TextSelection oldSelection, TextSelection newSelection, SelectionChangedCause? cause) { + switch (defaultTargetPlatform) { + case TargetPlatform.iOS: + case TargetPlatform.macOS: + if (cause == SelectionChangedCause.longPress || + cause == SelectionChangedCause.drag) { + bringIntoView(newSelection.extent); + } + break; + case TargetPlatform.linux: + case TargetPlatform.windows: + case TargetPlatform.fuchsia: + case TargetPlatform.android: + if (cause == SelectionChangedCause.drag) { + if (oldSelection.baseOffset != newSelection.baseOffset) { + bringIntoView(newSelection.base); + } else if (oldSelection.extentOffset != newSelection.extentOffset) { + bringIntoView(newSelection.extent); + } + } + break; + } + } + void _onCursorColorTick() { renderEditable.cursorColor = widget.cursorColor.withOpacity(_cursorBlinkOpacityController.value); _cursorVisibilityNotifier.value = widget.showCursor && _cursorBlinkOpacityController.value > 0; diff --git a/packages/flutter/lib/src/widgets/text_selection.dart b/packages/flutter/lib/src/widgets/text_selection.dart index 79ba17dab4..d9037e612f 100644 --- a/packages/flutter/lib/src/widgets/text_selection.dart +++ b/packages/flutter/lib/src/widgets/text_selection.dart @@ -718,7 +718,7 @@ class TextSelectionOverlay { )); final TextSelection currentSelection = TextSelection.fromPosition(position); - _handleSelectionHandleChanged(currentSelection, isEnd: true); + _handleSelectionHandleChanged(currentSelection); return; } @@ -749,7 +749,7 @@ class TextSelectionOverlay { break; } - _handleSelectionHandleChanged(newSelection, isEnd: true); + _handleSelectionHandleChanged(newSelection); _selectionOverlay.updateMagnifier(_buildMagnifier( currentTextPosition: newSelection.extent, @@ -814,7 +814,7 @@ class TextSelectionOverlay { )); final TextSelection currentSelection = TextSelection.fromPosition(position); - _handleSelectionHandleChanged(currentSelection, isEnd: false); + _handleSelectionHandleChanged(currentSelection); return; } @@ -851,7 +851,7 @@ class TextSelectionOverlay { renderEditable: renderObject, )); - _handleSelectionHandleChanged(newSelection, isEnd: false); + _handleSelectionHandleChanged(newSelection); } void _handleAnyDragEnd(DragEndDetails details) { @@ -874,13 +874,11 @@ class TextSelectionOverlay { } } - void _handleSelectionHandleChanged(TextSelection newSelection, {required bool isEnd}) { - final TextPosition textPosition = isEnd ? newSelection.extent : newSelection.base; + void _handleSelectionHandleChanged(TextSelection newSelection) { selectionDelegate.userUpdateTextEditingValue( _value.copyWith(selection: newSelection), SelectionChangedCause.drag, ); - selectionDelegate.bringIntoView(textPosition); } TextSelectionHandleType _chooseType( diff --git a/packages/flutter/test/cupertino/text_field_test.dart b/packages/flutter/test/cupertino/text_field_test.dart index cb6d8c0590..5929a685f8 100644 --- a/packages/flutter/test/cupertino/text_field_test.dart +++ b/packages/flutter/test/cupertino/text_field_test.dart @@ -4853,6 +4853,163 @@ void main() { ); }); + testWidgets( + 'Can drag the left handle while the right handle remains off-screen', + (WidgetTester tester) async { + // Text is longer than textfield width. + const String testValue = 'aaaaaaaaaaaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbbbbbbbbbbb'; + final TextEditingController controller = TextEditingController(text: testValue); + final ScrollController scrollController = ScrollController(); + + await tester.pumpWidget( + CupertinoApp( + home: Center( + child: CupertinoTextField( + dragStartBehavior: DragStartBehavior.down, + controller: controller, + scrollController: scrollController, + ), + ), + ), + ); + + // Double tap 'b' to show handles. + final Offset bPos = textOffsetToPosition(tester, testValue.indexOf('b')); + await tester.tapAt(bPos); + await tester.pump(kDoubleTapTimeout ~/ 2); + await tester.tapAt(bPos); + await tester.pumpAndSettle(); + + final TextSelection selection = controller.selection; + expect(selection.baseOffset, 28); + expect(selection.extentOffset, testValue.length); + + // Move to the left edge. + scrollController.jumpTo(0); + await tester.pumpAndSettle(); + + final RenderEditable renderEditable = findRenderEditable(tester); + final List endpoints = globalize( + renderEditable.getEndpointsForSelection(selection), + renderEditable, + ); + expect(endpoints.length, 2); + + // Left handle should appear between textfield's left and right position. + final Offset textFieldLeftPosition = + tester.getTopLeft(find.byType(CupertinoTextField)); + expect(endpoints[0].point.dx - textFieldLeftPosition.dx, isPositive); + final Offset textFieldRightPosition = + tester.getTopRight(find.byType(CupertinoTextField)); + expect(textFieldRightPosition.dx - endpoints[0].point.dx, isPositive); + // Right handle should remain off-screen. + expect(endpoints[1].point.dx - textFieldRightPosition.dx, isPositive); + + // Drag the left handle to the right by 25 offset. + const int toOffset = 25; + final double beforeScrollOffset = scrollController.offset; + final Offset handlePos = endpoints[0].point + const Offset(-1.0, 1.0); + final Offset newHandlePos = textOffsetToPosition(tester, toOffset); + final TestGesture gesture = await tester.startGesture(handlePos, pointer: 7); + await tester.pump(); + await gesture.moveTo(newHandlePos); + await tester.pump(); + await gesture.up(); + await tester.pump(); + + switch (defaultTargetPlatform) { + case TargetPlatform.iOS: + case TargetPlatform.macOS: + // On Apple platforms, dragging the base handle makes it the extent. + expect(controller.selection.baseOffset, testValue.length); + expect(controller.selection.extentOffset, toOffset); + break; + case TargetPlatform.android: + case TargetPlatform.fuchsia: + case TargetPlatform.linux: + case TargetPlatform.windows: + expect(controller.selection.baseOffset, toOffset); + expect(controller.selection.extentOffset, testValue.length); + break; + } + + // The scroll area of text field should not move. + expect(scrollController.offset, beforeScrollOffset); + }, + ); + + testWidgets( + 'Can drag the right handle while the left handle remains off-screen', + (WidgetTester tester) async { + // Text is longer than textfield width. + const String testValue = 'aaaaaaaaaaaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbbbbbbbbbbb'; + final TextEditingController controller = TextEditingController(text: testValue); + final ScrollController scrollController = ScrollController(); + + await tester.pumpWidget( + CupertinoApp( + home: Center( + child: CupertinoTextField( + dragStartBehavior: DragStartBehavior.down, + controller: controller, + scrollController: scrollController, + ), + ), + ), + ); + + // Double tap 'a' to show handles. + final Offset aPos = textOffsetToPosition(tester, testValue.indexOf('a')); + await tester.tapAt(aPos); + await tester.pump(kDoubleTapTimeout ~/ 2); + await tester.tapAt(aPos); + await tester.pumpAndSettle(); + + final TextSelection selection = controller.selection; + expect(selection.baseOffset, 0); + expect(selection.extentOffset, 27); + + // Move to the right edge. + scrollController.jumpTo(800); + await tester.pumpAndSettle(); + + final RenderEditable renderEditable = findRenderEditable(tester); + final List endpoints = globalize( + renderEditable.getEndpointsForSelection(selection), + renderEditable, + ); + expect(endpoints.length, 2); + + // Right handle should appear between textfield's left and right position. + final Offset textFieldLeftPosition = + tester.getTopLeft(find.byType(CupertinoTextField)); + expect(endpoints[1].point.dx - textFieldLeftPosition.dx, isPositive); + final Offset textFieldRightPosition = + tester.getTopRight(find.byType(CupertinoTextField)); + expect(textFieldRightPosition.dx - endpoints[1].point.dx, isPositive); + // Left handle should remain off-screen. + expect(endpoints[0].point.dx, isNegative); + + // Drag the right handle to the left by 50 offset. + const int toOffset = 50; + final double beforeScrollOffset = scrollController.offset; + final Offset handlePos = endpoints[1].point + const Offset(1.0, 1.0); + final Offset newHandlePos = textOffsetToPosition(tester, toOffset); + final TestGesture gesture = await tester.startGesture(handlePos, pointer: 7); + await tester.pump(); + await gesture.moveTo(newHandlePos); + await tester.pump(); + await gesture.up(); + await tester.pump(); + + expect(controller.selection.baseOffset, 0); + expect(controller.selection.extentOffset, toOffset); + + // The scroll area of text field should not move. + expect(scrollController.offset, beforeScrollOffset); + }, + ); + group('Text selection toolbar', () { testWidgets('Collapsed selection works', (WidgetTester tester) async { EditableText.debugDeterministicCursor = true; diff --git a/packages/flutter/test/material/text_field_test.dart b/packages/flutter/test/material/text_field_test.dart index f742bede79..727769d096 100644 --- a/packages/flutter/test/material/text_field_test.dart +++ b/packages/flutter/test/material/text_field_test.dart @@ -2566,6 +2566,171 @@ void main() { variant: TargetPlatformVariant.all(excluding: { TargetPlatform.iOS, TargetPlatform.macOS }), ); + testWidgets( + 'Can drag the left handle while the right handle remains off-screen', + (WidgetTester tester) async { + // Text is longer than textfield width. + const String testValue = + 'aaaaaaaaaaaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbbbbbbbbbbb'; + final TextEditingController controller = TextEditingController(text: testValue); + final ScrollController scrollController = ScrollController(); + + await tester.pumpWidget( + MaterialApp( + home: Material( + child: MediaQuery( + data: const MediaQueryData(size: Size(800.0, 600.0)), + child: TextField( + dragStartBehavior: DragStartBehavior.down, + controller: controller, + scrollController: scrollController, + ), + ), + ), + ), + ); + + // Double tap 'b' to show handles. + final Offset bPos = textOffsetToPosition(tester, testValue.indexOf('b')); + await tester.tapAt(bPos); + await tester.pump(kDoubleTapTimeout ~/ 2); + await tester.tapAt(bPos); + await tester.pumpAndSettle(); + + final TextSelection selection = controller.selection; + expect(selection.baseOffset, 28); + expect(selection.extentOffset, testValue.length); + + // Move to the left edge. + scrollController.jumpTo(0); + await tester.pumpAndSettle(); + + final RenderEditable renderEditable = findRenderEditable(tester); + final List endpoints = globalize( + renderEditable.getEndpointsForSelection(selection), + renderEditable, + ); + expect(endpoints.length, 2); + + // Left handle should appear between textfield's left and right position. + final Offset textFieldLeftPosition = + tester.getTopLeft(find.byType(TextField)); + expect(endpoints[0].point.dx - textFieldLeftPosition.dx, isPositive); + final Offset textFieldRightPosition = + tester.getTopRight(find.byType(TextField)); + expect(textFieldRightPosition.dx - endpoints[0].point.dx, isPositive); + // Right handle should remain off-screen. + expect(endpoints[1].point.dx - textFieldRightPosition.dx, isPositive); + + // Drag the left handle to the right by 25 offset. + const int toOffset = 25; + final double beforeScrollOffset = scrollController.offset; + final Offset handlePos = endpoints[0].point + const Offset(-1.0, 1.0); + final Offset newHandlePos = textOffsetToPosition(tester, toOffset); + final TestGesture gesture = await tester.startGesture(handlePos, pointer: 7); + await tester.pump(); + await gesture.moveTo(newHandlePos); + await tester.pump(); + await gesture.up(); + await tester.pump(); + + switch (defaultTargetPlatform) { + case TargetPlatform.iOS: + case TargetPlatform.macOS: + // On Apple platforms, dragging the base handle makes it the extent. + expect(controller.selection.baseOffset, testValue.length); + expect(controller.selection.extentOffset, toOffset); + break; + case TargetPlatform.android: + case TargetPlatform.fuchsia: + case TargetPlatform.linux: + case TargetPlatform.windows: + expect(controller.selection.baseOffset, toOffset); + expect(controller.selection.extentOffset, testValue.length); + break; + } + + // The scroll area of text field should not move. + expect(scrollController.offset, beforeScrollOffset); + }, + ); + + testWidgets( + 'Can drag the right handle while the left handle remains off-screen', + (WidgetTester tester) async { + // Text is longer than textfield width. + const String testValue = + 'aaaaaaaaaaaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbbbbbbbbbbb'; + final TextEditingController controller = TextEditingController(text: testValue); + final ScrollController scrollController = ScrollController(); + + await tester.pumpWidget( + MaterialApp( + home: Material( + child: MediaQuery( + data: const MediaQueryData(size: Size(800.0, 600.0)), + child: TextField( + dragStartBehavior: DragStartBehavior.down, + controller: controller, + scrollController: scrollController, + ), + ), + ), + ), + ); + + // Double tap 'a' to show handles. + final Offset aPos = textOffsetToPosition(tester, testValue.indexOf('a')); + await tester.tapAt(aPos); + await tester.pump(kDoubleTapTimeout ~/ 2); + await tester.tapAt(aPos); + await tester.pumpAndSettle(); + + final TextSelection selection = controller.selection; + expect(selection.baseOffset, 0); + expect(selection.extentOffset, 27); + + // Move to the right edge. + scrollController.jumpTo(800); + await tester.pumpAndSettle(); + + final RenderEditable renderEditable = findRenderEditable(tester); + final List endpoints = globalize( + renderEditable.getEndpointsForSelection(selection), + renderEditable, + ); + expect(endpoints.length, 2); + + // Right handle should appear between textfield's left and right position. + final Offset textFieldLeftPosition = + tester.getTopLeft(find.byType(TextField)); + expect(endpoints[1].point.dx - textFieldLeftPosition.dx, isPositive); + final Offset textFieldRightPosition = + tester.getTopRight(find.byType(TextField)); + expect(textFieldRightPosition.dx - endpoints[1].point.dx, isPositive); + // Left handle should remain off-screen. + expect(endpoints[0].point.dx, isNegative); + + // Drag the right handle to the left by 50 offset. + const int toOffset = 50; + final double beforeScrollOffset = scrollController.offset; + final Offset handlePos = endpoints[1].point + const Offset(1.0, 1.0); + final Offset newHandlePos = textOffsetToPosition(tester, toOffset); + final TestGesture gesture = await tester.startGesture(handlePos, pointer: 7); + await tester.pump(); + await gesture.moveTo(newHandlePos); + await tester.pump(); + await gesture.up(); + await tester.pump(); + + expect(controller.selection.baseOffset, 0); + expect(controller.selection.extentOffset, toOffset); + + // The scroll area of text field should not move. + expect(scrollController.offset, beforeScrollOffset); + }, + ); + testWidgets('Drag handles trigger feedback', (WidgetTester tester) async { final FeedbackTester feedback = FeedbackTester(); addTearDown(feedback.dispose);