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

Include an 'id' prop #95

Closed
wants to merge 1 commit into from
Closed

Conversation

danielfalk
Copy link

This is to address issue #86

Add an 'id' prop to the component so that the id used for accessibility
can be managed externally if necessary. At the moment, it is needed for
universal applications.

This prop is optional and will default to the old method of
autogenerating the identifier if one is not supplied.

Add an 'id' prop to the component so that the id used for accessibility
can be managed externally if necessary.  At the moment, it is needed for
universal applications.

This prop is optional and will default to the old method of
autogenerating the identifier if one is not supplied.
@@ -55,7 +56,7 @@ let Autocomplete = React.createClass({
},

componentWillMount () {
this.id = lodash.uniqueId('autocomplete-');
this.id = this.props.id || lodash.uniqueId('autocomplete-');
Copy link
Contributor

Choose a reason for hiding this comment

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

lodash is removed as part of #89

Copy link
Author

Choose a reason for hiding this comment

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

#89 appears to eliminate the label and labelText prop, which eliminates the need for a random id. If that gets merged as it stands now, then this change is probably not necessary anyway.

Copy link
Collaborator

Choose a reason for hiding this comment

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

After #89 is merged you'll still have the ability to assign an id to the input via inputProps (which you should to make labels focus the corresponding input).

@bfink13
Copy link
Contributor

bfink13 commented Apr 13, 2016

As someone who hasn't done much isomorphic rendering before, can you give a bit more explanation about why an id is necessary?

@danielfalk
Copy link
Author

Sure. In isomorphic rendering, you first render your components on the server as a string and serve this to the browser, allowing the page to appear immediately rendered. Once your application code is fully started up on the client side, the first thing React does is re-render the components and verify that the resulting dom tree is an *exact * match of what the server sent. If not, react gives us a nasty console message and has to throw away the dom and re-render the whole thing.

React however does a really good job of ensuring this works in every possible case. Unfortunately there are still some cases which it cannot account for. Introducing random id's into the resulting dom is one of those cases. If the random id generated on the server happens to be different from that on the client, we will have a mismatch.

The answer I've proposed here is to allow the application that uses react-autocomplete to specify an id, so that the component can reliably operate in an isomorphic setting.

@bfink13
Copy link
Contributor

bfink13 commented Apr 13, 2016

Got it, thanks for the context. Does the need for this go away with #89 (not generating a random ID) and the fact that the consumer can now set the ID on the wrapper and input (with inputProps and wrapperProps)?

@danielfalk
Copy link
Author

Yes. The need goes away.

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.

4 participants