From fe88a88a5ae941e44f1e755d87b342bc54f7de86 Mon Sep 17 00:00:00 2001 From: LongCatIsLooong <31859944+LongCatIsLooong@users.noreply.github.com> Date: Tue, 28 Jul 2020 10:23:17 -0700 Subject: [PATCH] Autofill save (#58731) --- .../flutter/lib/src/services/autofill.dart | 12 +- .../flutter/lib/src/services/text_input.dart | 66 ++++++++- .../flutter/lib/src/widgets/autofill.dart | 95 +++++++++++-- .../lib/src/widgets/editable_text.dart | 42 ++++-- .../test/widgets/autofill_group_test.dart | 131 +++++++++++++++++- .../test/widgets/editable_text_test.dart | 5 +- 6 files changed, 309 insertions(+), 42 deletions(-) diff --git a/packages/flutter/lib/src/services/autofill.dart b/packages/flutter/lib/src/services/autofill.dart index 61a54d58e9..7745419baf 100644 --- a/packages/flutter/lib/src/services/autofill.dart +++ b/packages/flutter/lib/src/services/autofill.dart @@ -807,13 +807,11 @@ mixin AutofillScopeMixin implements AutofillScope { !autofillClients.any((AutofillClient client) => client.textInputConfiguration.autofillConfiguration == null), 'Every client in AutofillScope.autofillClients must enable autofill', ); - return TextInput.attach( - trigger, - _AutofillScopeTextInputConfiguration( - allConfigurations: autofillClients - .map((AutofillClient client) => client.textInputConfiguration), - currentClientConfiguration: configuration, - ), + + final TextInputConfiguration inputConfiguration = _AutofillScopeTextInputConfiguration( + allConfigurations: autofillClients.map((AutofillClient client) => client.textInputConfiguration), + currentClientConfiguration: configuration, ); + return TextInput.attach(trigger, inputConfiguration); } } diff --git a/packages/flutter/lib/src/services/text_input.dart b/packages/flutter/lib/src/services/text_input.dart index 172ac7d861..d1b6ba9235 100644 --- a/packages/flutter/lib/src/services/text_input.dart +++ b/packages/flutter/lib/src/services/text_input.dart @@ -861,12 +861,14 @@ class TextInputConnection { TextInput._instance._show(); } - /// Requests the platform autofill UI to appear. + /// Requests the system autofill UI to appear. /// - /// The call has no effect unless the currently attached client supports - /// autofill, and the platform has a standalone autofill UI (for example, this - /// call has no effect on iOS since its autofill UI is part of the software - /// keyboard). + /// Currently only works on Android. Other platforms do not respond to this + /// message. + /// + /// See also: + /// + /// * [EditableText], a [TextInputClient] that calls this method when focused. void requestAutofill() { assert(attached); TextInput._instance._requestAutofill(); @@ -1228,4 +1230,58 @@ class TextInput { args, ); } + + /// Finishes the current autofill context, and potentially saves the user + /// input for future use if `shouldSave` is true. + /// + /// Typically, this method should be called when the user has finalized their + /// input. For example, in a [Form], it's typically done immediately before or + /// after its content is submitted. + /// + /// The topmost [AutofillGroup]s also call [finishAutofillContext] + /// automatically when they are disposed. The default behavior can be + /// overridden in [AutofillGroup.onDisposeAction]. + /// + /// {@template flutter.services.autofill.autofillContext} + /// An autofill context is a collection of input fields that live in the + /// platform's text input plugin. The platform is encouraged to save the user + /// input stored in the current autofill context before the context is + /// destroyed, when [finishAutofillContext] is called with `shouldSave` set to + /// true. + /// + /// Currently, there can only be at most one autofill context at any given + /// time. When any input field in an [AutofillGroup] requests for autofill + /// (which is done automatically when an autofillable [EditableText] gains + /// focus), the current autofill context will merge the content of that + /// [AutofillGroup] into itself. When there isn't an existing autofill context, + /// one will be created to hold the newly added input fields from the group. + /// + /// Once added to an autofill context, an input field will stay in the context + /// until the context is destroyed. To prevent leaks, call [finishAutofillContext] + /// to signal the text input plugin that the user has finalized their input in + /// the current autofill context. The platform text input plugin either + /// encourages or discourages the platform from saving the user input based on + /// the value of the `shouldSave` parameter. The platform usually shows a + /// "Save for autofill?" prompt for user confirmation. + /// {@endtemplate} + /// + /// On many platforms, calling [finishAutofillContext] shows the save user + /// input dialog and disrupts the user's flow. Ideally the dialog should only + /// be shown no more than once for every screen. Consider removing premature + /// [finishAutofillContext] calls to prevent showing the save user input UI + /// too frequently. However, calling [finishAutofillContext] when there's no + /// existing autofill context usually does not bring up the save user input + /// UI. + /// + /// See also: + /// + /// * [AutofillGroup.onDisposeAction], a configurable action that runs when a + /// topmost [AutofillGroup] is getting disposed. + static void finishAutofillContext({ bool shouldSave = true }) { + assert(shouldSave != null); + TextInput._instance._channel.invokeMethod( + 'TextInput.finishAutofillContext', + shouldSave , + ); + } } diff --git a/packages/flutter/lib/src/widgets/autofill.dart b/packages/flutter/lib/src/widgets/autofill.dart index ee89294fc0..b5c594308f 100644 --- a/packages/flutter/lib/src/widgets/autofill.dart +++ b/packages/flutter/lib/src/widgets/autofill.dart @@ -9,10 +9,26 @@ import 'framework.dart'; export 'package:flutter/services.dart' show AutofillHints; +/// Predefined autofill context clean up actions. +enum AutofillContextAction { + /// Destroys the current autofill context after informing the platform to save + /// the user input from it. + /// + /// Corresponds to calling [TextInput.finishAutofillContext] with + /// `shouldSave == true`. + commit, + + /// Destroys the current autofill context without saving the user input. + /// + /// Corresponds to calling [TextInput.finishAutofillContext] with + /// `shouldSave == false`. + cancel, +} + /// An [AutofillScope] widget that groups [AutofillClient]s together. /// -/// [AutofillClient]s within the same [AutofillScope] must be built together, and -/// they be will be autofilled together. +/// [AutofillClient]s that share the same closest [AutofillGroup] ancestor must +/// be built together, and they be will be autofilled together. /// /// {@macro flutter.services.autofill.AutofillScope} /// @@ -20,12 +36,27 @@ export 'package:flutter/services.dart' show AutofillHints; /// it using the [AutofillGroupState.register] API. Typically, [AutofillGroup] /// will not pick up [AutofillClient]s that are not mounted, for example, an /// [AutofillClient] within a [Scrollable] that has never been scrolled into the -/// viewport. To workaround this problem, ensure clients in the same [AutofillGroup] -/// are built together: +/// viewport. To workaround this problem, ensure clients in the same +/// [AutofillGroup] are built together. +/// +/// The topmost [AutofillGroup] widgets (the ones that are closest to the root +/// widget) can be used to clean up the current autofill context when the +/// current autofill context is no longer relevant. +/// +/// {@macro flutter.services.autofill.autofillContext} +/// +/// By default, [onDisposeAction] is set to [AutofillContextAction.commit], in +/// which case when any of the topmost [AutofillGroup]s is being disposed, the +/// platform will be informed to save the user input from the current autofill +/// context, then the current autofill context will be destroyed, to free +/// resources. You can, for example, wrap a route that contains a [Form] full of +/// autofillable input fields in an [AutofillGroup], so the user input of the +/// [Form] can be saved for future autofill by the platform. /// /// {@tool dartpad --template=stateful_widget_scaffold} /// -/// An example form with autofillable fields grouped into different `AutofillGroup`s. +/// An example form with autofillable fields grouped into different +/// `AutofillGroup`s. /// /// ```dart /// bool isSameAddress = true; @@ -44,8 +75,8 @@ export 'package:flutter/services.dart' show AutofillHints; /// return ListView( /// children: [ /// const Text('Shipping address'), -/// // The address fields are grouped together as some platforms are capable -/// // of autofilling all these fields in one go. +/// // The address fields are grouped together as some platforms are +/// // capable of autofilling all of these fields in one go. /// AutofillGroup( /// child: Column( /// children: [ @@ -83,8 +114,8 @@ export 'package:flutter/services.dart' show AutofillHints; /// ), /// ), /// const Text('Credit Card Information'), -/// // The credit card number and the security code are grouped together as -/// // some platforms are capable of autofilling both fields. +/// // The credit card number and the security code are grouped together +/// // as some platforms are capable of autofilling both fields. /// AutofillGroup( /// child: Column( /// children: [ @@ -111,6 +142,11 @@ export 'package:flutter/services.dart' show AutofillHints; /// } /// ``` /// {@end-tool} +/// +/// See also: +/// +/// * [AutofillContextAction], an enum that contains predefined autofill context +/// clean up actions to be run when a topmost [AutofillGroup] is disposed. class AutofillGroup extends StatefulWidget { /// Creates a scope for autofillable input fields. /// @@ -118,6 +154,7 @@ class AutofillGroup extends StatefulWidget { const AutofillGroup({ Key key, @required this.child, + this.onDisposeAction = AutofillContextAction.commit, }) : assert(child != null), super(key: key); @@ -137,6 +174,17 @@ class AutofillGroup extends StatefulWidget { /// {@macro flutter.widgets.child} final Widget child; + /// The [AutofillContextAction] to be run when this [AutofillGroup] is the + /// topmost [AutofillGroup] and it's being disposed, in order to clean up the + /// current autofill context. + /// + /// {@macro flutter.services.autofill.autofillContext} + /// + /// Defaults to [AutofillContextAction.commit], which prompts the platform to + /// save the user input and destroy the current autofill context. No action + /// will be taken if [onDisposeAction] is set to null. + final AutofillContextAction onDisposeAction; + @override AutofillGroupState createState() => AutofillGroupState(); } @@ -160,6 +208,11 @@ class AutofillGroup extends StatefulWidget { class AutofillGroupState extends State with AutofillScopeMixin { final Map _clients = {}; + // Whether this AutofillGroup widget is the topmost AutofillGroup (i.e., it + // has no AutofillGroup ancestor). Each topmost AutofillGroup runs its + // `AutofillGroup.onDisposeAction` when it gets disposed. + bool _isTopmostAutofillGroup = false; + @override AutofillClient getAutofillClient(String tag) => _clients[tag]; @@ -184,7 +237,7 @@ class AutofillGroupState extends State with AutofillScopeMixin { _clients.putIfAbsent(client.autofillId, () => client); } - /// Removes an [AutofillClient] with the given [autofillId] from this + /// Removes an [AutofillClient] with the given `autofillId` from this /// [AutofillGroup]. /// /// Typically, this should be called by autofillable [TextInputClient]s in @@ -203,6 +256,12 @@ class AutofillGroupState extends State with AutofillScopeMixin { _clients.remove(autofillId); } + @override + void didChangeDependencies() { + super.didChangeDependencies(); + _isTopmostAutofillGroup = AutofillGroup.of(context) == null; + } + @override Widget build(BuildContext context) { return _AutofillScope( @@ -210,6 +269,22 @@ class AutofillGroupState extends State with AutofillScopeMixin { child: widget.child, ); } + + @override + void dispose() { + super.dispose(); + + if (!_isTopmostAutofillGroup || widget.onDisposeAction == null) + return; + switch (widget.onDisposeAction) { + case AutofillContextAction.cancel: + TextInput.finishAutofillContext(shouldSave: false); + break; + case AutofillContextAction.commit: + TextInput.finishAutofillContext(shouldSave: true); + break; + } + } } class _AutofillScope extends InheritedWidget { diff --git a/packages/flutter/lib/src/widgets/editable_text.dart b/packages/flutter/lib/src/widgets/editable_text.dart index 2a652f98e8..f4fb42eb79 100644 --- a/packages/flutter/lib/src/widgets/editable_text.dart +++ b/packages/flutter/lib/src/widgets/editable_text.dart @@ -1410,6 +1410,9 @@ class EditableTextState extends State with AutomaticKeepAliveClien @override AutofillScope get currentAutofillScope => _currentAutofillScope; + // Is this field in the current autofill context. + bool _isInAutofillContext = false; + // This value is an eyeball estimation of the time it takes for the iOS cursor // to ease in and out. static const Duration _fadeDuration = Duration(milliseconds: 250); @@ -1470,6 +1473,7 @@ class EditableTextState extends State with AutomaticKeepAliveClien _currentAutofillScope?.unregister(autofillId); _currentAutofillScope = newAutofillGroup; newAutofillGroup?.register(this); + _isInAutofillContext = _isInAutofillContext || _shouldBeInAutofillContext; } if (!_didAutoFocus && widget.autofocus) { @@ -1494,6 +1498,8 @@ class EditableTextState extends State with AutomaticKeepAliveClien _selectionOverlay?.update(_value); } _selectionOverlay?.handlesVisible = widget.showSelectionHandles; + _isInAutofillContext = _isInAutofillContext || _shouldBeInAutofillContext; + if (widget.focusNode != oldWidget.focusNode) { oldWidget.focusNode.removeListener(_handleFocusChanged); _focusAttachment?.detach(); @@ -1776,6 +1782,8 @@ class EditableTextState extends State with AutomaticKeepAliveClien } bool get _hasInputConnection => _textInputConnection != null && _textInputConnection.attached; + bool get _needsAutofill => widget.autofillHints?.isNotEmpty ?? false; + bool get _shouldBeInAutofillContext => _needsAutofill && currentAutofillScope != null; void _openInputConnection() { if (widget.readOnly) { @@ -1785,14 +1793,24 @@ class EditableTextState extends State with AutomaticKeepAliveClien final TextEditingValue localValue = _value; _lastFormattedUnmodifiedTextEditingValue = localValue; - _textInputConnection = (widget.autofillHints?.isNotEmpty ?? false) && currentAutofillScope != null + // When _needsAutofill == true && currentAutofillScope == null, autofill + // is allowed but saving the user input from the text field is + // discouraged. + // + // In case the autofillScope changes from a non-null value to null, or + // _needsAutofill changes to false from true, the platform needs to be + // notified to exclude this field from the autofill context. So we need to + // provide the autofillId. + _textInputConnection = _needsAutofill && currentAutofillScope != null ? currentAutofillScope.attach(this, textInputConfiguration) - : TextInput.attach(this, textInputConfiguration); + : TextInput.attach(this, _createTextInputConfiguration(_isInAutofillContext || _needsAutofill)); _textInputConnection.show(); _updateSizeAndTransform(); - // Request autofill AFTER the size and the transform have been sent to the - // platform side. - _textInputConnection.requestAutofill(); + if (_needsAutofill) { + // Request autofill AFTER the size and the transform have been sent to + // the platform text input plugin. + _textInputConnection.requestAutofill(); + } final TextStyle style = widget.style; _textInputConnection @@ -2223,9 +2241,8 @@ class EditableTextState extends State with AutomaticKeepAliveClien @override String get autofillId => 'EditableText-$hashCode'; - @override - TextInputConfiguration get textInputConfiguration { - final bool isAutofillEnabled = widget.autofillHints?.isNotEmpty ?? false; + TextInputConfiguration _createTextInputConfiguration(bool needsAutofillConfiguration) { + assert(needsAutofillConfiguration != null); return TextInputConfiguration( inputType: widget.keyboardType, obscureText: widget.obscureText, @@ -2239,14 +2256,19 @@ class EditableTextState extends State with AutomaticKeepAliveClien ), textCapitalization: widget.textCapitalization, keyboardAppearance: widget.keyboardAppearance, - autofillConfiguration: !isAutofillEnabled ? null : AutofillConfiguration( + autofillConfiguration: !needsAutofillConfiguration ? null : AutofillConfiguration( uniqueIdentifier: autofillId, - autofillHints: widget.autofillHints.toList(growable: false), + autofillHints: widget.autofillHints?.toList(growable: false) ?? [], currentEditingValue: currentTextEditingValue, ), ); } + @override + TextInputConfiguration get textInputConfiguration { + return _createTextInputConfiguration(_needsAutofill); + } + // null if no promptRect should be shown. TextRange _currentPromptRectRange; diff --git a/packages/flutter/test/widgets/autofill_group_test.dart b/packages/flutter/test/widgets/autofill_group_test.dart index b4d8e866fb..ca1281bddf 100644 --- a/packages/flutter/test/widgets/autofill_group_test.dart +++ b/packages/flutter/test/widgets/autofill_group_test.dart @@ -5,8 +5,12 @@ // @dart = 2.8 import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; +final Matcher _matchesCommit = isMethodCall('TextInput.finishAutofillContext', arguments: true); +final Matcher _matchesCancel = isMethodCall('TextInput.finishAutofillContext', arguments: false); + void main() { testWidgets('AutofillGroup has the right clients', (WidgetTester tester) async { const Key outerKey = Key('outer'); @@ -43,16 +47,15 @@ void main() { ); expect(outerState.autofillClients, [clientState1]); + // The second TextField doesn't have autofill enabled. expect(innerState.autofillClients, [clientState2]); }); testWidgets('new clients can be added & removed to a scope', (WidgetTester tester) async { const Key scopeKey = Key('scope'); - final List hints = []; - const TextField client1 = TextField(autofillHints: ['1']); - final TextField client2 = TextField(autofillHints: hints); + TextField client2 = const TextField(autofillHints: []); StateSetter setState; @@ -84,16 +87,16 @@ void main() { expect(scopeState.autofillClients, [clientState1]); // Add to scope. - setState(() { hints.add('2'); }); + setState(() { client2 = const TextField(autofillHints: ['2']); }); await tester.pump(); - expect(scopeState.autofillClients.length, 2); expect(scopeState.autofillClients, contains(clientState1)); expect(scopeState.autofillClients, contains(clientState2)); + expect(scopeState.autofillClients.length, 2); // Remove from scope again. - setState(() { hints.clear(); }); + setState(() { client2 = const TextField(autofillHints: []); }); await tester.pump(); @@ -165,4 +168,120 @@ void main() { expect(outerState.autofillClients, contains(clientState3)); expect(innerState.autofillClients, [clientState2]); }); + + testWidgets('disposing AutofillGroups', (WidgetTester tester) async { + StateSetter setState; + const Key group1 = Key('group1'); + const Key group2 = Key('group2'); + const Key group3 = Key('group3'); + const TextField placeholder = TextField(autofillHints: [AutofillHints.name]); + + List children = const [ + AutofillGroup( + key: group1, + onDisposeAction: AutofillContextAction.commit, + child: AutofillGroup(child: placeholder), + ), + AutofillGroup(key: group2, onDisposeAction: AutofillContextAction.cancel, child: placeholder), + AutofillGroup( + key: group3, + onDisposeAction: AutofillContextAction.commit, + child: AutofillGroup(child: placeholder), + ), + ]; + + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: StatefulBuilder( + builder: (BuildContext context, StateSetter setter) { + setState = setter; + return Column(children: children); + }, + ), + ), + ), + ); + + expect( + tester.testTextInput.log, + isNot(contains(_matchesCommit)), + ); + + tester.testTextInput.log.clear(); + + // Remove the first topmost group group1. Should commit. + setState(() { + children = const [ + AutofillGroup(key: group2, onDisposeAction: AutofillContextAction.cancel, child: placeholder), + AutofillGroup( + key: group3, + onDisposeAction: AutofillContextAction.commit, + child: AutofillGroup(child: placeholder), + ), + ]; + }); + + await tester.pump(); + + expect( + tester.testTextInput.log.single, + _matchesCommit, + ); + + tester.testTextInput.log.clear(); + + // Remove the topmost group group2. Should cancel. + setState(() { + children = const [ + AutofillGroup( + key: group3, + onDisposeAction: AutofillContextAction.commit, + child: AutofillGroup(child: placeholder), + ), + ]; + }); + + await tester.pump(); + + expect( + tester.testTextInput.log.single, + _matchesCancel, + ); + + tester.testTextInput.log.clear(); + + // Remove the inner group within group3. No action. + setState(() { + children = const [ + AutofillGroup( + key: group3, + onDisposeAction: AutofillContextAction.commit, + child: placeholder, + ), + ]; + }); + + await tester.pump(); + + expect( + tester.testTextInput.log, + isNot(contains('TextInput.finishAutofillContext')), + ); + + tester.testTextInput.log.clear(); + + // Remove the topmosts group group3. Should commit. + setState(() { + children = const [ + ]; + }); + + await tester.pump(); + + expect( + tester.testTextInput.log.single, + _matchesCommit, + ); + }); } diff --git a/packages/flutter/test/widgets/editable_text_test.dart b/packages/flutter/test/widgets/editable_text_test.dart index 51e4263089..12b8ac8833 100644 --- a/packages/flutter/test/widgets/editable_text_test.dart +++ b/packages/flutter/test/widgets/editable_text_test.dart @@ -4407,13 +4407,12 @@ void main() { 'TextInput.setClient', 'TextInput.show', 'TextInput.setEditableSizeAndTransform', - 'TextInput.requestAutofill', 'TextInput.setStyle', 'TextInput.setEditingState', 'TextInput.setEditingState', 'TextInput.show', ]; - expect(tester.testTextInput.log.length, 8); + expect(tester.testTextInput.log.length, 7); int index = 0; for (final MethodCall m in tester.testTextInput.log) { expect(m.method, logOrder[index]); @@ -4452,7 +4451,6 @@ void main() { 'TextInput.setClient', 'TextInput.show', 'TextInput.setEditableSizeAndTransform', - 'TextInput.requestAutofill', 'TextInput.setStyle', 'TextInput.setEditingState', 'TextInput.setEditingState', @@ -4500,7 +4498,6 @@ void main() { 'TextInput.setClient', 'TextInput.show', 'TextInput.setEditableSizeAndTransform', - 'TextInput.requestAutofill', 'TextInput.setStyle', 'TextInput.setEditingState', 'TextInput.setEditingState',