-
Notifications
You must be signed in to change notification settings - Fork 25k
feat: Unify TextInput autoComplete and textContentType props #34523
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
feat: Unify TextInput autoComplete and textContentType props #34523
Conversation
Base commit: b6bf1fd |
Base commit: b6bf1fd |
|
@necolas has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
6e8761d to
21ba83a
Compare
21ba83a to
22b7359
Compare
|
@necolas has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@necolas do you mind checking what is causing internal builds and tests to fail? |
|
Someone else will have to import this PR. I closed that internal diff as I'm away for the rest of September |
|
Ping @cipolleschi or @jacdebug to import |
|
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
| } | ||
| autoComplete={ | ||
| Platform.OS === 'android' | ||
| ? // $FlowFixMe |
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.
What is this suppressing? If we do add a suppression, could we suppress the specific error?
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.
Hi @NickGerleman this is suppressing the prop-missing error because the autoCompleteWebToAutoCompleteAndroidMap object does not include all autoComplete keys/values. The reason for this is just because it would make the object super huge and we can just fallback to autoComplete when autoCompleteWebToTextContentTypeMap[autoComplete] is undefined. I've just updated the suppression to specify the prop-missing error tho
|
This pull request was successfully merged by @gabrieldonadel in 73abcba. When will my fix make it into a release? | Upcoming Releases |
…k#34523) Summary: This unifies the Android only `autoComplete` and the iOS only `textContentType` TextInput props with the web `autoComplete` values as requested on facebook#34424. I left the `textContentType` prop and the current supported `autoComplete` values untouched in order to avoid having a breaking change. This also updates RNTester to include test cases using the new `autoComplete` values ## Changelog <!-- Help reviewers and the release process by writing your own changelog entry. For an example, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [General] [Changed] - Unify TextInput autoComplete and textContentType props Pull Request resolved: facebook#34523 Test Plan: 1. Open the RNTester app and navigate to the TextInput page 2. Test the `TextInput` component through the `Text Auto Complete` section https://user-images.githubusercontent.com/11707729/187118267-3b509631-7b84-47b7-a580-567a7f5b483f.mov Reviewed By: NickGerleman Differential Revision: D39104545 Pulled By: cipolleschi fbshipit-source-id: a0d4b1b9ab336854a741a9efe4a62c3da0b5c0f4
Summary
This unifies the Android only
autoCompleteand the iOS onlytextContentTypeTextInput props with the webautoCompletevalues as requested on #34424. I left thetextContentTypeprop and the current supportedautoCompletevalues untouched in order to avoid having a breaking change. This also updates RNTester to include test cases using the newautoCompletevaluesChangelog
[General] [Changed] - Unify TextInput autoComplete and textContentType props
Test Plan
TextInputcomponent through theText Auto CompletesectionScreen.Recording.2022-08-29.at.00.37.52.mov