From 7b047111a0584d5a730764ff33fa37a74fc7b5fc Mon Sep 17 00:00:00 2001 From: Johannes Baiter Date: Mon, 13 Jan 2014 22:26:51 +0100 Subject: [PATCH] Support two-way binding for checkboxes in LinkedValueMixin via 'checkedLink' property --- src/dom/components/LinkedValueUtils.js | 54 +++++++++++++- src/dom/components/ReactDOMInput.js | 5 +- .../__tests__/ReactDOMInput-test.js | 73 +++++++++++++++++++ 3 files changed, 127 insertions(+), 5 deletions(-) diff --git a/src/dom/components/LinkedValueUtils.js b/src/dom/components/LinkedValueUtils.js index e0fc356e86..79a247cf53 100644 --- a/src/dom/components/LinkedValueUtils.js +++ b/src/dom/components/LinkedValueUtils.js @@ -27,7 +27,7 @@ var hasReadOnlyValue = { 'radio': true }; -function _assertLink(input) { +function _assertValueLink(input) { invariant( input.props.value == null && input.props.onChange == null, 'Cannot provide a valueLink and a value or onChange event. If you want ' + @@ -35,6 +35,15 @@ function _assertLink(input) { ); } +function _assertCheckedLink(input) { + invariant( + input.props.checked == null && input.props.onChange == null, + 'Cannot provide a checkedLink and a checked property or onChange event. ' + + 'If you want to use checked or onChange, you probably don\'t want to ' + + 'use checkedLink' + ); +} + /** * @param {SyntheticEvent} e change event to handle */ @@ -43,6 +52,14 @@ function _handleLinkedValueChange(e) { this.props.valueLink.requestChange(e.target.value); } +/** + * @param {SyntheticEvent} e change event to handle + */ +function _handleLinkedCheckChange(e) { + /*jshint validthis:true */ + this.props.checkedLink.requestChange(e.target.checked === 'true'); +} + /** * Provide a linked `value` attribute for controlled forms. You should not use * this outside of the ReactDOM controlled form components. @@ -65,6 +82,21 @@ var LinkedValueUtils = { ); } } + }, + checked: function(props, propName, componentName) { + if (__DEV__) { + if (props[propName] && + !props.onChange && + !props.readOnly && + !props.disabled) { + console.warn( + 'You provided a `checked` prop to a form field without an ' + + '`onChange` handler. This will render a read-only field. If ' + + 'the field should be mutable use `defaultChecked`. Otherwise, ' + + 'set either `onChange` or `readOnly`.' + ); + } + } } } }, @@ -75,20 +107,36 @@ var LinkedValueUtils = { */ getValue: function(input) { if (input.props.valueLink) { - _assertLink(input); + _assertValueLink(input); return input.props.valueLink.value; } return input.props.value; }, + /** + * @param {ReactComponent} input Form component + * @return {*} current checked status of the input either from checked prop + * or link. + */ + getChecked: function(input) { + if (input.props.checkedLink) { + _assertCheckedLink(input); + return input.props.checkedLink.value; + } + return input.props.checked; + }, + /** * @param {ReactComponent} input Form component * @return {function} change callback either from onChange prop or link. */ getOnChange: function(input) { if (input.props.valueLink) { - _assertLink(input); + _assertValueLink(input); return _handleLinkedValueChange; + } else if (input.props.checkedLink) { + _assertCheckedLink(input); + return _handleLinkedCheckChange; } return input.props.onChange; } diff --git a/src/dom/components/ReactDOMInput.js b/src/dom/components/ReactDOMInput.js index 58febb6372..27148e483b 100644 --- a/src/dom/components/ReactDOMInput.js +++ b/src/dom/components/ReactDOMInput.js @@ -73,12 +73,13 @@ var ReactDOMInput = ReactCompositeComponent.createClass({ props.defaultChecked = null; props.defaultValue = null; - props.checked = - this.props.checked != null ? this.props.checked : this.state.checked; var value = LinkedValueUtils.getValue(this); props.value = value != null ? value : this.state.value; + var checked = LinkedValueUtils.getChecked(this); + props.checked = checked != null ? checked : this.state.checked; + props.onChange = this._handleChange; return input(props, this.props.children); diff --git a/src/dom/components/__tests__/ReactDOMInput-test.js b/src/dom/components/__tests__/ReactDOMInput-test.js index 6898956c66..df86ab6f8d 100644 --- a/src/dom/components/__tests__/ReactDOMInput-test.js +++ b/src/dom/components/__tests__/ReactDOMInput-test.js @@ -252,4 +252,77 @@ describe('ReactDOMInput', function() { expect(React.renderComponent.bind(React, instance, node)).toThrow(); }); + + it('should support checkedLink', function() { + var container = document.createElement('div'); + var link = new ReactLink(true, mocks.getMockFunction()); + var instance = ; + + React.renderComponent(instance, container); + + expect(instance.getDOMNode().checked).toBe(true); + expect(link.value).toBe(true); + expect(link.requestChange.mock.calls.length).toBe(0); + + instance.getDOMNode().checked = false; + ReactTestUtils.Simulate.input(instance.getDOMNode()); + + expect(link.requestChange.mock.calls.length).toBe(1); + expect(link.requestChange.mock.calls[0][0]).toEqual(false); + }); + + it('should warn with checked and no onChange handler', function() { + var oldWarn = console.warn; + try { + console.warn = mocks.getMockFunction(); + + var node = document.createElement('div'); + var link = new ReactLink(true, mocks.getMockFunction()); + React.renderComponent(, node); + expect(console.warn.mock.calls.length).toBe(0); + + React.renderComponent( + , + node + ); + expect(console.warn.mock.calls.length).toBe(0); + + React.renderComponent( + , + node + ); + expect(console.warn.mock.calls.length).toBe(0); + + React.renderComponent(, node); + expect(console.warn.mock.calls.length).toBe(1); + + React.renderComponent( + , + node + ); + expect(console.warn.mock.calls.length).toBe(2); + } finally { + console.warn = oldWarn; + } + }); + + it('should throw if both checked and checkedLink are provided', function() { + // Silences console.error messages + // ReactErrorUtils.guard is applied to all methods of a React component + // and calls console.error in __DEV__ (true for test environment) + spyOn(console, 'error'); + + var node = document.createElement('div'); + var link = new ReactLink(true, mocks.getMockFunction()); + var instance = ; + + expect(React.renderComponent.bind(React, instance, node)).not.toThrow(); + + instance = ; + expect(React.renderComponent.bind(React, instance, node)).toThrow(); + + instance = ; + expect(React.renderComponent.bind(React, instance, node)).toThrow(); + + }); });