fix(RadioGroup): add callback on value update (#618)

* fix(RadioGroup): add callback on value update * fix(RadioButton): put callback trigger on whole wrapper
tillersystems · Jul 9, 2019 · 9de45f0 · 9de45f0
1 parent cf02257
commit 9de45f0
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 14 deletions.
diff --git a/src/RadioButton/index.js b/src/RadioButton/index.js
@@ -15,14 +15,13 @@ function RadioButton(props) {
   const { children, id, value, selectedValue, name, onChange, disabled } = props;
   const checked = selectedValue === value;
   return (
-    <Wrapper disabled={disabled}>
-      <Container onClick={() => onChange(value)} checked={checked} disabled={disabled}>
+    <Wrapper disabled={disabled} onClick={() => onChange(value)}>
+      <Container checked={checked} disabled={disabled}>
         <input
           defaultChecked={checked}
           type="radio"
           name={name}
           value={value}
-          onChange={() => onChange(value)}
           id={id}
           disabled={disabled}
         />

diff --git a/src/RadioGroup/__stories__/RadioGroup.stories.js b/src/RadioGroup/__stories__/RadioGroup.stories.js
@@ -1,6 +1,7 @@
 import React from 'react';
 import { storiesOf } from '@storybook/react';
 import { withKnobs, boolean, select } from '@storybook/addon-knobs';
+import { action } from '@storybook/addon-actions';
 import { State, Store } from '@sambego/storybook-state';
 
 import { RadioGroup, RadioButton } from '../..';
@@ -21,10 +22,11 @@ storiesOf('RadioGroup', module)
   .add('default', () => {
     const enabledValue = boolean('Enabled', true, 'state');
     const enabledRow = boolean('Is row ?', false, 'state');
+    const onChange = action('onChange');
 
     return (
       <State store={store}>
-        <RadioGroup groupName="vegetable" isRow={enabledRow}>
+        <RadioGroup groupName="vegetable" isRow={enabledRow} onChange={onChange}>
           <RadioButton disabled={!enabledValue} value="artichoke" id="artichoke">
             artichoke
           </RadioButton>
@@ -47,10 +49,16 @@ storiesOf('RadioGroup', module)
     const selectedValue = select('Selected value', options, 'artichoke', 'state');
     const enabledValue = boolean('Enabled', true, 'state');
     const enabledRow = boolean('Is row ?', false, 'state');
+    const onChange = action('onChange');
 
     return (
       <State store={store}>
-        <RadioGroup groupName="vegetable" isRow={enabledRow} selectedValue={selectedValue}>
+        <RadioGroup
+          groupName="vegetable"
+          isRow={enabledRow}
+          selectedValue={selectedValue}
+          onChange={onChange}
+        >
           <RadioButton disabled={!enabledValue} value="artichoke" id="artichoke">
             artichoke
           </RadioButton>

diff --git a/src/RadioGroup/__tests__/RadioGroup.test.js b/src/RadioGroup/__tests__/RadioGroup.test.js
@@ -168,4 +168,29 @@ describe('<RadioGroup />', () => {
     expect(beetrootNode).toHaveAttribute('checked');
     expect(pumpkinNode).not.toHaveAttribute('checked');
   });
+
+  test('should call onChange when another radio button is selected', () => {
+    const spy = jest.fn();
+
+    const { getByLabelText } = render(
+      <RadioGroup groupName="vegetable" selectedValue="artichoke" onChange={spy}>
+        <RadioButton value="artichoke" id="artichoke">
+          artichoke
+        </RadioButton>
+        <RadioButton value="beetroot" id="beetroot">
+          beetroot
+        </RadioButton>
+        <RadioButton value="pumpkin" id="pumpkin">
+          pumpkin
+        </RadioButton>
+      </RadioGroup>,
+    );
+
+    let pumpkinNode = getByLabelText('pumpkin');
+
+    fireEvent.click(pumpkinNode);
+
+    expect(spy).toHaveBeenCalledTimes(1);
+    expect(spy).toHaveBeenCalledWith('pumpkin');
+  });
 });
diff --git a/src/RadioGroup/index.js b/src/RadioGroup/index.js
@@ -3,23 +3,28 @@ import PropTypes from 'prop-types';
 
 import { Wrapper } from './elements';
 
+const { bool, func, node, string } = PropTypes;
+
 /**
- * Defines a RadioGroup component.
+ * A RadioGroup component groups radio buttons.
+ *
  */
 class RadioGroup extends PureComponent {
   /** Prop types. */
   static propTypes = {
-    children: PropTypes.node,
-    groupName: PropTypes.string,
-    isRow: PropTypes.bool,
-    selectedValue: PropTypes.string,
+    children: node,
+    groupName: string,
+    isRow: bool,
+    onChange: func,
+    selectedValue: string,
   };
 
   /** Default props. */
   static defaultProps = {
     children: null,
     groupName: null,
     isRow: false,
+    onChange: null,
     selectedValue: null,
   };
 
@@ -52,12 +57,21 @@ class RadioGroup extends PureComponent {
 
     if (selectedValue !== prevProps.selectedValue) {
       /* eslint-disable react/no-did-update-set-state */
-      this.setState({ selectedValue: selectedValue });
+      this.setState({ selectedValue });
     }
   }
 
-  handleChange = val => {
-    this.setState({ selectedValue: val });
+  handleChange = value => {
+    const { onChange } = this.props;
+
+    this.setState(
+      {
+        selectedValue: value,
+      },
+      () => {
+        onChange && onChange(value);
+      },
+    );
   };
 
   /**
@@ -72,7 +86,7 @@ class RadioGroup extends PureComponent {
     const radios = React.Children.map(children, radio =>
       React.cloneElement(radio, {
         onChange: this.handleChange,
-        selectedValue: selectedValue,
+        selectedValue,
         name: groupName,
       }),
     );