From c98f03889b8d59a84d42a2a694b5697dbb72c48e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Tegn=C3=A9r?= Date: Thu, 17 Dec 2015 13:42:45 +0100 Subject: [PATCH 1/4] [changed] Stop holding input value in state a.k.a. replace `props.initialValue` with `props.value`. This change makes the behaviour of Autocomplete closer to that of native input elements, i.e. you are forced to always supply the current value which the input should display, and the element will provide the updated value via the `onChange` callback (and `onSelect`). Upgrade path: Before: ```js class MyWidget extends Component { render() { return ( ) } } ``` After: ```js class MyWidget extends Component { constructor() { this.state = { value: 'hello' } } render() { return ( this.setState({ value: event.target.value })} onSelect={value => this.setState({ value })} /> ) } } ``` This is obviously a breaking change. Closes #49 --- lib/Autocomplete.js | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/lib/Autocomplete.js b/lib/Autocomplete.js index fe479517..65e02ddb 100644 --- a/lib/Autocomplete.js +++ b/lib/Autocomplete.js @@ -7,7 +7,7 @@ let _debugStates = [] let Autocomplete = React.createClass({ propTypes: { - initialValue: React.PropTypes.any, + value: React.PropTypes.any, onChange: React.PropTypes.func, onSelect: React.PropTypes.func, shouldItemRender: React.PropTypes.func, @@ -19,6 +19,7 @@ let Autocomplete = React.createClass({ getDefaultProps () { return { + value: '', inputProps: {}, labelText: '', onChange () {}, @@ -42,7 +43,6 @@ let Autocomplete = React.createClass({ getInitialState () { return { - value: this.props.initialValue || '', isOpen: false, highlightedIndex: null, } @@ -92,11 +92,7 @@ let Autocomplete = React.createClass({ handleChange (event) { this._performAutoCompleteOnKeyUp = true - this.setState({ - value: event.target.value, - }, () => { - this.props.onChange(event, this.state.value) - }) + this.props.onChange(event, event.target.value) }, handleKeyUp () { @@ -151,17 +147,17 @@ let Autocomplete = React.createClass({ else { // text entered + menu item has been highlighted + enter is hit -> update value to that of selected menu item, close the menu var item = this.getFilteredItems()[this.state.highlightedIndex] + var value = this.props.getItemValue(item) this.setState({ - value: this.props.getItemValue(item), isOpen: false, highlightedIndex: null }, () => { //this.refs.input.focus() // TODO: file issue this.refs.input.setSelectionRange( - this.state.value.length, - this.state.value.length + value.length, + value.length ) - this.props.onSelect(this.state.value, item) + this.props.onSelect(value, item) }) } }, @@ -179,13 +175,13 @@ let Autocomplete = React.createClass({ if (this.props.shouldItemRender) { items = items.filter((item) => ( - this.props.shouldItemRender(item, this.state.value) + this.props.shouldItemRender(item, this.props.value) )) } if (this.props.sortItems) { items.sort((a, b) => ( - this.props.sortItems(a, b, this.state.value) + this.props.sortItems(a, b, this.props.value) )) } @@ -193,7 +189,7 @@ let Autocomplete = React.createClass({ }, maybeAutoCompleteText () { - if (this.state.value === '') + if (this.props.value === '') return var { highlightedIndex } = this.state var items = this.getFilteredItems() @@ -203,13 +199,13 @@ let Autocomplete = React.createClass({ items[highlightedIndex] : items[0] var itemValue = this.props.getItemValue(matchedItem) var itemValueDoesMatch = (itemValue.toLowerCase().indexOf( - this.state.value.toLowerCase() + this.props.value.toLowerCase() ) === 0) if (itemValueDoesMatch) { var node = this.refs.input var setSelection = () => { node.value = itemValue - node.setSelectionRange(this.state.value.length, itemValue.length) + node.setSelectionRange(this.props.value.length, itemValue.length) } if (highlightedIndex === null) this.setState({ highlightedIndex: 0 }, setSelection) @@ -237,12 +233,12 @@ let Autocomplete = React.createClass({ }, selectItemFromMouse (item) { + var value = this.props.getItemValue(item); this.setState({ - value: this.props.getItemValue(item), isOpen: false, highlightedIndex: null }, () => { - this.props.onSelect(this.state.value, item) + this.props.onSelect(value, item) this.refs.input.focus() this.setIgnoreBlur(false) }) @@ -271,7 +267,7 @@ let Autocomplete = React.createClass({ top: this.state.menuTop, minWidth: this.state.menuWidth, } - var menu = this.props.renderMenu(items, this.state.value, style) + var menu = this.props.renderMenu(items, this.props.value, style) return React.cloneElement(menu, { ref: 'menu' }) }, @@ -318,7 +314,7 @@ let Autocomplete = React.createClass({ onKeyDown={(event) => this.handleKeyDown(event)} onKeyUp={(event) => this.handleKeyUp(event)} onClick={this.handleInputClick} - value={this.state.value} + value={this.props.value} id={this.id} /> {this.state.isOpen && this.renderMenu()} From 92d818e26f600ad11636c8393d0aaf25dfa451be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Tegn=C3=A9r?= Date: Thu, 17 Dec 2015 13:59:31 +0100 Subject: [PATCH 2/4] Updating examples to use new `value` prop --- examples/async-data/app.js | 6 ++++-- examples/custom-menu/app.js | 6 ++++-- examples/static-data/app.js | 7 ++++++- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/examples/async-data/app.js b/examples/async-data/app.js index c1ff0b17..05a149ed 100644 --- a/examples/async-data/app.js +++ b/examples/async-data/app.js @@ -6,6 +6,7 @@ let App = React.createClass({ getInitialState () { return { + value: '', unitedStates: getStates(), loading: false } @@ -27,16 +28,17 @@ let App = React.createClass({ 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} onSelect={(value, item) => { // set the menu to only the selected item - this.setState({ unitedStates: [ item ] }) + this.setState({ value, unitedStates: [ item ] }) // or you could reset it to a default list again // this.setState({ unitedStates: getStates() }) }} onChange={(event, value) => { - this.setState({loading: true}) + this.setState({ value, loading: true }) fakeRequest(value, (items) => { this.setState({ unitedStates: items, loading: false }) }) diff --git a/examples/custom-menu/app.js b/examples/custom-menu/app.js index 1ea62c24..4fe08a44 100644 --- a/examples/custom-menu/app.js +++ b/examples/custom-menu/app.js @@ -6,6 +6,7 @@ let App = React.createClass({ getInitialState () { return { + value: '', unitedStates: getStates(), loading: false } @@ -21,13 +22,14 @@ let App = React.createClass({ letter of the alphabet.

