From f8f5953d8002cb17b861486dff93e5456ca54efd Mon Sep 17 00:00:00 2001 From: Justin McCandless Date: Thu, 13 Dec 2018 11:09:13 -0800 Subject: [PATCH] obscureText and enableInteractiveSelection defaults (#24527) * obscureText true defaults to disabling selection * Tests and comments for selectable text field * Improve selection docs * Refactor so that all enableInteractiveSelection params are null by default, delegate to selectionEnabled * Fix selection param macros --- .../flutter/lib/src/material/text_field.dart | 18 +++--- .../flutter/lib/src/rendering/editable.dart | 25 ++++++-- .../lib/src/widgets/editable_text.dart | 20 ++++--- .../test/material/text_field_test.dart | 60 +++++++++++++++++++ 4 files changed, 101 insertions(+), 22 deletions(-) diff --git a/packages/flutter/lib/src/material/text_field.dart b/packages/flutter/lib/src/material/text_field.dart index ca5ce230549..170a478c6b6 100644 --- a/packages/flutter/lib/src/material/text_field.dart +++ b/packages/flutter/lib/src/material/text_field.dart @@ -92,8 +92,8 @@ class TextField extends StatefulWidget { /// switch to the [decoration.errorStyle] when the limit is exceeded. /// /// The [textAlign], [autofocus], [obscureText], [autocorrect], - /// [maxLengthEnforced], [scrollPadding], [maxLines], [maxLength], - /// and [enableInteractiveSelection] arguments must not be null. + /// [maxLengthEnforced], [scrollPadding], [maxLines], and [maxLength] + /// arguments must not be null. /// /// See also: /// @@ -126,7 +126,7 @@ class TextField extends StatefulWidget { this.cursorColor, this.keyboardAppearance, this.scrollPadding = const EdgeInsets.all(20.0), - this.enableInteractiveSelection = true, + this.enableInteractiveSelection, this.onTap, }) : assert(textAlign != null), assert(autofocus != null), @@ -137,7 +137,6 @@ class TextField extends StatefulWidget { assert(maxLines == null || maxLines > 0), assert(maxLength == null || maxLength > 0), keyboardType = keyboardType ?? (maxLines == 1 ? TextInputType.text : TextInputType.multiline), - assert(enableInteractiveSelection != null), super(key: key); /// Controls the text being edited. @@ -334,6 +333,11 @@ class TextField extends StatefulWidget { /// {@macro flutter.widgets.editableText.enableInteractiveSelection} final bool enableInteractiveSelection; + /// {@macro flutter.rendering.editable.selectionEnabled} + bool get selectionEnabled { + return enableInteractiveSelection ?? !obscureText; + } + /// Called when the user taps on this textfield. /// /// The textfield builds a [GestureDetector] to handle input events like tap, @@ -508,7 +512,7 @@ class _TextFieldState extends State with AutomaticKeepAliveClientMixi } void _handleTap() { - if (widget.enableInteractiveSelection) + if (widget.selectionEnabled) _renderEditable.handleTap(); _requestKeyboard(); _confirmCurrentSplash(); @@ -521,7 +525,7 @@ class _TextFieldState extends State with AutomaticKeepAliveClientMixi } void _handleLongPress() { - if (widget.enableInteractiveSelection) + if (widget.selectionEnabled) _renderEditable.handleLongPress(); _confirmCurrentSplash(); } @@ -599,7 +603,7 @@ class _TextFieldState extends State with AutomaticKeepAliveClientMixi autocorrect: widget.autocorrect, maxLines: widget.maxLines, selectionColor: themeData.textSelectionColor, - selectionControls: widget.enableInteractiveSelection + selectionControls: widget.selectionEnabled ? (themeData.platform == TargetPlatform.iOS ? cupertinoTextSelectionControls : materialTextSelectionControls) diff --git a/packages/flutter/lib/src/rendering/editable.dart b/packages/flutter/lib/src/rendering/editable.dart index 603f02890ac..d4feef67b0c 100644 --- a/packages/flutter/lib/src/rendering/editable.dart +++ b/packages/flutter/lib/src/rendering/editable.dart @@ -122,8 +122,6 @@ class RenderEditable extends RenderBox { /// /// The [offset] is required and must not be null. You can use [new /// ViewportOffset.zero] if you have no need for scrolling. - /// - /// The [enableInteractiveSelection] argument must not be null. RenderEditable({ TextSpan text, @required TextDirection textDirection, @@ -143,7 +141,7 @@ class RenderEditable extends RenderBox { Locale locale, double cursorWidth = 1.0, Radius cursorRadius, - bool enableInteractiveSelection = true, + bool enableInteractiveSelection, @required this.textSelectionDelegate, }) : assert(textAlign != null), assert(textDirection != null, 'RenderEditable created without a textDirection.'), @@ -152,7 +150,6 @@ class RenderEditable extends RenderBox { assert(offset != null), assert(ignorePointer != null), assert(obscureText != null), - assert(enableInteractiveSelection != null), assert(textSelectionDelegate != null), _textPainter = TextPainter( text: text, @@ -715,6 +712,22 @@ class RenderEditable extends RenderBox { markNeedsSemanticsUpdate(); } + /// {@template flutter.rendering.editable.selectionEnabled} + /// True if interactive selection is enabled based on the values of + /// [enableInteractiveSelection] and [obscureText]. + /// + /// By default [enableInteractiveSelection] is null, obscureText is false, + /// and this method returns true. + /// If [enableInteractiveSelection] is null and obscureText is true, then this + /// method returns false. This is the common case for password fields. + /// If [enableInteractiveSelection] is non-null then its value is returned. An + /// app might set it to true to enable interactive selection for a password + /// field, or to false to unconditionally disable interactive selection. + /// {@endtemplate} + bool get selectionEnabled { + return enableInteractiveSelection ?? !obscureText; + } + @override void describeSemanticsConfiguration(SemanticsConfiguration config) { super.describeSemanticsConfiguration(config); @@ -728,10 +741,10 @@ class RenderEditable extends RenderBox { ..isFocused = hasFocus ..isTextField = true; - if (hasFocus && enableInteractiveSelection) + if (hasFocus && selectionEnabled) config.onSetSelection = _handleSetSelection; - if (enableInteractiveSelection && _selection?.isValid == true) { + if (selectionEnabled && _selection?.isValid == true) { config.textSelection = _selection; if (_textPainter.getOffsetBefore(_selection.extentOffset) != null) { config diff --git a/packages/flutter/lib/src/widgets/editable_text.dart b/packages/flutter/lib/src/widgets/editable_text.dart index e4ed2b0d17b..c68435d0a1a 100644 --- a/packages/flutter/lib/src/widgets/editable_text.dart +++ b/packages/flutter/lib/src/widgets/editable_text.dart @@ -183,8 +183,7 @@ class EditableText extends StatefulWidget { /// default to [TextInputType.multiline]. /// /// The [controller], [focusNode], [style], [cursorColor], [textAlign], - /// [rendererIgnoresPointer], and [enableInteractiveSelection] arguments must - /// not be null. + /// [rendererIgnoresPointer] arguments must not be null. EditableText({ Key key, @required this.controller, @@ -214,7 +213,7 @@ class EditableText extends StatefulWidget { this.cursorRadius, this.scrollPadding = const EdgeInsets.all(20.0), this.keyboardAppearance = Brightness.light, - this.enableInteractiveSelection = true, + this.enableInteractiveSelection, }) : assert(controller != null), assert(focusNode != null), assert(obscureText != null), @@ -226,7 +225,6 @@ class EditableText extends StatefulWidget { assert(autofocus != null), assert(rendererIgnoresPointer != null), assert(scrollPadding != null), - assert(enableInteractiveSelection != null), keyboardType = keyboardType ?? (maxLines == 1 ? TextInputType.text : TextInputType.multiline), inputFormatters = maxLines == 1 ? ( @@ -469,6 +467,11 @@ class EditableText extends StatefulWidget { /// Defaults to false, resulting in a typical blinking cursor. static bool debugDeterministicCursor = false; + /// {@macro flutter.rendering.editable.selectionEnabled} + bool get selectionEnabled { + return enableInteractiveSelection ?? !obscureText; + } + @override EditableTextState createState() => EditableTextState(); @@ -929,19 +932,19 @@ class EditableTextState extends State with AutomaticKeepAliveClien } VoidCallback _semanticsOnCopy(TextSelectionControls controls) { - return widget.enableInteractiveSelection && _hasFocus && controls?.canCopy(this) == true + return widget.selectionEnabled && _hasFocus && controls?.canCopy(this) == true ? () => controls.handleCopy(this) : null; } VoidCallback _semanticsOnCut(TextSelectionControls controls) { - return widget.enableInteractiveSelection && _hasFocus && controls?.canCut(this) == true + return widget.selectionEnabled && _hasFocus && controls?.canCut(this) == true ? () => controls.handleCut(this) : null; } VoidCallback _semanticsOnPaste(TextSelectionControls controls) { - return widget.enableInteractiveSelection &&_hasFocus && controls?.canPaste(this) == true + return widget.selectionEnabled &&_hasFocus && controls?.canPaste(this) == true ? () => controls.handlePaste(this) : null; } @@ -1050,11 +1053,10 @@ class _Editable extends LeafRenderObjectWidget { this.rendererIgnoresPointer = false, this.cursorWidth, this.cursorRadius, - this.enableInteractiveSelection = true, + this.enableInteractiveSelection, this.textSelectionDelegate, }) : assert(textDirection != null), assert(rendererIgnoresPointer != null), - assert(enableInteractiveSelection != null), super(key: key); final TextSpan textSpan; diff --git a/packages/flutter/test/material/text_field_test.dart b/packages/flutter/test/material/text_field_test.dart index 64ff26242c1..3cd49379904 100644 --- a/packages/flutter/test/material/text_field_test.dart +++ b/packages/flutter/test/material/text_field_test.dart @@ -658,6 +658,66 @@ void main() { // End the test here to ensure the animation is properly disposed of. }); + testWidgets('An obscured TextField is not selectable by default', (WidgetTester tester) async { + // This is a regression test for + // https://github.com/flutter/flutter/issues/24100 + + final TextEditingController controller = TextEditingController(); + Widget buildFrame(bool obscureText, bool enableInteractiveSelection) { + return overlay( + child: TextField( + controller: controller, + obscureText: obscureText, + enableInteractiveSelection: enableInteractiveSelection, + ), + ); + } + + // Obscure text and don't enable or disable selection + await tester.pumpWidget(buildFrame(true, null)); + await tester.enterText(find.byType(TextField), 'abcdefghi'); + await skipPastScrollingAnimation(tester); + expect(controller.selection.isCollapsed, true); + + // Long press doesn't select anything + final Offset ePos = textOffsetToPosition(tester, 1); + final TestGesture gesture = await tester.startGesture(ePos, pointer: 7); + await tester.pump(const Duration(seconds: 2)); + await gesture.up(); + await tester.pump(); + expect(controller.selection.isCollapsed, true); + }); + + testWidgets('An obscured TextField is selectable when enabled', (WidgetTester tester) async { + // This is a regression test for + // https://github.com/flutter/flutter/issues/24100 + + final TextEditingController controller = TextEditingController(); + Widget buildFrame(bool obscureText, bool enableInteractiveSelection) { + return overlay( + child: TextField( + controller: controller, + obscureText: obscureText, + enableInteractiveSelection: enableInteractiveSelection, + ), + ); + } + + // Explicitly allow selection on obscured text + await tester.pumpWidget(buildFrame(true, true)); + await tester.enterText(find.byType(TextField), 'abcdefghi'); + await skipPastScrollingAnimation(tester); + expect(controller.selection.isCollapsed, true); + + // Long press does select text + final Offset ePos2 = textOffsetToPosition(tester, 1); + final TestGesture gesture2 = await tester.startGesture(ePos2, pointer: 7); + await tester.pump(const Duration(seconds: 2)); + await gesture2.up(); + await tester.pump(); + expect(controller.selection.isCollapsed, false); + }); + testWidgets('Multiline text will wrap up to maxLines', (WidgetTester tester) async { final Key textFieldKey = UniqueKey();