Skip to content

Commit

Permalink
Don't paint the cursor for an invalid selection (#143533)
Browse files Browse the repository at this point in the history
Fixes flutter/flutter#79495

This is basically a reland of flutter/flutter#79607.

Currently when the cursor is invalid, arrow key navigation / typing / backspacing doesn't work since the cursor position is unknown. 
Showing the cursor when the selection is invalid gives the user the wrong information about the current insert point in the text. 

This is going to break internal golden tests.
  • Loading branch information
LongCatIsLooong authored Feb 16, 2024
1 parent df4205e commit c61b950
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 12 deletions.
3 changes: 1 addition & 2 deletions packages/flutter/lib/src/rendering/editable.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2965,8 +2965,7 @@ class _CaretPainter extends RenderEditablePainter {

final TextSelection? selection = renderEditable.selection;

// TODO(LongCatIsLooong): skip painting caret when selection is (-1, -1): https://github.com/flutter/flutter/issues/79495
if (selection == null || !selection.isCollapsed) {
if (selection == null || !selection.isCollapsed || !selection.isValid) {
return;
}

Expand Down
22 changes: 12 additions & 10 deletions packages/flutter/test/material/text_field_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10868,6 +10868,7 @@ void main() {
expect(controller.value.text, testValueA);

final Offset firstLinePos = textOffsetToPosition(tester, 5);
final double lineHeight = findRenderEditable(tester).preferredLineHeight;

// Tap on text field to gain focus, and set selection to 'i|s' on the first line.
final TestGesture gesture = await tester.startGesture(
Expand Down Expand Up @@ -10902,35 +10903,35 @@ void main() {

// Drag, down after the triple tap, to select line by line.
// Moving down will extend the selection to the second line.
await gesture.moveTo(firstLinePos + const Offset(0, 10.0));
await gesture.moveTo(firstLinePos + Offset(0, lineHeight));
await tester.pumpAndSettle();

expect(controller.selection.baseOffset, 0);
expect(controller.selection.extentOffset, 35);

// Moving down will extend the selection to the third line.
await gesture.moveTo(firstLinePos + const Offset(0, 20.0));
await gesture.moveTo(firstLinePos + Offset(0, lineHeight * 2));
await tester.pumpAndSettle();

expect(controller.selection.baseOffset, 0);
expect(controller.selection.extentOffset, 54);

// Moving down will extend the selection to the last line.
await gesture.moveTo(firstLinePos + const Offset(0, 40.0));
await gesture.moveTo(firstLinePos + Offset(0, lineHeight * 4));
await tester.pumpAndSettle();

expect(controller.selection.baseOffset, 0);
expect(controller.selection.extentOffset, 72);

// Moving up will extend the selection to the third line.
await gesture.moveTo(firstLinePos + const Offset(0, 20.0));
await gesture.moveTo(firstLinePos + Offset(0, lineHeight * 2));
await tester.pumpAndSettle();

expect(controller.selection.baseOffset, 0);
expect(controller.selection.extentOffset, 54);

// Moving up will extend the selection to the second line.
await gesture.moveTo(firstLinePos + const Offset(0, 10.0));
await gesture.moveTo(firstLinePos + Offset(0, lineHeight * 1));
await tester.pumpAndSettle();

expect(controller.selection.baseOffset, 0);
Expand Down Expand Up @@ -10969,6 +10970,7 @@ void main() {
expect(controller.value.text, testValueA);

final Offset firstLinePos = textOffsetToPosition(tester, 5);
final double lineHeight = findRenderEditable(tester).preferredLineHeight;

// Tap on text field to gain focus, and set selection to 'i|s' on the first line.
final TestGesture gesture = await tester.startGesture(
Expand Down Expand Up @@ -11003,35 +11005,35 @@ void main() {

// Drag, down after the triple tap, to select paragraph by paragraph.
// Moving down will extend the selection to the second line.
await gesture.moveTo(firstLinePos + const Offset(0, 10.0));
await gesture.moveTo(firstLinePos + Offset(0, lineHeight));
await tester.pumpAndSettle();

expect(controller.selection.baseOffset, 0);
expect(controller.selection.extentOffset, 36);

// Moving down will extend the selection to the third line.
await gesture.moveTo(firstLinePos + const Offset(0, 20.0));
await gesture.moveTo(firstLinePos + Offset(0, lineHeight * 2));
await tester.pumpAndSettle();

expect(controller.selection.baseOffset, 0);
expect(controller.selection.extentOffset, 55);

// Moving down will extend the selection to the last line.
await gesture.moveTo(firstLinePos + const Offset(0, 40.0));
await gesture.moveTo(firstLinePos + Offset(0, lineHeight * 4));
await tester.pumpAndSettle();

expect(controller.selection.baseOffset, 0);
expect(controller.selection.extentOffset, 72);

// Moving up will extend the selection to the third line.
await gesture.moveTo(firstLinePos + const Offset(0, 20.0));
await gesture.moveTo(firstLinePos + Offset(0, lineHeight * 2));
await tester.pumpAndSettle();

expect(controller.selection.baseOffset, 0);
expect(controller.selection.extentOffset, 55);

// Moving up will extend the selection to the second line.
await gesture.moveTo(firstLinePos + const Offset(0, 10.0));
await gesture.moveTo(firstLinePos + Offset(0, lineHeight));
await tester.pumpAndSettle();

expect(controller.selection.baseOffset, 0);
Expand Down
55 changes: 55 additions & 0 deletions packages/flutter/test/rendering/editable_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,61 @@ void main() {
expect(editable, paintsExactlyCountTimes(#drawRect, 1));
});

test('does not paint the caret when selection is null or invalid', () async {
final TextSelectionDelegate delegate = _FakeEditableTextState();
final ValueNotifier<bool> showCursor = ValueNotifier<bool>(true);
final RenderEditable editable = RenderEditable(
backgroundCursorColor: Colors.grey,
selectionColor: Colors.black,
paintCursorAboveText: true,
textDirection: TextDirection.ltr,
cursorColor: Colors.red,
showCursor: showCursor,
offset: ViewportOffset.zero(),
textSelectionDelegate: delegate,
text: const TextSpan(
text: 'test',
style: TextStyle(height: 1.0, fontSize: 10.0),
),
startHandleLayerLink: LayerLink(),
endHandleLayerLink: LayerLink(),
selection: const TextSelection.collapsed(
offset: 2,
affinity: TextAffinity.upstream,
),
);

layout(editable);

expect(
editable,
paints
..paragraph()
// Red collapsed cursor is painted, not a selection box.
..rect(color: Colors.red[500]),
);

// Let the RenderEditable paint again. Setting the selection to null should
// prevent the caret from being painted.
editable.selection = null;
// Still paints the paragraph.
expect(editable, paints..paragraph());
// No longer paints the caret.
expect(editable, isNot(paints..rect(color: Colors.red[500])));

// Reset.
editable.selection = const TextSelection.collapsed(offset: 0);
expect(editable, paints..paragraph());
expect(editable, paints..rect(color: Colors.red[500]));

// Invalid cursor position.
editable.selection = const TextSelection.collapsed(offset: -1);
// Still paints the paragraph.
expect(editable, paints..paragraph());
// No longer paints the caret.
expect(editable, isNot(paints..rect(color: Colors.red[500])));
});

test('selects correct place with offsets', () {
const String text = 'test\ntest';
final _FakeEditableTextState delegate = _FakeEditableTextState()
Expand Down

0 comments on commit c61b950

Please sign in to comment.