From d3bdbed4985ca2ab174d65a6f556bd18ac03be5e Mon Sep 17 00:00:00 2001 From: LongCatIsLooong <31859944+LongCatIsLooong@users.noreply.github.com> Date: Fri, 6 Aug 2021 17:45:02 -0700 Subject: [PATCH] [EditableText] preserve selection/composition range on unfocus (#86796) --- .../flutter/lib/src/cupertino/text_field.dart | 16 ++++++- .../flutter/lib/src/material/text_field.dart | 19 +++++++- .../lib/src/widgets/editable_text.dart | 6 +-- .../lib/src/widgets/focus_manager.dart | 1 + .../test/material/text_field_focus_test.dart | 3 ++ .../test/material/text_field_test.dart | 33 +------------- .../material/text_selection_theme_test.dart | 34 +++++++++++---- .../test/widgets/editable_text_test.dart | 43 +++++++++++++++++++ .../test/widgets/selectable_text_test.dart | 39 +---------------- 9 files changed, 110 insertions(+), 84 deletions(-) diff --git a/packages/flutter/lib/src/cupertino/text_field.dart b/packages/flutter/lib/src/cupertino/text_field.dart index 914caaef7e4..cbfe083e6c7 100644 --- a/packages/flutter/lib/src/cupertino/text_field.dart +++ b/packages/flutter/lib/src/cupertino/text_field.dart @@ -884,6 +884,11 @@ class _CupertinoTextFieldState extends State with Restoratio _controller!.dispose(); _controller = null; } + + if (widget.focusNode != oldWidget.focusNode) { + (oldWidget.focusNode ?? _focusNode)?.removeListener(_handleFocusChanged); + (widget.focusNode ?? _focusNode)?.addListener(_handleFocusChanged); + } _effectiveFocusNode.canRequestFocus = widget.enabled ?? true; } @@ -915,6 +920,7 @@ class _CupertinoTextFieldState extends State with Restoratio @override void dispose() { + _effectiveFocusNode.removeListener(_handleFocusChanged); _focusNode?.dispose(); _controller?.dispose(); super.dispose(); @@ -926,6 +932,13 @@ class _CupertinoTextFieldState extends State with Restoratio _editableText.requestKeyboard(); } + void _handleFocusChanged() { + setState(() { + // Rebuild the widget on focus change to show/hide the text selection + // highlight. + }); + } + bool _shouldShowSelectionHandles(SelectionChangedCause? cause) { // When the text field is activated by something that doesn't trigger the // selection overlay, we shouldn't show the handles either. @@ -1202,7 +1215,8 @@ class _CupertinoTextFieldState extends State with Restoratio maxLines: widget.maxLines, minLines: widget.minLines, expands: widget.expands, - selectionColor: selectionColor, + // Only show the selection highlight when the text field is focused. + selectionColor: _effectiveFocusNode.hasFocus ? selectionColor : null, selectionControls: widget.selectionEnabled ? textSelectionControls : null, onChanged: widget.onChanged, diff --git a/packages/flutter/lib/src/material/text_field.dart b/packages/flutter/lib/src/material/text_field.dart index c85b97768e1..dc67d74080c 100644 --- a/packages/flutter/lib/src/material/text_field.dart +++ b/packages/flutter/lib/src/material/text_field.dart @@ -985,6 +985,7 @@ class _TextFieldState extends State with RestorationMixin implements _createLocalController(); } _effectiveFocusNode.canRequestFocus = _isEnabled; + _effectiveFocusNode.addListener(_handleFocusChanged); } bool get _canRequestFocus { @@ -1013,7 +1014,14 @@ class _TextFieldState extends State with RestorationMixin implements _controller!.dispose(); _controller = null; } + + if (widget.focusNode != oldWidget.focusNode) { + (oldWidget.focusNode ?? _focusNode)?.removeListener(_handleFocusChanged); + (widget.focusNode ?? _focusNode)?.addListener(_handleFocusChanged); + } + _effectiveFocusNode.canRequestFocus = _canRequestFocus; + if (_effectiveFocusNode.hasFocus && widget.readOnly != oldWidget.readOnly && _isEnabled) { if(_effectiveController.selection.isCollapsed) { _showSelectionHandles = !widget.readOnly; @@ -1048,6 +1056,7 @@ class _TextFieldState extends State with RestorationMixin implements @override void dispose() { + _effectiveFocusNode.removeListener(_handleFocusChanged); _focusNode?.dispose(); _controller?.dispose(); super.dispose(); @@ -1083,6 +1092,13 @@ class _TextFieldState extends State with RestorationMixin implements return false; } + void _handleFocusChanged() { + setState(() { + // Rebuild the widget on focus change to show/hide the text selection + // highlight. + }); + } + void _handleSelectionChanged(TextSelection selection, SelectionChangedCause? cause) { final bool willShowSelectionHandles = _shouldShowSelectionHandles(cause); if (willShowSelectionHandles != _showSelectionHandles) { @@ -1239,7 +1255,8 @@ class _TextFieldState extends State with RestorationMixin implements maxLines: widget.maxLines, minLines: widget.minLines, expands: widget.expands, - selectionColor: selectionColor, + // Only show the selection highlight when the text field is focused. + selectionColor: focusNode.hasFocus ? selectionColor : null, selectionControls: widget.selectionEnabled ? textSelectionControls : null, onChanged: widget.onChanged, onSelectionChanged: _handleSelectionChanged, diff --git a/packages/flutter/lib/src/widgets/editable_text.dart b/packages/flutter/lib/src/widgets/editable_text.dart index d84b6b386b2..5457b477bad 100644 --- a/packages/flutter/lib/src/widgets/editable_text.dart +++ b/packages/flutter/lib/src/widgets/editable_text.dart @@ -2454,9 +2454,7 @@ class EditableTextState extends State with AutomaticKeepAliveClien } } else { WidgetsBinding.instance!.removeObserver(this); - // Clear the selection and composition state if this widget lost focus. - _value = TextEditingValue(text: _value.text); - _currentPromptRectRange = null; + setState(() { _currentPromptRectRange = null; }); } updateKeepAlive(); } @@ -2754,7 +2752,7 @@ class EditableTextState extends State with AutomaticKeepAliveClien return widget.controller.buildTextSpan( context: context, style: widget.style, - withComposing: !widget.readOnly, + withComposing: !widget.readOnly && _hasFocus, ); } } diff --git a/packages/flutter/lib/src/widgets/focus_manager.dart b/packages/flutter/lib/src/widgets/focus_manager.dart index c12123d6e23..9efd4b7f1bc 100644 --- a/packages/flutter/lib/src/widgets/focus_manager.dart +++ b/packages/flutter/lib/src/widgets/focus_manager.dart @@ -1868,6 +1868,7 @@ class FocusManager with DiagnosticableTreeMixin, ChangeNotifier { _primaryFocus = _markedForFocus; _markedForFocus = null; } + assert(_markedForFocus == null); if (previousFocus != _primaryFocus) { assert(_focusDebug('Updating focus from $previousFocus to $_primaryFocus')); if (previousFocus != null) { diff --git a/packages/flutter/test/material/text_field_focus_test.dart b/packages/flutter/test/material/text_field_focus_test.dart index b4937a76c41..f22d118bab7 100644 --- a/packages/flutter/test/material/text_field_focus_test.dart +++ b/packages/flutter/test/material/text_field_focus_test.dart @@ -120,6 +120,9 @@ void main() { await tester.idle(); expect(tester.testTextInput.isVisible, isTrue); + // Prevent the gesture recognizer from recognizing the next tap as a + // double-tap. + await tester.pump(const Duration(seconds: 1)); tester.testTextInput.hide(); final EditableTextState state = tester.state(find.byType(EditableText)); diff --git a/packages/flutter/test/material/text_field_test.dart b/packages/flutter/test/material/text_field_test.dart index e16d93f8dca..21899d447f4 100644 --- a/packages/flutter/test/material/text_field_test.dart +++ b/packages/flutter/test/material/text_field_test.dart @@ -3948,37 +3948,6 @@ void main() { feedback.dispose(); }); - testWidgets('Text field drops selection when losing focus', (WidgetTester tester) async { - final Key key1 = UniqueKey(); - final TextEditingController controller1 = TextEditingController(); - final Key key2 = UniqueKey(); - - await tester.pumpWidget( - overlay( - child: Column( - children: [ - TextField( - key: key1, - controller: controller1, - ), - TextField(key: key2), - ], - ), - ), - ); - - await tester.tap(find.byKey(key1)); - await tester.enterText(find.byKey(key1), 'abcd'); - await tester.pump(); - controller1.selection = const TextSelection(baseOffset: 0, extentOffset: 3); - await tester.pump(); - expect(controller1.selection, isNot(equals(TextRange.empty))); - - await tester.tap(find.byKey(key2)); - await tester.pump(); - expect(controller1.selection, equals(TextRange.empty)); - }); - testWidgets('Selection is consistent with text length', (WidgetTester tester) async { final TextEditingController controller = TextEditingController(); @@ -5509,7 +5478,7 @@ void main() { } await tester.sendKeyUpEvent(LogicalKeyboardKey.shift); - expect(c1.selection.extentOffset - c1.selection.baseOffset, 0); + expect(c1.selection.extentOffset - c1.selection.baseOffset, -5); expect(c2.selection.extentOffset - c2.selection.baseOffset, -5); }, skip: areKeyEventsHandledByPlatform, // [intended] only applies to platforms where we handle key events. diff --git a/packages/flutter/test/material/text_selection_theme_test.dart b/packages/flutter/test/material/text_selection_theme_test.dart index 633d48e21c9..b815cde0655 100644 --- a/packages/flutter/test/material/text_selection_theme_test.dart +++ b/packages/flutter/test/material/text_selection_theme_test.dart @@ -54,23 +54,29 @@ void main() { }); testWidgets('Empty textSelectionTheme will use defaults', (WidgetTester tester) async { - const Color defaultCursorColor = Color(0x002196f3); + const Color defaultCursorColor = Color(0xff2196f3); const Color defaultSelectionColor = Color(0x662196f3); const Color defaultSelectionHandleColor = Color(0xff2196f3); + EditableText.debugDeterministicCursor = true; + addTearDown(() { + EditableText.debugDeterministicCursor = false; + }); // Test TextField's cursor & selection color. await tester.pumpWidget( const MaterialApp( home: Material( - child: TextField(), + child: TextField(autofocus: true), ), ), ); + await tester.pump(); await tester.pumpAndSettle(); + final EditableTextState editableTextState = tester.firstState(find.byType(EditableText)); final RenderEditable renderEditable = editableTextState.renderEditable; expect(renderEditable.cursorColor, defaultCursorColor); - expect(Color(renderEditable.selectionColor!.value), defaultSelectionColor); + expect(renderEditable.selectionColor?.value, defaultSelectionColor.value); // Test the selection handle color. await tester.pumpWidget( @@ -104,19 +110,25 @@ void main() { textSelectionTheme: textSelectionTheme, ); + EditableText.debugDeterministicCursor = true; + addTearDown(() { + EditableText.debugDeterministicCursor = false; + }); + // Test TextField's cursor & selection color. await tester.pumpWidget( MaterialApp( theme: theme, home: const Material( - child: TextField(), + child: TextField(autofocus: true), ), ), ); - await tester.pumpAndSettle(); + await tester.pump(); + final EditableTextState editableTextState = tester.firstState(find.byType(EditableText)); final RenderEditable renderEditable = editableTextState.renderEditable; - expect(renderEditable.cursorColor, textSelectionTheme.cursorColor!.withAlpha(0)); + expect(renderEditable.cursorColor, textSelectionTheme.cursorColor); expect(renderEditable.selectionColor, textSelectionTheme.selectionColor); // Test the selection handle color. @@ -157,6 +169,10 @@ void main() { selectionHandleColor: Color(0x00ffeedd), ); + EditableText.debugDeterministicCursor = true; + addTearDown(() { + EditableText.debugDeterministicCursor = false; + }); // Test TextField's cursor & selection color. await tester.pumpWidget( MaterialApp( @@ -164,15 +180,15 @@ void main() { home: const Material( child: TextSelectionTheme( data: widgetTextSelectionTheme, - child: TextField(), + child: TextField(autofocus: true), ), ), ), ); - await tester.pumpAndSettle(); + await tester.pump(); final EditableTextState editableTextState = tester.firstState(find.byType(EditableText)); final RenderEditable renderEditable = editableTextState.renderEditable; - expect(renderEditable.cursorColor, widgetTextSelectionTheme.cursorColor!.withAlpha(0)); + expect(renderEditable.cursorColor, widgetTextSelectionTheme.cursorColor); expect(renderEditable.selectionColor, widgetTextSelectionTheme.selectionColor); // Test the selection handle color. diff --git a/packages/flutter/test/widgets/editable_text_test.dart b/packages/flutter/test/widgets/editable_text_test.dart index d5e9200c983..57d3f63feda 100644 --- a/packages/flutter/test/widgets/editable_text_test.dart +++ b/packages/flutter/test/widgets/editable_text_test.dart @@ -516,6 +516,45 @@ void main() { expect(tester.testTextInput.setClientArgs!['inputAction'], equals('TextInputAction.newline')); }); + testWidgets('selection persists when unfocused', (WidgetTester tester) async { + const TextEditingValue value = TextEditingValue( + text: 'test test', + selection: TextSelection(affinity: TextAffinity.upstream, baseOffset: 5, extentOffset: 7), + ); + controller.value = value; + await tester.pumpWidget( + MediaQuery( + data: const MediaQueryData(devicePixelRatio: 1.0), + child: Directionality( + textDirection: TextDirection.ltr, + child: EditableText( + controller: controller, + backgroundCursorColor: Colors.grey, + focusNode: focusNode, + keyboardType: TextInputType.multiline, + style: textStyle, + cursorColor: cursorColor, + ), + ), + ), + ); + + expect(controller.value, value); + expect(focusNode.hasFocus, isFalse); + + focusNode.requestFocus(); + await tester.pump(); + + expect(controller.value, value); + expect(focusNode.hasFocus, isTrue); + + focusNode.unfocus(); + await tester.pump(); + + expect(controller.value, value); + expect(focusNode.hasFocus, isFalse); + }); + testWidgets('visiblePassword keyboard is requested when set explicitly', (WidgetTester tester) async { await tester.pumpWidget( MediaQuery( @@ -3906,6 +3945,8 @@ void main() { )); assert(focusNode.hasFocus); + // Autofocus has a one frame delay. + await tester.pump(); final RenderEditable renderEditable = findRenderEditable(tester); // The actual text span is split into 3 parts with the middle part underlined. @@ -3915,6 +3956,8 @@ void main() { expect(textSpan.style!.decoration, TextDecoration.underline); focusNode.unfocus(); + // Drain microtasks. + await tester.idle(); await tester.pump(); expect((renderEditable.text! as TextSpan).children, isNull); diff --git a/packages/flutter/test/widgets/selectable_text_test.dart b/packages/flutter/test/widgets/selectable_text_test.dart index 0242e3176a2..2132e920199 100644 --- a/packages/flutter/test/widgets/selectable_text_test.dart +++ b/packages/flutter/test/widgets/selectable_text_test.dart @@ -1388,42 +1388,7 @@ void main() { expect(topLeft.dx, equals(399.0)); }); - testWidgets('Selectable text drops selection when losing focus', (WidgetTester tester) async { - final Key key1 = UniqueKey(); - final Key key2 = UniqueKey(); - - await tester.pumpWidget( - overlay( - child: Column( - children: [ - SelectableText( - 'text 1', - key: key1, - ), - SelectableText( - 'text 2', - key: key2, - ), - ], - ), - ), - ); - - await tester.tap(find.byKey(key1)); - await tester.pump(); - final EditableText editableTextWidget = tester.widget(find.byType(EditableText).first); - final TextEditingController controller = editableTextWidget.controller; - controller.selection = const TextSelection(baseOffset: 0, extentOffset: 3); - await tester.pump(); - expect(controller.selection, isNot(equals(TextRange.empty))); - - await tester.tap(find.byKey(key2)); - await tester.pump(); - expect(controller.selection, equals(TextRange.empty)); - }); - - testWidgets('Selectable text is skipped during focus traversal', - (WidgetTester tester) async { + testWidgets('Selectable text is skipped during focus traversal', (WidgetTester tester) async { final FocusNode firstFieldFocus = FocusNode(); final FocusNode lastFieldFocus = FocusNode(); @@ -2028,7 +1993,7 @@ void main() { await tester.sendKeyUpEvent(LogicalKeyboardKey.shift); await tester.pumpAndSettle(); - expect(c1.selection.extentOffset - c1.selection.baseOffset, 0); + expect(c1.selection.extentOffset - c1.selection.baseOffset, -5); expect(c2.selection.extentOffset - c2.selection.baseOffset, -5); }, variant: KeySimulatorTransitModeVariant.all());