Repository: react-materialize/react-materialize
Pull request #809 (2019-04-03 22:46:25)

Checkbox component indeterminate state broken
# Description

`Checkbox`'s `indeterminate` prop had no effect on the checkbox.

This P.R fixes this. 

## Type of change

Please delete options that are not relevant.

- [x] Bug fix (non-breaking change which fixes an issue)

# How Has This Been Tested?

I've added a new spec to test if `indeterminate` is `true` when passing down this prop to the component.

# 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 529c6b2..66bd7d0 100644
--- a/src/Checkbox.js
+++ b/src/Checkbox.js
@@ -1,40 +1,65 @@
-import React from 'react';
+import React, { Component } from 'react';
 import PropTypes from 'prop-types';
 import cx from 'classnames';
 import idgen from './idgen';
 
-const Checkbox = ({
-  className,
-  checked,
-  onChange,
-  filledIn,
-  label,
-  disabled,
-  value,
-  id,
-  indeterminate,
-  ...other
-}) => {
-  const computedId =
-    id ||
-    (indeterminate ? `indeterminate-checkbox${idgen()}` : `checkbox${idgen()}`);
+class Checkbox extends Component {
+  constructor(props) {
+    super(props);
+  }
 
-  return (
-    <label htmlFor={computedId}>
-      <input
-        id={computedId}
-        className={cx({ 'filled-in': filledIn }, className)}
-        disabled={disabled}
-        onChange={onChange}
-        type="checkbox"
-        checked={checked}
-        value={value}
-        {...other}
-      />
-      <span>{label}</span>
-    </label>
-  );
-};
+  componentDidMount() {
+    const { indeterminate } = this.props;
+
+    if (this._input) {
+      this._input.indeterminate = indeterminate;
+      this._input.checked = false;
+    }
+  }
+
+  componentDidUpdate(prevProps) {
+    if (prevProps.indeterminate !== this.props.indeterminate) {
+      this._input.indeterminate = this.props.indeterminate;
+    }
+  }
+
+  render() {
+    const {
+      id,
+      indeterminate,
+      filledIn,
+      disabled,
+      onChange,
+      checked,
+      value,
+      label
+    } = this.props;
+
+    let computedId = id || idgen();
+
+    if (indeterminate) {
+      computedId = `indeterminate-checkbox-${idgen()}`;
+    }
+
+    return (
+      <label htmlFor={computedId}>
+        <input
+          id={computedId}
+          className={cx({
+            'filled-in': filledIn
+          })}
+          disabled={disabled}
+          onChange={onChange}
+          type="checkbox"
+          defaultChecked={checked}
+          value={value}
+          ref={input => (this._input = input)}
+        />
+        <span>{label}</span>
+      </label>
+    );
+  }
+}
 
 Checkbox.propTypes = {
   className: PropTypes.string,
diff --git a/stories/Collapsible.stories.js b/stories/Collapsible.stories.js
index 133adcd..eaa4386 100644
--- a/stories/Collapsible.stories.js
+++ b/stories/Collapsible.stories.js
@@ -14,7 +14,7 @@ stories.addParameters({
   }
 });
 
