Skip to content
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ export * from './ConnectedRadioGroup';
export * from './ConnectedRadioGroupInput';
export * from './ConnectedSelect';
export * from './ConnectedTextArea';
export * from './types';
4 changes: 2 additions & 2 deletions packages/gamut/src/ConnectedForm/ConnectedInputs/types.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { ReactNode } from 'react';

import {
CheckboxLabelProps,
CheckboxLabelUnion,
CheckboxProps,
InputWrapperProps,
RadioGroupProps,
Expand Down Expand Up @@ -30,7 +30,7 @@ export interface BaseConnectedCheckboxProps
ConnectedFieldProps {}

export type ConnectedCheckboxProps = BaseConnectedCheckboxProps &
CheckboxLabelProps;
CheckboxLabelUnion;

type FieldComponent<T> = Omit<T, 'defaultValue' | 'name' | 'validation'>;

Expand Down
13 changes: 13 additions & 0 deletions packages/gamut/src/Form/__tests__/Checkbox.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,19 @@ describe('<Checkbox>', () => {

expect(onChange).toHaveBeenCalled();
});
it('sets the input indeterminate state when the prop is passed', () => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it help to also check that checked is also false (or !true? falsey?) here when indeterminate is true?

E.g. as a safeguard in case types get changed?

const { view } = renderView({ indeterminate: true });

const checkbox = view.getByRole('checkbox');
expect(checkbox).toHaveProperty('indeterminate', true);
});

it('does not set indeterminate state when the prop is false', () => {
const { view } = renderView({ indeterminate: false });

const checkbox = view.getByRole('checkbox');
expect(checkbox).toHaveProperty('indeterminate', false);
});

