Merge pull request #1369 from Shopify/fix-checkbox-click-disabled

[Checkbox] Fix being clicked while disabled

Author: AndrewMusgrave

UNRELEASED.md

@@ -18,6 +18,7 @@ Use [the changelog guidelines](https://git.io/polaris-changelog-guidelines) to f
 
 ### Bug fixes
 
+- Fixed `Checkbox` being toggled when disabled ([#1369](https://github.com/Shopify/polaris-react/pull/1369))
 - Fixed `DropZone.FileUpload` from incorrectly displaying action hint and title when the default is used and removed ([#1233](https://github.com/Shopify/polaris-react/pull/1233))
 - Fixed `ResourceList.Item` interaction states from being incorrectly applied ([#1312](https://github.com/Shopify/polaris-react/pull/1312)
 - Fixed selected state for date picker in windows high contrast mode ([#1342](https://github.com/Shopify/polaris-react/pull/1342))

src/components/Checkbox/Checkbox.tsx

@@ -45,6 +45,23 @@ const getUniqueID = createUniqueIDFactory('Checkbox');
 class Checkbox extends React.PureComponent<CombinedProps, never> {
   private inputNode = React.createRef<HTMLInputElement>();
 
+  componentDidMount() {
+    const {checked, disabled} = this.props;
+
+    if (checked && disabled) this.handleInput();
+  }
+
+  componentDidUpdate({disabled: prevDisabled}: Props) {
+    if (
+      this.props.disabled &&
+      !prevDisabled !== true &&
+      this.inputNode.current &&
+      this.inputNode.current.checked
+    ) {
+      this.handleInput();
+    }
+  }
+
   handleInput = () => {
     const {onChange, id} = this.props;
 

src/components/Checkbox/tests/Checkbox.test.tsx

@@ -78,6 +78,33 @@ describe('<Checkbox />', () => {
       checkbox.simulate('click');
       expect(checkbox.find('input').instance()).toBe(document.activeElement);
     });
+
+    it('is not called when disabled', () => {
+      const spy = jest.fn();
+      const checkbox = mountWithAppProvider(
+        <Checkbox label="label" disabled onChange={spy} />,
+      );
+      checkbox.find('input').simulate('click');
+      expect(spy).not.toHaveBeenCalled();
+    });
+
+    it('is called when disabled and checked are both true on load', () => {
+      const spy = jest.fn();
+      mountWithAppProvider(
+        <Checkbox label="label" id="id" disabled onChange={spy} checked />,
+      );
+      expect(spy).toHaveBeenCalledWith(false, 'id');
+    });
+
+    it('it is called when disabled and checked are both true after initial load', () => {
+      const spy = jest.fn();
+      const checkbox = mountWithAppProvider(
+        <Checkbox label="label" disabled onChange={spy} id="id" />,
+      );
+
+      checkbox.setProps({checked: true});
+      expect(spy).toHaveBeenCalledWith(false, 'id');
+    });
   });
 
   describe('onFocus()', () => {

src/components/Choice/Choice.tsx

@@ -35,14 +35,20 @@ export default function Choice({
   helpText,
   onClick,
 }: Props) {
+  function handleClick() {
+    if (disabled || !onClick) return;
+
+    onClick();
+  }
+
   const className = classNames(
     styles.Choice,
     labelHidden && styles.labelHidden,
     disabled && styles.disabled,
   );
 
   const labelMarkup = (
-    <label className={className} htmlFor={id} onClick={onClick}>
+    <label className={className} htmlFor={id} onClick={handleClick}>
       <span className={styles.Control}>{children}</span>
       <span className={styles.Label}>{label}</span>
     </label>

src/components/Choice/tests/Choice.test.tsx

@@ -86,4 +86,13 @@ describe('<Choice />', () => {
       expect(label.find(blockLevelElements[i])).toHaveLength(0);
     }
   });
+
+  it('does not fire an onClick event when disabled is true', () => {
+    const spy = jest.fn();
+    const choice = mountWithAppProvider(
+      <Choice id="id" label="Label" onClick={spy} disabled />,
+    );
+    choice.find('label').simulate('click');
+    expect(spy).not.toHaveBeenCalled();
+  });
 });