Skip to content
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

[Android + Crossplatform][TextInput] Add autoComplete prop #21575

Closed
wants to merge 4 commits into from

Conversation

cassiozen
Copy link

@cassiozen cassiozen commented Oct 8, 2018

Motivation

TL;DR: Setting autoComplete will allow the system to suggest autofill options for the <TextInput> component.

Android Oreo introduced the AutoFill Framework, for secure communication between an app and autofill services (e.g. Password managers). When using <TextInput> on Android Oreo+, the system already tries to autofill (based on heuristics), but there is no way to set configuring options or disable.

The quick solution would be to just add the same Android attributes (autofillHints & importantForAutofill) in React Native TextInput, but that doesn't bond well with the cross-platform nature of the library.

This PR

Introduces an autoComplete prop based on HTML's autocomplete attribute, mapping to Android autofillHints & importantForAutofill and serving as a proper placeholder for autofill/autocomplete in other platforms:

Also gives you the ability to disable autofill by setting autocomplete="off".

Test Plan

Adding autoComplete equal to username, password or any other available options should give you an autofill-bar bellow the TextInput which will let you fill in values from an AutoFill Service.

(The image show an example using Google's sample AutoFill Service

screen shot 2018-10-08 at 10 16 44 am

Setting the appropriate autoComplete will fill in the correct value in the TextInput.

Usage:

<TextInput
    value={this.state.username}
    onChangeText={this.setUserName}
    autoComplete="username"
/>
<TextInput
    value={this.state.password}
    onChangeText={this.setPassword}
    secureTextEntry={true}
    autoComplete="password"
/>

To disable:

<TextInput
    value={this.state.password}
    onChangeText={this.setPassword}
    secureTextEntry={true}
    autoComplete="off"
/>

References

Related PR:

Docs PR: facebook/react-native-website#606

Release Notes:

Help reviewers and the release process by writing your own release notes. See below for an example.

[ANDROID] [FEATURE] [AutoComplete] - Allow control over autofill suggestions for the <TextInput> component.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 8, 2018
Copy link

@analysis-bot analysis-bot left a comment

Choose a reason for hiding this comment

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

Code analysis results:

  • eslint found some issues.
  • eslint found some issues.

Libraries/Components/TextInput/TextInput.js Show resolved Hide resolved
Libraries/Components/TextInput/TextInput.js Show resolved Hide resolved
@cassiozen cassiozen changed the title [Android + Crossplatform][TextInput] Add autoComplete for TextInput [Android + Crossplatform][TextInput] Add autoComplete prop Oct 8, 2018
@react-native-bot react-native-bot added Platform: Android Android applications. Component: TextInput Related to the TextInput component. 🔶Components labels Oct 8, 2018
@gaodeng
Copy link
Contributor

gaodeng commented Oct 12, 2018

+1

@janicduplessis
Copy link
Contributor

@cassiozen Thanks for the PR! We already have https://facebook.github.io/react-native/docs/textinput#textcontenttype for iOS, I think we could reuse this prop and make it cross platform. We probably just have to specify which values are platform specific.

@cassiozen
Copy link
Author

cassiozen commented Oct 15, 2018

Hi @janicduplessis - thanks for the feedback.

I considered reusing textContentType, but decided agains it because it's used mainly to convey contextual meaning to the system (which will adjust keyboard styles and behavior etc.).

Even on iOS, only username and password are guaranteed to fire an autofill.

On Android, the AutoFill means just safe autocompletion, it doesn't affect the system in any other way.

@cassiozen
Copy link
Author

Another argument in favor of a cross-platform solution: iOS is introducing password recipes (and Android might do something similar in the future) - we could embrace all those solutions with a single, unified API.

@nhayfield
Copy link

Hopefully this gets merged soon. I need it to fix a whole host of bugs involving the autofill manager:

android.os.RemoteException: Remote stack trace: at com.android.server.autofill.AutofillManagerServiceImpl.assertCallerLocked(AutofillManagerServiceImpl.java:552) at com.android.server.autofill.AutofillManagerServiceImpl.createSessionByTokenLocked(AutofillManagerServiceImpl.java:510) at com.android.server.autofill.AutofillManagerServiceImpl.startSessionLocked(AutofillManagerServiceImpl.java:375) at com.android.server.autofill.AutofillManagerService$AutoFillManagerServiceStub.startSession(AutofillManagerService.java:897) at android.view.autofill.IAutoFillManager$Stub.onTransact(IAutoFillManager.java:117) 

@the-felipeal
Copy link

Hi there,

I'm the technical lead of the Android Autofill Framework. See my replies below..

@cassiozen We already have https://facebook.github.io/react-native/docs/textinput#textcontenttype for iOS

When we designed the Autofill Framework, we consider using Android's InputType to convey the semantic of the field type, but we decide to add a new property (autofillHints) for 2 reasons:

1.The inputType is only associated with text fields, and the framework supports more types, so that information would not be available on those types (for example, a Spinner).
2.This "automatic" mapping is error prone. For example, an app could use a inputType="textPassword" in a CVC field, which would make its "hint" to be "password", not "creditCardVerificationNumber"

I think we could reuse this prop and make it cross platform.

I think an "autocomplete" property would be cross-platform as well, as this is a W3C standard. In fact, for Android apps using WebView, the WebView implementation automatically maps the autocomplete tag to autofillHints.

-- Felipe

Copy link

@lostatseajoshua lostatseajoshua left a comment

Choose a reason for hiding this comment

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

A quick change requested on the java side but everything else looks good. Thanks for the feature!

Copy link
Contributor

@hramos hramos left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Will give @cassiozen a chance to address feedback before importing this.

Copy link

@lostatseajoshua lostatseajoshua left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the changes! @cassiozen @hramos

@facebook-github-bot facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Feb 15, 2019
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@hramos is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Collaborator

Cassio Zen merged commit f151456 into facebook:master.

@facebook facebook locked as resolved and limited conversation to collaborators Feb 15, 2019
@react-native-bot react-native-bot added the Merged This PR has been merged. label Feb 15, 2019
grabbou pushed a commit that referenced this pull request Feb 18, 2019
Summary:
TL;DR: Setting `autoComplete` will allow the system to suggest autofill options for the `<TextInput>` component.

Android Oreo introduced the AutoFill Framework, for secure communication between an app and autofill services (e.g. Password managers). When using `<TextInput>` on Android Oreo+, the system already tries to autofill (based on heuristics), but there is no way to set configuring options or disable.

The quick solution would be to just add the same Android attributes (`autofillHints` & `importantForAutofill`) in React Native TextInput, but that doesn't bond well with the cross-platform nature of the library.

Introduces an `autoComplete` prop based on HTML's `autocomplete` attribute, mapping to Android `autofillHints` & `importantForAutofill` and serving as a proper placeholder for autofill/autocomplete in other platforms:

Also gives you the ability to disable autofill by setting autocomplete="off".
Pull Request resolved: #21575

Differential Revision: D14102949

Pulled By: hramos

fbshipit-source-id: 7601aeaca0332a1f3ce8da8020dba037b700853a
@hramos hramos removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Feb 23, 2019
@hramos
Copy link
Contributor

hramos commented Mar 15, 2019

We noticed some crashes on pre-26 devices. We're rolling out a fix to prevent setTextContentType from calling the Oreo+ APIs on pre-26.

@grabbou
Copy link
Contributor

grabbou commented Mar 15, 2019

Going to revert manually in 0.60-stable now.

EDIT: Had some issues doing git revert. I'll wait for your commit.

@hramos
Copy link
Contributor

hramos commented Mar 15, 2019

Fix is in d4aa1e7. Cross-posted to react-native-community/releases#99.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Component: TextInput Related to the TextInput component. Merged This PR has been merged. p: Facebook Partner: Facebook Partner Platform: Android Android applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.