item.name} - onSelect={() => this.setState({ unitedStates: [] }) } + onSelect={value => this.setState({ value, unitedStates: [] }) } onChange={(event, value) => { - this.setState({loading: true}) + this.setState({ value, loading: true }) fakeRequest(value, (items) => { this.setState({ unitedStates: items, loading: false }) }) diff --git a/examples/static-data/app.js b/examples/static-data/app.js index 9def5b08..e3022f2e 100644 --- a/examples/static-data/app.js +++ b/examples/static-data/app.js @@ -3,6 +3,9 @@ import { getStates, matchStateToTerm, sortStates, styles } from '../../lib/utils import Autocomplete from '../../lib/index' let App = React.createClass({ + getInitialState() { + return { value: 'Ma' } + }, render () { return (
@@ -14,13 +17,15 @@ let App = React.createClass({

item.name} shouldItemRender={matchStateToTerm} sortItems={sortStates} + onChange={(event, value) => this.setState({ value })} + onSelect={value => this.setState({ value })} renderItem={(item, isHighlighted) => (
Date: Sat, 12 Mar 2016 20:22:17 +0100 Subject: [PATCH 3/4] Update tests to reflect removal of `value` from state --- lib/__tests__/Autocomplete-test.js | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/lib/__tests__/Autocomplete-test.js b/lib/__tests__/Autocomplete-test.js index 0ed62408..70c1d14a 100644 --- a/lib/__tests__/Autocomplete-test.js +++ b/lib/__tests__/Autocomplete-test.js @@ -15,7 +15,6 @@ chai.use(chaiEnzyme()); function AutocompleteComponentJSX (extraProps) { return ( item.name} @@ -49,11 +48,11 @@ describe('Autocomplete acceptance tests', () => { expect(autocompleteWrapper.instance().refs.menu).to.exist; }); - it('should show results when partial match is typed in', () => { + it('should show results when value is a partial match', () => { // Render autocomplete results upon partial input match expect(autocompleteWrapper.ref('menu').children()).to.have.length(50); - autocompleteInputWrapper.simulate('change', { target: { value: 'Ar' } }); + autocompleteWrapper.setProps({ value: 'Ar' }); expect(autocompleteWrapper.ref('menu').children()).to.have.length(6); }); @@ -173,9 +172,10 @@ describe('Autocomplete kewDown->Enter event handlers', () => { }); it('should close menu if input has focus but no item has been selected and then the Enter key is hit', () => { + let value = ''; autocompleteWrapper.setState({'isOpen': true}); autocompleteInputWrapper.simulate('focus'); - autocompleteInputWrapper.simulate('change', { target: { value: '' } }); + autocompleteWrapper.setProps({ value, onSelect(v) { value = v; } }); // simulate keyUp of backspace, triggering autocomplete suggestion on an empty string, which should result in nothing highlighted autocompleteInputWrapper.simulate('keyUp', { key : 'Backspace', keyCode: 8, which: 8 }); @@ -183,22 +183,23 @@ describe('Autocomplete kewDown->Enter event handlers', () => { autocompleteInputWrapper.simulate('keyDown', { key : 'Enter', keyCode: 13, which: 13 }); - expect(autocompleteWrapper.state('value')).to.equal(''); + expect(value).to.equal(''); expect(autocompleteWrapper.state('isOpen')).to.be.false; }); - it('should update input value from selected menu item and close the menu', () => { + it('should invoke `onSelect` with the selected menu item and close the menu', () => { + let value = 'Ar'; autocompleteWrapper.setState({'isOpen': true}); autocompleteInputWrapper.simulate('focus'); - autocompleteInputWrapper.simulate('change', { target: { value: 'Ar' } }); + autocompleteWrapper.setProps({ value, onSelect(v) { value = v; } }); // simulate keyUp of last key, triggering autocomplete suggestion + selection of the suggestion in the menu autocompleteInputWrapper.simulate('keyUp', { key : 'r', keyCode: 82, which: 82 }); // Hit enter, updating state.value with the selected Autocomplete suggestion autocompleteInputWrapper.simulate('keyDown', { key : 'Enter', keyCode: 13, which: 13 }); - expect(autocompleteWrapper.state('value')).to.equal('Arizona'); + expect(value).to.equal('Arizona'); expect(autocompleteWrapper.state('isOpen')).to.be.false; }); @@ -238,7 +239,7 @@ describe('Autocomplete#renderMenu', () => { it('should return a menu ReactComponent with a subset of children when partial match text has been entered', () => { // Input 'Ar' should result in 6 items in the menu, populated from autocomplete. - autocompleteInputWrapper.simulate('change', { target: { value: 'Ar' } }); + autocompleteWrapper.setProps({ value: 'Ar' }); var autocompleteMenu = autocompleteWrapper.instance().renderMenu(); expect(autocompleteMenu.props.children.length).to.be.equal(6); From 16cf53ef867e4ace0167b2d8d58271c8bd2b33c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Tegn=C3=A9r?= Date: Sun, 13 Mar 2016 16:41:08 +0100 Subject: [PATCH 4/4] Add test for character input --- lib/__tests__/Autocomplete-test.js | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/lib/__tests__/Autocomplete-test.js b/lib/__tests__/Autocomplete-test.js index 70c1d14a..f35a04fd 100644 --- a/lib/__tests__/Autocomplete-test.js +++ b/lib/__tests__/Autocomplete-test.js @@ -70,6 +70,27 @@ describe('Autocomplete acceptance tests', () => { // Event handler unit tests +describe('Autocomplete keyPress-> event handlers', () => { + + var autocompleteWrapper = mount(AutocompleteComponentJSX({})); + var autocompleteInputWrapper = autocompleteWrapper.find('input'); + + it('should pass updated `input.value` to `onChange` and replace with `props.value`', done => { + + let value = ''; + autocompleteWrapper.setProps({ value, onChange(_, v) { value = v; } }); + + autocompleteInputWrapper.get(0).value = 'a'; + autocompleteInputWrapper.simulate('keyPress', { key : 'a', keyCode: 97, which: 97 }); + autocompleteInputWrapper.simulate('change'); + + expect(autocompleteInputWrapper.get(0).value).to.equal(''); + expect(value).to.equal('a'); + done(); + }); + +}); + describe('Autocomplete kewDown->ArrowDown event handlers', () => { var autocompleteWrapper = mount(AutocompleteComponentJSX({}));