From c61b9501e3e0f1b87f2e9876ebfc7d1044de285e Mon Sep 17 00:00:00 2001 From: LongCatIsLooong <31859944+LongCatIsLooong@users.noreply.github.com> Date: Fri, 16 Feb 2024 15:40:26 -0800 Subject: [PATCH] Don't paint the cursor for an invalid selection (#143533) Fixes https://github.com/flutter/flutter/issues/79495 This is basically a reland of https://github.com/flutter/flutter/pull/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. --- .../flutter/lib/src/rendering/editable.dart | 3 +- .../test/material/text_field_test.dart | 22 ++++---- .../flutter/test/rendering/editable_test.dart | 55 +++++++++++++++++++ 3 files changed, 68 insertions(+), 12 deletions(-) diff --git a/packages/flutter/lib/src/rendering/editable.dart b/packages/flutter/lib/src/rendering/editable.dart index 5c75d7b878..f17d587a0e 100644 --- a/packages/flutter/lib/src/rendering/editable.dart +++ b/packages/flutter/lib/src/rendering/editable.dart @@ -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; } diff --git a/packages/flutter/test/material/text_field_test.dart b/packages/flutter/test/material/text_field_test.dart index 8a4ac8b0f4..c7a0a4509c 100644 --- a/packages/flutter/test/material/text_field_test.dart +++ b/packages/flutter/test/material/text_field_test.dart @@ -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( @@ -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); @@ -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( @@ -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); diff --git a/packages/flutter/test/rendering/editable_test.dart b/packages/flutter/test/rendering/editable_test.dart index 7f1613778f..44bbd34b4e 100644 --- a/packages/flutter/test/rendering/editable_test.dart +++ b/packages/flutter/test/rendering/editable_test.dart @@ -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 showCursor = ValueNotifier(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()