Skip to content

[autofill] opt-out instead of opt-in#86312

Merged
fluttergithubbot merged 7 commits into
flutter:masterfrom
LongCatIsLooong:opt-out-autofill
Sep 5, 2021
Merged

[autofill] opt-out instead of opt-in#86312
fluttergithubbot merged 7 commits into
flutter:masterfrom
LongCatIsLooong:opt-out-autofill

Conversation

@LongCatIsLooong

Copy link
Copy Markdown
Contributor

To opt-out, specify a null autofillHints.

Additionally, allows high-level text input widgets to pass an
AutofillClient to EditableText. This allows these widgets to provide
additional autofill information that EditableTextState usually does
not have access to (for instance, the placeholder text).

Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.

List which issues are fixed by this PR. You must list at least one issue.

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@LongCatIsLooong LongCatIsLooong requested a review from justinmc July 12, 2021 20:18
@flutter-dashboard flutter-dashboard Bot added f: cupertino flutter/packages/flutter/cupertino repository f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Jul 12, 2021
@google-cla google-cla Bot added the cla: yes label Jul 12, 2021
- fixes flutter#85554

To opt-out, specify a null `autofillHints`.

Additionally, allows high-level text input widgets to pass an
`AutofillClient` to `EditableText`. This allows these widgets to provide
additional autofill information that `EditableTextState` usually does
not have access to (for instance, the placeholder text).
@skia-gold

Copy link
Copy Markdown

Gold has detected about 4 new digest(s) on patchset 3.
View them at https://flutter-gold.skia.org/cl/github/86312

@flutter-dashboard

Copy link
Copy Markdown

This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@justinmc justinmc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. One question about hintText. I really think making this opt-out is a great idea, it should make many Flutter apps much more likely to have working autofill.

uniqueIdentifier: autofillId,
autofillHints: autofillHints,
currentEditingValue: _effectiveController.value,
hintText: widget.placeholder,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is hintText actually used for, I guess somewhere in the engine already? I understand autofillHints is the main list of hints that's used by the platform to determine what to autofill.

@LongCatIsLooong LongCatIsLooong Aug 24, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK currently we don't send the hint text (expect maybe for semantics). The reason the text input plugin needs it is when the autofill hints list is empty it's a good indication for what the text field is actually for: #85554

Comment on lines +728 to +729
/// The optional hint text placed on the view that typically suggests what
/// sort of input the field accepts.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I asked about hintText above, maybe these docs need some more info. I could see someone confusing this for autofillHints if they saw this first.

@justinmc

Copy link
Copy Markdown
Contributor

@LongCatIsLooong I think the Linux failures may be infrastructure flakes? Not sure about Google testing failures. Also you have conflicts FYI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

f: cupertino flutter/packages/flutter/cupertino repository f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Let Autofill be Opt-out

4 participants