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

Include an 'id' prop #95

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions examples/async-data/app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,19 @@ let App = React.createClass({
attempt to autocomplete the first one.
</p>

{/*
Note that the 'id' prop is optional, but if you intend to use this
component in a universal (isomorphic) application, omitting it will
probably cause a server-client mismatch.
*/}
<Autocomplete
labelText="Choose a state from the US"
inputProps={{name: "US state"}}
ref="autocomplete"
value={this.state.value}
items={this.state.unitedStates}
getItemValue={(item) => item.name}
id="autocomplete-us-state"
onSelect={(value, item) => {
// set the menu to only the selected item
this.setState({ value, unitedStates: [ item ] })
Expand Down
3 changes: 2 additions & 1 deletion lib/Autocomplete.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ let Autocomplete = React.createClass({

propTypes: {
value: React.PropTypes.any,
id: React.PropTypes.string,
onChange: React.PropTypes.func,
onSelect: React.PropTypes.func,
shouldItemRender: React.PropTypes.func,
Expand Down Expand Up @@ -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).

this._ignoreBlur = false
this._performAutoCompleteOnUpdate = false
this._performAutoCompleteOnKeyUp = false
Expand Down
7 changes: 7 additions & 0 deletions lib/__tests__/Autocomplete-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ function AutocompleteComponentJSX (extraProps) {
labelText="Choose a state from the US"
inputProps={{name: "US state"}}
getItemValue={(item) => item.name}
id="autocomplete-us-state"
items={getStates()}
renderItem={(item, isHighlighted) => (
<div
Expand Down Expand Up @@ -66,6 +67,12 @@ describe('Autocomplete acceptance tests', () => {

});

it('should use the id prop, if supplied', () => {

expect(autocompleteWrapper.instance().id).to.equal('autocomplete-us-state');

});

});

// Event handler unit tests
Expand Down