Skip to content

Commit 8df407d

Browse files
committed
Merge pull request #281 from spicyj/radio
Fix controlled radio button behavior
2 parents 2ba405d + b56b588 commit 8df407d

File tree

4 files changed

+105
-6
lines changed

4 files changed

+105
-6
lines changed

src/core/ReactMount.js

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ var nodeCache = {};
3131
var $ = require('$');
3232

3333
/** Mapping from reactRootID to React component instance. */
34-
var instanceByReactRootID = {};
34+
var instancesByReactRootID = {};
3535

3636
/** Mapping from reactRootID to `container` nodes. */
3737
var containersByReactRootID = {};
@@ -212,7 +212,7 @@ var ReactMount = {
212212
useTouchEvents: false,
213213

214214
/** Exposed for debugging purposes **/
215-
_instanceByReactRootID: instanceByReactRootID,
215+
_instancesByReactRootID: instancesByReactRootID,
216216

217217
/**
218218
* This is a hook provided to support rendering React components while
@@ -272,7 +272,7 @@ var ReactMount = {
272272
ReactMount.prepareEnvironmentForDOM();
273273

274274
var reactRootID = ReactMount.registerContainer(container);
275-
instanceByReactRootID[reactRootID] = nextComponent;
275+
instancesByReactRootID[reactRootID] = nextComponent;
276276
return reactRootID;
277277
},
278278

@@ -316,7 +316,7 @@ var ReactMount = {
316316
* @return {ReactComponent} Component instance rendered in `container`.
317317
*/
318318
renderComponent: function(nextComponent, container, callback) {
319-
var registeredComponent = instanceByReactRootID[getReactRootID(container)];
319+
var registeredComponent = instancesByReactRootID[getReactRootID(container)];
320320

321321
if (registeredComponent) {
322322
if (registeredComponent.constructor === nextComponent.constructor) {
@@ -403,12 +403,12 @@ var ReactMount = {
403403
*/
404404
unmountAndReleaseReactRootNode: function(container) {
405405
var reactRootID = getReactRootID(container);
406-
var component = instanceByReactRootID[reactRootID];
406+
var component = instancesByReactRootID[reactRootID];
407407
if (!component) {
408408
return false;
409409
}
410410
ReactMount.unmountComponentFromNode(component, container);
411-
delete instanceByReactRootID[reactRootID];
411+
delete instancesByReactRootID[reactRootID];
412412
delete containersByReactRootID[reactRootID];
413413
if (__DEV__) {
414414
delete rootElementsByReactRootID[reactRootID];

src/dom/DefaultDOMPropertyConfig.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ var DefaultDOMPropertyConfig = {
5959
disabled: MUST_USE_PROPERTY | HAS_BOOLEAN_VALUE,
6060
draggable: null,
6161
encType: null,
62+
form: MUST_USE_ATTRIBUTE,
6263
frameBorder: MUST_USE_ATTRIBUTE,
6364
height: MUST_USE_ATTRIBUTE,
6465
hidden: MUST_USE_ATTRIBUTE | HAS_BOOLEAN_VALUE,

src/dom/components/ReactDOMInput.js

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,16 @@
2121
var DOMPropertyOperations = require('DOMPropertyOperations');
2222
var ReactCompositeComponent = require('ReactCompositeComponent');
2323
var ReactDOM = require('ReactDOM');
24+
var ReactMount = require('ReactMount');
2425

26+
var invariant = require('invariant');
2527
var merge = require('merge');
2628

2729
// Store a reference to the <input> `ReactNativeComponent`.
2830
var input = ReactDOM.input;
2931

32+
var instancesByReactID = {};
33+
3034
/**
3135
* Implements an <input> native component that allows setting these optional
3236
* props: `checked`, `value`, `defaultChecked`, and `defaultValue`.
@@ -73,6 +77,17 @@ var ReactDOMInput = ReactCompositeComponent.createClass({
7377
return input(props, this.props.children);
7478
},
7579

80+
componentDidMount: function(rootNode) {
81+
var id = ReactMount.getID(rootNode);
82+
instancesByReactID[id] = this;
83+
},
84+
85+
componentWillUnmount: function() {
86+
var rootNode = this.getDOMNode();
87+
var id = ReactMount.getID(rootNode);
88+
delete instancesByReactID[id];
89+
},
90+
7691
componentDidUpdate: function(prevProps, prevState, rootNode) {
7792
if (this.props.checked != null) {
7893
DOMPropertyOperations.setValueForProperty(
@@ -103,6 +118,46 @@ var ReactDOMInput = ReactCompositeComponent.createClass({
103118
checked: event.target.checked,
104119
value: event.target.value
105120
});
121+
122+
var name = this.props.name;
123+
if (this.props.type === 'radio' && name != null) {
124+
var rootNode = this.getDOMNode();
125+
// If `rootNode.form` was non-null, then we could try `form.elements`,
126+
// but that sometimes behaves strangely in IE8. We could also try using
127+
// `form.getElementsByName`, but that will only return direct children
128+
// and won't include inputs that use the HTML5 `form=` attribute. Since
129+
// the input might not even be in a form, let's just use the global
130+
// `getElementsByName` to ensure we don't miss anything.
131+
var group = document.getElementsByName(name);
132+
for (var i = 0; i < group.length; i++) {
133+
var otherNode = group[i];
134+
if (otherNode === rootNode ||
135+
otherNode.nodeName !== 'INPUT' || otherNode.type !== 'radio' ||
136+
otherNode.form !== rootNode.form) {
137+
continue;
138+
}
139+
var otherID = ReactMount.getID(otherNode);
140+
invariant(
141+
otherID,
142+
'ReactDOMInput: Mixing React and non-React radio inputs with the ' +
143+
'same `name` is not supported.'
144+
);
145+
var otherInstance = instancesByReactID[otherID];
146+
invariant(
147+
otherInstance,
148+
'ReactDOMInput: Unknown radio button ID %s.',
149+
otherID
150+
);
151+
// In some cases, this will actually change the `checked` state value.
152+
// In other cases, there's no change but this forces a reconcile upon
153+
// which componentDidUpdate will reset the DOM property to whatever it
154+
// should be.
155+
otherInstance.setState({
156+
checked: false
157+
});
158+
}
159+
}
160+
106161
return returnValue;
107162
}
108163

src/dom/components/__tests__/ReactDOMInput-test.js

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,4 +52,47 @@ describe('ReactDOMInput', function() {
5252
expect(node.value).toBe('0');
5353
});
5454

55+
it('should control radio buttons', function() {
56+
var RadioGroup = React.createClass({
57+
render: function() {
58+
return (
59+
<div>
60+
<input ref="a" type="radio" name="fruit" checked={true} />A
61+
<input ref="b" type="radio" name="fruit" />B
62+
63+
<form>
64+
<input ref="c" type="radio" name="fruit" form="pluto"
65+
defaultChecked={true} />
66+
</form>
67+
</div>
68+
);
69+
}
70+
});
71+
72+
var stub = ReactTestUtils.renderIntoDocument(<RadioGroup />);
73+
var aNode = stub.refs.a.getDOMNode();
74+
var bNode = stub.refs.b.getDOMNode();
75+
var cNode = stub.refs.c.getDOMNode();
76+
77+
expect(aNode.checked).toBe(true);
78+
expect(bNode.checked).toBe(false);
79+
// c is in a separate form and shouldn't be affected at all here
80+
expect(cNode.checked).toBe(true);
81+
82+
bNode.checked = true;
83+
// This next line isn't necessary in a proper browser environment, but
84+
// jsdom doesn't uncheck the others in a group (which makes this whole test
85+
// a little less effective)
86+
aNode.checked = false;
87+
expect(cNode.checked).toBe(true);
88+
89+
// Now let's run the actual ReactDOMInput change event handler (on radio
90+
// inputs, ChangeEventPlugin listens for the `click` event so trigger that)
91+
ReactTestUtils.Simulate.click(bNode);
92+
93+
// The original state should have been restored
94+
expect(aNode.checked).toBe(true);
95+
expect(cNode.checked).toBe(true);
96+
});
97+
5598
});

0 commit comments

Comments
 (0)