From 6c12e399474fafb8aac317fae5e2dbe0efae44f4 Mon Sep 17 00:00:00 2001 From: Renzo Olivares Date: Mon, 30 Jan 2023 14:44:26 -0800 Subject: [PATCH] Introduce ParagraphBoundary subclass for text editing (#116549) * attempt to extend to paragraph * second attempt * clean up implementation * clean up * updates * updates * Fix implementation * remove old * update docs * update docs * fix analyzer * Fix bug where new line character was selected and backwards selection failed * remove print * Add test for paragraph boundary * Add text editing test for extending selection to paragraph for mac and ios * rename to ExtendSelectionToParagraphBoundaryIntent * fix analyzer * Should default to downstream when collapsing selection * get rid of _getParagraphAtOffset and move into getTextBoundaryAt * Search for all line terminators * iterate through code units instead of characters * Address some reviewer comments" * Add separate implementations for leading and trailing paragraph boundary methods * Do not break after a carriage return if it is followed by a line feed * test carriage return followed by a line feed * more tests * Do not continue if the line terminator is at the target text offset * add hack to extend highlight to line terminator * Revert "add hack to extend highlight to line terminator" This reverts commit b4d3c434539b66c3c81c215e87c645b425902825. * Revert "Do not continue if the line terminator is at the target text offset" This reverts commit 789e1b838e54e7c25600bfa8852e59431ccaf5dc. * Update ParagraphBoundary with latest TextBoundary changes * Update implementation to iterate through indexes * update getTrailingTextBoundaryAt to include the line terminator * Updates * more updates * more updates * updates * updates * Lets try this again * clean up * updates * more updates * updates * fix * Re-implement using custom paragraph boundary applying method * Revert "Re-implement using custom paragraph boundary applying method" This reverts commit cd2f7f4b6eb6726b28f82a43708812e06a49df95. * Revert "fix" This reverts commit 8ec1f8f58935cfb3eb86dc6afd2894537af4cf7b. * updates * Revert "updates" This reverts commit 9dcca4a0031fe18ada9d6ffbbe77ba09918e82ae. * Revert "Revert "fix"" This reverts commit 9cc1332cd3041badc472d0d223a106203e46afb8. * Revert "Revert "Re-implement using custom paragraph boundary applying method"" This reverts commit 1acb606fb743fd840da20cca26d9a7c26accb71d. * Fix paragraph boundaries * Add failing test * Address some comments * group tests and fix analyzer * fix typo * fix remaining test * updates * more fixes and logs * clean up and add another test * Fix last test * Add new test * Clean up * more clean up * clean up comments * address comments * updates * return null when position is out of bounds and 0 or end of text if appropriate * Clean up cases * Do not return null when OOB in the direction of iteration * clean up * simplify implementation thanks to LongCatIsLooong feedback * Address comments * Add line and paragraph separator * Use _moveBeyondTextBoundary instead of custom _moveToParagraphBoundary * Change some intent names and revert fromPosition change * clean up docs --------- Co-authored-by: Renzo Olivares --- .../lib/src/services/text_boundary.dart | 80 ++++++++- .../lib/src/services/text_layout_metrics.dart | 19 +++ .../default_text_editing_shortcuts.dart | 8 +- .../lib/src/widgets/editable_text.dart | 2 + .../lib/src/widgets/text_editing_intents.dart | 15 ++ .../test/services/text_boundary_test.dart | 107 ++++++++++++ .../test/widgets/editable_text_test.dart | 155 ++++++++++++++++++ 7 files changed, 381 insertions(+), 5 deletions(-) diff --git a/packages/flutter/lib/src/services/text_boundary.dart b/packages/flutter/lib/src/services/text_boundary.dart index e1f4a77ba8..efbea24505 100644 --- a/packages/flutter/lib/src/services/text_boundary.dart +++ b/packages/flutter/lib/src/services/text_boundary.dart @@ -123,9 +123,87 @@ class LineBoundary extends TextBoundary { TextRange getTextBoundaryAt(int position) => _textLayout.getLineAtOffset(TextPosition(offset: max(position, 0))); } +/// A text boundary that uses paragraphs as logical boundaries. +/// +/// A paragraph is defined as the range between line terminators. If no +/// line terminators exist then the paragraph boundary is the entire document. +class ParagraphBoundary extends TextBoundary { + /// Creates a [ParagraphBoundary] with the text. + const ParagraphBoundary(this._text); + + final String _text; + + /// Returns the [int] representing the start position of the paragraph that + /// bounds the given `position`. The returned [int] is the position of the code unit + /// that follows the line terminator that encloses the desired paragraph. + @override + int? getLeadingTextBoundaryAt(int position) { + if (position < 0 || _text.isEmpty) { + return null; + } + + if (position >= _text.length) { + return _text.length; + } + + if (position == 0) { + return 0; + } + + final List codeUnits = _text.codeUnits; + int index = position; + + if (index > 1 && codeUnits[index] == 0xA && codeUnits[index - 1] == 0xD) { + index -= 2; + } else if (TextLayoutMetrics.isLineTerminator(codeUnits[index])) { + index -= 1; + } + + while (index > 0) { + if (TextLayoutMetrics.isLineTerminator(codeUnits[index])) { + return index + 1; + } + index -= 1; + } + + return max(index, 0); + } + + /// Returns the [int] representing the end position of the paragraph that + /// bounds the given `position`. The returned [int] is the position of the + /// code unit representing the trailing line terminator that encloses the + /// desired paragraph. + @override + int? getTrailingTextBoundaryAt(int position) { + if (position >= _text.length || _text.isEmpty) { + return null; + } + + if (position < 0) { + return 0; + } + + final List codeUnits = _text.codeUnits; + int index = position; + + while (!TextLayoutMetrics.isLineTerminator(codeUnits[index])) { + index += 1; + if (index == codeUnits.length) { + return index; + } + } + + return index < codeUnits.length - 1 + && codeUnits[index] == 0xD + && codeUnits[index + 1] == 0xA + ? index + 2 + : index + 1; + } +} + /// A text boundary that uses the entire document as logical boundary. class DocumentBoundary extends TextBoundary { - /// Creates a [DocumentBoundary] with the text + /// Creates a [DocumentBoundary] with the text. const DocumentBoundary(this._text); final String _text; diff --git a/packages/flutter/lib/src/services/text_layout_metrics.dart b/packages/flutter/lib/src/services/text_layout_metrics.dart index c5bc3d1bab..f4ee0a7b9e 100644 --- a/packages/flutter/lib/src/services/text_layout_metrics.dart +++ b/packages/flutter/lib/src/services/text_layout_metrics.dart @@ -54,6 +54,25 @@ abstract class TextLayoutMetrics { return true; } + /// Check if the given code unit is a line terminator character. + /// + /// Includes newline characters from ASCII + /// (https://www.unicode.org/standard/reports/tr13/tr13-5.html). + static bool isLineTerminator(int codeUnit) { + switch (codeUnit) { + case 0xA: // line feed + case 0xB: // vertical feed + case 0xC: // form feed + case 0xD: // carriage return + case 0x85: // new line + case 0x2028: // line separator + case 0x2029: // paragraph separator + return true; + default: + return false; + } + } + /// {@template flutter.services.TextLayoutMetrics.getLineAtOffset} /// Return a [TextSelection] containing the line of the given [TextPosition]. /// {@endtemplate} diff --git a/packages/flutter/lib/src/widgets/default_text_editing_shortcuts.dart b/packages/flutter/lib/src/widgets/default_text_editing_shortcuts.dart index a670037543..51f9569df1 100644 --- a/packages/flutter/lib/src/widgets/default_text_editing_shortcuts.dart +++ b/packages/flutter/lib/src/widgets/default_text_editing_shortcuts.dart @@ -303,8 +303,8 @@ class DefaultTextEditingShortcuts extends StatelessWidget { const SingleActivator(LogicalKeyboardKey.arrowLeft, shift: true, alt: true): const ExtendSelectionToNextWordBoundaryOrCaretLocationIntent(forward: false), const SingleActivator(LogicalKeyboardKey.arrowRight, shift: true, alt: true): const ExtendSelectionToNextWordBoundaryOrCaretLocationIntent(forward: true), - const SingleActivator(LogicalKeyboardKey.arrowUp, shift: true, alt: true): const ExtendSelectionVerticallyToAdjacentLineIntent(forward: false, collapseSelection: false), - const SingleActivator(LogicalKeyboardKey.arrowDown, shift: true, alt: true): const ExtendSelectionVerticallyToAdjacentLineIntent(forward: true, collapseSelection: false), + const SingleActivator(LogicalKeyboardKey.arrowUp, shift: true, alt: true): const ExtendSelectionToNextParagraphBoundaryOrCaretLocationIntent(forward: false), + const SingleActivator(LogicalKeyboardKey.arrowDown, shift: true, alt: true): const ExtendSelectionToNextParagraphBoundaryOrCaretLocationIntent(forward: true), const SingleActivator(LogicalKeyboardKey.arrowLeft, meta: true): const ExtendSelectionToLineBreakIntent(forward: false, collapseSelection: true), const SingleActivator(LogicalKeyboardKey.arrowRight, meta: true): const ExtendSelectionToLineBreakIntent(forward: true, collapseSelection: true), @@ -560,8 +560,8 @@ Intent? intentForMacOSSelector(String selectorName) { 'moveWordLeftAndModifySelection:': ExtendSelectionToNextWordBoundaryOrCaretLocationIntent(forward: false), 'moveWordRightAndModifySelection:': ExtendSelectionToNextWordBoundaryOrCaretLocationIntent(forward: true), - 'moveParagraphBackwardAndModifySelection:': ExtendSelectionToLineBreakIntent(forward: false, collapseSelection: false, collapseAtReversal: true), - 'moveParagraphForwardAndModifySelection:': ExtendSelectionToLineBreakIntent(forward: true, collapseSelection: false, collapseAtReversal: true), + 'moveParagraphBackwardAndModifySelection:': ExtendSelectionToNextParagraphBoundaryOrCaretLocationIntent(forward: false), + 'moveParagraphForwardAndModifySelection:': ExtendSelectionToNextParagraphBoundaryOrCaretLocationIntent(forward: true), 'moveToLeftEndOfLine:': ExtendSelectionToLineBreakIntent(forward: false, collapseSelection: true), 'moveToRightEndOfLine:': ExtendSelectionToLineBreakIntent(forward: true, collapseSelection: true), diff --git a/packages/flutter/lib/src/widgets/editable_text.dart b/packages/flutter/lib/src/widgets/editable_text.dart index 01e5111d8a..3b7197609e 100644 --- a/packages/flutter/lib/src/widgets/editable_text.dart +++ b/packages/flutter/lib/src/widgets/editable_text.dart @@ -3978,6 +3978,7 @@ class EditableTextState extends State with AutomaticKeepAliveClien TextBoundary _characterBoundary() => widget.obscureText ? _CodeUnitBoundary(_value.text) : CharacterBoundary(_value.text); TextBoundary _nextWordBoundary() => widget.obscureText ? _documentBoundary() : renderEditable.wordBoundaries.moveByWordBoundary; TextBoundary _linebreak() => widget.obscureText ? _documentBoundary() : LineBoundary(renderEditable); + TextBoundary _paragraphBoundary() => ParagraphBoundary(_value.text); TextBoundary _documentBoundary() => DocumentBoundary(_value.text); Action _makeOverridable(Action defaultAction) { @@ -4218,6 +4219,7 @@ class EditableTextState extends State with AutomaticKeepAliveClien ExtendSelectionToLineBreakIntent: _makeOverridable(_UpdateTextSelectionAction(this, _linebreak, _moveToTextBoundary, ignoreNonCollapsedSelection: true)), ExtendSelectionVerticallyToAdjacentLineIntent: _makeOverridable(_verticalSelectionUpdateAction), ExtendSelectionVerticallyToAdjacentPageIntent: _makeOverridable(_verticalSelectionUpdateAction), + ExtendSelectionToNextParagraphBoundaryOrCaretLocationIntent: _makeOverridable(_UpdateTextSelectionAction(this, _paragraphBoundary, _moveBeyondTextBoundary, ignoreNonCollapsedSelection: true)), ExtendSelectionToDocumentBoundaryIntent: _makeOverridable(_UpdateTextSelectionAction(this, _documentBoundary, _moveBeyondTextBoundary, ignoreNonCollapsedSelection: true)), ExtendSelectionToNextWordBoundaryOrCaretLocationIntent: _makeOverridable(_UpdateTextSelectionAction(this, _nextWordBoundary, _moveBeyondTextBoundary, ignoreNonCollapsedSelection: true)), ScrollToDocumentBoundaryIntent: _makeOverridable(CallbackAction(onInvoke: _scrollToDocumentBoundary)), diff --git a/packages/flutter/lib/src/widgets/text_editing_intents.dart b/packages/flutter/lib/src/widgets/text_editing_intents.dart index 40d7394c78..b1a13335f5 100644 --- a/packages/flutter/lib/src/widgets/text_editing_intents.dart +++ b/packages/flutter/lib/src/widgets/text_editing_intents.dart @@ -221,6 +221,21 @@ class ExtendSelectionVerticallyToAdjacentPageIntent extends DirectionalCaretMove }) : super(forward, collapseSelection); } +/// Extends, or moves the current selection from the current +/// [TextSelection.extent] position to the previous or the next paragraph +/// boundary depending on the [forward] parameter. +/// +/// This [Intent] collapses the selection when the order of [TextSelection.base] +/// and [TextSelection.extent] would reverse. +/// +/// This is typically only used on MacOS. +class ExtendSelectionToNextParagraphBoundaryOrCaretLocationIntent extends DirectionalCaretMovementIntent { + /// Creates an [ExtendSelectionToNextParagraphBoundaryOrCaretLocationIntent]. + const ExtendSelectionToNextParagraphBoundaryOrCaretLocationIntent({ + required bool forward, + }) : super(forward, false, true); +} + /// Extends, or moves the current selection from the current /// [TextSelection.extent] position to the start or the end of the document. /// diff --git a/packages/flutter/test/services/text_boundary_test.dart b/packages/flutter/test/services/text_boundary_test.dart index f02c0cf3ca..fe3fa133b8 100644 --- a/packages/flutter/test/services/text_boundary_test.dart +++ b/packages/flutter/test/services/text_boundary_test.dart @@ -146,6 +146,113 @@ void main() { expect(boundary.getTextBoundaryAt(3), TestTextLayoutMetrics.lineAt3); }); + group('paragraph boundary', () { + test('works for simple cases', () { + const String textA= 'abcd efg hi\njklmno\npqrstuv'; + const ParagraphBoundary boundaryA = ParagraphBoundary(textA); + + // Position enclosed inside of paragraph, 'abcd efg h|i\n'. + const int position = 10; + + // The range includes the line terminator. + expect(boundaryA.getLeadingTextBoundaryAt(position), 0); + expect(boundaryA.getTrailingTextBoundaryAt(position), 12); + + // This text includes a carriage return followed by a line feed. + const String textB = 'abcd efg hi\r\njklmno\npqrstuv'; + const ParagraphBoundary boundaryB = ParagraphBoundary(textB); + expect(boundaryB.getLeadingTextBoundaryAt(position), 0); + expect(boundaryB.getTrailingTextBoundaryAt(position), 13); + + const String textF = 'Now is the time for\n' // 20 + 'all good people\n' // 20 + 16 => 36 + 'to come to the aid\n' // 36 + 19 => 55 + 'of their country.'; // 55 + 17 => 72 + const ParagraphBoundary boundaryF = ParagraphBoundary(textF); + const int positionF = 11; + expect(boundaryF.getLeadingTextBoundaryAt(positionF), 0); + expect(boundaryF.getTrailingTextBoundaryAt(positionF), 20); + }); + + test('works for consecutive line terminators involving CRLF', () { + const String textI = 'Now is the time for\n' // 20 + 'all good people\n\r\n' // 20 + 16 => 38 + 'to come to the aid\n' // 38 + 19 => 57 + 'of their country.'; // 57 + 17 => 74 + const ParagraphBoundary boundaryI = ParagraphBoundary(textI); + const int positionI = 56;// \n at the end of the third line. + const int positionJ = 38;// t at beginning of third line. + const int positionK = 37;// \n at end of second line. + expect(boundaryI.getLeadingTextBoundaryAt(positionI), 38); + expect(boundaryI.getTrailingTextBoundaryAt(positionI), 57); + expect(boundaryI.getLeadingTextBoundaryAt(positionJ), 38); + expect(boundaryI.getTrailingTextBoundaryAt(positionJ), 57); + expect(boundaryI.getLeadingTextBoundaryAt(positionK), 36); + expect(boundaryI.getTrailingTextBoundaryAt(positionK), 38); + }); + + test('works for consecutive line terminators', () { + const String textI = 'Now is the time for\n' // 20 + 'all good people\n\n' // 20 + 16 => 37 + 'to come to the aid\n' // 37 + 19 => 56 + 'of their country.'; // 56 + 17 => 73 + const ParagraphBoundary boundaryI = ParagraphBoundary(textI); + const int positionI = 55;// \n at the end of the third line. + const int positionJ = 37;// t at beginning of third line. + const int positionK = 36;// \n at end of second line. + expect(boundaryI.getLeadingTextBoundaryAt(positionI), 37); + expect(boundaryI.getTrailingTextBoundaryAt(positionI), 56); + expect(boundaryI.getLeadingTextBoundaryAt(positionJ), 37); + expect(boundaryI.getTrailingTextBoundaryAt(positionJ), 56); + expect(boundaryI.getLeadingTextBoundaryAt(positionK), 36); + expect(boundaryI.getTrailingTextBoundaryAt(positionK), 37); + }); + + test('leading boundary works for consecutive CRLF', () { + // This text includes multiple consecutive carriage returns followed by line feeds (CRLF). + const String textH = 'abcd efg hi\r\n\r\n\r\n\r\n\r\n\r\n\r\n\n\n\n\n\njklmno\npqrstuv'; + const ParagraphBoundary boundaryH = ParagraphBoundary(textH); + const int positionH = 18; + expect(boundaryH.getLeadingTextBoundaryAt(positionH), 17); + expect(boundaryH.getTrailingTextBoundaryAt(positionH), 19); + }); + + test('trailing boundary works for consecutive CRLF', () { + // This text includes multiple consecutive carriage returns followed by line feeds (CRLF). + const String textG = 'abcd efg hi\r\n\n\n\n\n\n\r\n\r\n\r\n\r\n\n\n\n\n\njklmno\npqrstuv'; + const ParagraphBoundary boundaryG = ParagraphBoundary(textG); + const int positionG = 18; + expect(boundaryG.getLeadingTextBoundaryAt(positionG), 18); + expect(boundaryG.getTrailingTextBoundaryAt(positionG), 20); + }); + + test('works when position is between two CRLF', () { + const String textE = 'abcd efg hi\r\nhello\r\n\n'; + const ParagraphBoundary boundaryE = ParagraphBoundary(textE); + // Position enclosed inside of paragraph, 'abcd efg hi\r\nhello\r\n\n'. + const int positionE = 16; + expect(boundaryE.getLeadingTextBoundaryAt(positionE), 13); + expect(boundaryE.getTrailingTextBoundaryAt(positionE), 20); + }); + + test('works for multiple consecutive line terminators', () { + // This text includes multiple consecutive line terminators. + const String textC = 'abcd efg hi\r\n\n\n\n\n\n\n\n\n\n\n\njklmno\npqrstuv'; + const ParagraphBoundary boundaryC = ParagraphBoundary(textC); + // Position enclosed inside of paragraph, 'abcd efg hi\r\n\n\n\n\n\n|\n\n\n\n\n\njklmno\npqrstuv'. + const int positionC = 18; + expect(boundaryC.getLeadingTextBoundaryAt(positionC), 18); + expect(boundaryC.getTrailingTextBoundaryAt(positionC), 19); + + const String textD = 'abcd efg hi\r\n\n\n\n'; + const ParagraphBoundary boundaryD = ParagraphBoundary(textD); + // Position enclosed inside of paragraph, 'abcd efg hi\r\n\n|\n\n'. + const int positionD = 14; + expect(boundaryD.getLeadingTextBoundaryAt(positionD), 14); + expect(boundaryD.getTrailingTextBoundaryAt(positionD), 15); + }); + }); + test('document boundary works', () { const String text = 'abcd efg hi\njklmno\npqrstuv'; const DocumentBoundary boundary = DocumentBoundary(text); diff --git a/packages/flutter/test/widgets/editable_text_test.dart b/packages/flutter/test/widgets/editable_text_test.dart index 794171655d..16cfe506ed 100644 --- a/packages/flutter/test/widgets/editable_text_test.dart +++ b/packages/flutter/test/widgets/editable_text_test.dart @@ -6365,6 +6365,161 @@ void main() { ); expect(controller.text, equals(testText), reason: 'on $platform'); + final bool platformCanSelectByParagraph = defaultTargetPlatform == TargetPlatform.iOS || defaultTargetPlatform == TargetPlatform.macOS; + // Move down one paragraph. + await sendKeys( + tester, + [ + LogicalKeyboardKey.arrowDown, + ], + shift: true, + wordModifier: true, + targetPlatform: defaultTargetPlatform, + ); + + expect( + selection, + equals( + TextSelection( + baseOffset: 10, + extentOffset: platformCanSelectByParagraph ? 20 : 10, + ), + ), + reason: 'on $platform', + ); + + // Move down another paragraph. + await sendKeys( + tester, + [ + LogicalKeyboardKey.arrowDown, + ], + shift: true, + wordModifier: true, + targetPlatform: defaultTargetPlatform, + ); + + expect( + selection, + equals( + TextSelection( + baseOffset: 10, + extentOffset: platformCanSelectByParagraph ? 36 : 10, + ), + ), + reason: 'on $platform', + ); + + // Move down another paragraph. + await sendKeys( + tester, + [ + LogicalKeyboardKey.arrowDown, + ], + shift: true, + wordModifier: true, + targetPlatform: defaultTargetPlatform, + ); + + expect( + selection, + equals( + TextSelection( + baseOffset: 10, + extentOffset: platformCanSelectByParagraph ? 55 : 10, + ), + ), + reason: 'on $platform', + ); + + // Move up a paragraph. + await sendKeys( + tester, + [ + LogicalKeyboardKey.arrowUp, + ], + shift: true, + wordModifier: true, + targetPlatform: defaultTargetPlatform, + ); + + expect( + selection, + equals( + TextSelection( + baseOffset: 10, + extentOffset: platformCanSelectByParagraph ? 36 : 10, + ), + ), + reason: 'on $platform', + ); + + // Move up a paragraph. + await sendKeys( + tester, + [ + LogicalKeyboardKey.arrowUp, + ], + shift: true, + wordModifier: true, + targetPlatform: defaultTargetPlatform, + ); + + expect( + selection, + equals( + TextSelection( + baseOffset: 10, + extentOffset: platformCanSelectByParagraph ? 20 : 10, + ), + ), + reason: 'on $platform', + ); + + // Move up back to the origin. + await sendKeys( + tester, + [ + LogicalKeyboardKey.arrowUp, + ], + shift: true, + wordModifier: true, + targetPlatform: defaultTargetPlatform, + ); + + expect( + selection, + equals( + TextSelection( + baseOffset: 10, + extentOffset: platformCanSelectByParagraph ? 10 : 10, + ), + ), + reason: 'on $platform', + ); + + // Move up, extending the selection backwards to the next paragraph. + await sendKeys( + tester, + [ + LogicalKeyboardKey.arrowUp, + ], + shift: true, + wordModifier: true, + targetPlatform: defaultTargetPlatform, + ); + + expect( + selection, + equals( + TextSelection( + baseOffset: 10, + extentOffset: platformCanSelectByParagraph ? 0 : 10, + ), + ), + reason: 'on $platform', + ); + // Copy All await sendKeys( tester,