From 9a2abc76ef7d96fe5afee82f3c1cea15850dad99 Mon Sep 17 00:00:00 2001 From: Jeremy Liberman Date: Fri, 13 Nov 2015 16:46:59 -0600 Subject: [PATCH 01/31] Fixes #594 Navigation issues with non-searchable controls --- src/Select.js | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/Select.js b/src/Select.js index d647fda3e5..b21321a3d2 100644 --- a/src/Select.js +++ b/src/Select.js @@ -63,6 +63,7 @@ const Select = React.createClass({ searchable: React.PropTypes.bool, // whether to enable searching feature or not simpleValue: React.PropTypes.bool, // pass the value to onChange as a simple value (legacy pre 1.0 mode), defaults to false style: React.PropTypes.object, // optional style to apply to the control + tabIndex: React.PropTypes.string, // optional tab index of the control value: React.PropTypes.any, // initial field value valueComponent: React.PropTypes.func, // value component to render valueKey: React.PropTypes.string, // path of the label value in option objects @@ -150,6 +151,7 @@ const Select = React.createClass({ // for the non-searchable select, toggle the menu if (!this.props.searchable) { + this.focus(); return this.setState({ isOpen: !this.state.isOpen, }); @@ -474,7 +476,19 @@ const Select = React.createClass({ var className = classNames('Select-input', this.props.inputProps.className); if (this.props.disabled || !this.props.searchable) { if (this.props.multi && valueArray.length) return; - return
 
