Skip to content

Commit ab5fe17

Browse files
kulek1nhunzaker
authored andcommitted
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
1 parent 935f600 commit ab5fe17

File tree

3 files changed

+71
-6
lines changed

3 files changed

+71
-6
lines changed

fixtures/dom/src/components/fixtures/selects/index.js

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,34 @@ class SelectFixture extends React.Component {
202202
</select>
203203
</div>
204204
</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>
205233
</FixtureSet>
206234
);
207235
}

packages/react-dom/src/__tests__/ReactDOMSelect-test.js

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,32 @@ describe('ReactDOMSelect', () => {
362362
expect(node.options[2].selected).toBe(true); // gorilla
363363
});
364364

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+
365391
it('should remember value when switching to uncontrolled', () => {
366392
let stub = (
367393
<select value={'giraffe'} onChange={noop}>

packages/react-dom/src/client/ReactDOMComponent.js

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -419,14 +419,25 @@ export function createElement(
419419
// See discussion in https://github.com/facebook/react/pull/6896
420420
// and discussion in https://bugzilla.mozilla.org/show_bug.cgi?id=1276240
421421
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
426428
// 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') {
428431
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+
}
430441
}
431442
}
432443
} else {

0 commit comments

Comments
 (0)