Add 2 new keyboard types and infer keyboardType from autofill hints #56641

LongCatIsLooong · 2020-05-08T07:06:40Z

Description

Add TextInputType.address and TextInputType.name.
Infer keyboardType from AutofillHints if left unspecified.
Fix docs for autofillHints

engine PR: flutter/engine#18202

Related Issues

Fixes #41508

Tests

I added the following tests:

Infer keyboardType from autofillHints

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.

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I signed the CLA.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I updated/added relevant documentation (doc comments with ///).
All existing and new tests are passing.
The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read Handling breaking changes.

No, no existing tests failed, so this is not a breaking change.
Yes, this is a breaking change. If not, delete the remainder of this section.
- I wrote a design doc: https://flutter.dev/go/template Replace this with a link to your design doc's short link
- I got input from the developer relations team, specifically from: Replace with the names of who gave advice
- I wrote a migration guide: Replace with a link to your migration guide

LongCatIsLooong · 2020-05-08T07:38:07Z

packages/flutter/lib/src/widgets/editable_text.dart

+  /// - Some autofill hints only work with specific [keyboardType]s. For example,
+  ///   [AutofillHints.name] requires [TextInputType.name] and [AutofillHints.email]
+  ///   works only with [TextInputType.email]. Make sure the input field has a
+  ///   compatible [keyboardType].


I think the 2nd bullet point needs better documentation on which keyboard types and autofill hints are compatible. I did some tests on an iOS 13.5 device using a native app, most UITextContentType only works with UIKeyboardType.namePhonePad (which is mapped to TextInputType.name in this PR), including content types like .streetAddress, .zipCode and .telephoneNumber (despite there're more fitting keyboard types). I can't really make sense of it so I'm not sure we should put that behavior in public documentation.

Maybe our users should be seeking out native iOS documentation on this stuff anyway, and we should just be clear about how we map to native.

Unfortunately the behavior isn't documented there either. Maybe I should change the name autofillHints to contentType to emphasize that it's not only for autofill, and it doesn't guarantee the platform autofill will trigger?

LongCatIsLooong · 2020-05-08T07:59:21Z

packages/flutter/lib/src/widgets/editable_text.dart

+        case TargetPlatform.macOS:
+          const Map<String, TextInputType> iOSKeyboardType = <String, TextInputType> {
+            AutofillHints.addressCity : TextInputType.name,
+            AutofillHints.addressCityAndState : TextInputType.name, // Autofill not working.


There're surprisingly many UITextContentTypes that don't trigger autofill (at least I couldn't find a way to, I had added all the fields, middle name, name prefix, etc., in Contacts). Maybe I should rename the field to textContentType?

Not sure what textContentType field you're referring to. Calling the enum that's used to select a keyboard TextInputType seems reasonable to me.

justinmc

LGTM

We should keep an eye on our issues after this is merged to see how users are understanding this.

justinmc · 2020-05-08T16:27:10Z

packages/flutter/test/widgets/editable_text_test.dart

+        await tester.idle();
+        expect(tester.testTextInput.editingState['text'], equals('test'));
+        expect(tester.testTextInput.setClientArgs['inputType']['name'], equals('TextInputType.address'));
+      });


Will these tests work even when they're run on other platforms after the PR is merged? I would have expected you to need to use TargetPlatformVariant, but maybe I'm wrong.

From the looks of it, they do the same thing. Changed to TargetPlatformVariant as it seems to be more conventional.

justinmc · 2020-05-08T16:35:24Z

packages/flutter/lib/src/widgets/editable_text.dart

+  /// - Some autofill hints only work with specific [keyboardType]s. For example,
+  ///   [AutofillHints.name] requires [TextInputType.name] and [AutofillHints.email]
+  ///   works only with [TextInputType.email]. Make sure the input field has a
+  ///   compatible [keyboardType].


Maybe our users should be seeking out native iOS documentation on this stuff anyway, and we should just be clear about how we map to native.

LongCatIsLooong · 2020-05-08T22:06:45Z

packages/flutter/lib/src/services/autofill.dart

looking for a better word for "availability"

I think availability is pretty good choice here. Alternatively you could say "how well they're supported on different platforms".

dla-kirito

Nothing to preview.

…ypes

HansMuller

LGTM

HansMuller · 2020-05-29T00:10:33Z

packages/flutter/lib/src/widgets/editable_text.dart

+        case TargetPlatform.macOS:
+          const Map<String, TextInputType> iOSKeyboardType = <String, TextInputType> {
+            AutofillHints.addressCity : TextInputType.name,
+            AutofillHints.addressCityAndState : TextInputType.name, // Autofill not working.


Not sure what textContentType field you're referring to. Calling the enum that's used to select a keyboard TextInputType seems reasonable to me.

…lutter#56641)

Add 2 new keyboard types and keyboardType inference

be647dd

LongCatIsLooong requested review from HansMuller and justinmc May 8, 2020 07:06

fluttergithubbot added the framework flutter/packages/flutter repository. See also f: labels. label May 8, 2020

googlebot added the cla: yes label May 8, 2020

LongCatIsLooong commented May 8, 2020

View reviewed changes

justinmc approved these changes May 8, 2020

View reviewed changes

remove trailing spaces

b792e2e

LongCatIsLooong force-pushed the more-text-input-types branch from ff21a3e to b792e2e Compare May 8, 2020 17:44

fluttergithubbot added f: cupertino flutter/packages/flutter/cupertino repository f: material design flutter/packages/flutter/material repository. labels May 8, 2020

LongCatIsLooong commented May 8, 2020

View reviewed changes

docs

e61461a

LongCatIsLooong force-pushed the more-text-input-types branch 2 times, most recently from ed44476 to c1d7a98 Compare May 9, 2020 07:35

additional ios docs

94cf7a3

LongCatIsLooong force-pushed the more-text-input-types branch from c1d7a98 to 94cf7a3 Compare May 9, 2020 07:36

dla-kirito reviewed May 14, 2020

View reviewed changes

Merge remote-tracking branch 'upstream/master' into more-text-input-t…

6c36b63

…ypes

flutter deleted a comment from dla-kirito May 29, 2020

HansMuller approved these changes May 29, 2020

View reviewed changes

LongCatIsLooong added the waiting for tree to go green label Jun 8, 2020

fluttergithubbot merged commit b64d652 into flutter:master Jun 8, 2020

zljj0818 pushed a commit to zljj0818/flutter that referenced this pull request Jun 22, 2020

Add 2 new keyboard types and infer keyboardType from autofill hints (f…

bd7124c

…lutter#56641)

mingwandroid pushed a commit to mingwandroid/flutter that referenced this pull request Sep 6, 2020

Add 2 new keyboard types and infer keyboardType from autofill hints (f…

8d09f75

…lutter#56641)

github-actions bot locked as resolved and limited conversation to collaborators Jul 30, 2021

Add 2 new keyboard types and infer keyboardType from autofill hints #56641

Add 2 new keyboard types and infer keyboardType from autofill hints #56641

Uh oh!

Conversation

LongCatIsLooong commented May 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Tests

Checklist

Breaking Change

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dla-kirito left a comment

Choose a reason for hiding this comment

Uh oh!

HansMuller left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

LongCatIsLooong commented May 8, 2020 •

edited

Loading