Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 60 additions & 3 deletions fixtures/dom/src/components/fixtures/selects/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,28 @@ import TestCase from '../../TestCase';

const React = window.React;
const ReactDOM = window.ReactDOM;
const ReactDOMClient = window.ReactDOMClient;

class ControlledSelect extends React.Component {
constructor(props) {
super(props);
this.state = {value: 'bar'};
}

handleChange = event => {
this.setState({value: event.target.value});
};

render() {
return (
<select value={this.state.value} onChange={this.handleChange}>
<option value="foo">foo</option>
<option value="bar">bar</option>
<option value="baz">baz</option>
</select>
);
}
}

class SelectFixture extends React.Component {
state = {value: ''};
Expand Down Expand Up @@ -33,15 +55,24 @@ class SelectFixture extends React.Component {
}

_renderNestedSelect() {
ReactDOM.render(
const element = (
<select value={this.state.value} onChange={this.onChange}>
<option value="">Select a color</option>
<option value="red">Red</option>
<option value="blue">Blue</option>
<option value="green">Green</option>
</select>,
this._nestedDOMNode
</select>
);
const container = this._nestedDOMNode;
if (ReactDOMClient === undefined) {
ReactDOM.render(element, container);
} else {
const root = ReactDOMClient.createRoot(container);
// eslint-disable-next-line no-undef -- queueMicrotask is supported in modern browsers
queueMicrotask(
ReactDOM.flushSync.bind(null, root.render.bind(root, element))
);
}
}

render() {
Expand Down Expand Up @@ -230,6 +261,32 @@ class SelectFixture extends React.Component {
select attribute assignment.
</p>
</TestCase>

<TestCase
title="A controlled select should keep its value on form reset"
relatedIssues="30580"
introducedIn="?">
<TestCase.ExpectedResult>
No options should be selected.
</TestCase.ExpectedResult>

<div className="test-fixture">
<form>
<ControlledSelect />
<input type="reset" />
</form>
</div>

<p className="footnote">
<b>Notes:</b> <code>form.reset()</code> resets each associated form
element to its default value. For HTMLInputElement that's the value
attribute. For HTMLSELECTElement it's option with defaultSelected
set to true.
</p>
<p className="footnote">
Bug became more pressing when React introduced automatic form reset.
</p>
</TestCase>
</FixtureSet>
);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/react-dom-bindings/src/client/ReactDOMOption.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export function validateOptionProps(element: Element, props: Object) {
if (props.selected != null && !didWarnSelectedSetOnOption) {
console.error(
'Use the `defaultValue` or `value` props on <select> instead of ' +
'setting `selected` on <option>.',
'setting `selected` on <option>. Otherwise form reset may not work as expected.',
);
didWarnSelectedSetOnOption = true;
}
Expand Down
33 changes: 21 additions & 12 deletions packages/react-dom-bindings/src/client/ReactDOMSelect.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,29 +80,38 @@ function updateOptions(
if (options[i].selected !== selected) {
options[i].selected = selected;
}
if (selected && setDefaultSelected) {
options[i].defaultSelected = true;
if (setDefaultSelected) {
options[i].defaultSelected = selected;
}
}
} else {
// Do not set `select.value` as exact behavior isn't consistent across all
// browsers for all cases.
const selectedValue = toString(getToStringValue(propValue));
let defaultSelected = null;
// We use null as a signal that an option is explicitly selected.
let defaultSelected: void | null | HTMLOptionElement = undefined;
for (let i = 0; i < options.length; i++) {
if (options[i].value === selectedValue) {
const selected = options[i].value === selectedValue;
if (setDefaultSelected) {
options[i].defaultSelected = selected;
}
if (selected) {
options[i].selected = true;
if (setDefaultSelected) {
options[i].defaultSelected = true;
defaultSelected = null;
if (!setDefaultSelected) {
// We need to loop through all options when updating defaultSelected.
// Otherwise multiple options may end up with defaultSelected=true
// and resetting won't work properly.
return;
}
return;
}
if (defaultSelected === null && !options[i].disabled) {
if (defaultSelected === undefined && !options[i].disabled) {
defaultSelected = options[i];
}
}
if (defaultSelected !== null) {
if (defaultSelected !== null && defaultSelected !== undefined) {
defaultSelected.selected = true;
defaultSelected.defaultSelected = true;
}
}
}
Expand Down Expand Up @@ -152,7 +161,7 @@ export function initSelect(
const node: HTMLSelectElement = (element: any);
node.multiple = !!multiple;
if (value != null) {
updateOptions(node, !!multiple, value, false);
updateOptions(node, !!multiple, value, true);
} else if (defaultValue != null) {
updateOptions(node, !!multiple, defaultValue, true);
}
Expand Down Expand Up @@ -221,7 +230,7 @@ export function updateSelect(
const node: HTMLSelectElement = (element: any);

if (value != null) {
updateOptions(node, !!multiple, value, false);
updateOptions(node, !!multiple, value, true);
} else if (!!wasMultiple !== !!multiple) {
// For simplicity, reapply `defaultValue` if `multiple` is toggled.
if (defaultValue != null) {
Expand All @@ -238,6 +247,6 @@ export function restoreControlledSelectState(element: Element, props: Object) {
const value = props.value;

if (value != null) {
updateOptions(node, !!props.multiple, value, false);
updateOptions(node, !!props.multiple, value, true);
}
}
2 changes: 1 addition & 1 deletion packages/react-dom/src/__tests__/ReactDOMSelect-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -792,7 +792,7 @@ describe('ReactDOMSelect', () => {
});
assertConsoleErrorDev([
'Use the `defaultValue` or `value` props on <select> instead of ' +
'setting `selected` on <option>.\n' +
'setting `selected` on <option>. Otherwise form reset may not work as expected.\n' +
' in option (at **)\n' +
' in App (at **)',
]);
Expand Down
Loading