-stories.add('Accordion', () => (
+stories.add('Default', () => (
   <Collapsible>
     <CollapsibleItem
       header="Better safe than sorry. That's my motto."
@@ -61,52 +61,3 @@ stories.add('Expandable', () => (
     </CollapsibleItem>
   </Collapsible>
 ));
-
-stories.add('Popout', () => (
-  <Collapsible popout>
-    <CollapsibleItem
-      header="Better safe than sorry. That's my motto."
-      icon="filter_drama"
-    >
-      Better safe than sorry. That's my motto.
-    </CollapsibleItem>
-    <CollapsibleItem
-      header="Yeah, you do seem to have a little 'shit creek' action going."
-      icon="place"
-    >
-      Yeah, you do seem to have a little 'shit creek' action going.
-    </CollapsibleItem>
-    <CollapsibleItem
-      header="You know, FYI, you can buy a paddle. Did you not plan for this contingency?"
-      icon="whatshot"
-    >
-      You know, FYI, you can buy a paddle. Did you not plan for this
-      contingency?
-    </CollapsibleItem>
-  </Collapsible>
-));
-
-stories.add('Preselected Section', () => (
-  <Collapsible>
-    <CollapsibleItem
-      header="Better safe than sorry. That's my motto."
-      icon="filter_drama"
-    >
-      Better safe than sorry. That's my motto.
-    </CollapsibleItem>
-    <CollapsibleItem
-      header="Yeah, you do seem to have a little 'shit creek' action going."
-      icon="place"
-      expanded
-    >
-      Yeah, you do seem to have a little 'shit creek' action going.
-    </CollapsibleItem>
-    <CollapsibleItem
-      header="You know, FYI, you can buy a paddle. Did you not plan for this contingency?"
-      icon="whatshot"
-    >
-      You know, FYI, you can buy a paddle. Did you not plan for this
-      contingency?
-    </CollapsibleItem>
-  </Collapsible>
-));


Test patch:
diff --git a/test/Checkbox.spec.js b/test/Checkbox.spec.js
index b7d79a8..a586e72 100644
--- a/test/Checkbox.spec.js
+++ b/test/Checkbox.spec.js
@@ -35,8 +35,9 @@ describe('<Checkbox />', () => {
   });
 
   test('with indeterminate', () => {
-    wrapper = shallow(<Checkbox value="red" label="red" indeterminate />);
-    expect(wrapper).toMatchSnapshot();
+    wrapper = mount(<Checkbox value="red" label="red" indeterminate />);
+    expect(wrapper.find('input').instance().checked).toBe(false);
+    expect(wrapper.find('input').instance().indeterminate).toBe(true);
   });
 
   test('does not overwrite materialize-css classnames', () => {
diff --git a/test/__snapshots__/Checkbox.spec.js.snap b/test/__snapshots__/Checkbox.spec.js.snap
index 82795bb..82e9564 100644
--- a/test/__snapshots__/Checkbox.spec.js.snap
+++ b/test/__snapshots__/Checkbox.spec.js.snap
@@ -8,11 +8,11 @@ exports[`<Checkbox /> does not overwrite materialize-css classnames 1`] = `
   value="red"
 >
   <label
-    htmlFor="checkboxmockId"
+    htmlFor="mockId"
   >
     <input
-      className="filled-in my-class"
-      id="checkboxmockId"
+      className="filled-in"
+      id="mockId"
       type="checkbox"
       value="red"
     />
@@ -25,11 +25,11 @@ exports[`<Checkbox /> does not overwrite materialize-css classnames 1`] = `
 
 exports[`<Checkbox /> filled-in 1`] = `
 <label
-  htmlFor="checkboxmockId"
+  htmlFor="mockId"
 >
   <input
     className="filled-in"
-    id="checkboxmockId"
+    id="mockId"
     type="checkbox"
     value="red"
   />
@@ -41,11 +41,11 @@ exports[`<Checkbox /> filled-in 1`] = `
 
 exports[`<Checkbox /> renders 1`] = `
 <label
-  htmlFor="checkboxmockId"
+  htmlFor="mockId"
 >
   <input
     className=""
-    id="checkboxmockId"
+    id="mockId"
     type="checkbox"
     value="red"
   />
@@ -57,12 +57,12 @@ exports[`<Checkbox /> renders 1`] = `
 
 exports[`<Checkbox /> renders disabled 1`] = `
 <label
-  htmlFor="checkboxmockId"
+  htmlFor="mockId"
 >
   <input
     className=""
     disabled={true}
-    id="checkboxmockId"
+    id="mockId"
     type="checkbox"
     value="red"
   />
@@ -87,19 +87,3 @@ exports[`<Checkbox /> with id 1`] = `
   </span>
 </label>
 `;
-
-exports[`<Checkbox /> with indeterminate 1`] = `
-<label
-  htmlFor="indeterminate-checkboxmockId"
->
-  <input
-    className=""
-    id="indeterminate-checkboxmockId"
-    type="checkbox"
-    value="red"
-  />
-  <span>
-    red
-  </span>
-</label>
-`;