ReactDOMSelect makeover, fix edge-case inconsistencies and remove state
Andreas Svensson committed
Sep 25, 2014 at 11:59 UTC
2601b6a0b0c2b0bccaca67fbbfab6987ac5bef30
2 files changed
+122
-64
src/browser/ui/dom/components/ReactDOMSelect.js
+51
-58
@@ -22,11 +22,14 @@ var assign = require('Object.assign');
22
23
var select = ReactElement.createFactory('select');
24
25
-function updateWithPendingValueIfMounted() {
25
+function updateOptionsIfPendingUpdateAndMounted() {
26
/*jshint validthis:true */
27
- if (this.isMounted()) {
28
- this.setState({value: this._pendingValue});
29
- this._pendingValue = 0;
27
+ if (this._pendingUpdate) {
28
+ this._pendingUpdate = false;
29
+ var value = LinkedValueUtils.getValue(this);
30
+ if (value != null && this.isMounted()) {
31
+ updateOptions(this, value);
32
+ }
33
}
34
}
35
@@ -56,40 +59,43 @@ function selectValueType(props, propName, componentName) {
59
}
60
61
/**
59
- * If `value` is supplied, updates <option> elements on mount and update.
62
* @param {ReactComponent} component Instance of ReactDOMSelect
61
- * @param {?*} propValue For uncontrolled components, null/undefined. For
62
- * controlled components, a string (or with `multiple`, a list of strings).
63
+ * @param {*} propValue A stringable (with `multiple`, a list of stringables).
64
* @private
65
*/
66
function updateOptions(component, propValue) {
66
- var multiple = component.props.multiple;
67
- var value = propValue != null ? propValue : component.state.value;
68
- var options = component.getDOMNode().options;
67
var selectedValue, i, l;
70
- if (multiple) {
68
+ var options = component.getDOMNode().options;
69
+
70
+ if (component.props.multiple) {
71
selectedValue = {};
72
- for (i = 0, l = value.length; i < l; ++i) {
73
- selectedValue['' + value[i]] = true;
72
+ for (i = 0, l = propValue.length; i < l; i++) {
73
+ selectedValue['' + propValue[i]] = true;
74
+ }
75
+ for (i = 0, l = options.length; i < l; i++) {
76
+ var selected = selectedValue.hasOwnProperty(options[i].value);
77
+ if (options[i].selected !== selected) {
78
+ options[i].selected = selected;
79
+ }
80
}
81
} else {
76
- selectedValue = '' + value;
77
- }
78
- for (i = 0, l = options.length; i < l; i++) {
79
- var selected = multiple ?
80
- selectedValue.hasOwnProperty(options[i].value) :
81
- options[i].value === selectedValue;
82
-
83
- if (selected !== options[i].selected) {
84
- options[i].selected = selected;
82
+ // Do not set `select.value` as exact behavior isn't consistent across all
83
+ // browsers for all cases.
84
+ selectedValue = '' + propValue;
85
+ for (i = 0, l = options.length; i < l; i++) {
86
+ if (options[i].value === selectedValue) {
87
+ options[i].selected = true;
88
+ return;
89
+ }
90
}
91
+ options[0].selected = true;
92
}
93
}
94
95
/**
96
* Implements a <select> native component that allows optionally setting the
97
* props `value` and `defaultValue`. If `multiple` is false, the prop must be a
92
- * string. If `multiple` is true, the prop must be an array of strings.
98
+ * stringable. If `multiple` is true, the prop must be an array of stringables.
99
*
100
* If `value` is not supplied (or null/undefined), user actions that change the
101
* selected option will trigger updates to the rendered options.
@@ -111,22 +117,6 @@ var ReactDOMSelect = ReactClass.createClass({
117
value: selectValueType
118
},
119
114
- getInitialState: function() {
115
- return {value: this.props.defaultValue || (this.props.multiple ? [] : '')};
116
- },
117
-
118
- componentWillMount: function() {
119
- this._pendingValue = null;
120
- },
121
-
122
- componentWillReceiveProps: function(nextProps) {
123
- if (!this.props.multiple && nextProps.multiple) {
124
- this.setState({value: [this.state.value]});
125
- } else if (this.props.multiple && !nextProps.multiple) {
126
- this.setState({value: this.state.value[0]});
127
- }
128
- },
129
-
120
render: function() {
121
// Clone `this.props` so we don't mutate the input.
122
var props = assign({}, this.props);
@@ -137,16 +127,32 @@ var ReactDOMSelect = ReactClass.createClass({
127
return select(props, this.props.children);
128
},
129
130
+ componentWillMount: function() {
131
+ this._pendingUpdate = false;
132
+ },
133
+
134
componentDidMount: function() {
141
- updateOptions(this, LinkedValueUtils.getValue(this));
135
+ var value = LinkedValueUtils.getValue(this);
136
+ if (value != null) {
137
+ updateOptions(this, value);
138
+ } else if (this.props.defaultValue != null) {
139
+ updateOptions(this, this.props.defaultValue);
140
+ }
141
},
142
143
componentDidUpdate: function(prevProps) {
144
var value = LinkedValueUtils.getValue(this);
146
- var prevMultiple = !!prevProps.multiple;
147
- var multiple = !!this.props.multiple;
148
- if (value != null || prevMultiple !== multiple) {
145
+ if (value != null) {
146
+ this._pendingUpdate = false;
147
updateOptions(this, value);
148
+ } else if (!prevProps.multiple !== !this.props.multiple) {
149
+ // For simplicity, reapply `defaultValue` if `multiple` is toggled.
150
+ if (this.props.defaultValue != null) {
151
+ updateOptions(this, this.props.defaultValue);
152
+ } else {
153
+ // Revert the select back to its default unselected state.
154
+ updateOptions(this, this.props.multiple ? [] : '');
155
+ }
156
}
157
},
158
@@ -157,21 +163,8 @@ var ReactDOMSelect = ReactClass.createClass({
163
returnValue = onChange.call(this, event);
164
}
165
160
- var selectedValue;
161
- if (this.props.multiple) {
162
- selectedValue = [];
163
- var options = event.target.options;
164
- for (var i = 0, l = options.length; i < l; i++) {
165
- if (options[i].selected) {
166
- selectedValue.push(options[i].value);
167
- }
168
- }
169
- } else {
170
- selectedValue = event.target.value;
171
- }
172
-
173
- this._pendingValue = selectedValue;
174
- ReactUpdates.asap(updateWithPendingValueIfMounted, this);
166
+ this._pendingUpdate = true;
167
+ ReactUpdates.asap(updateOptionsIfPendingUpdateAndMounted, this);
168
return returnValue;
169
}
170
src/browser/ui/dom/components/__tests__/ReactDOMSelect-test.js
+71
-6
@@ -137,6 +137,27 @@ describe('ReactDOMSelect', function() {
137
expect(node.options[2].selected).toBe(true); // twelve
138
});
139
140
+ it('should reset child options selected when they are changed and `value` is set', function() {
141
+ var stub =
142
+ <select multiple={true} value={["a", "b"]}>
143
+ </select>
144
+ stub = ReactTestUtils.renderIntoDocument(stub);
145
+
146
+ stub.setProps({
147
+ children: [
148
+ <option value="a">a</option>,
149
+ <option value="b">b</option>,
150
+ <option value="c">c</option>
151
+ ]
152
+ })
153
+
154
+ var node = stub.getDOMNode()
155
+
156
+ expect(node.options[0].selected).toBe(true); // a
157
+ expect(node.options[1].selected).toBe(true); // b
158
+ expect(node.options[2].selected).toBe(false); // c
159
+ });
160
+
161
it('should allow setting `value` with `objectToString`', function() {
162
var objectToString = {
163
animal: "giraffe",
@@ -181,12 +202,12 @@ describe('ReactDOMSelect', function() {
202
expect(node.options[1].selected).toBe(true); // giraffe
203
expect(node.options[2].selected).toBe(false); // gorilla
204
184
- // When making it multiple, giraffe should still be selected
185
- stub.setProps({multiple: true, defaultValue: null});
205
+ // When making it multiple, giraffe and gorilla should be selected
206
+ stub.setProps({multiple: true, defaultValue: ['giraffe', 'gorilla']});
207
208
expect(node.options[0].selected).toBe(false); // monkey
209
expect(node.options[1].selected).toBe(true); // giraffe
189
- expect(node.options[2].selected).toBe(false); // gorilla
210
+ expect(node.options[2].selected).toBe(true); // gorilla
211
});
212
213
it('should allow switching from multiple', function() {
@@ -203,13 +224,57 @@ describe('ReactDOMSelect', function() {
224
expect(node.options[1].selected).toBe(true); // giraffe
225
expect(node.options[2].selected).toBe(true); // gorilla
226
206
- // When removing multiple, giraffe should still be selected (but gorilla
207
- // will no longer be)
208
- stub.setProps({multiple: false, defaultValue: null});
227
+ // When removing multiple, defaultValue is applied again, being omitted
228
+ // means that "monkey" will be selected
229
+ stub.setProps({multiple: false, defaultValue: 'gorilla'});
230
+
231
+ expect(node.options[0].selected).toBe(false); // monkey
232
+ expect(node.options[1].selected).toBe(false); // giraffe
233
+ expect(node.options[2].selected).toBe(true); // gorilla
234
+ });
235
+
236
+ it('should remember value when switching to uncontrolled', function() {
237
+ var stub =
238
+ <select value={'giraffe'}>
239
+ <option value="monkey">A monkey!</option>
240
+ <option value="giraffe">A giraffe!</option>
241
+ <option value="gorilla">A gorilla!</option>
242
+ </select>;
243
+ stub = ReactTestUtils.renderIntoDocument(stub);
244
+ var node = stub.getDOMNode();
245
246
expect(node.options[0].selected).toBe(false); // monkey
247
expect(node.options[1].selected).toBe(true); // giraffe
248
expect(node.options[2].selected).toBe(false); // gorilla
249
+
250
+ stub.setProps({value: null});
251
+
252
+ expect(node.options[0].selected).toBe(false); // monkey
253
+ expect(node.options[1].selected).toBe(true); // giraffe
254
+ expect(node.options[2].selected).toBe(false); // gorilla
255
+ });
256
+
257
+ it('should remember updated value when switching to uncontrolled', function() {
258
+ var stub =
259
+ <select value={'giraffe'}>
260
+ <option value="monkey">A monkey!</option>
261
+ <option value="giraffe">A giraffe!</option>
262
+ <option value="gorilla">A gorilla!</option>
263
+ </select>;
264
+ stub = ReactTestUtils.renderIntoDocument(stub);
265
+ var node = stub.getDOMNode();
266
+
267
+ stub.setProps({value: 'gorilla'});
268
+
269
+ expect(node.options[0].selected).toBe(false); // monkey
270
+ expect(node.options[1].selected).toBe(false); // giraffe
271
+ expect(node.options[2].selected).toBe(true); // gorilla
272
+
273
+ stub.setProps({value: null});
274
+
275
+ expect(node.options[0].selected).toBe(false); // monkey
276
+ expect(node.options[1].selected).toBe(false); // giraffe
277
+ expect(node.options[2].selected).toBe(true); // gorilla
278
});
279
280
it('should support ReactLink', function() {