Don't set the first option as selected in select tag with `size` attribute (#14242)
* Set 'size' attribute to select tag if it occurs before appending options * Add comment about why size is assigned on select create. Tests I added some more clarification for why size must be set on select element creation: - In the source code - In the DOM test fixture - In a unit test * Use let, not const in select tag stub assignment
Mateusz committed
Mar 14, 2019 at 22:40 UTC
ab5fe174c69ea4ea5019390f4234a83ef7ce8d52
3 files changed
+71
-6
fixtures/dom/src/components/fixtures/selects/index.js
+28
@@ -202,6 +202,34 @@ class SelectFixture extends React.Component {
202
</select>
203
</div>
204
</TestCase>
205
+
206
+ <TestCase
207
+ title="A select with the size attribute should not set first option as selected"
208
+ relatedIssues="14239"
209
+ introducedIn="16.0.0">
210
+ <TestCase.ExpectedResult>
211
+ No options should be selected.
212
+ </TestCase.ExpectedResult>
213
+
214
+ <div className="test-fixture">
215
+ <select size="3">
216
+ <option>0</option>
217
+ <option>1</option>
218
+ <option>2</option>
219
+ </select>
220
+ </div>
221
+
222
+ <p className="footnote">
223
+ <b>Notes:</b> This happens if <code>size</code> is assigned after
224
+ options are selected. The select element picks the first item by
225
+ default, then it is expanded to show more options when{' '}
226
+ <code>size</code> is assigned, preserving the default selection.
227
+ </p>
228
+ <p className="footnote">
229
+ This was introduced in React 16.0.0 when options were added before
230
+ select attribute assignment.
231
+ </p>
232
+ </TestCase>
233
</FixtureSet>
234
);
235
}
packages/react-dom/src/__tests__/ReactDOMSelect-test.js
+26
@@ -362,6 +362,32 @@ describe('ReactDOMSelect', () => {
362
expect(node.options[2].selected).toBe(true); // gorilla
363
});
364
365
+ it('does not select an item when size is initially set to greater than 1', () => {
366
+ const stub = (
367
+ <select size="2">
368
+ <option value="monkey">A monkey!</option>
369
+ <option value="giraffe">A giraffe!</option>
370
+ <option value="gorilla">A gorilla!</option>
371
+ </select>
372
+ );
373
+ const container = document.createElement('div');
374
+ const select = ReactDOM.render(stub, container);
375
+
376
+ expect(select.options[0].selected).toBe(false);
377
+ expect(select.options[1].selected).toBe(false);
378
+ expect(select.options[2].selected).toBe(false);
379
+
380
+ // Note: There is an inconsistency between JSDOM and Chrome where
381
+ // Chrome reports an empty string when no value is selected for a
382
+ // single-select with a size greater than 0. JSDOM reports the first
383
+ // value
384
+ //
385
+ // This assertion exists only for clarity of JSDOM behavior:
386
+ expect(select.value).toBe('monkey'); // "" in Chrome
387
+ // Despite this, the selection index is correct:
388
+ expect(select.selectedIndex).toBe(-1);
389
+ });
390
+
391
it('should remember value when switching to uncontrolled', () => {
392
let stub = (
393
<select value={'giraffe'} onChange={noop}>
packages/react-dom/src/client/ReactDOMComponent.js
+17
-6
@@ -419,14 +419,25 @@ export function createElement(
419
// See discussion in https://github.com/facebook/react/pull/6896
420
// and discussion in https://bugzilla.mozilla.org/show_bug.cgi?id=1276240
421
domElement = ownerDocument.createElement(type);
422
- // Normally attributes are assigned in `setInitialDOMProperties`, however the `multiple`
423
- // attribute on `select`s needs to be added before `option`s are inserted. This prevents
424
- // a bug where the `select` does not scroll to the correct option because singular
425
- // `select` elements automatically pick the first item.
422
+ // Normally attributes are assigned in `setInitialDOMProperties`, however the `multiple` and `size`
423
+ // attributes on `select`s needs to be added before `option`s are inserted.
424
+ // This prevents:
425
+ // - a bug where the `select` does not scroll to the correct option because singular
426
+ // `select` elements automatically pick the first item #13222
427
+ // - a bug where the `select` set the first item as selected despite the `size` attribute #14239
428
// See https://github.com/facebook/react/issues/13222
427
- if (type === 'select' && props.multiple) {
429
+ // and https://github.com/facebook/react/issues/14239
430
+ if (type === 'select') {
431
const node = ((domElement: any): HTMLSelectElement);
429
- node.multiple = true;
432
+ if (props.multiple) {
433
+ node.multiple = true;
434
+ } else if (props.size) {
435
+ // Setting a size greater than 1 causes a select to behave like `multiple=true`, where
436
+ // it is possible that no option is selected.
437
+ //
438
+ // This is only necessary when a select in "single selection mode".
439
+ node.size = props.size;
440
+ }
441
}
442
}
443
} else {