; + return ( + + ); } return ( Date: Mon, 16 Nov 2015 10:31:01 -0600 Subject: [PATCH 02/31] Fix keyboard nav for non-searchable multi controls --- src/Select.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Select.js b/src/Select.js index b21321a3d2..9140edd3b9 100644 --- a/src/Select.js +++ b/src/Select.js @@ -475,7 +475,6 @@ const Select = React.createClass({ renderInput (valueArray) { var className = classNames('Select-input', this.props.inputProps.className); if (this.props.disabled || !this.props.searchable) { - if (this.props.multi && valueArray.length) return; return ( Date: Fri, 20 Nov 2015 09:19:41 -0600 Subject: [PATCH 03/31] Fix failing Value.js tests --- src/Value.js | 2 +- test/Value-test.js | 10 ++-------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/src/Value.js b/src/Value.js index 1bb82c463a..6d2556ea12 100644 --- a/src/Value.js +++ b/src/Value.js @@ -46,7 +46,7 @@ const Value = React.createClass({ renderLabel () { let className = 'Select-value-label'; return this.props.onClick || this.props.value.href ? ( - + {this.props.children} ) : ( diff --git a/test/Value-test.js b/test/Value-test.js index a5d31a69b5..9b02308a67 100644 --- a/test/Value-test.js +++ b/test/Value-test.js @@ -32,12 +32,12 @@ describe('Value component', function() { value: OPTION, onRemove: sinon.spy() }; - value = TestUtils.renderIntoDocument(); + value = TestUtils.renderIntoDocument({OPTION.label}); }); it('requests its own removal when the remove icon is clicked', function() { var selectItemIcon = TestUtils.findRenderedDOMComponentWithClass(value, 'Select-value-icon'); - TestUtils.Simulate.click(selectItemIcon); + TestUtils.Simulate.mouseDown(selectItemIcon); expect(props.onRemove, 'was called'); }); @@ -47,12 +47,6 @@ describe('Value component', function() { expect(props.onRemove, 'was called'); }); - it('prevents event propagation, pt 1', function() { - var mockEvent = { stopPropagation: sinon.spy() }; - value.blockEvent(mockEvent); - expect(mockEvent.stopPropagation, 'was called'); - }); - describe('without a custom click handler', function() { it('presents the given label', function() { From f4a6fb6c1b17038dccfd87ad81e6cc50f1d32ef0 Mon Sep 17 00:00:00 2001 From: Dave Brotherstone Date: Sun, 8 Nov 2015 20:31:21 +0100 Subject: [PATCH 04/31] Update wallaby.js --- wallaby.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/wallaby.js b/wallaby.js index 863f67800f..c17e8eeeb7 100644 --- a/wallaby.js +++ b/wallaby.js @@ -6,11 +6,11 @@ var babel = require('babel'); module.exports = function (wallaby) { // eslint-disable-line no-unused-vars return { - files: ['src/*.js', 'testHelpers/*.js'], + files: ['src/**/*.js', 'testHelpers/*.js'], tests: ['test/*-test.js' ], env: { type: 'node', - runner: 'node' + runner: '/home/dave/.nvm/versions/node/v4.2.1/bin/node' }, compilers: { '**/*.js': wallaby.compilers.babel({ From edb91fd9bf5eb0455bffea084dbf7748efcbb3ee Mon Sep 17 00:00:00 2001 From: Dave Brotherstone Date: Mon, 9 Nov 2015 07:01:33 +0100 Subject: [PATCH 05/31] Test removed - relies on changing value --- test/Select-test.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/Select-test.js b/test/Select-test.js index 81ccf34521..5de2d76dd2 100644 --- a/test/Select-test.js +++ b/test/Select-test.js @@ -248,10 +248,6 @@ describe('Select', () => { expect(onChange, 'was called with', null); }); - it('should clear the display', () => { - expect(ReactDOM.findDOMNode(instance), 'queried for', PLACEHOLDER_SELECTOR, - 'to have text', 'Select...'); - }); }); it('should focus the first value on mouse click', () => { From 309f7c35e21af57a8692146996a56377b5a9669d Mon Sep 17 00:00:00 2001 From: Dave Brotherstone Date: Mon, 9 Nov 2015 07:03:21 +0100 Subject: [PATCH 06/31] Tests removed - behaviour changed These tests were to do with entering and leaving the options, where the behaviour is now that the option remains focussed. Probably need some new tests to cover this --- test/Select-test.js | 40 ---------------------------------------- 1 file changed, 40 deletions(-) diff --git a/test/Select-test.js b/test/Select-test.js index 5de2d76dd2..d4a4f1ecd8 100644 --- a/test/Select-test.js +++ b/test/Select-test.js @@ -348,46 +348,6 @@ describe('Select', () => { expect(ReactDOM.findDOMNode(instance), 'to contain no elements matching', '.Select-option'); }); - - describe('after mouseEnter and leave of an option', () => { - - // TODO: this behaviour has changed, and we no longer lose option focus on mouse out - return; - - beforeEach(() => { - // Show the options - var selectControl = getSelectControl(instance); - TestUtils.Simulate.keyDown(selectControl, { keyCode: 40, key: 'ArrowDown' }); - - var optionTwo = ReactDOM.findDOMNode(instance).querySelectorAll('.Select-option')[1]; - TestUtils.SimulateNative.mouseOver(optionTwo); - TestUtils.SimulateNative.mouseOut(optionTwo); - }); - - it('should have no focused options', () => { - var domNode = ReactDOM.findDOMNode(instance); - expect(domNode, 'to contain no elements matching', '.Select-option.is-focused'); - }); - - it('should focus top option after down arrow pressed', () => { - var selectControl = getSelectControl(instance); - TestUtils.Simulate.keyDown(selectControl, { keyCode: 40, key: 'ArrowDown' }); - var domNode = ReactDOM.findDOMNode(instance); - expect(domNode, 'queried for', '.Select-option.is-focused', - 'to have items satisfying', - 'to have text', 'One'); - - }); - - it('should focus last option after up arrow pressed', () => { - var selectControl = getSelectControl(instance); - TestUtils.Simulate.keyDown(selectControl, { keyCode: 38, key: 'ArrowUp' }); - var domNode = ReactDOM.findDOMNode(instance); - expect(domNode, 'queried for', '.Select-option.is-focused', - 'to have items satisfying', - 'to have text', 'Three'); - }); - }); }); describe('with values as numbers', () => { From 825b6036b6c886978d1091b275b22c24e4470204 Mon Sep 17 00:00:00 2001 From: Dave Brotherstone Date: Mon, 9 Nov 2015 07:03:45 +0100 Subject: [PATCH 07/31] Two tests marked as "broken" that actually weren't :) --- test/Select-test.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/Select-test.js b/test/Select-test.js index d4a4f1ecd8..b644bba04c 100644 --- a/test/Select-test.js +++ b/test/Select-test.js @@ -373,7 +373,6 @@ describe('Select', () => { }); it('set the initial value of the hidden input control', () => { - return; // TODO; broken test? expect(ReactDOM.findDOMNode(wrapper).querySelector(FORM_VALUE_SELECTOR).value, 'to equal', '2' ); }); @@ -384,7 +383,6 @@ describe('Select', () => { }); it('updates the value of the hidden input control after new value prop', () => { - return; // TODO; broken test? wrapper.setPropsForChild({ value: 3 }); expect(ReactDOM.findDOMNode(wrapper).querySelector(FORM_VALUE_SELECTOR).value, 'to equal', '3' ); }); From d29743fec36a7af2cc50cb4548083c40a966baf9 Mon Sep 17 00:00:00 2001 From: Dave Brotherstone Date: Mon, 9 Nov 2015 07:04:36 +0100 Subject: [PATCH 08/31] The hidden input is only rendered with a name property --- test/Select-test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/Select-test.js b/test/Select-test.js index b644bba04c..ceb392329f 100644 --- a/test/Select-test.js +++ b/test/Select-test.js @@ -634,6 +634,7 @@ describe('Select', () => { // Render an instance of the component wrapper = createControlWithWrapper({ + name: 'out-select-control', value: 'one', options: options, searchable: true From ebf7046d2a87596701b0e7cc32aeb9024567757b Mon Sep 17 00:00:00 2001 From: Dave Brotherstone Date: Mon, 9 Nov 2015 07:06:23 +0100 Subject: [PATCH 09/31] Wrong selector was used in test This is a hangover from the old version where it produced the same result. --- test/Select-test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/Select-test.js b/test/Select-test.js index ceb392329f..1cf8711575 100644 --- a/test/Select-test.js +++ b/test/Select-test.js @@ -731,7 +731,7 @@ describe('Select', () => { TestUtils.Simulate.mouseDown(ReactDOM.findDOMNode(instance).querySelectorAll('.Select-option')[1]); expect(onChange, 'was not called'); - expect(ReactDOM.findDOMNode(instance), 'queried for first', DISPLAYED_SELECTION_SELECTOR, + expect(ReactDOM.findDOMNode(instance), 'queried for first', PLACEHOLDER_SELECTOR, 'to have text', 'Select...'); }); @@ -838,7 +838,7 @@ describe('Select', () => { TestUtils.Simulate.keyDown(getSelectControl(instance), { keyCode: 13, key: 'Enter' }); expect(onChange, 'was not called'); - expect(ReactDOM.findDOMNode(instance), 'queried for first', DISPLAYED_SELECTION_SELECTOR, + expect(ReactDOM.findDOMNode(instance), 'queried for first', PLACEHOLDER_SELECTOR, 'to have text', 'Select...'); }); From 5778276f290e6b56e7ed8a6c354463f588505073 Mon Sep 17 00:00:00 2001 From: Dave Brotherstone Date: Mon, 9 Nov 2015 07:07:27 +0100 Subject: [PATCH 10/31] Add TODO marking a genuine regression When the `value` prop is set to a value not in the options, previously the value would be shown as the label, now nothing is selected. Need to decide if this is a valid change. --- test/Select-test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/Select-test.js b/test/Select-test.js index 1cf8711575..9cf4e8c0b1 100644 --- a/test/Select-test.js +++ b/test/Select-test.js @@ -675,6 +675,8 @@ describe('Select', () => { value: 'something new' }); + // TODO: This is a genuine regression / feature change + // When setting the value to something not in the options, it used to use the value as the label, now nothing is selected expect(ReactDOM.findDOMNode(instance), 'queried for', DISPLAYED_SELECTION_SELECTOR, 'to have items satisfying', 'to have text', 'something new'); }); From 983396fdef9ffbeb135beab7617d4c17615542d6 Mon Sep 17 00:00:00 2001 From: Dave Brotherstone Date: Mon, 9 Nov 2015 07:14:28 +0100 Subject: [PATCH 11/31] Add `wireUpOnChangeToValue` option for `createControlWithWrapper` This is as passed as a prop to `createControlWithWrapper`, but is then removed This means that onChange calls cause the value to be set. --- test/Select-test.js | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/test/Select-test.js b/test/Select-test.js index 9cf4e8c0b1..fffc1b36c4 100644 --- a/test/Select-test.js +++ b/test/Select-test.js @@ -128,8 +128,17 @@ describe('Select', () => { }; + var setValueProp = value => wrapper.setPropsForChild({ value }); + var createControlWithWrapper = (props) => { - onChange = sinon.spy(); + + if (props.wireUpOnChangeToValue) { + delete props.wireUpOnChangeToValue; + onChange = sinon.spy(setValueProp); + } else { + onChange = sinon.spy(); + } + onInputChange = sinon.spy(); wrapper = TestUtils.renderIntoDocument( @@ -915,7 +924,8 @@ describe('Select', () => { ]; wrapper = createControlWithWrapper({ - options: options + options: options, + wireUpOnChangeToValue: true }); }); @@ -1362,7 +1372,7 @@ describe('Select', () => { return new Promise((resolve, reject) => { input === '_FAIL'? reject('nope') : resolve({options: options}); - }) + }); }); }); From ad2ad88c40d8d6febff0ceb5dc6cef435cb034b0 Mon Sep 17 00:00:00 2001 From: Dave Brotherstone Date: Fri, 20 Nov 2015 17:52:49 +0100 Subject: [PATCH 12/31] value prop for multi should now be an array, or a single value --- test/Select-test.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/test/Select-test.js b/test/Select-test.js index fffc1b36c4..356c0ac55f 100644 --- a/test/Select-test.js +++ b/test/Select-test.js @@ -433,7 +433,7 @@ describe('Select', () => { ]; wrapper = createControlWithWrapper({ - value: '2,1', + value: [2, 1], options: options, multi: true, searchable: true @@ -452,15 +452,14 @@ describe('Select', () => { it('calls onChange with the correct value when 1 option is selected', () => { var removeIcons = ReactDOM.findDOMNode(instance).querySelectorAll('.Select-value .Select-value-icon'); - TestUtils.Simulate.click(removeIcons[0]); - // For multi-select, the "value" (first arg) to onChange is always a string - expect(onChange, 'was called with', '1', [{ value: 1, label: 'One' }]); + TestUtils.Simulate.mouseDown(removeIcons[0]); + expect(onChange, 'was called with', [{ value: 1, label: 'One' }]); }); it('supports updating the values via props', () => { wrapper.setPropsForChild({ - value: '3,4' + value: [3, 4] }); expect(ReactDOM.findDOMNode(instance), 'queried for', '.Select-value .Select-value-label', @@ -2831,7 +2830,7 @@ describe('Select', () => { // TODO: Need to use the new Select.Async control for this return; - + var asyncOptions; beforeEach(() => { From 4d5eaf596525d3359790eb0bef8bdc8a3d2a5152 Mon Sep 17 00:00:00 2001 From: Dave Brotherstone Date: Fri, 20 Nov 2015 17:57:17 +0100 Subject: [PATCH 13/31] Fix for value prop being set to zero The test for a single value of 0 was already there, and it caught the bug for the simple test of `if (!value) ...`. Yay for tests :) --- src/Select.js | 2 +- test/Select-test.js | 14 +++++++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/Select.js b/src/Select.js index 9140edd3b9..dd5178f534 100644 --- a/src/Select.js +++ b/src/Select.js @@ -297,7 +297,7 @@ const Select = React.createClass({ if (this.props.multi) { if (typeof value === 'string') value = value.split(this.props.delimiter); if (!Array.isArray(value)) { - if (!value) return []; + if (value === null || value === undefined) return []; value = [value]; } return value.map(this.expandValue).filter(i => i); diff --git a/test/Select-test.js b/test/Select-test.js index 356c0ac55f..ea44a4f4bb 100644 --- a/test/Select-test.js +++ b/test/Select-test.js @@ -469,7 +469,19 @@ describe('Select', () => { ]); }); - it('supports updating the value to 0', () => { + it('supports updating the value to a single value', () => { + + wrapper.setPropsForChild({ + value: 1 + }); + + expect(ReactDOM.findDOMNode(instance), 'queried for', '.Select-value .Select-value-label', + 'to satisfy', [ + expect.it('to have text', 'One') + ]); + }); + + it('supports updating the value to single value of 0', () => { // This test is specifically in case there's a "if (value) {... " somewhere wrapper.setPropsForChild({ From ef21e3830835609cd957d76a9a5df30bc0c4361a Mon Sep 17 00:00:00 2001 From: Dave Brotherstone Date: Fri, 20 Nov 2015 18:32:16 +0100 Subject: [PATCH 14/31] Fix tests for param changes for onChange onChange now called with array of option objects --- test/Select-test.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/Select-test.js b/test/Select-test.js index ea44a4f4bb..b3c662c2e5 100644 --- a/test/Select-test.js +++ b/test/Select-test.js @@ -499,7 +499,7 @@ describe('Select', () => { typeSearchText('fo'); pressEnterToAccept(); // Select 'Four' - expect(onChange, 'was called with', '2,1,4', [ + expect(onChange, 'was called with', [ { value: 2, label: 'Two' }, { value: 1, label: 'One' }, { value: 4, label: 'Four' } @@ -1548,7 +1548,6 @@ describe('Select', () => { value: '', options: options, searchable: true, - allowCreate: true, multi: true }); }); @@ -1557,7 +1556,7 @@ describe('Select', () => { typeSearchText('fo'); pressEnterToAccept(); - expect(onChange, 'was called with', 'four', [{ label: 'Four', value: 'four' }]); + expect(onChange, 'was called with', [{ label: 'Four', value: 'four' }]); }); it('selects a second option', () => { From c7b2d840abac7630847e949a58ceae3101d8740f Mon Sep 17 00:00:00 2001 From: Dave Brotherstone Date: Fri, 20 Nov 2015 18:35:08 +0100 Subject: [PATCH 15/31] Fix tests for `multi=true` with value prop set There are now two ways to pass the value, either as the original object, or as the value property from the option. Both are now explicitely tested. --- test/Select-test.js | 57 +++++++++++++++++++++++++++++++-------------- 1 file changed, 40 insertions(+), 17 deletions(-) diff --git a/test/Select-test.js b/test/Select-test.js index b3c662c2e5..30f1d946a4 100644 --- a/test/Select-test.js +++ b/test/Select-test.js @@ -1559,27 +1559,50 @@ describe('Select', () => { expect(onChange, 'was called with', [{ label: 'Four', value: 'four' }]); }); - it('selects a second option', () => { + describe('when using the option value object', () => { - typeSearchText('fo'); - pressEnterToAccept(); - typeSearchText('th'); - onChange.reset(); // Ignore previous onChange calls - pressEnterToAccept(); - expect(onChange, 'was called with', 'four,three', - [{ label: 'Four', value: 'four' }, { label: 'Three', value: 'three' }]); + it('selects an additional option', () => { + + setValueProp(options[3]); + typeSearchText('th'); + onChange.reset(); // Ignore previous onChange calls + pressEnterToAccept(); + + expect(onChange, 'was called with', + [{ label: 'Four', value: 'four' }, { label: 'Three', value: 'three' }]); + }); + + it('displays both selected options', () => { + + setValueProp([options[3], options[2]]); + expect(ReactDOM.findDOMNode(instance).querySelectorAll('.Select-value-label')[0], + 'to have text', 'Four'); + expect(ReactDOM.findDOMNode(instance).querySelectorAll('.Select-value-label')[1], + 'to have text', 'Three'); + }); }); - it('displays both selected options', () => { + describe('when using the option value', () => { - typeSearchText('fo'); - pressEnterToAccept(); - typeSearchText('th'); - pressEnterToAccept(); - expect(ReactDOM.findDOMNode(instance).querySelectorAll('.Select-value-label')[0], - 'to have text', 'Four'); - expect(ReactDOM.findDOMNode(instance).querySelectorAll('.Select-value-label')[1], - 'to have text', 'Three'); + it('selects an additional option', () => { + + setValueProp('four'); + typeSearchText('th'); + onChange.reset(); // Ignore previous onChange calls + pressEnterToAccept(); + + expect(onChange, 'was called with', + [{ label: 'Four', value: 'four' }, { label: 'Three', value: 'three' }]); + }); + + it('displays both selected options', () => { + + setValueProp(['four', 'three']); + expect(ReactDOM.findDOMNode(instance).querySelectorAll('.Select-value-label')[0], + 'to have text', 'Four'); + expect(ReactDOM.findDOMNode(instance).querySelectorAll('.Select-value-label')[1], + 'to have text', 'Three'); + }); }); it('filters the existing selections from the options', () => { From b74f272d0448a700b78ec697f517892c739bf223 Mon Sep 17 00:00:00 2001 From: Dave Brotherstone Date: Fri, 20 Nov 2015 18:35:36 +0100 Subject: [PATCH 16/31] More value prop / onChange test fixes --- test/Select-test.js | 39 ++++++++++++--------------------------- 1 file changed, 12 insertions(+), 27 deletions(-) diff --git a/test/Select-test.js b/test/Select-test.js index 30f1d946a4..9c358d1a96 100644 --- a/test/Select-test.js +++ b/test/Select-test.js @@ -1607,45 +1607,35 @@ describe('Select', () => { it('filters the existing selections from the options', () => { - wrapper.setPropsForChild({ - value: 'four,three' - }); + setValueProp(['four','three']); typeSearchText('o'); var options = ReactDOM.findDOMNode(instance).querySelectorAll('.Select-option'); - expect(options[0], 'to have text', 'Add "o"?'); - expect(options[1], 'to have text', 'One'); - expect(options[2], 'to have text', 'Two'); - expect(options, 'to have length', 3); // No "Four", as already selected + + expect(options[0], 'to have text', 'One'); + expect(options[1], 'to have text', 'Two'); + expect(options, 'to have length', 2); // No "Four", as already selected }); it('removes the last selected option with backspace', () => { - typeSearchText('fo'); - pressEnterToAccept(); - typeSearchText('th'); - pressEnterToAccept(); + setValueProp(['four','three']); onChange.reset(); // Ignore previous onChange calls pressBackspace(); - expect(onChange, 'was called with', 'four', [{ label: 'Four', value: 'four' }]); + expect(onChange, 'was called with', [{ label: 'Four', value: 'four' }]); }); it('does not remove the last selected option with backspace when backspaceRemoves=false', () => { // Disable backspace wrapper.setPropsForChild({ - backspaceRemoves: false + backspaceRemoves: false, + value: ['four', 'three'] }); - - typeSearchText('fo'); - pressEnterToAccept(); - typeSearchText('th'); - pressEnterToAccept(); onChange.reset(); // Ignore previous onChange calls pressBackspace(); - expect(onChange, 'was not called'); var items = ReactDOM.findDOMNode(instance).querySelectorAll('.Select-value-label'); expect(items[0], 'to have text', 'Four'); @@ -1654,18 +1644,13 @@ describe('Select', () => { it('removes an item when clicking on the X', () => { - typeSearchText('fo'); - pressEnterToAccept(); - typeSearchText('th'); - pressEnterToAccept(); - typeSearchText('tw'); - pressEnterToAccept(); + setValueProp(['four', 'three', 'two']); onChange.reset(); // Ignore previous onChange calls var threeDeleteButton = ReactDOM.findDOMNode(instance).querySelectorAll('.Select-value-icon')[1]; - TestUtils.Simulate.click(threeDeleteButton); + TestUtils.Simulate.mouseDown(threeDeleteButton); - expect(onChange, 'was called with', 'four,two', [ + expect(onChange, 'was called with', [ { label: 'Four', value: 'four' }, { label: 'Two', value: 'two' } ]); From f4810d9702ca84a13ae434d70840ed9c0bc31ea2 Mon Sep 17 00:00:00 2001 From: Dave Brotherstone Date: Sun, 22 Nov 2015 15:50:49 +0100 Subject: [PATCH 17/31] Comma to create an item is no longer supported --- test/Select-test.js | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/test/Select-test.js b/test/Select-test.js index 9c358d1a96..e582176ede 100644 --- a/test/Select-test.js +++ b/test/Select-test.js @@ -1656,21 +1656,6 @@ describe('Select', () => { ]); }); - it('uses the selected text as an item when comma is pressed', () => { - - typeSearchText('fo'); - pressEnterToAccept(); - typeSearchText('new item'); - onChange.reset(); - - TestUtils.Simulate.keyDown(searchInputNode, { keyCode: 188, key: ',' }); - expect(onChange, 'was called with', 'four,new item', [ - { value: 'four', label: 'Four' }, - { value: 'new item', label: 'new item' } - ]); - - }); - describe('with late options', () => { beforeEach(() => { From 1dfa63a2d0f2261d2658348daa631061e2fba498 Mon Sep 17 00:00:00 2001 From: Dave Brotherstone Date: Sun, 22 Nov 2015 15:57:38 +0100 Subject: [PATCH 18/31] Behaviour change: multi-select non-searchable remains open after select After selecting an item in a non-searchable Selectbox, the menu is no longer closed Tests changed to use this behaviour. --- test/Select-test.js | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/test/Select-test.js b/test/Select-test.js index e582176ede..08e34c560d 100644 --- a/test/Select-test.js +++ b/test/Select-test.js @@ -1707,7 +1707,8 @@ describe('Select', () => { value: '', options: options, searchable: false, - multi: true + multi: true, + wireUpOnChangeToValue: true }); // We need a hack here. @@ -1726,8 +1727,6 @@ describe('Select', () => { var items = ReactDOM.findDOMNode(instance).querySelectorAll('.Select-option'); TestUtils.Simulate.mouseDown(items[1]); - // The menu is now closed, click the arrow to open it again - clickArrowToOpen(); items = ReactDOM.findDOMNode(instance).querySelectorAll('.Select-option'); TestUtils.Simulate.mouseDown(items[0]); @@ -1744,12 +1743,10 @@ describe('Select', () => { var items = ReactDOM.findDOMNode(instance).querySelectorAll('.Select-option'); TestUtils.Simulate.mouseDown(items[1]); expect(onChange, 'was called once'); - expect(onChange, 'was called with', 'two', [{ value: 'two', label: 'Two' }]); + expect(onChange, 'was called with', [{ value: 'two', label: 'Two' }]); // Second item - // The menu is now closed, click the arrow to open it again - clickArrowToOpen(); items = ReactDOM.findDOMNode(instance).querySelectorAll('.Select-option'); TestUtils.Simulate.mouseDown(items[0]); expect(onChange, 'was called twice'); @@ -1766,9 +1763,7 @@ describe('Select', () => { TestUtils.Simulate.mouseDown(items[1]); expect(onChange, 'was called times', 1); - // Now get the list again, - // The menu is now closed, click the arrow to open it again - clickArrowToOpen(); + // Now get the list again items = ReactDOM.findDOMNode(instance).querySelectorAll('.Select-option'); expect(items[0], 'to have text', 'One'); expect(items[1], 'to have text', 'Three'); @@ -1779,8 +1774,6 @@ describe('Select', () => { TestUtils.Simulate.mouseDown(items[0]); expect(onChange, 'was called times', 2); - // The menu is now closed, click the arrow to open it again - clickArrowToOpen(); items = ReactDOM.findDOMNode(instance).querySelectorAll('.Select-option'); expect(items[0], 'to have text', 'Three'); expect(items[1], 'to have text', 'Four'); @@ -1790,8 +1783,6 @@ describe('Select', () => { TestUtils.Simulate.mouseDown(items[1]); expect(onChange, 'was called times', 3); - // The menu is now closed, click the arrow to open it again - clickArrowToOpen(); items = ReactDOM.findDOMNode(instance).querySelectorAll('.Select-option'); expect(items[0], 'to have text', 'Three'); expect(items, 'to have length', 1); From 420a70a9e9157ba60e9c85e2884f06e90b651dfe Mon Sep 17 00:00:00 2001 From: Dave Brotherstone Date: Sun, 22 Nov 2015 16:06:32 +0100 Subject: [PATCH 19/31] When selection is empty, onChange now called with null --- test/Select-test.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/test/Select-test.js b/test/Select-test.js index 08e34c560d..42a3a17c7f 100644 --- a/test/Select-test.js +++ b/test/Select-test.js @@ -1808,10 +1808,11 @@ describe('Select', () => { beforeEach(() => { - var instance = createControl({ + var wrapper = createControlWithWrapper({ clearable: true, options: defaultOptions, - value: 'three' + value: 'three', + wireUpOnChangeToValue: true }); expect(ReactDOM.findDOMNode(instance), 'queried for', DISPLAYED_SELECTION_SELECTOR, @@ -1826,14 +1827,14 @@ describe('Select', () => { pressEscape(); }); - it('calls onChange with empty', () => { + it('calls onChange with null', () => { - expect(onChange, 'was called with', ''); + expect(onChange, 'was called with', null); }); it('resets the display value', () => { - expect(ReactDOM.findDOMNode(instance), 'queried for', DISPLAYED_SELECTION_SELECTOR, + expect(ReactDOM.findDOMNode(instance), 'queried for', PLACEHOLDER_SELECTOR, 'to have items satisfying', 'to have text', 'Select...'); }); From 73c7447d1faa4f96a91f0701ec227a31a2984333 Mon Sep 17 00:00:00 2001 From: Dave Brotherstone Date: Sun, 22 Nov 2015 16:09:06 +0100 Subject: [PATCH 20/31] onChange called with null, and placeholder selector changed Fixed tests to match new behaviour. --- test/Select-test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/Select-test.js b/test/Select-test.js index 42a3a17c7f..f6d4ea7d57 100644 --- a/test/Select-test.js +++ b/test/Select-test.js @@ -1846,15 +1846,15 @@ describe('Select', () => { describe('on clicking `clear`', () => { beforeEach(() => { - TestUtils.Simulate.click(ReactDOM.findDOMNode(instance).querySelector('.Select-clear')); + TestUtils.Simulate.mouseDown(ReactDOM.findDOMNode(instance).querySelector('.Select-clear')); }); it('calls onChange with empty', () => { - expect(onChange, 'was called with', ''); + expect(onChange, 'was called with', null); }); it('resets the display value', () => { - expect(ReactDOM.findDOMNode(instance), 'queried for', DISPLAYED_SELECTION_SELECTOR, + expect(ReactDOM.findDOMNode(instance), 'queried for', PLACEHOLDER_SELECTOR, 'to have items satisfying', 'to have text', 'Select...'); }); From 9767a8d7111f7dcb1a899f863ae06a01f1e79af1 Mon Sep 17 00:00:00 2001 From: Dave Brotherstone Date: Sun, 22 Nov 2015 18:10:10 +0100 Subject: [PATCH 21/31] Fix bug where escape doesn't clear the input After searching, this left the select box closed, but displaying no value, even though the value was actually selected. Tests corrected. Added `clearable` to single select example --- examples/src/components/States.js | 9 +++++++-- src/Select.js | 3 ++- test/Select-test.js | 12 ++++++++++-- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/examples/src/components/States.js b/examples/src/components/States.js index 21ae542b12..8103c2eb90 100644 --- a/examples/src/components/States.js +++ b/examples/src/components/States.js @@ -20,7 +20,8 @@ var StatesField = React.createClass({ country: 'AU', disabled: false, searchable: this.props.searchable, - selectValue: 'new-south-wales' + selectValue: 'new-south-wales', + clearable: true, }; }, switchCountry (e) { @@ -50,7 +51,7 @@ var StatesField = React.createClass({ return (

{this.props.label}

-
@@ -62,6 +63,10 @@ var StatesField = React.createClass({ Disabled +