Skip to content

Commit

Permalink
feat!: add indeterminate prop to <Checkbox> that's separate from `c…
Browse files Browse the repository at this point in the history
…hecked` (#1520)

https://html.spec.whatwg.org/multipage/input.html#checkbox-state-(type=checkbox)
considers indeterminate to be a separate thing from a checkbox's checked state.

In fact, it calls out that "The indeterminate IDL attribute only gives the
appearance of a third state."

Basically you can end up with all 4 combinatinos of checked and indeterminate,
although visually there are only 3 states (both indeterminate states look the
same).

We can also see clearly at https://codepen.io/ahuth/pen/MWqmwyv that browsers
behave this way.
  • Loading branch information
ahuth authored Mar 7, 2023
1 parent 289400e commit d8e2cc4
Show file tree
Hide file tree
Showing 7 changed files with 160 additions and 60 deletions.
53 changes: 30 additions & 23 deletions src/components/Checkbox/Checkbox.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,6 @@ export default {
title: 'Components/Checkbox',
component: Checkbox,
args: defaultArgs,
argTypes: {
// For some reason Storybook does not infer all `checked` correctly;
checked: {
control: 'radio',
options: [true, false, 'indeterminate'],
},
},
decorators: [
(Story) => (
<div
Expand Down Expand Up @@ -75,27 +68,41 @@ export const LargeChecked: StoryObj<Args> = {

export const Indeterminate: StoryObj<Args> = {
args: {
checked: 'indeterminate',
readOnly: true, // Prevent console warning about the field being read only
indeterminate: true,
},
};

export const Disabled: StoryObj<Args> = {
render: () => (
<table className="border-spacing-8">
<tbody>
{[false, true, 'indeterminate' as const].map((checked, i) => (
// FIXME
// eslint-disable-next-line react/no-array-index-key
<tr key={i}>
<td>
<Checkbox checked={checked} disabled label="Disabled" />
</td>
<td>
<Checkbox checked={checked} label="Default" readOnly />
</td>
</tr>
))}
{/* Un-checked */}
<tr>
<td>
<Checkbox checked={false} disabled label="Disabled" />
</td>
<td>
<Checkbox checked={false} label="Default" readOnly />
</td>
</tr>
{/* Checked */}
<tr>
<td>
<Checkbox checked disabled label="Disabled" />
</td>
<td>
<Checkbox checked label="Default" readOnly />
</td>
</tr>
{/* Indeterminate */}
<tr>
<td>
<Checkbox disabled indeterminate label="Disabled" />
</td>
<td>
<Checkbox indeterminate label="Default" readOnly />
</td>
</tr>
</tbody>
</table>
),
Expand All @@ -115,10 +122,10 @@ export const WithoutVisibleLabel: StoryObj<Args> = {
<div className="flex flex-col gap-2">
<Checkbox {...args} readOnly />
<Checkbox {...args} checked readOnly />
<Checkbox {...args} checked="indeterminate" readOnly />
<Checkbox {...args} indeterminate />
<Checkbox {...args} disabled />
<Checkbox {...args} checked disabled />
<Checkbox {...args} checked="indeterminate" disabled />
<Checkbox {...args} disabled indeterminate />
</div>
),
};
Expand Down
12 changes: 0 additions & 12 deletions src/components/Checkbox/__snapshots__/Checkbox.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,6 @@ exports[`<Checkbox /> Disabled story renders snapshot 1`] = `
class="checkbox"
>
<input
aria-checked="mixed"
checked=""
class="checkbox__input"
disabled=""
id=":rb:"
Expand All @@ -157,8 +155,6 @@ exports[`<Checkbox /> Disabled story renders snapshot 1`] = `
class="checkbox"
>
<input
aria-checked="mixed"
checked=""
class="checkbox__input"
id=":rc:"
readonly=""
Expand Down Expand Up @@ -205,11 +201,8 @@ exports[`<Checkbox /> Indeterminate story renders snapshot 1`] = `
class="checkbox"
>
<input
aria-checked="mixed"
checked=""
class="checkbox__input"
id=":r6:"
readonly=""
type="checkbox"
/>
<label
Expand Down Expand Up @@ -443,12 +436,9 @@ exports[`<Checkbox /> WithoutVisibleLabel story renders snapshot 1`] = `
class="checkbox"
>
<input
aria-checked="mixed"
aria-label="a checkbox has no name"
checked=""
class="checkbox__input"
id=":rf:"
readonly=""
type="checkbox"
/>
</div>
Expand Down Expand Up @@ -479,9 +469,7 @@ exports[`<Checkbox /> WithoutVisibleLabel story renders snapshot 1`] = `
class="checkbox"
>
<input
aria-checked="mixed"
aria-label="a checkbox has no name"
checked=""
class="checkbox__input"
disabled=""
id=":ri:"
Expand Down
2 changes: 1 addition & 1 deletion src/components/CheckboxInput/CheckboxInput.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
width: 24px;
}

