Skip to content

Commit 70abda5

Browse files
landvibenhunzaker
authored andcommitted
Switching the name property preserves radio selection
Fixes a case where changing the name and checked value of a radio button in the same update would lead to checking the wrong radio input. Also adds a DOM test fixture for related issue. Related issues: #7630
1 parent 3f405da commit 70abda5

File tree

5 files changed

+124
-8
lines changed

5 files changed

+124
-8
lines changed
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
const React = window.React;
2+
const noop = n => n;
3+
4+
class RadioNameChangeFixture extends React.Component {
5+
state = {
6+
updated: false,
7+
};
8+
onClick = () => {
9+
this.setState(state => {
10+
return {updated: !state.updated};
11+
});
12+
};
13+
render() {
14+
const {updated} = this.state;
15+
const radioName = updated ? 'firstName' : 'secondName';
16+
return (
17+
<div>
18+
<label>
19+
<input
20+
type="radio"
21+
name={radioName}
22+
onChange={noop}
23+
checked={updated === true}
24+
/>
25+
First Radio
26+
</label>
27+
28+
<label>
29+
<input
30+
type="radio"
31+
name={radioName}
32+
onChange={noop}
33+
checked={updated === false}
34+
/>
35+
Second Radio
36+
</label>
37+
38+
<div>
39+
<button type="button" onClick={this.onClick}>Toggle</button>
40+
</div>
41+
</div>
42+
);
43+
}
44+
}
45+
46+
export default RadioNameChangeFixture;

fixtures/dom/src/components/fixtures/input-change-events/index.js

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import TestCase from '../../TestCase';
33
import RangeKeyboardFixture from './RangeKeyboardFixture';
44
import RadioClickFixture from './RadioClickFixture';
55
import RadioGroupFixture from './RadioGroupFixture';
6+
import RadioNameChangeFixture from './RadioNameChangeFixture';
67
import InputPlaceholderFixture from './InputPlaceholderFixture';
78
const React = window.React;
89

@@ -88,6 +89,24 @@ class InputChangeEvents extends React.Component {
8889

8990
<InputPlaceholderFixture />
9091
</TestCase>
92+
<TestCase
93+
title="Radio button groups with name changes"
94+
description={`
95+
A radio button group should have correct checked value when
96+
the names changes
97+
`}
98+
resolvedBy="#11227"
99+
affectedBrowsers="IE9+">
100+
<TestCase.Steps>
101+
<li>Click the toggle button</li>
102+
</TestCase.Steps>
103+
104+
<TestCase.ExpectedResult>
105+
The checked radio button should switch between the first and second radio button
106+
</TestCase.ExpectedResult>
107+
108+
<RadioNameChangeFixture />
109+
</TestCase>
91110
</FixtureSet>
92111
);
93112
}

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -668,6 +668,45 @@ describe('ReactDOMInput', () => {
668668
expect(cNode.checked).toBe(true);
669669
});
670670

671+
it('should check the correct radio when the selected name moves', () => {
672+
class App extends React.Component {
673+
state = {
674+
updated: false,
675+
};
676+
onClick = () => {
677+
this.setState({updated: true});
678+
};
679+
render() {
680+
const {updated} = this.state;
681+
const radioName = updated ? 'secondName' : 'firstName';
682+
return (
683+
<div>
684+
<button type="button" onClick={this.onClick} />
685+
<input
686+
type="radio"
687+
name={radioName}
688+
onChange={emptyFunction}
689+
checked={updated === true}
690+
/>
691+
<input
692+
type="radio"
693+
name={radioName}
694+
onChange={emptyFunction}
695+
checked={updated === false}
696+
/>
697+
</div>
698+
);
699+
}
700+
}
701+
702+
var stub = ReactTestUtils.renderIntoDocument(<App />);
703+
var buttonNode = ReactDOM.findDOMNode(stub).childNodes[0];
704+
var firstRadioNode = ReactDOM.findDOMNode(stub).childNodes[1];
705+
expect(firstRadioNode.checked).toBe(false);
706+
ReactTestUtils.Simulate.click(buttonNode);
707+
expect(firstRadioNode.checked).toBe(true);
708+
});
709+
671710
it('should control radio buttons if the tree updates during render', () => {
672711
var sharedParent = document.createElement('div');
673712
var container1 = document.createElement('div');

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -764,6 +764,17 @@ export function updateProperties(
764764
lastRawProps: Object,
765765
nextRawProps: Object,
766766
): void {
767+
// Update checked *before* name.
768+
// In the middle of an update, it is possible to have multiple checked.
769+
// When a checked radio tries to change name, browser makes another radio's checked false.
770+
if (
771+
tag === 'input' &&
772+
nextRawProps.type === 'radio' &&
773+
nextRawProps.name != null
774+
) {
775+
ReactDOMFiberInput.updateChecked(domElement, nextRawProps);
776+
}
777+
767778
var wasCustomComponentTag = isCustomComponent(tag, lastRawProps);
768779
var isCustomComponentTag = isCustomComponent(tag, nextRawProps);
769780
// Apply the diff.

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

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,14 @@ export function initWrapperState(element: Element, props: Object) {
142142
};
143143
}
144144

145+
export function updateChecked(element: Element, props: Object) {
146+
var node = ((element: any): InputWithWrapperState);
147+
var checked = props.checked;
148+
if (checked != null) {
149+
DOMPropertyOperations.setValueForProperty(node, 'checked', checked);
150+
}
151+
}
152+
145153
export function updateWrapper(element: Element, props: Object) {
146154
var node = ((element: any): InputWithWrapperState);
147155
if (__DEV__) {
@@ -181,14 +189,7 @@ export function updateWrapper(element: Element, props: Object) {
181189
}
182190
}
183191

184-
var checked = props.checked;
185-
if (checked != null) {
186-
DOMPropertyOperations.setValueForProperty(
187-
node,
188-
'checked',
189-
checked || false,
190-
);
191-
}
192+
updateChecked(element, props);
192193

193194
var value = props.value;
194195
if (value != null) {

0 commit comments

Comments
 (0)