-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[TextField] Use autocomplete="nope" instead of autocomplete="off" #708
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
Conversation
Some browsers (I'm looking at you Chrome) ignore autocomplete="off" because they think they know better. As a result, setting autocomplete="off" does not disable autocomplete. A better way to disable autocomplete is to use any string that does not match a valid autocomplete value such as "nope". See https://developer.mozilla.org/en-US/docs/Web/Security/Securing_your_site/Turning_off_form_autocompletion#Disabling_autocompletion for more details.
👋 Thanks for opening your first pull request. A contributor should give feedback soon. If you haven’t already, please check out the contributing guidelines. You can also join #polaris on the Shopify Partners Slack. |
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.
Thanks for the contribution @jineshshah36! This looks great. Just one change on the UNRELEASED.md
and this should be good to go
🎩 this PR on branch import * as React from 'react';
import {Page, TextField} from '@shopify/polaris';
interface State {
value: string | undefined;
}
export default class Playground extends React.Component<{}, State> {
state: State = {
value: undefined,
};
render() {
return (
<Page
title="Playground"
primaryAction={{content: 'View Examples', url: '/examples'}}
>
<TextField
label="Email"
value={this.state.value}
onChange={this.handleChange}
autoComplete={false}
type="email"
/>
</Page>
);
}
handleChange = (value: string) => {
this.setState({value});
};
} |
🎉 Thanks for your contribution to Polaris React! |
@danrosenthal @jineshshah36 I'm actually seeing the reverse situation in Chrome 71. When I'm specifically testing this with the |
I'm also seeing this now. I'm thinking we should also revert this |
If you change it to |
WHY are these changes introduced?
Some browsers (I'm looking at you Chrome) ignore autocomplete="off" because they think they know better. As a result, setting autocomplete="off" does not disable autocomplete. A better way to disable autocomplete is to use any string that does not match a valid autocomplete value such as "nope". See https://developer.mozilla.org/en-US/docs/Web/Security/Securing_your_site/Turning_off_form_autocompletion#Disabling_autocompletion for more details.
WHAT is this pull request doing?
This will not make any UI changes