Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug fix on input radio #11227

Merged
merged 9 commits into from
Nov 19, 2017
Merged
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
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 @@ -5,6 +5,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';

class InputChangeEvents extends React.Component {
Expand Down Expand Up @@ -87,6 +88,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 @@ -669,6 +669,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 @@ -771,6 +771,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 @@ -144,6 +144,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 @@ -183,14 +191,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