@samitouri / QOS-React-2 / commits / b56b5885d0

Fix controlled radio button behavior

Fixes #242.

Ben Alpert committed Aug 19, 2013 at 23:31 UTC b56b5885d01c14041a5281fd60a46207b1b680dd
4 files changed +105 -6
src/core/ReactMount.js
+6 -6
@@ -31,7 +31,7 @@ var nodeCache = {};
31 var $ = require('$');
32
33 /** Mapping from reactRootID to React component instance. */
34 -var instanceByReactRootID = {};
34 +var instancesByReactRootID = {};
35
36 /** Mapping from reactRootID to `container` nodes. */
37 var containersByReactRootID = {};
@@ -212,7 +212,7 @@ var ReactMount = {
212 useTouchEvents: false,
213
214 /** Exposed for debugging purposes **/
215 - _instanceByReactRootID: instanceByReactRootID,
215 + _instancesByReactRootID: instancesByReactRootID,
216
217 /**
218 * This is a hook provided to support rendering React components while
@@ -272,7 +272,7 @@ var ReactMount = {
272 ReactMount.prepareEnvironmentForDOM();
273
274 var reactRootID = ReactMount.registerContainer(container);
275 - instanceByReactRootID[reactRootID] = nextComponent;
275 + instancesByReactRootID[reactRootID] = nextComponent;
276 return reactRootID;
277 },
278
@@ -316,7 +316,7 @@ var ReactMount = {
316 * @return {ReactComponent} Component instance rendered in `container`.
317 */
318 renderComponent: function(nextComponent, container, callback) {
319 - var registeredComponent = instanceByReactRootID[getReactRootID(container)];
319 + var registeredComponent = instancesByReactRootID[getReactRootID(container)];
320
321 if (registeredComponent) {
322 if (registeredComponent.constructor === nextComponent.constructor) {
@@ -403,12 +403,12 @@ var ReactMount = {
403 */
404 unmountAndReleaseReactRootNode: function(container) {
405 var reactRootID = getReactRootID(container);
406 - var component = instanceByReactRootID[reactRootID];
406 + var component = instancesByReactRootID[reactRootID];
407 if (!component) {
408 return false;
409 }
410 ReactMount.unmountComponentFromNode(component, container);
411 - delete instanceByReactRootID[reactRootID];
411 + delete instancesByReactRootID[reactRootID];
412 delete containersByReactRootID[reactRootID];
413 if (__DEV__) {
414 delete rootElementsByReactRootID[reactRootID];
src/dom/DefaultDOMPropertyConfig.js
+1
@@ -59,6 +59,7 @@ var DefaultDOMPropertyConfig = {
59 disabled: MUST_USE_PROPERTY | HAS_BOOLEAN_VALUE,
60 draggable: null,
61 encType: null,
62 + form: MUST_USE_ATTRIBUTE,
63 frameBorder: MUST_USE_ATTRIBUTE,
64 height: MUST_USE_ATTRIBUTE,
65 hidden: MUST_USE_ATTRIBUTE | HAS_BOOLEAN_VALUE,
src/dom/components/ReactDOMInput.js
+55
@@ -21,12 +21,16 @@
21 var DOMPropertyOperations = require('DOMPropertyOperations');
22 var ReactCompositeComponent = require('ReactCompositeComponent');
23 var ReactDOM = require('ReactDOM');
24 +var ReactMount = require('ReactMount');
25
26 +var invariant = require('invariant');
27 var merge = require('merge');
28
29 // Store a reference to the <input> `ReactNativeComponent`.
30 var input = ReactDOM.input;
31
32 +var instancesByReactID = {};
33 +
34 /**
35 * Implements an <input> native component that allows setting these optional
36 * props: `checked`, `value`, `defaultChecked`, and `defaultValue`.
@@ -71,6 +75,17 @@ var ReactDOMInput = ReactCompositeComponent.createClass({
75 return input(props, this.props.children);
76 },
77
78 + componentDidMount: function(rootNode) {
79 + var id = ReactMount.getID(rootNode);
80 + instancesByReactID[id] = this;
81 + },
82 +
83 + componentWillUnmount: function() {
84 + var rootNode = this.getDOMNode();
85 + var id = ReactMount.getID(rootNode);
86 + delete instancesByReactID[id];
87 + },
88 +
89 componentDidUpdate: function(prevProps, prevState, rootNode) {
90 if (this.props.checked != null) {
91 DOMPropertyOperations.setValueForProperty(
@@ -101,6 +116,46 @@ var ReactDOMInput = ReactCompositeComponent.createClass({
116 checked: event.target.checked,
117 value: event.target.value
118 });
119 +
120 + var name = this.props.name;
121 + if (this.props.type === 'radio' && name != null) {
122 + var rootNode = this.getDOMNode();
123 + // If `rootNode.form` was non-null, then we could try `form.elements`,
124 + // but that sometimes behaves strangely in IE8. We could also try using
125 + // `form.getElementsByName`, but that will only return direct children
126 + // and won't include inputs that use the HTML5 `form=` attribute. Since
127 + // the input might not even be in a form, let's just use the global
128 + // `getElementsByName` to ensure we don't miss anything.
129 + var group = document.getElementsByName(name);
130 + for (var i = 0; i < group.length; i++) {
131 + var otherNode = group[i];
132 + if (otherNode === rootNode ||
133 + otherNode.nodeName !== 'INPUT' || otherNode.type !== 'radio' ||
134 + otherNode.form !== rootNode.form) {
135 + continue;
136 + }
137 + var otherID = ReactMount.getID(otherNode);
138 + invariant(
139 + otherID,
140 + 'ReactDOMInput: Mixing React and non-React radio inputs with the ' +
141 + 'same `name` is not supported.'
142 + );
143 + var otherInstance = instancesByReactID[otherID];
144 + invariant(
145 + otherInstance,
146 + 'ReactDOMInput: Unknown radio button ID %s.',
147 + otherID
148 + );
149 + // In some cases, this will actually change the `checked` state value.
150 + // In other cases, there's no change but this forces a reconcile upon
151 + // which componentDidUpdate will reset the DOM property to whatever it
152 + // should be.
153 + otherInstance.setState({
154 + checked: false
155 + });
156 + }
157 + }
158 +
159 return returnValue;
160 }
161
src/dom/components/__tests__/ReactDOMInput-test.js
+43
@@ -52,4 +52,47 @@ describe('ReactDOMInput', function() {
52 expect(node.value).toBe('0');
53 });
54
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 +
98 });