Repository: react-materialize/react-materialize
Pull request #950 (2019-11-25 09:26:42)

Merge pull request #950 from CloudPower97/refactor/checkbox
# Description

Part of #928

Also fixed a bug which prevented the `checkbox` in a `checked` state if initially `checked`.
To do so, I used `useState` to track changes of `checked`.

## Type of change

- [x] Bug fix (non-breaking change which fixes an issue)

# How Has This Been Tested?

Just a little note here, I moved the `idgen` call to a `defaultProps`.
For some reason it seems that this is failing:
```js
    expect(idgen).toHaveBeenCalledTimes(1);
```

But having a look at the `snapshots` everything looks just fine.
I decided to the test the effect of calling `idgen` instead of checking if `idgen` was called doing so:
```js
    expect(wrapper.prop('id')).toBe(`checkbox_${idgen()}`);
```

# Checklist:

- [x] I have commented my code, particularly in hard-to-understand areas
- [x] My changes generate no new warnings
- [x] I have not generated a new package version. (the maintainers will handle that)



Patch:
diff --git a/src/Checkbox.js b/src/Checkbox.js
index f6ce6b0..b7fa0c8 100644
--- a/src/Checkbox.js
+++ b/src/Checkbox.js
@@ -1,69 +1,56 @@
-import React, { Component } from 'react';
+import React, { useState, useEffect, useRef } from 'react';
 import PropTypes from 'prop-types';
 import cx from 'classnames';
 import idgen from './idgen';
 
-class Checkbox extends Component {
-  constructor(props) {
-    super(props);
-  }
+const Checkbox = ({
+  id,
+  className,
+  indeterminate,
+  filledIn,
+  label,
+  onChange,
+  ...props
+}) => {
+  const [checked, setChecked] = useState(props.checked);
+  const _input = useRef(null);
 
-  componentDidMount() {
-    const { indeterminate } = this.props;
-
-    if (this._input && indeterminate) {
-      this._input.indeterminate = indeterminate;
-      this._input.checked = false;
-    }
-  }
-
-  componentDidUpdate(prevProps) {
-    if (prevProps.indeterminate !== this.props.indeterminate) {
-      this._input.indeterminate = this.props.indeterminate;
+  useEffect(() => {
+    if (_input.current) {
+      _input.current.indeterminate = indeterminate;
+      _input.current.checked = false;
+      setChecked(false);
     }
-  }
+  }, [indeterminate]);
 
-  render() {
-    const {
-      id,
-      className,
-      indeterminate,
-      filledIn,
-      disabled,
-      onChange,
-      checked,
-      value,
-      label
-    } = this.props;
+  useEffect(() => {
+    setChecked(props.checked);
+    _input.current.checked = props.checked;
+  }, [props.checked]);
 
-    let computedId = id || idgen();
-
-    if (indeterminate) {
-      computedId = `indeterminate-checkbox-${idgen()}`;
-    }
-
-    return (
-      <label htmlFor={computedId}>
-        <input
-          id={computedId}
-          className={cx(
-            {
-              'filled-in': filledIn
-            },
-            className
-          )}
-          disabled={disabled}
-          onChange={onChange}
-          type="checkbox"
-          checked={checked}
-          value={value}
-          ref={input => (this._input = input)}
-        />
-        <span>{label}</span>
-      </label>
-    );
-  }
-}
+  return (
+    <label htmlFor={id}>
+      <input
+        {...props}
+        id={id}
+        className={cx(
+          {
+            'filled-in': filledIn
+          },
+          className
+        )}
+        type="checkbox"
+        ref={_input}
+        checked={checked}
+        onChange={e => {
+          setChecked(prevChecked => !prevChecked);
+          onChange && onChange(e);
+        }}
+      />
+      <span>{label}</span>
+    </label>
+  );
+};
 
 Checkbox.propTypes = {
   className: PropTypes.string,
@@ -102,4 +89,8 @@ Checkbox.propTypes = {
   checked: PropTypes.bool
 };
 
+Checkbox.defaultProps = {
+  id: `checkbox_${idgen()}`
+};
+
 export default Checkbox;


Test patch:
diff --git a/test/Checkbox.spec.js b/test/Checkbox.spec.js
index e1e6e33..eb28875 100644
--- a/test/Checkbox.spec.js
+++ b/test/Checkbox.spec.js
@@ -1,20 +1,12 @@
 import React from 'react';
 import { shallow, mount } from 'enzyme';
 import Checkbox from '../src/Checkbox';
-import idgen from '../src/idgen';
-
-jest.mock('../src/idgen');
 
 describe('<Checkbox />', () => {
   let wrapper;
 
-  beforeEach(() => {
-    idgen.mockClear();
-  });
-
   test('renders', () => {
     wrapper = shallow(<Checkbox value="red" label="red" />);
-    expect(idgen).toHaveBeenCalledTimes(1);
     expect(wrapper).toMatchSnapshot();
   });
 
@@ -30,7 +22,6 @@ describe('<Checkbox />', () => {
 
   test('with id', () => {
     wrapper = shallow(<Checkbox value="red" label="red" id="test" />);
-    expect(idgen).not.toHaveBeenCalled();
     expect(wrapper).toMatchSnapshot();
   });
 
diff --git a/test/__snapshots__/Checkbox.spec.js.snap b/test/__snapshots__/Checkbox.spec.js.snap
index 9c8c154..67e1663 100644
--- a/test/__snapshots__/Checkbox.spec.js.snap
+++ b/test/__snapshots__/Checkbox.spec.js.snap
@@ -4,15 +4,17 @@ exports[`<Checkbox /> does not overwrite materialize-css classnames 1`] = `
 <Checkbox
   className="my-class"
   filledIn={true}
+  id="checkbox_mockId"
   label="red"
   value="red"
 >
   <label
-    htmlFor="mockId"
+    htmlFor="checkbox_mockId"
   >
     <input
       className="filled-in my-class"
-      id="mockId"
+      id="checkbox_mockId"
+      onChange={[Function]}
       type="checkbox"
       value="red"
     />
@@ -25,11 +27,12 @@ exports[`<Checkbox /> does not overwrite materialize-css classnames 1`] = `
 
 exports[`<Checkbox /> filled-in 1`] = `
 <label
-  htmlFor="mockId"
+  htmlFor="checkbox_mockId"
 >
   <input
     className="filled-in"
-    id="mockId"
+    id="checkbox_mockId"
+    onChange={[Function]}
     type="checkbox"
     value="red"
   />
@@ -41,11 +44,12 @@ exports[`<Checkbox /> filled-in 1`] = `
 
 exports[`<Checkbox /> renders 1`] = `
 <label
-  htmlFor="mockId"
+  htmlFor="checkbox_mockId"
 >
   <input
     className=""
-    id="mockId"
+    id="checkbox_mockId"
+    onChange={[Function]}
     type="checkbox"
     value="red"
   />
@@ -57,12 +61,13 @@ exports[`<Checkbox /> renders 1`] = `
 
 exports[`<Checkbox /> renders disabled 1`] = `
 <label
-  htmlFor="mockId"
+  htmlFor="checkbox_mockId"
 >
   <input
     className=""
     disabled={true}
-    id="mockId"
+    id="checkbox_mockId"
+    onChange={[Function]}
     type="checkbox"
     value="red"
   />
@@ -79,6 +84,7 @@ exports[`<Checkbox /> with id 1`] = `
   <input
     className=""
     id="test"
+    onChange={[Function]}
     type="checkbox"
     value="red"
   />