fix(Checkbox): Fix Checkbox & Checkbox group functionality

heydoctor · Aug 1, 2018 · f8ea91c · f8ea91c
1 parent 0ff2d49
commit f8ea91c
Show file tree

Hide file tree

Showing 3 changed files with 75 additions and 72 deletions.
diff --git a/src/Form/Checkbox.js b/src/Form/Checkbox.js
@@ -29,16 +29,7 @@ const StyledInput = createComponent({
 `;
 
 const StyledIcon = styled(Icon).attrs({
-  color: ({ checked, variant = 'primary', theme, fill }) => {
-    const config = theme.variants[variant];
-
-    if (!fill && !config) {
-      throw new Error(`Refractal: variant "${variant}" not found.`);
-    }
-    const color = fill || config.fontColor;
-
-    return checked ? color : theme.colors.grayMid;
-  },
+  color: ({ checked, color = 'primary', theme }) => (checked ? theme.colors[color] : theme.colors.grayMid),
 })``;
 
 const StyledLabel = createComponent({
@@ -50,71 +41,73 @@ const StyledLabel = createComponent({
 `;
 
 export default class Checkbox extends React.Component {
-  state = {
-    checked: this.props.value,
-  };
-
   static propTypes = {
     id: PropTypes.string.isRequired,
     name: PropTypes.string.isRequired,
     label: PropTypes.oneOfType([PropTypes.string, PropTypes.element]),
     value: PropTypes.oneOfType([PropTypes.number, PropTypes.string, PropTypes.bool]),
+    valueTrue: PropTypes.oneOfType([PropTypes.number, PropTypes.string, PropTypes.bool]),
+    valueFalse: PropTypes.oneOfType([PropTypes.number, PropTypes.string, PropTypes.bool]),
     onChange: PropTypes.func,
     iconOn: PropTypes.string,
     iconOff: PropTypes.string,
     iconSize: PropTypes.number,
     fontSize: PropTypes.number,
-    variant: PropTypes.string,
+    color: PropTypes.string,
     horizontal: PropTypes.bool,
   };
 
   static defaultProps = {
-    value: false,
-    variant: 'primary',
+    color: 'primary',
     iconOn: 'checkbox-marked',
     iconOff: 'checkbox-blank-outline',
+    valueTrue: true,
+    valueFalse: false,
     iconSize: 18,
     horizontal: false,
     onChange() {},
   };
 
-  componentDidUpdate(prevProps) {
-    // update checked state based on props passed in
-    if ('value' in this.props) {
-      if (this.props.value !== this.state.checked && this.props.value !== prevProps.value) {
-        this.update(this.props);
-      }
-    }
+  state = {
+    value: this.props.value,
+  };
+
+  get checked() {
+    return this.state.value === this.props.valueTrue;
   }
 
-  update(newProps) {
-    this.setState({ checked: newProps.value });
+  componentDidUpdate() {
+    if (this.props.value !== undefined && this.state.value !== this.props.value) {
+      this.setState({
+        value: this.props.value,
+      });
+    }
   }
 
   handleChange = () => {
-    const { onChange, name, value } = this.props;
-    const { checked } = this.state;
-
-    // if value component is being controlled via parent
-    if (value) return;
-
-    this.setState({ checked: !checked }, () => {
-      // only pass an update if onChange is defined
-      if (typeof onChange === 'function') {
-        this.props.onChange(name, checked);
+    const { valueTrue, valueFalse, onChange } = this.props;
+    const newValue = this.checked ? valueFalse : valueTrue;
+
+    this.setState(
+      {
+        value: newValue,
+      },
+      () => {
+        onChange(this.props.name, newValue);
       }
-    });
+    );
   };
 
   render() {
-    const { label, id, error, name, fontSize, iconOn, iconOff, iconSize, variant, horizontal } = this.props;
-    const { checked } = this.state;
+    const { label, id, error, name, fontSize, iconOn, iconOff, iconSize, color, horizontal } = this.props;
+    const { checked } = this;
 
     return (
       <CheckboxContainer horizontal={horizontal}>
         <StyledInput id={id} name={name} type="checkbox" checked={checked} onChange={this.handleChange} />
+
         <Flex alignItems="center">
-          <StyledIcon size={iconSize} variant={variant} checked={checked} name={checked ? iconOn : iconOff} />
+          <StyledIcon size={iconSize} color={color} checked={checked} name={checked ? iconOn : iconOff} />
 
           {label && <StyledLabel fontSize={fontSize}>{label}</StyledLabel>}
         </Flex>

diff --git a/src/Form/Checkbox.mdx b/src/Form/Checkbox.mdx
@@ -66,7 +66,7 @@ Form Checkbox component. Different sizes are available.
     ];
     return (
       <React.Fragment>
-        <Checkbox id="checkbox" name="checkbox" variant="success" />
+        <Checkbox id="checkbox" name="checkbox" color="green" />
       </React.Fragment>
     )
   }}

diff --git a/src/Form/CheckboxGroup.js b/src/Form/CheckboxGroup.js
@@ -7,6 +7,7 @@ import Flex from '../Flex';
 
 export default class CheckboxGroup extends Component {
   static propTypes = {
+    name: PropTypes.string.isRequired,
     onChange: PropTypes.func,
     value: PropTypes.arrayOf(PropTypes.oneOfType([PropTypes.string, PropTypes.number])),
     horizontal: PropTypes.bool,
@@ -16,11 +17,10 @@ export default class CheckboxGroup extends Component {
         label: PropTypes.string,
         disabled: PropTypes.bool,
       })
-    ),
+    ).isRequired,
   };
 
   static defaultProps = {
-    choices: [],
     onChange() {},
     horizontal: false,
   };
@@ -29,25 +29,58 @@ export default class CheckboxGroup extends Component {
     selected: Array.isArray(this.props.value) ? [...this.props.value] : [],
   };
 
+  createChangeHandler = choice => (name, value) => {
+    const { selected } = this.state;
+    let newSelected;
+    if (!selected.includes(value)) {
+      if (choice.exclusive) {
+        newSelected = [value];
+      } else {
+        newSelected = this.props.choices
+          .filter(c => !c.exclusive && (c.value === value || selected.includes(c.value)))
+          .map(c => c.value);
+      }
+    } else {
+      const index = selected.indexOf(value);
+      newSelected = [...selected.slice(0, index), ...selected.slice(index + 1)];
+    }
+
+    this.setState(
+      {
+        selected: newSelected,
+      },
+      () => {
+        this.props.onChange(this.props.name, newSelected);
+      }
+    );
+
+    console.log(newSelected);
+  };
+
   render() {
-    const { choices, error, horizontal } = this.props;
+    const { name, color, choices, error, horizontal } = this.props;
+    const { selected } = this.state;
 
     return (
       <Box>
         <Flex flexDirection={horizontal ? 'row' : 'column'}>
           {choices.length &&
             choices.map(choice => {
               const { value = choice.id } = choice;
+              const key = `checkbox-${name}-${value}`;
 
               return (
                 <Checkbox
-                  key={`Checkbox${value}`}
-                  id={`Checkbox${value}`}
-                  name={`Checkbox${value}`}
-                  value={this.state.selected && this.state.selected.includes(value)}
+                  key={key}
+                  id={key}
+                  name={key}
                   label={choice.label}
-                  onChange={this.createChangeHandler(choice)}
+                  color={color}
                   horizontal={horizontal}
+                  value={selected && selected.includes(value) ? value : null}
+                  valueTrue={value}
+                  valueFalse={value}
+                  onChange={this.createChangeHandler(choice)}
                 />
               );
             })}
@@ -56,27 +89,4 @@ export default class CheckboxGroup extends Component {
       </Box>
     );
   }
-
-  createChangeHandler = choice => (name, value) => {
-    const { selected } = this.state;
-    let newSelected;
-    if (!selected.includes(value)) {
-      if (choice.exclusive) {
-        newSelected = [value];
-      } else {
-        newSelected = this.props.choices
-          .filter(c => !c.exclusive && (c.value === value || selected.includes(c.value)))
-          .map(c => c.value);
-      }
-    } else {
-      const index = selected.indexOf(value);
-      newSelected = [...selected.slice(0, index), ...selected.slice(index + 1)];
-    }
-
-    this.setState({
-      selected: newSelected,
-    });
-
-    this.props.onChange(this.props.name, newSelected);
-  };
 }