Skip to content

Commit

Permalink
Switching the name property preserves radio selection
Browse files Browse the repository at this point in the history
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
  • Loading branch information
landvibe authored and nhunzaker committed Nov 19, 2017
1 parent 3f405da commit 70abda5
Show file tree
Hide file tree
Showing 5 changed files with 124 additions and 8 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
const React = window.React;
const noop = n => n;

class RadioNameChangeFixture extends React.Component {
state = {
updated: false,
};
onClick = () => {
this.setState(state => {
return {updated: !state.updated};
});
};
render() {
const {updated} = this.state;
const radioName = updated ? 'firstName' : 'secondName';
return (
<div>
<label>
<input
type="radio"
name={radioName}
onChange={noop}
checked={updated === true}
/>
First Radio
</label>

<label>
<input
type="radio"
name={radioName}
onChange={noop}
checked={updated === false}
/>
Second Radio
</label>

<div>
<button type="button" onClick={this.onClick}>Toggle</button>
</div>
</div>
);
}
}

export default RadioNameChangeFixture;
19 changes: 19 additions & 0 deletions fixtures/dom/src/components/fixtures/input-change-events/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import TestCase from '../../TestCase';
import RangeKeyboardFixture from './RangeKeyboardFixture';
import RadioClickFixture from './RadioClickFixture';
import RadioGroupFixture from './RadioGroupFixture';
import RadioNameChangeFixture from './RadioNameChangeFixture';
import InputPlaceholderFixture from './InputPlaceholderFixture';
const React = window.React;

Expand Down Expand Up @@ -88,6 +89,24 @@ class InputChangeEvents extends React.Component {

<InputPlaceholderFixture />
</TestCase>
<TestCase
title="Radio button groups with name changes"
description={`
A radio button group should have correct checked value when
the names changes
`}
resolvedBy="#11227"
affectedBrowsers="IE9+">
<TestCase.Steps>
<li>Click the toggle button</li>
</TestCase.Steps>

<TestCase.ExpectedResult>
The checked radio button should switch between the first and second radio button
</TestCase.ExpectedResult>

<RadioNameChangeFixture />
</TestCase>
</FixtureSet>
);
}
Expand Down
39 changes: 39 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMInput-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -668,6 +668,45 @@ describe('ReactDOMInput', () => {
expect(cNode.checked).toBe(true);
});

it('should check the correct radio when the selected name moves', () => {
class App extends React.Component {
state = {
updated: false,
};
onClick = () => {
this.setState({updated: true});
};
render() {
const {updated} = this.state;
const radioName = updated ? 'secondName' : 'firstName';
return (
<div>
<button type="button" onClick={this.onClick} />
<input
type="radio"
name={radioName}
onChange={emptyFunction}
checked={updated === true}
/>
<input
type="radio"
name={radioName}
onChange={emptyFunction}
checked={updated === false}
/>
</div>
);
}
}

var stub = ReactTestUtils.renderIntoDocument(<App />);
var buttonNode = ReactDOM.findDOMNode(stub).childNodes[0];
var firstRadioNode = ReactDOM.findDOMNode(stub).childNodes[1];
expect(firstRadioNode.checked).toBe(false);
ReactTestUtils.Simulate.click(buttonNode);
expect(firstRadioNode.checked).toBe(true);
});

it('should control radio buttons if the tree updates during render', () => {
var sharedParent = document.createElement('div');
var container1 = document.createElement('div');
Expand Down
11 changes: 11 additions & 0 deletions packages/react-dom/src/client/ReactDOMFiberComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -764,6 +764,17 @@ export function updateProperties(
lastRawProps: Object,
nextRawProps: Object,
): void {
// Update checked *before* name.
// In the middle of an update, it is possible to have multiple checked.
// When a checked radio tries to change name, browser makes another radio's checked false.
if (
tag === 'input' &&
nextRawProps.type === 'radio' &&
nextRawProps.name != null
) {
ReactDOMFiberInput.updateChecked(domElement, nextRawProps);
}

var wasCustomComponentTag = isCustomComponent(tag, lastRawProps);
var isCustomComponentTag = isCustomComponent(tag, nextRawProps);
// Apply the diff.
Expand Down
17 changes: 9 additions & 8 deletions packages/react-dom/src/client/ReactDOMFiberInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,14 @@ export function initWrapperState(element: Element, props: Object) {
};
}

export function updateChecked(element: Element, props: Object) {
var node = ((element: any): InputWithWrapperState);
var checked = props.checked;
if (checked != null) {
DOMPropertyOperations.setValueForProperty(node, 'checked', checked);
}
}

export function updateWrapper(element: Element, props: Object) {
var node = ((element: any): InputWithWrapperState);
if (__DEV__) {
Expand Down Expand Up @@ -181,14 +189,7 @@ export function updateWrapper(element: Element, props: Object) {
}
}

var checked = props.checked;
if (checked != null) {
DOMPropertyOperations.setValueForProperty(
node,
'checked',
checked || false,
);
}
updateChecked(element, props);

var value = props.value;
if (value != null) {
Expand Down

0 comments on commit 70abda5

Please sign in to comment.