Skip to content

Commit

Permalink
Text selection located wrong position when selecting multiple lines o…
Browse files Browse the repository at this point in the history
…ver max lines (#102747)

Fix for text selection toolbar position in cases with overflowing text.
  • Loading branch information
takassh authored Jun 7, 2022
1 parent 3f9ec41 commit 4b4c876
Show file tree
Hide file tree
Showing 5 changed files with 149 additions and 2 deletions.
5 changes: 4 additions & 1 deletion packages/flutter/lib/src/cupertino/text_selection.dart
Original file line number Diff line number Diff line change
Expand Up @@ -97,13 +97,16 @@ class _CupertinoTextSelectionControlsToolbarState extends State<_CupertinoTextSe
mediaQuery.size.width - mediaQuery.padding.right - _kArrowScreenPadding,
);

final double topAmountInEditableRegion = widget.endpoints.first.point.dy - widget.textLineHeight;
final double anchorTop = math.max(topAmountInEditableRegion, 0) + widget.globalEditableRegion.top;

// The y-coordinate has to be calculated instead of directly quoting
// selectionMidpoint.dy, since the caller
// (TextSelectionOverlay._buildToolbar) does not know whether the toolbar is
// going to be facing up or down.
final Offset anchorAbove = Offset(
anchorX,
widget.endpoints.first.point.dy - widget.textLineHeight + widget.globalEditableRegion.top,
anchorTop,
);
final Offset anchorBelow = Offset(
anchorX,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ class CupertinoTextSelectionToolbar extends StatelessWidget {
delegate: TextSelectionToolbarLayoutDelegate(
anchorAbove: anchorAbove - localAdjustment - contentPaddingAdjustment,
anchorBelow: anchorBelow - localAdjustment + contentPaddingAdjustment,
fitsAbove: fitsAbove,
),
child: _CupertinoTextSelectionToolbarContent(
anchor: fitsAbove ? anchorAbove : anchorBelow,
Expand Down
5 changes: 4 additions & 1 deletion packages/flutter/lib/src/material/text_selection.dart
Original file line number Diff line number Diff line change
Expand Up @@ -205,9 +205,12 @@ class _TextSelectionControlsToolbarState extends State<_TextSelectionControlsToo
final TextSelectionPoint endTextSelectionPoint = widget.endpoints.length > 1
? widget.endpoints[1]
: widget.endpoints[0];
final double topAmountInEditableRegion = startTextSelectionPoint.point.dy - widget.textLineHeight;
final double anchorTop = math.max(topAmountInEditableRegion, 0) + widget.globalEditableRegion.top - _kToolbarContentDistance;

final Offset anchorAbove = Offset(
widget.globalEditableRegion.left + widget.selectionMidpoint.dx,
widget.globalEditableRegion.top + startTextSelectionPoint.point.dy - widget.textLineHeight - _kToolbarContentDistance,
anchorTop,
);
final Offset anchorBelow = Offset(
widget.globalEditableRegion.left + widget.selectionMidpoint.dx,
Expand Down
63 changes: 63 additions & 0 deletions packages/flutter/test/cupertino/text_selection_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,69 @@ void main() {
skip: isBrowser, // [intended] We do not use Flutter-rendered context menu on the Web.
variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.iOS }),
);

testWidgets(
'When selecting multiple lines over max lines',
(WidgetTester tester) async {
final TextEditingController controller = TextEditingController(text: 'abc\ndef\nghi\njkl\nmno\npqr');
await tester.pumpWidget(CupertinoApp(
home: Directionality(
textDirection: TextDirection.ltr,
child: MediaQuery(
data: const MediaQueryData(size: Size(800.0, 600.0)),
child: Center(
child: CupertinoTextField(
padding: const EdgeInsets.all(8.0),
controller: controller,
maxLines: 2,
),
),
),
),
));

// Initially, the menu isn't shown at all.
expect(find.text('Cut'), findsNothing);
expect(find.text('Copy'), findsNothing);
expect(find.text('Paste'), findsNothing);
expect(find.text('Select All'), findsNothing);
expect(find.text('◀'), findsNothing);
expect(find.text('▶'), findsNothing);

// Long press on an space to show the selection menu.
await tester.longPressAt(textOffsetToPosition(tester, 1));
await tester.pumpAndSettle();
expect(find.text('Cut'), findsNothing);
expect(find.text('Copy'), findsNothing);
expect(find.text('Paste'), findsOneWidget);
expect(find.text('Select All'), findsOneWidget);
expect(find.text('◀'), findsNothing);
expect(find.text('▶'), findsNothing);

// Tap to select all.
await tester.tap(find.text('Select All'));
await tester.pumpAndSettle();

// Only Cut, Copy, and Paste are shown.
expect(find.text('Cut'), findsOneWidget);
expect(find.text('Copy'), findsOneWidget);
expect(find.text('Paste'), findsOneWidget);
expect(find.text('Select All'), findsNothing);
expect(find.text('◀'), findsNothing);
expect(find.text('▶'), findsNothing);

// The menu appears at the top of the visible selection.
final Offset selectionOffset = tester
.getTopLeft(find.byType(CupertinoTextSelectionToolbarButton).first);
final Offset textFieldOffset =
tester.getTopLeft(find.byType(CupertinoTextField));

// 7.0 + 43.0 + 8.0 - 8.0 = _kToolbarArrowSize + _kToolbarHeight + _kToolbarContentDistance - padding
expect(selectionOffset.dy + 7.0 + 43.0 + 8.0 - 8.0, equals(textFieldOffset.dy));
},
skip: isBrowser, // [intended] the selection menu isn't required by web
variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.iOS }),
);
});

