-
Notifications
You must be signed in to change notification settings - Fork 529
Conversation
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-'); |
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.
lodash is removed as part of #89
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.
#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.
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.
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).
As someone who hasn't done much isomorphic rendering before, can you give a bit more explanation about why an id is necessary? |
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. |
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)? |
Yes. The need goes away. |
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.