fix(radio-group): fixes bug where onChange callback would be called t…

…wice once with the change event, once with the radio button value [fixes #100254792]
pivotal-cf · Oct 1, 2015 · 4832cfb · 4832cfb
1 parent 258fc99
commit 4832cfb
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 24 deletions.
diff --git a/spec/pivotal-ui-react/radio-group/radio-group_spec.js b/spec/pivotal-ui-react/radio-group/radio-group_spec.js
@@ -4,10 +4,12 @@ var Radio = require('../../../src/pivotal-ui-react/radio/radio').Radio;
 
 describe('RadioGroup', function() {
   describe('basic RadioGroup', function() {
-    var changeSpy;
+    var changeSpy, callValue;
     beforeEach(function() {
 
-      changeSpy = jasmine.createSpy('change');
+      changeSpy = jasmine.createSpy('change').and.callFake(function(e) {
+        callValue = e.target.value;
+      });
       React.render(
         <RadioGroup name="bananas" onChange={changeSpy}>
           <Radio value="1">One!!!</Radio>
@@ -35,11 +37,12 @@ describe('RadioGroup', function() {
 
     describe('when the radio button is changed', function() {
       beforeEach(function() {
-        $('.radio-group :radio').simulate('change').simulate('click');
+        $('.radio-group :radio:eq(0)').simulate('click');
       });
 
       it('calls the change callback', function() {
-        expect(changeSpy).toHaveBeenCalledWith('1');
+        expect(callValue).toEqual('1');
+        expect(changeSpy.calls.count()).toEqual(1);
       });
     });
   });

diff --git a/src/pivotal-ui-react/radio-group/radio-group.js b/src/pivotal-ui-react/radio-group/radio-group.js
@@ -36,17 +36,13 @@ var RadioGroup = React.createClass({
     onChange: React.PropTypes.func
   },
 
-  getDefaultProps: function() {
-    return {onChange: () => null};
-  },
+  render: function() {
+    var {name, children, onChange, ...others} = this.props;
 
-  onChange: function(e) {
-    this.props.onChange(e.target.value);
-  },
+    children = React.Children.map(children,
+      (child) => cloneWithProps(child, {name, onChange: onChange})
+    );
 
-  render: function() {
-    var {name, children, ...others} = this.props;
-    children = React.Children.map(children, (child) => cloneWithProps(child, {name, onChange: this.onChange}));
     var props = mergeProps(others, {className: 'radio-group'});
 
 
@@ -110,8 +106,8 @@ Similar to the `name` property, the `onChange` handlers is passed down to all ch
 var MyComponent = React.createClass({
   getInitialState: function() { return {selection: null}; },
 
-  change: function(value) {
-    this.setState({selection: value});
+  change: function(e) {
+    this.setState({selection: e.target.value});
   },
 
   render: function() {
@@ -129,15 +125,19 @@ var MyComponent = React.createClass({
             </RadioGroup>
           </Col>
         </div>
-        {this.state.selection === 'special' && (<div className="form-group">
-          <Col md={3}>
-            <label>Stuff that appears</label>
-          </Col>
-          <Col md={21}>
-            <label htmlFor="exampleInputEmail1">Email address</label>
-            <input type="email" className="form-control" id="exampleInputEmail1" placeholder="Enter email" />
-          </Col>
-        </div>)}
+        {this.state.selection === 'special' && (
+
+          <div className="form-group">
+            <Col md={3}>
+              <label>Stuff that appears</label>
+            </Col>
+            <Col md={21}>
+              <label htmlFor="exampleInputEmail1">Email address</label>
+              <input type="email" className="form-control" id="exampleInputEmail1" placeholder="Enter email" />
+            </Col>
+          </div>
+
+        )}
       </form>
     );
   }