Merge pull request JedWatson#1891 from banderson/remove-selected

louisbl · Oct 10, 2017 · ca30469 · ca30469
1 parent 7523c20
commit ca30469
Show file tree

Hide file tree

Showing 3 changed files with 116 additions and 5 deletions.
diff --git a/src/Select.js b/src/Select.js
@@ -526,7 +526,12 @@ class Select extends React.Component {
 				inputValue: this.handleInputValueChange(updatedValue),
 				isOpen: !this.props.closeOnSelect,
 			}, () => {
-				this.addValue(value);
+				var valueArray = this.getValueArray(this.props.value);
+				if (valueArray.some(i => i[this.props.valueKey] === value[this.props.valueKey])) {
+					this.removeValue(value);
+				} else {
+					this.addValue(value);
+				}
 			});
 		} else {
 			this.setState({
@@ -562,7 +567,7 @@ class Select extends React.Component {
 
 	removeValue (value) {
 		var valueArray = this.getValueArray(this.props.value);
-		this.setValue(valueArray.filter(i => i !== value));
+		this.setValue(valueArray.filter(i => i[this.props.valueKey] !== value[this.props.valueKey]));
 		this.focus();
 	}
 
@@ -979,7 +984,7 @@ class Select extends React.Component {
 
 	render () {
 		let valueArray = this.getValueArray(this.props.value);
-		let options = this._visibleOptions = this.filterOptions(this.props.multi ? this.getValueArray(this.props.value) : null);
+		let options = this._visibleOptions = this.filterOptions(this.props.multi && this.props.removeSelected ? valueArray : null);
 		let isOpen = this.state.isOpen;
 		if (this.props.multi && !options.length && valueArray.length && !this.state.inputValue) isOpen = false;
 		const focusedOptionIndex = this.getFocusableOptionIndex(valueArray[0]);
@@ -1040,7 +1045,7 @@ class Select extends React.Component {
 					{this.renderClear()}
 					{this.renderArrow()}
 				</div>
-				{isOpen ? this.renderOuter(options, !this.props.multi ? valueArray : null, focusedOption) : null}
+				{isOpen ? this.renderOuter(options, valueArray, focusedOption) : null}
 			</div>
 		);
 	}
@@ -1107,6 +1112,7 @@ Select.propTypes = {
 	options: PropTypes.array,             // array of options
 	pageSize: PropTypes.number,           // number of entries to page when using page up/down keys
 	placeholder: stringOrNode,            // field placeholder, displayed when there's no value
+	removeSelected: PropTypes.bool,       // whether the selected option is removed from the dropdown on multi selects
 	required: PropTypes.bool,             // applies HTML5 required attribute when needed
 	resetValue: PropTypes.any,            // value to use when you clear the control
 	scrollMenuIntoView: PropTypes.bool,   // boolean to enable the viewport to shift so that the full menu fully visible when engaged
@@ -1157,6 +1163,7 @@ Select.defaultProps = {
 	optionComponent: Option,
 	pageSize: 5,
 	placeholder: 'Select...',
+	removeSelected: true,
 	required: false,
 	scrollMenuIntoView: true,
 	searchable: true,

diff --git a/src/utils/defaultMenuRenderer.js b/src/utils/defaultMenuRenderer.js
@@ -18,7 +18,7 @@ function menuRenderer ({
 	let Option = optionComponent;
 
 	return options.map((option, i) => {
-		let isSelected = valueArray && valueArray.indexOf(option) > -1;
+		let isSelected = valueArray && valueArray.some(x => x[valueKey] == option[valueKey]);
 		let isFocused = option === focusedOption;
 		let optionClass = classNames(optionClassName, {
 			'Select-option': true,

diff --git a/test/Select-test.js b/test/Select-test.js
@@ -2041,6 +2041,110 @@ describe('Select', () => {
 
 	});
 
+		describe('with removeSelected=false', () => {
+			beforeEach(() => {
+				options = [
+					{ value: 'one', label: 'One' },
+					{ value: 'two', label: 'Two' },
+					{ value: 'three', label: 'Three' },
+					{ value: 'four', label: 'Four' }
+				];
+
+				// Render an instance of the component
+				wrapper = createControlWithWrapper({
+					value: '',
+					options: options,
+					multi: true,
+					removeSelected: false,
+					closeOnSelect: false
+				}, {
+					wireUpOnChangeToValue: true
+				});
+
+				// We need a hack here.
+				// JSDOM (at least v3.x) doesn't appear to support div's with tabindex
+				// This just hacks that we are focused
+				// This is (obviously) implementation dependent, and may need to change
+				instance.setState({
+					isFocused: true
+				});
+			});
+
+			it('does not remove the selected options from the menu', () => {
+
+				clickArrowToOpen();
+
+				var items = ReactDOM.findDOMNode(instance).querySelectorAll('.Select-option');
+
+				// Click the option "Two" to select it
+				expect(items[1], 'to have text', 'Two');
+				TestUtils.Simulate.mouseDown(items[1]);
+				expect(onChange, 'was called times', 1);
+
+				// 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', 'Two');
+				expect(items[2], 'to have text', 'Three');
+				expect(items[3], 'to have text', 'Four');
+				expect(items, 'to have length', 4);
+
+				// Click first item, 'One'
+				TestUtils.Simulate.mouseDown(items[0]);
+				expect(onChange, 'was called times', 2);
+
+				items = ReactDOM.findDOMNode(instance).querySelectorAll('.Select-option');
+				expect(items[0], 'to have text', 'One');
+				expect(items[1], 'to have text', 'Two');
+				expect(items[2], 'to have text', 'Three');
+				expect(items[3], 'to have text', 'Four');
+				expect(items, 'to have length', 4);
+
+				// Click last item, 'Four'
+				TestUtils.Simulate.mouseDown(items[3]);
+				expect(onChange, 'was called times', 3);
+
+				items = ReactDOM.findDOMNode(instance).querySelectorAll('.Select-option');
+				expect(items[0], 'to have text', 'One');
+				expect(items[1], 'to have text', 'Two');
+				expect(items[2], 'to have text', 'Three');
+				expect(items[3], 'to have text', 'Four');
+				expect(items, 'to have length', 4);
+
+				expect(onChange.args, 'to equal', [
+					[[{ value: 'two', label: 'Two' }]],
+					[[{ value: 'two', label: 'Two' }, { value: 'one', label: 'One' }]],
+					[
+						[
+							{ value: 'two', label: 'Two' },
+							{ value: 'one', label: 'One' },
+							{ value: 'four', label: 'Four' },
+						],
+					],
+				]);
+			});
+
+			it('removes a selected value if chosen again', () => {
+
+				clickArrowToOpen();
+
+				var items = ReactDOM.findDOMNode(instance).querySelectorAll('.Select-option');
+
+				// Click the option "Two" to select it
+				TestUtils.Simulate.mouseDown(items[1]);
+				expect(onChange, 'was called times', 1);
+
+				// Click the option "Two" again to deselect it
+				TestUtils.Simulate.mouseDown(items[1]);
+				expect(onChange, 'was called times', 2);
+
+				expect(onChange.args, 'to equal', [
+					[[{ value: 'two', label: 'Two' }]],
+					[[]],
+				]);
+			});
+		});
+
 	describe('with props', () => {
 
 		describe('className', () => {