Skip to content

Commit

Permalink
ReactDOMSelect makeover, fix edge-case inconsistencies and remove state
Browse files Browse the repository at this point in the history
  • Loading branch information
@syranide
syranide committed Nov 3, 2014
1 parent cb50a86 commit 2601b6a
Show file tree
Hide file tree
Showing 2 changed files with 122 additions and 64 deletions.
109 changes: 51 additions & 58 deletions src/browser/ui/dom/components/ReactDOMSelect.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,14 @@ var assign = require('Object.assign');

var select = ReactElement.createFactory('select');

function updateWithPendingValueIfMounted() {
function updateOptionsIfPendingUpdateAndMounted() {
/*jshint validthis:true */
if (this.isMounted()) {
this.setState({value: this._pendingValue});
this._pendingValue = 0;
if (this._pendingUpdate) {
this._pendingUpdate = false;
var value = LinkedValueUtils.getValue(this);
if (value != null && this.isMounted()) {
updateOptions(this, value);
}
}
}

Expand Down Expand Up @@ -56,40 +59,43 @@ function selectValueType(props, propName, componentName) {
}

/**
* If `value` is supplied, updates <option> elements on mount and update.
* @param {ReactComponent} component Instance of ReactDOMSelect
* @param {?*} propValue For uncontrolled components, null/undefined. For
* controlled components, a string (or with `multiple`, a list of strings).
* @param {*} propValue A stringable (with `multiple`, a list of stringables).
* @private
*/
function updateOptions(component, propValue) {
var multiple = component.props.multiple;
var value = propValue != null ? propValue : component.state.value;
var options = component.getDOMNode().options;
var selectedValue, i, l;
if (multiple) {
var options = component.getDOMNode().options;

if (component.props.multiple) {
selectedValue = {};
for (i = 0, l = value.length; i < l; ++i) {
selectedValue['' + value[i]] = true;
for (i = 0, l = propValue.length; i < l; i++) {
selectedValue['' + propValue[i]] = true;
}
for (i = 0, l = options.length; i < l; i++) {
var selected = selectedValue.hasOwnProperty(options[i].value);
if (options[i].selected !== selected) {
options[i].selected = selected;
}
}
} else {
selectedValue = '' + value;
}
for (i = 0, l = options.length; i < l; i++) {
var selected = multiple ?
selectedValue.hasOwnProperty(options[i].value) :
options[i].value === selectedValue;

if (selected !== options[i].selected) {
options[i].selected = selected;
// Do not set `select.value` as exact behavior isn't consistent across all
// browsers for all cases.
selectedValue = '' + propValue;
for (i = 0, l = options.length; i < l; i++) {
if (options[i].value === selectedValue) {
options[i].selected = true;
return;
}
}
options[0].selected = true;
}
}

/**
* Implements a <select> native component that allows optionally setting the
* props `value` and `defaultValue`. If `multiple` is false, the prop must be a
* string. If `multiple` is true, the prop must be an array of strings.
* stringable. If `multiple` is true, the prop must be an array of stringables.
*
* If `value` is not supplied (or null/undefined), user actions that change the
* selected option will trigger updates to the rendered options.
Expand All @@ -111,22 +117,6 @@ var ReactDOMSelect = ReactClass.createClass({
value: selectValueType
},

getInitialState: function() {
return {value: this.props.defaultValue || (this.props.multiple ? [] : '')};
},

componentWillMount: function() {
this._pendingValue = null;
},

componentWillReceiveProps: function(nextProps) {
if (!this.props.multiple && nextProps.multiple) {
this.setState({value: [this.state.value]});
} else if (this.props.multiple && !nextProps.multiple) {
this.setState({value: this.state.value[0]});
}
},

render: function() {
// Clone `this.props` so we don't mutate the input.
var props = assign({}, this.props);
Expand All @@ -137,16 +127,32 @@ var ReactDOMSelect = ReactClass.createClass({
return select(props, this.props.children);
},

componentWillMount: function() {
this._pendingUpdate = false;
},

componentDidMount: function() {
updateOptions(this, LinkedValueUtils.getValue(this));
var value = LinkedValueUtils.getValue(this);
if (value != null) {
updateOptions(this, value);
} else if (this.props.defaultValue != null) {
updateOptions(this, this.props.defaultValue);
}
},

componentDidUpdate: function(prevProps) {
var value = LinkedValueUtils.getValue(this);
var prevMultiple = !!prevProps.multiple;
var multiple = !!this.props.multiple;
if (value != null || prevMultiple !== multiple) {
if (value != null) {
this._pendingUpdate = false;
updateOptions(this, value);
} else if (!prevProps.multiple !== !this.props.multiple) {
// For simplicity, reapply `defaultValue` if `multiple` is toggled.
if (this.props.defaultValue != null) {
updateOptions(this, this.props.defaultValue);
} else {
// Revert the select back to its default unselected state.
updateOptions(this, this.props.multiple ? [] : '');
}
}
},

Expand All @@ -157,21 +163,8 @@ var ReactDOMSelect = ReactClass.createClass({
returnValue = onChange.call(this, event);
}

var selectedValue;
if (this.props.multiple) {
selectedValue = [];
var options = event.target.options;
for (var i = 0, l = options.length; i < l; i++) {
if (options[i].selected) {
selectedValue.push(options[i].value);
}
}
} else {
selectedValue = event.target.value;
}

this._pendingValue = selectedValue;
ReactUpdates.asap(updateWithPendingValueIfMounted, this);
this._pendingUpdate = true;
ReactUpdates.asap(updateOptionsIfPendingUpdateAndMounted, this);
return returnValue;
}

Expand Down
77 changes: 71 additions & 6 deletions src/browser/ui/dom/components/__tests__/ReactDOMSelect-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,27 @@ describe('ReactDOMSelect', function() {
expect(node.options[2].selected).toBe(true); // twelve
});

it('should reset child options selected when they are changed and `value` is set', function() {
var stub =
<select multiple={true} value={["a", "b"]}>
</select>
stub = ReactTestUtils.renderIntoDocument(stub);

stub.setProps({
children: [
<option value="a">a</option>,
<option value="b">b</option>,
<option value="c">c</option>
]
})

var node = stub.getDOMNode()

expect(node.options[0].selected).toBe(true); // a
expect(node.options[1].selected).toBe(true); // b
expect(node.options[2].selected).toBe(false); // c
});

it('should allow setting `value` with `objectToString`', function() {
var objectToString = {
animal: "giraffe",
Expand Down Expand Up @@ -181,12 +202,12 @@ describe('ReactDOMSelect', function() {
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});
// When making it multiple, giraffe and gorilla should be selected
stub.setProps({multiple: true, defaultValue: ['giraffe', 'gorilla']});

expect(node.options[0].selected).toBe(false); // monkey
expect(node.options[1].selected).toBe(true); // giraffe
expect(node.options[2].selected).toBe(false); // gorilla
expect(node.options[2].selected).toBe(true); // gorilla
});

it('should allow switching from multiple', function() {
Expand All @@ -203,13 +224,57 @@ describe('ReactDOMSelect', function() {
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});
// When removing multiple, defaultValue is applied again, being omitted
// means that "monkey" will be selected
stub.setProps({multiple: false, defaultValue: 'gorilla'});

expect(node.options[0].selected).toBe(false); // monkey
expect(node.options[1].selected).toBe(false); // giraffe
expect(node.options[2].selected).toBe(true); // gorilla
});

it('should remember value when switching to uncontrolled', function() {
var stub =
<select value={'giraffe'}>
<option value="monkey">A monkey!</option>
<option value="giraffe">A giraffe!</option>
<option value="gorilla">A gorilla!</option>
</select>;
stub = ReactTestUtils.renderIntoDocument(stub);
var node = stub.getDOMNode();

expect(node.options[0].selected).toBe(false); // monkey
expect(node.options[1].selected).toBe(true); // giraffe
expect(node.options[2].selected).toBe(false); // gorilla

stub.setProps({value: 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 remember updated value when switching to uncontrolled', function() {
var stub =
<select value={'giraffe'}>
<option value="monkey">A monkey!</option>
<option value="giraffe">A giraffe!</option>
<option value="gorilla">A gorilla!</option>
</select>;
stub = ReactTestUtils.renderIntoDocument(stub);
var node = stub.getDOMNode();

stub.setProps({value: 'gorilla'});

expect(node.options[0].selected).toBe(false); // monkey
expect(node.options[1].selected).toBe(false); // giraffe
expect(node.options[2].selected).toBe(true); // gorilla

stub.setProps({value: null});

expect(node.options[0].selected).toBe(false); // monkey
expect(node.options[1].selected).toBe(false); // giraffe
expect(node.options[2].selected).toBe(true); // gorilla
});

it('should support ReactLink', function() {
Expand Down

0 comments on commit 2601b6a

Please sign in to comment.