Skip to content
This repository has been archived by the owner on Jul 19, 2019. It is now read-only.

[changed] Disable native autocomplete #93

Merged
merged 1 commit into from
Jun 5, 2016

Conversation

CMTegner
Copy link
Collaborator

@CMTegner CMTegner commented Apr 9, 2016

You can do this via inputProps, but since the whole point of this component
is to provide a custom autocomplete solution it makes sense to disable the
native counterpart. If this proves to be problematic we can change the ordering
so that the consumer can override it via inputProps: { autoComplete: 'on' }.

You can do this via `inputProps`, but since the whole point of this component
is to provide a custom autocomplete solution it makes sense to disable the
native counterpart. If this proves to be problematic we can change the ordering
so that the consumer can override it via `inputProps: { autoComplete: 'on' }`.
@@ -322,6 +322,7 @@ let Autocomplete = React.createClass({
{...this.props.inputProps}
Copy link
Contributor

Choose a reason for hiding this comment

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

Later props take precedence: https://facebook.github.io/react/docs/jsx-spread.html#spread-attributes

This should come at the end of the props list so that the consumer can override autoComplete (and everything else for that matter).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As I said in the description:

If this proves to be problematic we can change the ordering so that the consumer can override it via inputProps: { autoComplete: 'on' }.

Some props we don't want to let the user override (e.g. onFocus, onBlur), some we probably don't want them to override (e.g. role, aria-autocomplete), while others we may want to let them override (e.g. autoComplete). I don't know if there has been put any thought into this, so I just followed the existing convention of putting "our" props last.

Copy link
Contributor

Choose a reason for hiding this comment

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

Bah, I totally misread the original description, sorry about that. Sounds good.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Np :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants