From 25e2cd0db6673f0ae80534b983b57a8cde1a0be4 Mon Sep 17 00:00:00 2001 From: Ben Alpert Date: Sun, 4 Aug 2013 18:07:33 -0700 Subject: [PATCH] Fix behavior of ReactDOMSelect Closes #250. Test Plan: With multiple and not, verified: With defaultValue, the correct option is picked initially, user changes change the selection, and changes to defaultValue have no effect. With value, the correct option is picked initially, user changes do nothing, and changes to value change the selection. Also ran the tests. --- src/dom/components/ReactDOMSelect.js | 6 +- .../__tests__/ReactDOMSelect-test.js | 157 ++++++++++++++++++ 2 files changed, 159 insertions(+), 4 deletions(-) create mode 100644 src/dom/components/__tests__/ReactDOMSelect-test.js diff --git a/src/dom/components/ReactDOMSelect.js b/src/dom/components/ReactDOMSelect.js index 0709eaf5b1d7..22a7b2e3d28e 100644 --- a/src/dom/components/ReactDOMSelect.js +++ b/src/dom/components/ReactDOMSelect.js @@ -58,11 +58,9 @@ function selectValueType(props, propName, componentName) { */ function updateOptions() { /*jshint validthis:true */ - if (this.props.value == null) { - return; - } + var value = this.props.value != null ? this.props.value : this.state.value; var options = this.getDOMNode().options; - var selectedValue = '' + this.props.value; + var selectedValue = '' + value; for (var i = 0, l = options.length; i < l; i++) { var selected = this.props.multiple ? diff --git a/src/dom/components/__tests__/ReactDOMSelect-test.js b/src/dom/components/__tests__/ReactDOMSelect-test.js new file mode 100644 index 000000000000..9c8cb0875137 --- /dev/null +++ b/src/dom/components/__tests__/ReactDOMSelect-test.js @@ -0,0 +1,157 @@ +/** + * Copyright 2013 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * @jsx React.DOM + * @emails react-core + */ + +"use strict"; + +/*jshint evil:true */ + +describe('ReactDOMSelect', function() { + var React; + var ReactTestUtils; + + var renderSelect; + + beforeEach(function() { + React = require('React'); + ReactTestUtils = require('ReactTestUtils'); + + renderSelect = function(component) { + var stub = ReactTestUtils.renderIntoDocument(component); + var node = stub.getDOMNode(); + return node; + }; + }); + + it('should allow setting `defaultValue`', function() { + var stub = + ; + var node = renderSelect(stub); + + expect(node.value).toBe('giraffe'); + + // Changing `defaultValue` should do nothing. + stub.setProps({defaultValue: 'gorilla'}); + expect(node.value).toEqual('giraffe'); + }); + + it('should allow setting `defaultValue` with multiple', function() { + var stub = + ; + var node = renderSelect(stub); + + expect(node.options[0].selected).toBe(false); // monkey + expect(node.options[1].selected).toBe(true); // giraffe + expect(node.options[2].selected).toBe(true); // gorilla + + // Changing `defaultValue` should do nothing. + stub.setProps({defaultValue: ['monkey']}); + + expect(node.options[0].selected).toBe(false); // monkey + expect(node.options[1].selected).toBe(true); // giraffe + expect(node.options[2].selected).toBe(true); // gorilla + }); + + it('should allow setting `value`', function() { + var stub = + ; + var node = renderSelect(stub); + + expect(node.value).toBe('giraffe'); + + // Changing the `value` prop should change the selected option. + stub.setProps({value: 'gorilla'}); + expect(node.value).toEqual('gorilla'); + }); + + it('should allow setting `value` with multiple', function() { + var stub = + ; + var node = renderSelect(stub); + + expect(node.options[0].selected).toBe(false); // monkey + expect(node.options[1].selected).toBe(true); // giraffe + expect(node.options[2].selected).toBe(true); // gorilla + + // Changing the `value` prop should change the selected options. + stub.setProps({value: ['monkey']}); + + expect(node.options[0].selected).toBe(true); // monkey + expect(node.options[1].selected).toBe(false); // giraffe + expect(node.options[2].selected).toBe(false); // gorilla + }); + + it('should allow switching to multiple', function() { + var stub = + ; + var node = renderSelect(stub); + + expect(node.options[0].selected).toBe(false); // monkey + expect(node.options[1].selected).toBe(true); // giraffe + expect(node.options[2].selected).toBe(false); // gorilla + + // When making it multiple, giraffe should still be selected + stub.setProps({multiple: true, defaultValue: null}); + + expect(node.options[0].selected).toBe(false); // monkey + expect(node.options[1].selected).toBe(true); // giraffe + expect(node.options[2].selected).toBe(false); // gorilla + }); + + it('should allow switching from multiple', function() { + var stub = + ; + var node = renderSelect(stub); + + expect(node.options[0].selected).toBe(false); // monkey + expect(node.options[1].selected).toBe(true); // giraffe + expect(node.options[2].selected).toBe(true); // gorilla + + // When removing multiple, giraffe should still be selected (but gorilla + // will no longer be) + stub.setProps({multiple: false, defaultValue: null}); + + expect(node.options[0].selected).toBe(false); // monkey + expect(node.options[1].selected).toBe(true); // giraffe + expect(node.options[2].selected).toBe(false); // gorilla + }); +});