Unverified Commit 5c48d906 authored by Justin McCandless's avatar Justin McCandless Committed by GitHub

Multiline Selection Menu Position Bug (#36974)

Fix bug where selection menu appeared at top of screen for multiline inputs.
parent 975156e9
d35b254e39541083238f6a6e7b20d142b7ab4855 ead5d5df3236f6d9e619e640029f9811e4eb0716
...@@ -305,11 +305,12 @@ class _CupertinoTextSelectionControls extends TextSelectionControls { ...@@ -305,11 +305,12 @@ class _CupertinoTextSelectionControls extends TextSelectionControls {
// The toolbar should appear below the TextField when there is not enough // The toolbar should appear below the TextField when there is not enough
// space above the TextField to show it, assuming there's always enough space // space above the TextField to show it, assuming there's always enough space
// at the bottom in this case. // at the bottom in this case.
final bool isArrowPointingDown = final double toolbarHeightNeeded = mediaQuery.padding.top
mediaQuery.padding.top
+ _kToolbarScreenPadding + _kToolbarScreenPadding
+ _kToolbarHeight + _kToolbarHeight
+ _kToolbarContentDistance <= globalEditableRegion.top + endpoints.first.point.dy - textLineHeight; + _kToolbarContentDistance;
final double availableHeight = globalEditableRegion.top + endpoints.first.point.dy - textLineHeight;
final bool isArrowPointingDown = toolbarHeightNeeded <= availableHeight;
final double arrowTipX = (position.dx + globalEditableRegion.left).clamp( final double arrowTipX = (position.dx + globalEditableRegion.left).clamp(
_kArrowScreenPadding + mediaQuery.padding.left, _kArrowScreenPadding + mediaQuery.padding.left,
......
...@@ -19,6 +19,9 @@ const double _kHandleSize = 22.0; ...@@ -19,6 +19,9 @@ const double _kHandleSize = 22.0;
// viewport. // viewport.
const double _kToolbarScreenPadding = 8.0; const double _kToolbarScreenPadding = 8.0;
const double _kToolbarHeight = 44.0; const double _kToolbarHeight = 44.0;
// Padding when positioning toolbar below selection.
const double _kToolbarContentDistanceBelow = 16.0;
const double _kToolbarContentDistance = 8.0;
/// Manages a copy/paste text selection toolbar. /// Manages a copy/paste text selection toolbar.
class _TextSelectionToolbar extends StatelessWidget { class _TextSelectionToolbar extends StatelessWidget {
...@@ -152,18 +155,16 @@ class _MaterialTextSelectionControls extends TextSelectionControls { ...@@ -152,18 +155,16 @@ class _MaterialTextSelectionControls extends TextSelectionControls {
// The toolbar should appear below the TextField // The toolbar should appear below the TextField
// when there is not enough space above the TextField to show it. // when there is not enough space above the TextField to show it.
final TextSelectionPoint startTextSelectionPoint = endpoints[0]; final TextSelectionPoint startTextSelectionPoint = endpoints[0];
final TextSelectionPoint endTextSelectionPoint = (endpoints.length > 1) final double toolbarHeightNeeded = MediaQuery.of(context).padding.top
? endpoints[1] + _kToolbarScreenPadding
: null; + _kToolbarHeight
final double x = (endTextSelectionPoint == null) + _kToolbarContentDistance;
? startTextSelectionPoint.point.dx final double availableHeight = globalEditableRegion.top + endpoints.first.point.dy - textLineHeight;
: (startTextSelectionPoint.point.dx + endTextSelectionPoint.point.dx) / 2.0; final bool fitsAbove = toolbarHeightNeeded <= availableHeight;
final double availableHeight final double y = fitsAbove
= globalEditableRegion.top - MediaQuery.of(context).padding.top - _kToolbarScreenPadding; ? startTextSelectionPoint.point.dy - _kToolbarContentDistance - textLineHeight
final double y = (availableHeight < _kToolbarHeight) : startTextSelectionPoint.point.dy + _kToolbarHeight + _kToolbarContentDistanceBelow;
? startTextSelectionPoint.point.dy + globalEditableRegion.height + _kToolbarHeight + _kToolbarScreenPadding final Offset preciseMidpoint = Offset(position.dx, y);
: startTextSelectionPoint.point.dy - globalEditableRegion.height;
final Offset preciseMidpoint = Offset(x, y);
return ConstrainedBox( return ConstrainedBox(
constraints: BoxConstraints.tight(globalEditableRegion.size), constraints: BoxConstraints.tight(globalEditableRegion.size),
......
...@@ -499,7 +499,7 @@ void main() { ...@@ -499,7 +499,7 @@ void main() {
find.byType(Overlay), find.byType(Overlay),
matchesGoldenFile( matchesGoldenFile(
'text_field_opacity_test.0.png', 'text_field_opacity_test.0.png',
version: 2, version: 3,
), ),
); );
}, skip: isBrowser); }, skip: isBrowser);
...@@ -1478,6 +1478,32 @@ void main() { ...@@ -1478,6 +1478,32 @@ void main() {
expect(controller.text, 'abc d${testValue}ef ghi'); expect(controller.text, 'abc d${testValue}ef ghi');
}); });
// Show the selection menu at the given index into the text by tapping to
// place the cursor and then tapping on the handle.
Future<void> _showSelectionMenuAt(WidgetTester tester, TextEditingController controller, int index) async {
await tester.tapAt(tester.getCenter(find.byType(EditableText)));
await tester.pump();
await tester.pump(const Duration(milliseconds: 200)); // skip past the frame where the opacity is zero
expect(find.text('SELECT ALL'), findsNothing);
// Tap the selection handle to bring up the "paste / select all" menu for
// the last line of text.
await tester.tapAt(textOffsetToPosition(tester, index));
await tester.pump();
await tester.pump(const Duration(milliseconds: 200)); // skip past the frame where the opacity is zero
final RenderEditable renderEditable = findRenderEditable(tester);
final List<TextSelectionPoint> endpoints = globalize(
renderEditable.getEndpointsForSelection(controller.selection),
renderEditable,
);
// Tapping on the part of the handle's GestureDetector where it overlaps
// with the text itself does not show the menu, so add a small vertical
// offset to tap below the text.
await tester.tapAt(endpoints[0].point + const Offset(1.0, 13.0));
await tester.pump();
await tester.pump(const Duration(milliseconds: 200)); // skip past the frame where the opacity is zero
}
testWidgets( testWidgets(
'Check the toolbar appears below the TextField when there is not enough space above the TextField to show it', 'Check the toolbar appears below the TextField when there is not enough space above the TextField to show it',
(WidgetTester tester) async { (WidgetTester tester) async {
...@@ -1501,23 +1527,9 @@ void main() { ...@@ -1501,23 +1527,9 @@ void main() {
await tester.enterText(find.byType(TextField), testValue); await tester.enterText(find.byType(TextField), testValue);
await skipPastScrollingAnimation(tester); await skipPastScrollingAnimation(tester);
// Tap the selection handle to bring up the "paste / select all" menu. await _showSelectionMenuAt(tester, controller, testValue.indexOf('e'));
await tester.tapAt(textOffsetToPosition(tester, testValue.indexOf('e')));
await tester.pump();
await tester.pump(const Duration(milliseconds: 200)); // skip past the frame where the opacity is zero
RenderEditable renderEditable = findRenderEditable(tester);
List<TextSelectionPoint> endpoints = globalize(
renderEditable.getEndpointsForSelection(controller.selection),
renderEditable,
);
// Tapping on the part of the handle's GestureDetector where it overlaps
// with the text itself does not show the menu, so add a small vertical
// offset to tap below the text.
await tester.tapAt(endpoints[0].point + const Offset(1.0, 13.0));
await tester.pump();
await tester.pump(const Duration(milliseconds: 200)); // skip past the frame where the opacity is zero
// Verify the selection toolbar position // Verify the selection toolbar position is below the text.
Offset toolbarTopLeft = tester.getTopLeft(find.text('SELECT ALL')); Offset toolbarTopLeft = tester.getTopLeft(find.text('SELECT ALL'));
Offset textFieldTopLeft = tester.getTopLeft(find.byType(TextField)); Offset textFieldTopLeft = tester.getTopLeft(find.byType(TextField));
expect(textFieldTopLeft.dy, lessThan(toolbarTopLeft.dy)); expect(textFieldTopLeft.dy, lessThan(toolbarTopLeft.dy));
...@@ -1528,36 +1540,76 @@ void main() { ...@@ -1528,36 +1540,76 @@ void main() {
padding: const EdgeInsets.all(150.0), padding: const EdgeInsets.all(150.0),
child: TextField( child: TextField(
controller: controller, controller: controller,
),
), ),
), ),
), ),
); ));
await tester.enterText(find.byType(TextField), testValue); await tester.enterText(find.byType(TextField), testValue);
await skipPastScrollingAnimation(tester); await skipPastScrollingAnimation(tester);
// Tap the selection handle to bring up the "paste / select all" menu. await _showSelectionMenuAt(tester, controller, testValue.indexOf('e'));
await tester.tapAt(textOffsetToPosition(tester, testValue.indexOf('e')));
await tester.pump();
await tester.pump(const Duration(milliseconds: 200)); // skip past the frame where the opacity is zero
renderEditable = findRenderEditable(tester);
endpoints = globalize(
renderEditable.getEndpointsForSelection(controller.selection),
renderEditable,
);
// Tapping on the part of the handle's GestureDetector where it overlaps
// with the text itself does not show the menu, so add a small vertical
// offset to tap below the text.
await tester.tapAt(endpoints[0].point + const Offset(1.0, 13.0));
await tester.pump();
await tester.pump(const Duration(milliseconds: 200)); // skip past the frame where the opacity is zero
// Verify the selection toolbar position // Verify the selection toolbar position
toolbarTopLeft = tester.getTopLeft(find.text('SELECT ALL')); toolbarTopLeft = tester.getTopLeft(find.text('SELECT ALL'));
textFieldTopLeft = tester.getTopLeft(find.byType(TextField)); textFieldTopLeft = tester.getTopLeft(find.byType(TextField));
expect(toolbarTopLeft.dy, lessThan(textFieldTopLeft.dy)); expect(toolbarTopLeft.dy, lessThan(textFieldTopLeft.dy));
} },
);
testWidgets(
'Toolbar appears in the right places in multiline inputs',
(WidgetTester tester) async {
// This is a regression test for
// https://github.com/flutter/flutter/issues/36749
final TextEditingController controller = TextEditingController();
await tester.pumpWidget(MaterialApp(
home: Scaffold(
body: Padding(
padding: const EdgeInsets.all(30.0),
child: TextField(
controller: controller,
minLines: 6,
maxLines: 6,
),
),
),
));
expect(find.text('SELECT ALL'), findsNothing);
const String testValue = 'abc\ndef\nghi\njkl\nmno\npqr';
await tester.enterText(find.byType(TextField), testValue);
await skipPastScrollingAnimation(tester);
// Show the selection menu on the first line and verify the selection
// toolbar position is below the first line.
await _showSelectionMenuAt(tester, controller, testValue.indexOf('c'));
expect(find.text('SELECT ALL'), findsOneWidget);
final Offset firstLineToolbarTopLeft = tester.getTopLeft(find.text('SELECT ALL'));
final Offset firstLineTopLeft = textOffsetToPosition(tester, testValue.indexOf('a'));
expect(firstLineTopLeft.dy, lessThan(firstLineToolbarTopLeft.dy));
// Show the selection menu on the second to last line and verify the
// selection toolbar position is above that line and above the first
// line's toolbar.
await _showSelectionMenuAt(tester, controller, testValue.indexOf('o'));
expect(find.text('SELECT ALL'), findsOneWidget);
final Offset penultimateLineToolbarTopLeft = tester.getTopLeft(find.text('SELECT ALL'));
final Offset penultimateLineTopLeft = textOffsetToPosition(tester, testValue.indexOf('p'));
expect(penultimateLineToolbarTopLeft.dy, lessThan(penultimateLineTopLeft.dy));
expect(penultimateLineToolbarTopLeft.dy, lessThan(firstLineToolbarTopLeft.dy));
// Show the selection menu on the last line and verify the selection
// toolbar position is above that line and below the position of the
// second to last line's toolbar.
await _showSelectionMenuAt(tester, controller, testValue.indexOf('r'));
expect(find.text('SELECT ALL'), findsOneWidget);
final Offset lastLineToolbarTopLeft = tester.getTopLeft(find.text('SELECT ALL'));
final Offset lastLineTopLeft = textOffsetToPosition(tester, testValue.indexOf('p'));
expect(lastLineToolbarTopLeft.dy, lessThan(lastLineTopLeft.dy));
expect(lastLineToolbarTopLeft.dy, greaterThan(penultimateLineToolbarTopLeft.dy));
},
); );
testWidgets('Selection toolbar fades in', (WidgetTester tester) async { testWidgets('Selection toolbar fades in', (WidgetTester tester) async {
......
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