.checkbox__input[aria-checked="mixed"]::before {
.checkbox__input:indeterminate::before {
background-color: currentColor;
content: '';

Expand Down
29 changes: 29 additions & 0 deletions src/components/CheckboxInput/CheckboxInput.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { render, screen } from '@testing-library/react';
import React from 'react';
import { CheckboxInput } from './CheckboxInput';

it('forwards refs', () => {
const someRef = React.createRef<HTMLInputElement>();
render(<CheckboxInput id="test" ref={someRef} />);
expect(someRef.current).toBeInstanceOf(HTMLInputElement);
});

describe('indeterminancy', () => {
it('is indeterminate when "indeterminate" is true', () => {
render(<CheckboxInput id="test" indeterminate />);
const checkbox = screen.getByRole<HTMLInputElement>('checkbox');
expect(checkbox.indeterminate).toEqual(true);
});

it('is not indeterminate when "indeterminate" is false', () => {
render(<CheckboxInput id="test" indeterminate={false} />);
const checkbox = screen.getByRole<HTMLInputElement>('checkbox');
expect(checkbox.indeterminate).toEqual(false);
});

it('is not indeterminate when "indeterminate" is not present', () => {
render(<CheckboxInput id="test" />);
const checkbox = screen.getByRole<HTMLInputElement>('checkbox');
expect(checkbox.indeterminate).toEqual(false);
});
});
44 changes: 20 additions & 24 deletions src/components/CheckboxInput/CheckboxInput.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import clsx from 'clsx';
import React from 'react';
import useForwardedRef from '../../util/useForwardedRef';
import styles from './CheckboxInput.module.css';

type CheckboxHTMLElementProps = Omit<
Expand All @@ -9,11 +10,9 @@ type CheckboxHTMLElementProps = Omit<

export type CheckboxInputProps = CheckboxHTMLElementProps & {
/**
* Whether checkbox is checked. Defaults to false.
* "indeterminate" can be used when a checkbox visually represents
* a list of checkboxes that is "partially" checked.
* Whether checkbox is checked.
*/
checked?: boolean | 'indeterminate';
checked?: boolean;
/**
* Additional classnames passed in for styling.
*/
Expand All @@ -22,6 +21,11 @@ export type CheckboxInputProps = CheckboxHTMLElementProps & {
* Checkbox ID. Used to connect the input with a label for accessibility purposes.
*/
id: string;
/**
* Whether the checkbox is "indeterminate". Neither checked nor unchecked. The most common use
* case for this is when a checkbox has sub-checkboxes, to represent a "partially checked" state.
*/
indeterminate?: boolean;
};

/**
Expand All @@ -31,32 +35,24 @@ export type CheckboxInputProps = CheckboxHTMLElementProps & {
export const CheckboxInput = React.forwardRef<
HTMLInputElement,
CheckboxInputProps
>(({ checked, className, disabled, ...other }, ref) => {
// Make indeterminate checkbox visually match the colors of a checked state, but announce itself
// as "mixed" to screen readers
//
// However, https://html.spec.whatwg.org/multipage/input.html#checkbox-state-(type=checkbox)
// seems to consider `indeterminate` as a separate bit of information than `checked`. They can be
// in all 4 combinations (only 3 visual states, though).
//
// Should we make them separate props?
const checkedProps =
checked === 'indeterminate'
? {
'aria-checked': 'mixed' as const,
checked: true,
}
: {
checked,
};
>(({ checked, className, disabled, indeterminate, ...other }, ref) => {
const forwardedRef = useForwardedRef(ref);

// Make this checkbox indeterminate. Can only be done with JS for some reason.
// See https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/checkbox#indeterminate_state_checkboxes.
React.useEffect(() => {
if (forwardedRef.current) {
forwardedRef.current.indeterminate = !!indeterminate;
}
}, [forwardedRef, indeterminate]);

return (
<input
checked={checked}
className={clsx(className, styles['checkbox__input'])}
disabled={disabled}
ref={ref}
ref={forwardedRef}
type="checkbox"
{...checkedProps}
{...other}
/>
);
Expand Down
59 changes: 59 additions & 0 deletions src/util/useForwardedRef.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { render, screen } from '@testing-library/react';
import React from 'react';
import useForwardedRef from './useForwardedRef';

type Props = {
callbackWithRef?: (ref: React.RefObject<HTMLElement>) => void;
};

/**
* Component for testing our forwarded ref.
*/
const TestComponent = React.forwardRef<HTMLButtonElement, Props>(
({ callbackWithRef }, ref) => {
// Access the forwarded ref.
const anotherRef = useForwardedRef(ref);

// If present, pass the forwarded ref to a callback so we can do things with it in our tests.
React.useEffect(() => {
if (callbackWithRef) {
callbackWithRef(anotherRef);
}
}, [anotherRef, callbackWithRef]);

return <button ref={anotherRef}></button>;
},
);

it('allows refs to be forwarded', () => {
const someRef = React.createRef<HTMLButtonElement>();
render(<TestComponent ref={someRef} />);
expect(screen.getByRole('button')).toBeInTheDocument();
expect(someRef.current).toBeInstanceOf(HTMLButtonElement);
});

it('does not require a ref', () => {
render(<TestComponent />);
expect(screen.getByRole('button')).toBeInTheDocument();
});

it('allows forward refs to be intercepted and used by the component', () => {
const someRef = React.createRef<HTMLButtonElement>();

render(
<TestComponent
// We've exposed the ref outside of the compnent via a callback for testing purposes.
// Normally the component will do things with the ref internally.
callbackWithRef={(ref) => {
// Add a data attribute we can write assertions against.
if (ref.current) {
ref.current.dataset.coffee = '5';
}
}}
ref={someRef}
/>,
);

expect(someRef.current?.dataset.coffee).toEqual('5');
expect(screen.getByRole('button').dataset.coffee).toEqual('5');
});
21 changes: 21 additions & 0 deletions src/util/useForwardedRef.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { useRef, useEffect, type ForwardedRef, type RefObject } from 'react';

/**
* Safely access a forwarded ref, which may or may not be present.
*/
export default function useForwardedRef<T>(ref: ForwardedRef<T>): RefObject<T> {
const innerRef = useRef<T>(null);

// Keep the internal and forwarded refs in sync.
useEffect(() => {
if (!ref) {
return;
} else if (typeof ref === 'function') {
ref(innerRef.current);
} else {
ref.current = innerRef.current;
}
}, [ref]);

return innerRef;
}

0 comments on commit d8e2cc4

Please sign in to comment.