Unverified Commit b2adffa9 authored by Justin McCandless's avatar Justin McCandless Committed by GitHub

Keyboard text selection and wordwrap (#85653)

Keyboard shortcuts near wordwrapped lines could be buggy before this fix.
parent df4f130f
...@@ -1747,18 +1747,26 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin, ...@@ -1747,18 +1747,26 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin,
return moveSelectionLeftByLine(cause); return moveSelectionLeftByLine(cause);
} }
final int firstOffset = math.min(selection!.baseOffset, selection!.extentOffset); // If the lowest edge of the selection is at the start of a line, don't do
final int startPoint = previousCharacter(firstOffset, _plainText, false); // anything.
final TextSelection selectedLine = _getLineAtOffset(TextPosition(offset: startPoint)); // TODO(justinmc): Support selection with multiple TextAffinities.
// https://github.com/flutter/flutter/issues/88135
final TextSelection currentLine = _getLineAtOffset(TextPosition(
offset: selection!.start,
affinity: selection!.isCollapsed ? selection!.affinity : TextAffinity.downstream,
));
if (currentLine.baseOffset == selection!.start) {
return;
}
late final TextSelection nextSelection; late final TextSelection nextSelection;
if (selection!.extentOffset <= selection!.baseOffset) { if (selection!.extentOffset <= selection!.baseOffset) {
nextSelection = selection!.copyWith( nextSelection = selection!.copyWith(
extentOffset: selectedLine.baseOffset, extentOffset: currentLine.baseOffset,
); );
} else { } else {
nextSelection = selection!.copyWith( nextSelection = selection!.copyWith(
baseOffset: selectedLine.baseOffset, baseOffset: currentLine.baseOffset,
); );
} }
...@@ -1867,18 +1875,30 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin, ...@@ -1867,18 +1875,30 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin,
return moveSelectionRightByLine(cause); return moveSelectionRightByLine(cause);
} }
final int lastOffset = math.max(selection!.baseOffset, selection!.extentOffset); // If greatest edge is already at the end of a line, don't do anything.
final int startPoint = nextCharacter(lastOffset, _plainText, false); // TODO(justinmc): Support selection with multiple TextAffinities.
// https://github.com/flutter/flutter/issues/88135
final TextSelection currentLine = _getLineAtOffset(TextPosition(
offset: selection!.end,
affinity: selection!.isCollapsed ? selection!.affinity : TextAffinity.upstream,
));
if (currentLine.extentOffset == selection!.end) {
return;
}
final int startPoint = nextCharacter(selection!.end, _plainText, false);
final TextSelection selectedLine = _getLineAtOffset(TextPosition(offset: startPoint)); final TextSelection selectedLine = _getLineAtOffset(TextPosition(offset: startPoint));
late final TextSelection nextSelection; late final TextSelection nextSelection;
if (selection!.extentOffset >= selection!.baseOffset) { if (selection!.baseOffset <= selection!.extentOffset) {
nextSelection = selection!.copyWith( nextSelection = selection!.copyWith(
extentOffset: selectedLine.extentOffset, extentOffset: selectedLine.extentOffset,
affinity: TextAffinity.upstream,
); );
} else { } else {
nextSelection = selection!.copyWith( nextSelection = selection!.copyWith(
baseOffset: selectedLine.extentOffset, baseOffset: selectedLine.extentOffset,
affinity: TextAffinity.upstream,
); );
} }
...@@ -1950,10 +1970,9 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin, ...@@ -1950,10 +1970,9 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin,
void moveSelectionLeftByLine(SelectionChangedCause cause) { void moveSelectionLeftByLine(SelectionChangedCause cause) {
assert(selection != null); assert(selection != null);
// If the previous character is the edge of a line, don't do anything. // If already at the left edge of the line, do nothing.
final int previousPoint = previousCharacter(selection!.extentOffset, _plainText, true); final TextSelection currentLine = _getLineAtOffset(selection!.extent);
final TextSelection line = _getLineAtOffset(TextPosition(offset: previousPoint)); if (currentLine.baseOffset == selection!.extentOffset) {
if (line.extentOffset == previousPoint) {
return; return;
} }
...@@ -2037,9 +2056,7 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin, ...@@ -2037,9 +2056,7 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin,
assert(selection != null); assert(selection != null);
// If already at the right edge of the line, do nothing. // If already at the right edge of the line, do nothing.
final TextSelection currentLine = _getLineAtOffset(TextPosition( final TextSelection currentLine = _getLineAtOffset(selection!.extent);
offset: selection!.extentOffset,
));
if (currentLine.extentOffset == selection!.extentOffset) { if (currentLine.extentOffset == selection!.extentOffset) {
return; return;
} }
...@@ -2049,7 +2066,10 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin, ...@@ -2049,7 +2066,10 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin,
// for the line bounds, since _getLineAtOffset finds the line // for the line bounds, since _getLineAtOffset finds the line
// boundaries without including whitespace (like the newline). // boundaries without including whitespace (like the newline).
final int startPoint = nextCharacter(selection!.extentOffset, _plainText, false); final int startPoint = nextCharacter(selection!.extentOffset, _plainText, false);
final TextSelection selectedLine = _getLineAtOffset(TextPosition(offset: startPoint)); final TextSelection selectedLine = _getLineAtOffset(TextPosition(
offset: startPoint,
affinity: TextAffinity.upstream,
));
final TextSelection nextSelection = TextSelection.collapsed( final TextSelection nextSelection = TextSelection.collapsed(
offset: selectedLine.extentOffset, offset: selectedLine.extentOffset,
affinity: TextAffinity.upstream, affinity: TextAffinity.upstream,
...@@ -3537,8 +3557,6 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin, ...@@ -3537,8 +3557,6 @@ class RenderEditable extends RenderBox with RelayoutWhenSystemFontsChangeMixin,
'Last width ($_textLayoutLastMinWidth, $_textLayoutLastMaxWidth) not the same as max width constraint (${constraints.minWidth}, ${constraints.maxWidth}).', 'Last width ($_textLayoutLastMinWidth, $_textLayoutLastMaxWidth) not the same as max width constraint (${constraints.minWidth}, ${constraints.maxWidth}).',
); );
final TextRange line = _textPainter.getLineBoundary(position); final TextRange line = _textPainter.getLineBoundary(position);
if (position.offset >= line.end)
return TextSelection.fromPosition(position);
// If text is obscured, the entire string should be treated as one line. // If text is obscured, the entire string should be treated as one line.
if (obscureText) { if (obscureText) {
return TextSelection(baseOffset: 0, extentOffset: _plainText.length); return TextSelection(baseOffset: 0, extentOffset: _plainText.length);
......
...@@ -454,8 +454,8 @@ class DefaultTextEditingShortcuts extends Shortcuts { ...@@ -454,8 +454,8 @@ class DefaultTextEditingShortcuts extends Shortcuts {
SingleActivator(LogicalKeyboardKey.arrowLeft, shift: true): ExtendSelectionLeftTextIntent(), SingleActivator(LogicalKeyboardKey.arrowLeft, shift: true): ExtendSelectionLeftTextIntent(),
SingleActivator(LogicalKeyboardKey.arrowRight, shift: true): ExtendSelectionRightTextIntent(), SingleActivator(LogicalKeyboardKey.arrowRight, shift: true): ExtendSelectionRightTextIntent(),
SingleActivator(LogicalKeyboardKey.arrowUp, shift: true): ExtendSelectionUpTextIntent(), SingleActivator(LogicalKeyboardKey.arrowUp, shift: true): ExtendSelectionUpTextIntent(),
SingleActivator(LogicalKeyboardKey.end, shift: true): ExpandSelectionRightByLineTextIntent(), SingleActivator(LogicalKeyboardKey.end, shift: true): ExtendSelectionRightByLineTextIntent(),
SingleActivator(LogicalKeyboardKey.home, shift: true): ExpandSelectionLeftByLineTextIntent(), SingleActivator(LogicalKeyboardKey.home, shift: true): ExtendSelectionLeftByLineTextIntent(),
SingleActivator(LogicalKeyboardKey.keyX, control: true): CutSelectionTextIntent(), SingleActivator(LogicalKeyboardKey.keyX, control: true): CutSelectionTextIntent(),
SingleActivator(LogicalKeyboardKey.keyC, control: true): CopySelectionTextIntent(), SingleActivator(LogicalKeyboardKey.keyC, control: true): CopySelectionTextIntent(),
SingleActivator(LogicalKeyboardKey.keyV, control: true): PasteTextIntent(), SingleActivator(LogicalKeyboardKey.keyV, control: true): PasteTextIntent(),
......
...@@ -4547,13 +4547,13 @@ void main() { ...@@ -4547,13 +4547,13 @@ void main() {
const TextSelection( const TextSelection(
baseOffset: 20, baseOffset: 20,
extentOffset: 72, extentOffset: 72,
affinity: TextAffinity.downstream, affinity: TextAffinity.upstream,
), ),
), ),
reason: 'on $platform', reason: 'on $platform',
); );
// Select to the beginning of the first line. // Can't move left by line because we're already at the beginning of a line.
await sendKeys( await sendKeys(
tester, tester,
<LogicalKeyboardKey>[ <LogicalKeyboardKey>[
...@@ -4568,9 +4568,9 @@ void main() { ...@@ -4568,9 +4568,9 @@ void main() {
selection, selection,
equals( equals(
const TextSelection( const TextSelection(
baseOffset: 0, baseOffset: 20,
extentOffset: 72, extentOffset: 72,
affinity: TextAffinity.downstream, affinity: TextAffinity.upstream,
), ),
), ),
reason: 'on $platform', reason: 'on $platform',
...@@ -4592,7 +4592,7 @@ void main() { ...@@ -4592,7 +4592,7 @@ void main() {
const TextSelection( const TextSelection(
baseOffset: 0, baseOffset: 0,
extentOffset: testText.length, extentOffset: testText.length,
affinity: TextAffinity.downstream, affinity: TextAffinity.upstream,
), ),
), ),
reason: 'on $platform', reason: 'on $platform',
...@@ -7891,6 +7891,153 @@ void main() { ...@@ -7891,6 +7891,153 @@ void main() {
// On web, using keyboard for selection is handled by the browser. // On web, using keyboard for selection is handled by the browser.
}, variant: TargetPlatformVariant.all(), skip: kIsWeb); // [intended] }, variant: TargetPlatformVariant.all(), skip: kIsWeb); // [intended]
testWidgets('navigating multiline text', (WidgetTester tester) async {
const String multilineText = 'word word word\nword word\nword';
final TextEditingController controller = TextEditingController(text: multilineText);
// wo|rd wo|rd
controller.selection = const TextSelection(
baseOffset: 17,
extentOffset: 22,
affinity: TextAffinity.upstream,
);
await tester.pumpWidget(MaterialApp(
home: Align(
alignment: Alignment.topLeft,
child: SizedBox(
width: 400,
child: EditableText(
maxLines: 10,
controller: controller,
autofocus: true,
focusNode: focusNode,
style: Typography.material2018(platform: TargetPlatform.android).black.subtitle1!,
cursorColor: Colors.blue,
backgroundCursorColor: Colors.grey,
keyboardType: TextInputType.text,
),
),
),
));
await tester.pump(); // Wait for autofocus to take effect.
expect(controller.selection.isCollapsed, false);
expect(controller.selection.baseOffset, 17);
expect(controller.selection.extentOffset, 22);
// Multiple expandRightByLine shortcuts only move to the end of the line and
// not to the next line.
await sendKeys(
tester,
<LogicalKeyboardKey>[
LogicalKeyboardKey.arrowRight,
LogicalKeyboardKey.arrowRight,
LogicalKeyboardKey.arrowRight,
],
shift: true,
lineModifier: true,
targetPlatform: defaultTargetPlatform,
);
expect(controller.selection.isCollapsed, false);
expect(controller.selection.baseOffset, 17);
expect(controller.selection.extentOffset, 24);
// Multiple expandLeftByLine shortcuts only move ot the start of the line
// and not to the previous line.
await sendKeys(
tester,
<LogicalKeyboardKey>[
LogicalKeyboardKey.arrowLeft,
LogicalKeyboardKey.arrowLeft,
LogicalKeyboardKey.arrowLeft,
],
shift: true,
lineModifier: true,
targetPlatform: defaultTargetPlatform,
);
expect(controller.selection.isCollapsed, false);
expect(controller.selection.baseOffset, 15);
expect(controller.selection.extentOffset, 24);
// Set the caret to the end of a line.
controller.selection = const TextSelection(
baseOffset: 24,
extentOffset: 24,
affinity: TextAffinity.upstream,
);
await tester.pump();
expect(controller.selection.isCollapsed, true);
expect(controller.selection.baseOffset, 24);
expect(controller.selection.extentOffset, 24);
// Can't expand right by line any further.
await sendKeys(
tester,
<LogicalKeyboardKey>[
LogicalKeyboardKey.arrowRight,
],
shift: true,
lineModifier: true,
targetPlatform: defaultTargetPlatform,
);
expect(controller.selection.isCollapsed, true);
expect(controller.selection.baseOffset, 24);
expect(controller.selection.extentOffset, 24);
// Can select the entire line from the end.
await sendKeys(
tester,
<LogicalKeyboardKey>[
LogicalKeyboardKey.arrowLeft,
],
shift: true,
lineModifier: true,
targetPlatform: defaultTargetPlatform,
);
expect(controller.selection.isCollapsed, false);
expect(controller.selection.baseOffset, 24);
expect(controller.selection.extentOffset, 15);
// Set the caret to the start of a line.
controller.selection = const TextSelection(
baseOffset: 15,
extentOffset: 15,
affinity: TextAffinity.upstream,
);
await tester.pump();
expect(controller.selection.isCollapsed, true);
expect(controller.selection.baseOffset, 15);
expect(controller.selection.extentOffset, 15);
// Can't expand let any further.
await sendKeys(
tester,
<LogicalKeyboardKey>[
LogicalKeyboardKey.arrowLeft,
],
shift: true,
lineModifier: true,
targetPlatform: defaultTargetPlatform,
);
expect(controller.selection.isCollapsed, true);
expect(controller.selection.baseOffset, 15);
expect(controller.selection.extentOffset, 15);
// Can select the entire line from the start.
await sendKeys(
tester,
<LogicalKeyboardKey>[
LogicalKeyboardKey.arrowRight,
],
shift: true,
lineModifier: true,
targetPlatform: defaultTargetPlatform,
);
expect(controller.selection.isCollapsed, false);
expect(controller.selection.baseOffset, 15);
expect(controller.selection.extentOffset, 24);
// On web, using keyboard for selection is handled by the browser.
}, variant: TargetPlatformVariant.all(), skip: kIsWeb); // [intended]
testWidgets('expanding selection to start/end', (WidgetTester tester) async { testWidgets('expanding selection to start/end', (WidgetTester tester) async {
final TextEditingController controller = TextEditingController(text: 'word word word'); final TextEditingController controller = TextEditingController(text: 'word word word');
// word wo|rd| word // word wo|rd| word
......
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