it('accepts JSX in the label', () => {
const { view } = renderView({
Expand Down
1 change: 1 addition & 0 deletions packages/gamut/src/Form/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ export * from './elements/Form';
export * from './inputs/TextArea';
export * from './inputs/Radio';
export * from './inputs/RadioGroup';
export * from './inputs/types';
102 changes: 71 additions & 31 deletions packages/gamut/src/Form/inputs/Checkbox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
} from '@codecademy/gamut-styles';
import { StyleProps } from '@codecademy/variance';
import styled from '@emotion/styled';
import { forwardRef, InputHTMLAttributes, ReactNode } from 'react';
import { forwardRef, InputHTMLAttributes, useEffect, useRef } from 'react';

import {
checkboxElement,
Expand All @@ -19,29 +19,18 @@ import {
polyline,
} from '../styles';
import { BaseInputProps } from '../types';
import { CheckboxCheckedUnion, CheckboxLabelUnion } from './types';

/** Something will happen here */
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leftover comment

export type CheckboxTextProps = StyleProps<typeof checkboxTextStates>;
export type CheckboxPaddingProps = StyleProps<typeof checkboxPadding>;

export type CheckboxStringLabelProps = {
label: string;
'aria-label'?: string;
};

export type CheckboxReactNodeLabelProps = {
label: ReactNode;
'aria-label': string;
};

export type CheckboxLabelProps =
| CheckboxStringLabelProps
| CheckboxReactNodeLabelProps;

export type CheckboxProps = Omit<
InputHTMLAttributes<HTMLInputElement>,
'value' | 'label' | 'aria-label'
'checked' | 'value' | 'label' | 'aria-label'
> &
CheckboxLabelProps &
CheckboxLabelUnion &
CheckboxCheckedUnion &
CheckboxPaddingProps &
Pick<BaseInputProps, 'name' | 'required'> & {
multiline?: boolean;
Expand Down Expand Up @@ -78,6 +67,10 @@ export type CheckboxProps = Omit<
*/
value?: string | boolean;
id?: string;
/**
* Use if you want both the aria-label and text label to be read by voiceover - this component assumes that the aria-label and visual text label are identical.
* If you have a link in the Checkbox options, you should set this as true.
*/
dontAriaHideLabel?: boolean;
};

Expand All @@ -88,17 +81,20 @@ const CheckboxLabel = styled.label<Pick<CheckboxProps, 'disabled' | 'spacing'>>(
checkboxLabelStates
);

const CheckboxElement = styled('div', styledOptions)<
Pick<CheckboxProps, 'checked' | 'multiline' | 'disabled'>
>(checkboxElement, checkboxElementStates);
type CheckboxElementProps = StyleProps<typeof checkboxElementStates>;

const CheckboxElement = styled('div', styledOptions)<CheckboxElementProps>(
checkboxElement,
checkboxElementStates
);

const CheckboxVector = styled.svg`
position: absolute;
top: -1px;
left: -1px;
`;

const Polyline = styled.polyline<Pick<CheckboxProps, 'checked'>>`
const Checkmark = styled.polyline<Pick<CheckboxProps, 'checked'>>`
${polyline}
fill: none;
stroke: currentColor;
Expand All @@ -110,6 +106,16 @@ const Polyline = styled.polyline<Pick<CheckboxProps, 'checked'>>`
transition: stroke-dashoffset ${timing.fast};
`;

const Line = styled.line<Pick<CheckboxProps, 'indeterminate'>>`
${polyline}
fill: none;
stroke: currentColor;
stroke-width: 2;
stroke-dasharray: 18px;
stroke-dashoffset: ${({ indeterminate }) => (indeterminate ? 0 : `18px`)};
transition: stroke-dashoffset ${timing.fast};
`;

const Input = styled.input`
${screenReaderOnly}
${checkboxInput}
Expand All @@ -121,20 +127,42 @@ export const Checkbox = forwardRef<HTMLInputElement, CheckboxProps>(
(
{
'aria-label': ariaLabel,
checked,
indeterminate,
className,
label,
disabled,
dontAriaHideLabel,
htmlFor,
multiline,
id,
checked,
disabled,
label,
multiline,
spacing,
value,
dontAriaHideLabel,
...rest
},
ref
) => {
const intRef = useRef<HTMLInputElement | null>(null);

function syncedRefs(element: HTMLInputElement | null) {
intRef.current = element;
if (ref) {
if (typeof ref === 'object') {
ref.current = element;
} else {
ref(element);
}
}
}
Comment on lines +146 to +155
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: when would ref be passed in as a function?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

callback refs are sometimes used in animations and other things - here's some react official docs with fancy examples: Manipulating the DOM with Refs


useEffect(() => {
if (intRef.current && indeterminate !== undefined) {
intRef.current.indeterminate = indeterminate;
}
}, [indeterminate]);

const active = checked || indeterminate;

return (
<div className={className}>
<Input
Expand All @@ -152,27 +180,39 @@ export const Checkbox = forwardRef<HTMLInputElement, CheckboxProps>(
type="checkbox"
value={`${value}`}
{...rest}
ref={ref}
ref={syncedRefs}
/>
<CheckboxLabel
disabled={disabled}
htmlFor={id || htmlFor}
spacing={spacing}
>
<CheckboxElement
checked={checked}
active={active}
disabled={disabled}
hasBg={checked || indeterminate}
hideBorder={disabled && (checked || indeterminate)}
multiline={multiline}
>
<CheckboxVector
aria-hidden
color={checked ? 'currentColor' : 'transparent'}
color={active ? 'currentColor' : 'transparent'}
height="19px"
viewBox="0 0 19 19"
width="19px"
>
<path d="M1 1h19v19h-19z" fill="currentColor" />
<Polyline checked={checked} points="4 11 8 15 16 6" />
<Checkmark
// This should never happen if the types are working, but is a good back-up.
checked={checked && !indeterminate}
points="4 11 8 15 16 6"
/>
<Line
indeterminate={indeterminate}
x1="4"
x2="16"
y1="10"
y2="10"
/>
</CheckboxVector>
</CheckboxElement>
<CheckboxText
Expand Down
27 changes: 27 additions & 0 deletions packages/gamut/src/Form/inputs/types.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { ReactNode } from 'react';

export type CheckboxStringLabelProps = {
label: string;
'aria-label'?: string;
};

export type CheckboxReactNodeLabelProps = {
label: ReactNode;
'aria-label': string;
};

type CheckboxIndeterminate = {
indeterminate: boolean;
checked?: false;
};

type CheckboxRegular = {
indeterminate?: never;
checked?: boolean;
};

export type CheckboxCheckedUnion = CheckboxRegular | CheckboxIndeterminate;

export type CheckboxLabelUnion =
| CheckboxStringLabelProps
| CheckboxReactNodeLabelProps;
8 changes: 7 additions & 1 deletion packages/gamut/src/Form/styles/Checkbox-styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export const checkboxElementStates = system.states({
multiline: {
mt: 4,
},
checked: {
active: {
color: 'primary',
},
disabled: {
Expand All @@ -76,6 +76,12 @@ export const checkboxElementStates = system.states({
outline: 'none',
},
},
hasBg: {
bg: 'currentColor',
},
hideBorder: {
borderColor: 'transparent',
},
});

export const checkboxInput = system.css({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,93 @@ const CustomCheckbox: React.FC<CustomCheckboxProps> = ({

<Canvas of={CheckboxStories.Checked} />

### Indeterminate

Checkboxes have a third state - `indeterminate` - represented by a dash.

<Canvas of={CheckboxStories.Indeterminate} />

`indeterminate` is used for Checkboxes that are the header of a list of checkboxes. When clicked, all of its child checkboxes should be selected or unselected. Clicking on only some of the child checkboxes should make the header checkbox show the indeterminate state.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeterminate is used for Checkboxes that are the header of a list of checkboxes.

I think calling the parent a "header" might be confusing — maybe sticking with "parent" and "nested" like in your example works. Something like:

indeterminate is used for a Checkbox that is the parent of a list of checkboxes.
or GPT has this suggestion:

indeterminate is used for a parent Checkbox that represents the combined state of a group of nested checkboxes.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When clicked, all of its child checkboxes should be selected or unselected. Clicking on only some of the child checkboxes should make the header checkbox show the indeterminate state.

Maybe: "When the parent checkbox is checked or blank, all of its child checkboxes should be selected or unselected, respectively. The indeterminate state is shown when only some of the child checkboxes are checked."


<Canvas of={CheckboxStories.NestedCheckboxes} />

```tsx
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comment on exporting the example directly — but I don't think you need to add this

const NestedCheckboxExample: React.FC = () => {
const [childrenChecked, setChildrenChecked] = useState<boolean[]>([
false,
false,
false,
]);

const allChecked = childrenChecked.every(Boolean);
const someChecked = childrenChecked.some(Boolean);

const isIndeterminate = !allChecked && someChecked;

const toggleAll = () => {
const next = !allChecked;
setChildrenChecked(childrenChecked.map(() => next));
};

const toggleChild = (index: number) => () => {
setChildrenChecked((prev) => {
const next = [...prev];
next[index] = !prev[index];
return next;
});
};

return (
<Box as="fieldset" border={1} borderRadius="sm" maxWidth="340px" p={16}>
<legend>My fave Gamut components</legend>
<Box ml={8}>
<Checkbox
htmlFor="nested-parent"
label="Select all components"
name="nested-parent"
onChange={toggleAll}
{...(isIndeterminate
? { indeterminate: true as const, checked: false as const }
: { checked: allChecked })}
/>
</Box>
<Box as="ul" listStyle="none" ml={32} p={0}>
{['Boxes', 'ToolTips', 'Pagination'].map((component, i) => (
<Box as="li" key={component}>
<Checkbox
checked={childrenChecked[i]}
htmlFor={`nested-child-${i}`}
label={component}
name={`nested-child-${i}`}
onChange={toggleChild(i)}
/>
</Box>
))}
</Box>
</Box>
);
};
```

There are several accessibility considerations to take when using indeterminate checkboxes.

#### Accessibility considerations

- **Programmatically set the `indeterminate` property**
The mixed state is a JavaScript-only property on the native `<input type="checkbox">`. When you set `indeterminate` on the Gamut `Checkbox`, the component applies the underlying DOM property for you, but you must update this prop whenever the selection state of the child checkboxes changes so that assistive technologies receive an accurate announcement.

- **Keep `checked` and `indeterminate` mutually exclusive**
There are Typescript types that guard against this behavior but keep in mind this shouldn't happen.

- **Group related checkboxes with `fieldset` & `legend`**
Wrapping the parent and its children in a `<fieldset>` with a `<legend>` communicates their relationship to screen-reader users.

- **Use a descriptive label**
The parent checkbox's label should describe what will be selected, e.g. "Select all lessons", so the purpose is clear when a screen reader announces "mixed".

- **Nested checkboxes should be in a list**
The checkboxs should be wrapped in a `ul` and `li` components as shown in the example above.

### Disabled

<Canvas of={CheckboxStories.Disabled} />
Expand Down
Loading
Loading