From 05813d2090a756877a9ea7257c46d18f92949be0 Mon Sep 17 00:00:00 2001 From: w3-3w Date: Tue, 28 Mar 2017 16:19:42 +0900 Subject: [PATCH 1/7] add configurable prop for Async whether clear option list or not on selection --- src/Async.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Async.js b/src/Async.js index 23d8e9c61a..f189537568 100644 --- a/src/Async.js +++ b/src/Async.js @@ -14,6 +14,7 @@ const propTypes = { ]), loadOptions: React.PropTypes.func.isRequired, // callback to load options asynchronously; (inputValue: string, callback: Function): ?Promise multi: React.PropTypes.bool, // multi-value input + clearOptionsOnSelection: React.PropTypes.bool, // clears options after selecting a value when `multi` is true; defaults to true options: PropTypes.array.isRequired, // array of options placeholder: React.PropTypes.oneOfType([ // field placeholder, displayed when there's no value (shared with Select) React.PropTypes.string, @@ -43,6 +44,7 @@ const defaultProps = { loadingPlaceholder: 'Loading...', options: [], searchPromptText: 'Type to search', + clearOptionsOnSelection: true, }; export default class Async extends Component { @@ -191,7 +193,7 @@ export default class Async extends Component { options: (isLoading && loadingPlaceholder) ? [] : options, ref: (ref) => (this.select = ref), onChange: (newValues) => { - if (this.props.multi && this.props.value && (newValues.length > this.props.value.length)) { + if (this.props.multi && this.props.clearOptionsOnSelection && this.props.value && (newValues.length > this.props.value.length)) { this.clearOptions(); } this.props.onChange(newValues); From 7cb42ae29a84fe04e4ee05245e68dd51d404beb8 Mon Sep 17 00:00:00 2001 From: w3-3w Date: Tue, 28 Mar 2017 17:05:31 +0900 Subject: [PATCH 2/7] fix #1561 --- examples/src/components/GithubUsers.js | 2 +- src/Async.js | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/examples/src/components/GithubUsers.js b/examples/src/components/GithubUsers.js index 05939d7174..d06ab935d4 100644 --- a/examples/src/components/GithubUsers.js +++ b/examples/src/components/GithubUsers.js @@ -22,7 +22,7 @@ const GithubUsers = React.createClass({ switchToMulti () { this.setState({ multi: true, - value: [this.state.value], + value: this.state.value ? [this.state.value] : null }); }, switchToSingle () { diff --git a/src/Async.js b/src/Async.js index f189537568..184998fd5e 100644 --- a/src/Async.js +++ b/src/Async.js @@ -193,8 +193,12 @@ export default class Async extends Component { options: (isLoading && loadingPlaceholder) ? [] : options, ref: (ref) => (this.select = ref), onChange: (newValues) => { - if (this.props.multi && this.props.clearOptionsOnSelection && this.props.value && (newValues.length > this.props.value.length)) { - this.clearOptions(); + if (this.props.multi && this.props.clearOptionsOnSelection) { + // this.props.value may be null or undefined, so we have to confirm it has length prop + const prevValueLength = (this.props.value && this.props.value.length) ? this.props.value.length : 0; + if (newValues.length > prevValueLength) { + this.clearOptions(); + } } this.props.onChange(newValues); } From 58e6e4bbe010d727f27553d32fe66927a71fe700 Mon Sep 17 00:00:00 2001 From: w3-3w Date: Tue, 28 Mar 2017 23:00:13 +0900 Subject: [PATCH 3/7] reset to initial options for multi Async on blur By now, on blur, the pulldown will be closed with options state unmodified. If we focus on it again, the previous options will be shown, even if nothing was typed as keyword. This makes it looks inconsistent. --- src/Async.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Async.js b/src/Async.js index 184998fd5e..2db1231177 100644 --- a/src/Async.js +++ b/src/Async.js @@ -208,7 +208,8 @@ export default class Async extends Component { ...this.props, ...props, isLoading, - onInputChange: this._onInputChange + onInputChange: this._onInputChange, + onBlur: () => { this._onInputChange('') } }); } } From 86674a4455cea3021596aabb184edfa26cb6863e Mon Sep 17 00:00:00 2001 From: w3-3w Date: Tue, 4 Apr 2017 17:55:34 +0900 Subject: [PATCH 4/7] fix onBlur, take onBlurResetsInput into consideration didn't realize the presence of onBlurResetsInput prop. --- src/Async.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Async.js b/src/Async.js index 2db1231177..f09163a94a 100644 --- a/src/Async.js +++ b/src/Async.js @@ -209,7 +209,14 @@ export default class Async extends Component { ...props, isLoading, onInputChange: this._onInputChange, - onBlur: () => { this._onInputChange('') } + onBlur: (...args) => { + if (this.props.onBlurResetsInput !== false) { + this._onInputChange(''); + } + if (typeof this.props.onBlur === 'function') { + this.props.onBlur(...args); + } + } }); } } From 970938e4af960cb4af995e6fbe987f9b05350dfe Mon Sep 17 00:00:00 2001 From: w3-3w Date: Tue, 4 Apr 2017 18:06:07 +0900 Subject: [PATCH 5/7] move onBlur to proper place --- src/Async.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Async.js b/src/Async.js index f09163a94a..5681ce6000 100644 --- a/src/Async.js +++ b/src/Async.js @@ -201,14 +201,7 @@ export default class Async extends Component { } } this.props.onChange(newValues); - } - }; - - return children({ - ...this.props, - ...props, - isLoading, - onInputChange: this._onInputChange, + }, onBlur: (...args) => { if (this.props.onBlurResetsInput !== false) { this._onInputChange(''); @@ -217,6 +210,13 @@ export default class Async extends Component { this.props.onBlur(...args); } } + }; + + return children({ + ...this.props, + ...props, + isLoading, + onInputChange: this._onInputChange, }); } } From 5c60650aa2eaa6db0f0465c9299deb88421c6ea6 Mon Sep 17 00:00:00 2001 From: w3-3w Date: Tue, 4 Apr 2017 23:07:21 +0900 Subject: [PATCH 6/7] add test --- src/Async.js | 8 ++++-- test/Async-test.js | 64 ++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 68 insertions(+), 4 deletions(-) diff --git a/src/Async.js b/src/Async.js index 5681ce6000..6759e1c363 100644 --- a/src/Async.js +++ b/src/Async.js @@ -31,6 +31,8 @@ const propTypes = { ]), onInputChange: React.PropTypes.func, // optional for keeping track of what is being typed value: React.PropTypes.any, // initial field value + onBlur: React.PropTypes.func, // onBlur handler: function (event) {} + onBlurResetsInput: React.PropTypes.bool, // whether input is cleared on blur }; const defaultCache = {}; @@ -200,13 +202,15 @@ export default class Async extends Component { this.clearOptions(); } } - this.props.onChange(newValues); + if (this.props.onChange) { + this.props.onChange(newValues); + } }, onBlur: (...args) => { if (this.props.onBlurResetsInput !== false) { this._onInputChange(''); } - if (typeof this.props.onBlur === 'function') { + if (this.props.onBlur) { this.props.onBlur(...args); } } diff --git a/test/Async-test.js b/test/Async-test.js index 8742e315ff..a797f19f6a 100644 --- a/test/Async-test.js +++ b/test/Async-test.js @@ -41,9 +41,9 @@ describe('Async', () => { function createOptionsResponse (options) { return { - options: options.map((option) => ({ + options: options.map((option, idx) => ({ label: option, - value: option + value: idx })) }; } @@ -341,6 +341,66 @@ describe('Async', () => { }); }); + describe('multi', () => { + describe('options', () => { + function loadOptions (input, resolve) { + resolve(null, createOptionsResponse(['foo', 'bar', 'fog'])); + } + + it('should be cleared on selection by default', () => { + createControl({ + multi: true, + loadOptions + }); + expect(asyncNode.querySelectorAll('[role=option]').length, 'to equal', 0); // autoLoad is false, no options + typeSearchText('fo'); + expect(asyncNode.querySelectorAll('[role=option]').length, 'to equal', 2); // input changes to 'fo', filter from ['foo', 'bar', 'fog'] by input + TestUtils.Simulate.keyDown(filterInputNode, { keyCode: 13, key: 'Enter' }); + expect(asyncNode.querySelectorAll('[role=option]').length, 'to equal', 0); // on selection, input reset to ''(hard coded behavior), force clearing options + }); + + it('should not be cleared on selection when clearOptionsOnSelection is false', () => { + createControl({ + multi: true, + loadOptions, + clearOptionsOnSelection: false + }); + expect(asyncNode.querySelectorAll('[role=option]').length, 'to equal', 0); // autoLoad is false, no options + typeSearchText('fo'); + expect(asyncNode.querySelectorAll('[role=option]').length, 'to equal', 2); // input changes to 'fo', filter from ['foo', 'bar', 'fog'] by input + TestUtils.Simulate.keyDown(filterInputNode, { keyCode: 13, key: 'Enter' }); + expect(asyncNode.querySelectorAll('[role=option]').length, 'to equal', 3); // on selection, input reset to ''(hard coded behavior), filter from ['foo', 'bar', 'fog'] by input + }); + + it('should be reset on blur by default', () => { + createControl({ + multi: true, + loadOptions + }); + expect(asyncNode.querySelectorAll('[role=option]').length, 'to equal', 0); // autoLoad is false, no options + typeSearchText('bar'); + expect(asyncNode.querySelectorAll('[role=option]').length, 'to equal', 1); // input changes to 'bar', filter from ['foo', 'bar', 'fog'] by input + TestUtils.Simulate.blur(filterInputNode); + findAndFocusInputControl(); + expect(asyncNode.querySelectorAll('[role=option]').length, 'to equal', 3); // input resets to '', filter from ['foo', 'bar', 'fog'] by input + }); + + it('should not be reset on blur when onBlurResetsInput is false', () => { + createControl({ + multi: true, + loadOptions, + onBlurResetsInput: false + }); + expect(asyncNode.querySelectorAll('[role=option]').length, 'to equal', 0); // autoLoad is false, no options + typeSearchText('bar'); + expect(asyncNode.querySelectorAll('[role=option]').length, 'to equal', 1); // input changes to 'bar', filter from ['foo', 'bar', 'fog'] by input + TestUtils.Simulate.blur(filterInputNode); + findAndFocusInputControl(); + expect(asyncNode.querySelectorAll('[role=option]').length, 'to equal', 1); // input remains 'bar', filter from ['foo', 'bar', 'fog'] by input + }); + }); + }); + describe('noResultsText', () => { beforeEach(() => { From 3e361601d5663878f86e51ac18e36fd3cae525b1 Mon Sep 17 00:00:00 2001 From: w3-3w Date: Tue, 4 Apr 2017 23:11:24 +0900 Subject: [PATCH 7/7] update README --- README.md | 1 + src/Async.js | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 5a0202284b..a646291117 100644 --- a/README.md +++ b/README.md @@ -358,6 +358,7 @@ function onInputKeyDown(event) { className | string | undefined | className for the outer element clearable | bool | true | should it be possible to reset value clearAllText | string | 'Clear all' | title for the "clear" control when `multi` is true + clearOptionsOnSelection | bool | true | clears options after selecting an option for Async component when `multi` is true clearRenderer | func | undefined | Renders a custom clear to be shown in the right-hand side of the select when clearable true: `clearRenderer()` clearValueText | string | 'Clear value' | title for the "clear" control resetValue | any | null | value to use when you clear the control diff --git a/src/Async.js b/src/Async.js index 6759e1c363..5c2956e2c4 100644 --- a/src/Async.js +++ b/src/Async.js @@ -14,7 +14,7 @@ const propTypes = { ]), loadOptions: React.PropTypes.func.isRequired, // callback to load options asynchronously; (inputValue: string, callback: Function): ?Promise multi: React.PropTypes.bool, // multi-value input - clearOptionsOnSelection: React.PropTypes.bool, // clears options after selecting a value when `multi` is true; defaults to true + clearOptionsOnSelection: React.PropTypes.bool, // clears options after selecting an option when `multi` is true; defaults to true options: PropTypes.array.isRequired, // array of options placeholder: React.PropTypes.oneOfType([ // field placeholder, displayed when there's no value (shared with Select) React.PropTypes.string,