-
Notifications
You must be signed in to change notification settings - Fork 26.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Autofill Part 1 #52126
Autofill Part 1 #52126
Conversation
Why does the |
@goderbauer the controllers in the list may change as the state of the form changes. In most cases it should simply follow the logic in the |
If the controllers change, wouldn't you have to call setState anyways to assign those new controllers to the textfields? When you do that, the Autofill would also rebuild with the new list of controllers. |
Then in that case the list of controllers will be updated in the next build phase? If the state has already changed and the scope queries the list of controllers before the next build phase, the list we return should reflect the new state I think? |
hintText: 'What do people call you?', | ||
labelText: 'Name *', | ||
// Since the fields are in a Column, it's easier to | ||
// uses the non-lazy version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case since we're using a Column
, we can just wrap the entire Column
inside an Autofill
and omit clientListBuilder
examples/flutter_gallery/lib/demo/material/text_form_field_demo.dart
Outdated
Show resolved
Hide resolved
6bcb4f7
to
2cf28ff
Compare
6133e83
to
87af84f
Compare
2bbf2da
to
89ee788
Compare
cac5fa1
to
8b11268
Compare
8b11268
to
af1e1f9
Compare
/// | ||
/// {@template flutter.widgets.autofill.AutofillGroupState} | ||
/// An [AutofillGroupState] can be used to register an [AutofillClient] when it | ||
/// enteres this [AutofillGroup] (for example, when an [EditableText] is mounted or |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"enteres" => "enters"
@@ -0,0 +1,166 @@ | |||
// Copyright 2014 The Flutter Authors. All rights reserved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file's name is missing an underscore, should be: autofill_group_test.dart
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the new registration-based approach.
if (method == 'TextInputClient.updateEditingStateWithTag') { | ||
final TextInputClient client = _currentConnection._client; | ||
assert(client != null); | ||
if (client is AutofillTrigger) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should all TextInputClient
just have a currentAutofillScope getter and return null
by default if they don't participate in AutoFill? (Not sure what the AutofillTrigger abstraction is adding)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functionality-wise I think it's the same. Adding AutofillTrigger
makes it non-breaking?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just landed a very similar breaking change a few days ago. I think that makes breaking this less bad?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I would prefer the cleaner API here without the type check. Implementing your own TextInputClient should be fairly un-common, so migration should be fairly easy?
|
||
/// Adds the [AutofillClient] to this [AutofillGroup]. | ||
/// | ||
/// Typically, this is be called by [AutofillTrigger]s (for example, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: remove the "be"?
370190d
to
e88c584
Compare
e88c584
to
1260a80
Compare
Can anyone tell me which version has these changes or is this hasn't been released yet? |
@BirjuVachhani simply check the merge commit e31f708 which will show you all tags it is in, right now: "1.18.0-8.0.pre 1.18.0-7.0.pre 1.18.0-6.0.pre" - ie. already in the dev channel. |
@hpoul Thank you for the info. I did check it out and I got those changes. However I was wondering about the same in a |
@hpoul @BirjuVachhani one of the commits was reverted as part of the engine roll revert, it's probably not working on android. I'll post updates in the related issue thread and hopefully close that issue soon. |
@LongCatIsLooong got it. Thank you for the info. 😊 |
Which one is the related issue? 🤔 |
Thank you very much, I was totally freaked out for such a solution, it is working properly just upgrade the flutter to master and that's it, even one-time password is also working. Thank you once again you saved my ass. |
How to invoke a prompt for saving the new username and password? I've wrapped textfields into autofillgroup, now the passwords are shown but the current is not saved |
@nausharipov Thanks for asking! We are still working on that part. The tracking issue: #55613 |
Description
iOS PR: flutter/engine#17493
Android PR: flutter/engine#17465
Design Doc: https://flutter.dev/go/input-field-autofill
Scope
This PR (as well as the engine PRs) adds the most basic autofill functionality. It does not include some platform-specific configurations (e.g., passwordRules on iOS).
Example Usage:
Related Issues
Part of #13015
Tests
I added the following tests:
Replace this with a list of the tests that you added as part of this PR. A change in behaviour with no test covering it
will likely get reverted accidentally sooner or later. PRs must include tests for all changed/updated/fixed behaviors. See Test Coverage.
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]
). This will ensure a smooth and quick review process.///
).flutter analyze --flutter-repo
) does not report any problems on my PR.Breaking Change
Did any tests fail when you ran them? Please read Handling breaking changes.