@samitouri / QOS-React / commits / 5c6cb597f9

Fix form reset for uncontrolled select elements (#11057)

* Set defaultSelected on option element from select's defaultValue * Add form reset fixture for selects * Pass explicit value for setDefaultSelected

Brandon Dail committed Oct 10, 2017 at 08:54 UTC 5c6cb597f9a4e6eef4d1461b2af234498e52ac7f
2 files changed +68 -6
fixtures/dom/src/components/fixtures/selects/index.js
+55
@@ -7,11 +7,23 @@ const ReactDOM = window.ReactDOM;
7 class SelectFixture extends React.Component {
8 state = {value: ''};
9 _nestedDOMNode = null;
10 + _singleFormDOMNode = null;
11 + _multipleFormDOMNode = null;
12
13 onChange = event => {
14 this.setState({value: event.target.value});
15 };
16
17 + resetSingleOptionForm = event => {
18 + event.preventDefault();
19 + this._singleFormDOMNode.reset();
20 + };
21 +
22 + resetMultipleOptionForm = event => {
23 + event.preventDefault();
24 + this._multipleFormDOMNode.reset();
25 + };
26 +
27 componentDidMount() {
28 this._renderNestedSelect();
29 }
@@ -100,6 +112,49 @@ class SelectFixture extends React.Component {
112 </select>
113 </div>
114 </TestCase>
115 +
116 + <TestCase title="A single select being reset">
117 + <TestCase.Steps>
118 + <li>Open the select</li>
119 + <li>Select "baz" or "foo"</li>
120 + <li>Click the "Reset" button</li>
121 + </TestCase.Steps>
122 + <TestCase.ExpectedResult>
123 + The select should be reset to the inital value, "bar"
124 + </TestCase.ExpectedResult>
125 +
126 + <div className="test-fixture">
127 + <form ref={n => (this._singleFormDOMNode = n)}>
128 + <select defaultValue="bar">
129 + <option value="foo">foo</option>
130 + <option value="bar">bar</option>
131 + <option value="baz">baz</option>
132 + </select>
133 + <button onClick={this.resetSingleOptionForm}>Reset</button>
134 + </form>
135 + </div>
136 + </TestCase>
137 +
138 + <TestCase title="A multiple select being reset">
139 + <TestCase.Steps>
140 + <li>Select any combination of options</li>
141 + <li>Click the "Reset" button</li>
142 + </TestCase.Steps>
143 + <TestCase.ExpectedResult>
144 + The select should be reset to the initial values "foo" and "baz"
145 + </TestCase.ExpectedResult>
146 +
147 + <div className="test-fixture">
148 + <form ref={n => (this._multipleFormDOMNode = n)}>
149 + <select multiple defaultValue={['foo', 'baz']}>
150 + <option value="foo">foo</option>
151 + <option value="bar">bar</option>
152 + <option value="baz">baz</option>
153 + </select>
154 + <button onClick={this.resetMultipleOptionForm}>Reset</button>
155 + </form>
156 + </div>
157 + </TestCase>
158 </FixtureSet>
159 );
160 }
src/renderers/dom/fiber/wrappers/ReactDOMFiberSelect.js
+13 -6
@@ -76,6 +76,7 @@ function updateOptions(
76 node: HTMLSelectElement,
77 multiple: boolean,
78 propValue: any,
79 + setDefaultSelected: boolean,
80 ) {
81 type IndexableHTMLOptionsCollection = HTMLOptionsCollection & {
82 [key: number]: HTMLOptionElement,
@@ -94,6 +95,9 @@ function updateOptions(
95 if (options[i].selected !== selected) {
96 options[i].selected = selected;
97 }
98 + if (selected && setDefaultSelected) {
99 + options[i].defaultSelected = true;
100 + }
101 }
102 } else {
103 // Do not set `select.value` as exact behavior isn't consistent across all
@@ -103,6 +107,9 @@ function updateOptions(
107 for (let i = 0; i < options.length; i++) {
108 if (options[i].value === selectedValue) {
109 options[i].selected = true;
110 + if (setDefaultSelected) {
111 + options[i].defaultSelected = true;
112 + }
113 return;
114 }
115 if (defaultSelected === null && !options[i].disabled) {
@@ -173,9 +180,9 @@ var ReactDOMSelect = {
180 node.multiple = !!props.multiple;
181 var value = props.value;
182 if (value != null) {
176 - updateOptions(node, !!props.multiple, value);
183 + updateOptions(node, !!props.multiple, value, false);
184 } else if (props.defaultValue != null) {
178 - updateOptions(node, !!props.multiple, props.defaultValue);
185 + updateOptions(node, !!props.multiple, props.defaultValue, true);
186 }
187 },
188
@@ -190,14 +197,14 @@ var ReactDOMSelect = {
197
198 var value = props.value;
199 if (value != null) {
193 - updateOptions(node, !!props.multiple, value);
200 + updateOptions(node, !!props.multiple, value, false);
201 } else if (wasMultiple !== !!props.multiple) {
202 // For simplicity, reapply `defaultValue` if `multiple` is toggled.
203 if (props.defaultValue != null) {
197 - updateOptions(node, !!props.multiple, props.defaultValue);
204 + updateOptions(node, !!props.multiple, props.defaultValue, true);
205 } else {
206 // Revert the select back to its default unselected state.
200 - updateOptions(node, !!props.multiple, props.multiple ? [] : '');
207 + updateOptions(node, !!props.multiple, props.multiple ? [] : '', false);
208 }
209 }
210 },
@@ -207,7 +214,7 @@ var ReactDOMSelect = {
214 var value = props.value;
215
216 if (value != null) {
210 - updateOptions(node, !!props.multiple, value);
217 + updateOptions(node, !!props.multiple, value, false);
218 }
219 },
220 };