fix(checkbox): hide icon when not checked (#2167)

twilio-labs · Feb 8, 2022 · 02285a8 · 02285a8
1 parent be459ec
commit 02285a8
Show file tree

Hide file tree

Showing 5 changed files with 163 additions and 22 deletions.
diff --git a/.changeset/fifty-shirts-know.md b/.changeset/fifty-shirts-know.md
@@ -0,0 +1,6 @@
+---
+'@twilio-paste/checkbox': minor
+'@twilio-paste/core': minor
+---
+
+[Checkbox] Hide check icon when not checked, add the defaultChecked prop.
diff --git a/packages/paste-core/components/checkbox/__tests__/checkbox.test.tsx b/packages/paste-core/components/checkbox/__tests__/checkbox.test.tsx
@@ -34,8 +34,11 @@ const defaultGroupProps = {
 
 describe('Checkbox', () => {
   it('should render', () => {
-    const {getByRole} = render(<Checkbox {...defaultProps}>foo</Checkbox>);
+    const {getByRole, container} = render(<Checkbox {...defaultProps}>foo</Checkbox>);
     expect(getByRole('checkbox')).not.toBeNull();
+
+    const checkIcon = container.querySelector('[data-paste-element="CHECKBOX_ICON"]');
+    expect(checkIcon).toHaveStyleRule('display', 'none');
   });
 
   it('should render as invalid', () => {
@@ -47,13 +50,28 @@ describe('Checkbox', () => {
     expect(getByRole('checkbox').getAttribute('aria-invalid')).toBeTruthy();
   });
 
-  it('should render as checked', () => {
-    const {getByLabelText} = render(
+  it('should render as checked when defaultChecked', () => {
+    const {getByLabelText, container} = render(
+      <Checkbox {...defaultProps} defaultChecked onChange={NOOP}>
+        foo
+      </Checkbox>
+    );
+    expect((getByLabelText('foo') as HTMLInputElement).checked).toBeTruthy();
+
+    const checkIcon = container.querySelector('[data-paste-element="CHECKBOX_ICON"]');
+    expect(checkIcon).toHaveStyleRule('display', 'block');
+  });
+
+  it('should render as checked when controlled', () => {
+    const {getByLabelText, container} = render(
       <Checkbox {...defaultProps} checked onChange={NOOP}>
         foo
       </Checkbox>
     );
     expect((getByLabelText('foo') as HTMLInputElement).checked).toBeTruthy();
+
+    const checkIcon = container.querySelector('[data-paste-element="CHECKBOX_ICON"');
+    expect(checkIcon).toHaveStyleRule('display', 'block');
   });
 
   it('should render a required dot', () => {
@@ -195,6 +213,7 @@ describe('Checkbox event handlers', () => {
         data-testid="checkbox-button"
         id="foo"
         name="foo"
+        checked
         onChange={onChangeMock}
         onFocus={onFocusMock}
         onBlur={onBlurMock}
@@ -211,8 +230,20 @@ describe('Checkbox event handlers', () => {
     expect(onBlurMock).toHaveBeenCalledTimes(1);
   });
 
-  it('Should check the checkbox', () => {
-    // const onChangeMock: jest.Mock = jest.fn();
+  it('Should not call onChange handler when uncontrolled', () => {
+    const onChangeMock: jest.Mock = jest.fn();
+
+    const {getByTestId} = render(
+      <Checkbox data-testid="checkbox-button" id="foo" name="foo" onChange={onChangeMock}>
+        foo
+      </Checkbox>
+    );
+
+    fireEvent.click(getByTestId('checkbox-button'));
+    expect(onChangeMock).toHaveBeenCalledTimes(0);
+  });
+
+  it('Should check the checkbox when controlled', () => {
     const MockCheckBox: React.FC = () => {
       const [checked, setChecked] = React.useState(false);
       return (
@@ -235,6 +266,21 @@ describe('Checkbox event handlers', () => {
     fireEvent.click(getByTestId('checkbox-button'));
     expect((getByTestId('checkbox-button') as HTMLInputElement).checked).toBe(true);
   });
+
+  it('Should check the checkbox when uncontrolled', () => {
+    const MockCheckBox: React.FC = () => {
+      return (
+        <Checkbox data-testid="checkbox-button" id="foo" name="foo">
+          foo
+        </Checkbox>
+      );
+    };
+
+    const {getByTestId} = render(<MockCheckBox />);
+
+    fireEvent.click(getByTestId('checkbox-button'));
+    expect((getByTestId('checkbox-button') as HTMLInputElement).checked).toBe(true);
+  });
 });
 
 describe('Customization', () => {

diff --git a/packages/paste-core/components/checkbox/src/Checkbox.tsx b/packages/paste-core/components/checkbox/src/Checkbox.tsx
@@ -25,6 +25,7 @@ export interface CheckboxProps extends React.InputHTMLAttributes<HTMLInputElemen
   isSelectAll?: boolean;
   isSelectAllChild?: boolean;
   checked?: boolean;
+  defaultChecked?: boolean;
 }
 
 type HiddenCheckboxProps = Pick<
@@ -64,13 +65,21 @@ const CheckboxIcon: React.FC<{
   if (indeterminate) {
     return <MinusIcon element={element} decorative color={color} size="sizeIcon10" />;
   }
-  return <CheckboxCheckIcon element={element} hidden={!checked} decorative color={color} size="sizeIcon10" />;
+  return (
+    <CheckboxCheckIcon
+      element={element}
+      display={!checked ? 'none' : 'block'}
+      decorative
+      color={color}
+      size="sizeIcon10"
+    />
+  );
 };
 
 const Checkbox = React.forwardRef<HTMLInputElement, CheckboxProps>(
   (
     {
-      checked,
+      checked: checkedProp,
       element = 'CHECKBOX',
       children,
       helpText,
@@ -90,15 +99,29 @@ const Checkbox = React.forwardRef<HTMLInputElement, CheckboxProps>(
     const disabled = props.disabled != null ? props.disabled : checkboxGroupContext.disabled;
     const name = props.name != null ? props.name : checkboxGroupContext.name;
     const hasError = props.hasError != null ? props.hasError : checkboxGroupContext.hasError;
-    const handleChange = props.onChange != null ? props.onChange : checkboxGroupContext.onChange;
+
+    // Manages what the value of checked is depending on if it is controlled or uncontrolled
+    const [checkedState, setCheckedState] = React.useState(!!props.defaultChecked);
+    const isControlled = checkedProp !== undefined;
+    const mergedChecked = isControlled ? checkedProp : checkedState;
+
+    const handleChange = (event: React.ChangeEvent<HTMLInputElement>): void => {
+      if (!isControlled) {
+        setCheckedState(event.target.checked);
+      } else if (props.onChange) {
+        props.onChange(event);
+      } else {
+        checkboxGroupContext.onChange(event);
+      }
+    };
 
     let paddingLeft: SpaceOptions | null = null;
     let checkboxBackground: BackgroundColorOptions | null = null;
 
     if (isSelectAll) {
       paddingLeft = 'space20';
 
-      if (checked || indeterminate) {
+      if (mergedChecked || indeterminate) {
         checkboxBackground = 'colorBackgroundPrimaryWeakest';
       } else if (!disabled) {
         checkboxBackground = 'colorBackground';
@@ -124,12 +147,12 @@ const Checkbox = React.forwardRef<HTMLInputElement, CheckboxProps>(
       >
         <HiddenCheckbox
           {...safelySpreadBoxProps(props)}
-          checked={checked}
+          checked={checkedProp}
           disabled={disabled}
           name={name}
           onChange={handleChange}
           aria-describedby={helpTextId}
-          aria-checked={indeterminate ? 'mixed' : checked}
+          aria-checked={indeterminate ? 'mixed' : checkedProp}
           aria-invalid={hasError}
           id={id}
           required={required}
@@ -146,7 +169,7 @@ const Checkbox = React.forwardRef<HTMLInputElement, CheckboxProps>(
           >
             <CheckboxIcon
               element={`${element}_ICON`}
-              checked={checked}
+              checked={mergedChecked}
               disabled={disabled}
               indeterminate={indeterminate}
             />
@@ -188,6 +211,7 @@ Checkbox.propTypes = {
   isSelectAll: PropTypes.bool,
   isSelectAllChild: PropTypes.bool,
   element: PropTypes.string,
+  defaultChecked: PropTypes.bool,
 };
 
 export {Checkbox};
diff --git a/packages/paste-website/src/component-examples/CheckboxExamples.ts b/packages/paste-website/src/component-examples/CheckboxExamples.ts
@@ -21,6 +21,48 @@ render(
 )
 `.trim();
 
+export const uncontrolledCheckbox = `
+const UncontrolledCheckbox = () => {
+  return (
+    <Checkbox
+      id="uncontrolled"
+      value="uncontrolled"
+      name="uncontrolled"
+      defaultChecked
+    >
+      I am an uncontrolled checkbox
+    </Checkbox>
+  );
+};
+
+render(
+  <UncontrolledCheckbox />
+)
+`.trim();
+
+export const controlledCheckbox = `
+const ControlledCheckbox = () => {
+  const [checked, setChecked] = React.useState(true);
+  return (
+    <Checkbox
+      id="controlled"
+      value="controlled"
+      name="controlled"
+      checked={checked}
+      onChange={(event) => {
+        setChecked(event.target.checked);
+      }}
+    >
+      I am a controlled checkbox
+    </Checkbox>
+  );
+};
+
+render(
+  <ControlledCheckbox />
+)
+`.trim();
+
 export const indeterminateExample = `
 const Results = () => {
   const [checkedItems, setCheckedItems] = React.useState([true, false]);

diff --git a/packages/paste-website/src/pages/components/checkbox/index.mdx b/packages/paste-website/src/pages/components/checkbox/index.mdx
@@ -15,7 +15,12 @@ import {UnorderedList, ListItem} from '@twilio-paste/list';
 import {DoDont, Do, Dont} from '../../../components/DoDont';
 import {SidebarCategoryRoutes} from '../../../constants';
 import Changelog from '@twilio-paste/checkbox/CHANGELOG.md';
-import {checkedCheckbox, indeterminateExample} from '../../../component-examples/CheckboxExamples';
+import {
+  checkedCheckbox,
+  indeterminateExample,
+  uncontrolledCheckbox,
+  controlledCheckbox,
+} from '../../../component-examples/CheckboxExamples';
 
 export const pageQuery = graphql`
   {
@@ -101,6 +106,22 @@ Use a checkbox to present a user with a binary (e.g. on/off) choice that is part
 - When in an error state display an inline error message below the offending checkbox that clearly describes the error along with an icon that depicts the severity.
 - When displaying additional content based on checking a checkbox, be sure that the new content appears after the checkbox so that it is naturally discoverable by assistive technology users.
 
+## Controlled vs. uncontrolled checkboxes
+
+The checkbox can either be controlled, meaning there is an external state that determines if the checkbox is checked or not, or uncontrolled, meaning the checkbox manages its own state.
+
+To make an uncontrolled checkbox, you do not pass the `checked` or `onChange` props. If you need the checkbox to be checked by default, use the `defaultChecked` prop.
+
+<LivePreview scope={{Checkbox}} noInline>
+  {uncontrolledCheckbox}
+</LivePreview>
+
+To make a controlled checkbox, you must pass the `checked` and `onChange` props. You cannot use the `defaultChecked` prop in a controlled checkbox.
+
+<LivePreview scope={{Checkbox}} noInline>
+  {controlledCheckbox}
+</LivePreview>
+
 ## Examples
 
 ### Basic checkbox with label
@@ -400,6 +421,7 @@ All the valid HTML attributes for `input[type=checkbox]` are supported including
 | children          | `ReactNode`                              |             | null    |
 | id?               | `string`                                 |             | null    |
 | checked?          | `boolean`                                |             | false   |
+| defaultChecked?   | `boolean`                                |             | null    |
 | hasError?         | `boolean`                                |             | false   |
 | helpText?         | `string` or `ReactNode`                  |             | null    |
 | indeterminate?    | `boolean`                                |             | false   |
@@ -428,15 +450,16 @@ All the valid HTML attributes for `input[type=checkbox]` are supported including
 
 All the valid HTML attributes for `input[type=checkbox]` are supported including the following props:
 
-| Prop       | Type                                     | Description | Default |
-| ---------- | ---------------------------------------- | ----------- | ------- |
-| children   | `ReactNode`                              |             | null    |
-| id?        | `string`                                 |             | null    |
-| checked?   | `boolean`                                |             | false   |
-| errorText? | `string` or `ReactNode`                  |             | null    |
-| onChange?  | `(event: React.MouseEvent<HTMLElement>)` |             | null    |
-| onFocus?   | `(event: React.MouseEvent<HTMLElement>)` |             | null    |
-| onBlur?    | `(event: React.MouseEvent<HTMLElement>)` |             | null    |
+| Prop            | Type                                     | Description | Default |
+| --------------- | ---------------------------------------- | ----------- | ------- |
+| children        | `ReactNode`                              |             | null    |
+| id?             | `string`                                 |             | null    |
+| checked?        | `boolean`                                |             | false   |
+| defaultChecked? | `boolean`                                |             | null    |
+| errorText?      | `string` or `ReactNode`                  |             | null    |
+| onChange?       | `(event: React.MouseEvent<HTMLElement>)` |             | null    |
+| onFocus?        | `(event: React.MouseEvent<HTMLElement>)` |             | null    |
+| onBlur?         | `(event: React.MouseEvent<HTMLElement>)` |             | null    |
 
 <ChangelogRevealer>
   <Changelog />