testWidgets('iOS selection handles scale with rich text (selection style 1)', (WidgetTester tester) async {
Expand Down
77 changes: 77 additions & 0 deletions packages/flutter/test/material/text_selection_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -547,6 +547,83 @@ void main() {
skip: isBrowser, // [intended] We do not use Flutter-rendered context menu on the Web.
variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.android }),
);

testWidgets(
'When selecting multiple lines over max lines',
(WidgetTester tester) async {
final TextEditingController controller =
TextEditingController(text: 'abc\ndef\nghi\njkl\nmno\npqr');
await tester.pumpWidget(MaterialApp(
theme: ThemeData(platform: TargetPlatform.android),
home: Directionality(
textDirection: TextDirection.ltr,
child: MediaQuery(
data: const MediaQueryData(size: Size(800.0, 600.0)),
child: Align(
alignment: Alignment.bottomCenter,
child: Material(
child: TextField(
decoration: const InputDecoration(contentPadding: EdgeInsets.all(8.0)),
style: const TextStyle(fontSize: 32, height: 1),
maxLines: 2,
controller: controller,
),
),
),
),
),
));

// Initially, the menu isn't shown at all.
expect(find.text('Cut'), findsNothing);
expect(find.text('Copy'), findsNothing);
expect(find.text('Paste'), findsNothing);
expect(find.text('Select all'), findsNothing);
expect(find.byType(IconButton), findsNothing);

// Tap to place the cursor in the field, then tap the handle to show the
// selection menu.
await tester.tap(find.byType(TextField));
await tester.pumpAndSettle();
final RenderEditable renderEditable = findRenderEditable(tester);
final List<TextSelectionPoint> endpoints = globalize(
renderEditable.getEndpointsForSelection(controller.selection),
renderEditable,
);
expect(endpoints.length, 1);
final Offset handlePos = endpoints[0].point + const Offset(0.0, 1.0);
await tester.tapAt(handlePos, pointer: 7);
await tester.pumpAndSettle();
expect(find.text('Cut'), findsNothing);
expect(find.text('Copy'), findsNothing);
expect(find.text('Paste'), findsOneWidget);
expect(find.text('Select all'), findsOneWidget);
expect(find.byType(IconButton), findsNothing);

// Tap to select all.
await tester.tap(find.text('Select all'));
await tester.pumpAndSettle();

// Only Cut, Copy, and Paste are shown.
expect(find.text('Cut'), findsOneWidget);
expect(find.text('Copy'), findsOneWidget);
expect(find.text('Paste'), findsOneWidget);
expect(find.text('Select all'), findsNothing);
expect(find.byType(IconButton), findsNothing);


// The menu appears at the top of the visible selection.
final Offset selectionOffset = tester
.getTopLeft(find.byType(TextSelectionToolbarTextButton).first);
final Offset textFieldOffset =
tester.getTopLeft(find.byType(TextField));

// 44.0 + 8.0 - 8.0 = _kToolbarHeight + _kToolbarContentDistance - contentPadding
expect(selectionOffset.dy + 44.0 + 8.0 - 8.0, equals(textFieldOffset.dy));
},
skip: isBrowser, // [intended] the selection menu isn't required by web
variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.android }),
);
});

group('material handles', () {
Expand Down

0 comments on commit 4b4c876

Please sign in to comment.