diff --git a/.changeset/checkbox-ref-callback.md b/.changeset/checkbox-ref-callback.md new file mode 100644 index 00000000000..f17830c9665 --- /dev/null +++ b/.changeset/checkbox-ref-callback.md @@ -0,0 +1,5 @@ +--- +"@primer/react": patch +--- + +Checkbox: replace useLayoutEffect with ref callback for setting indeterminate state diff --git a/packages/react/src/Checkbox/Checkbox.test.tsx b/packages/react/src/Checkbox/Checkbox.test.tsx index 253baa55b83..3f3e887693a 100644 --- a/packages/react/src/Checkbox/Checkbox.test.tsx +++ b/packages/react/src/Checkbox/Checkbox.test.tsx @@ -103,11 +103,12 @@ describe('Checkbox', () => { const checkbox = getByRole('checkbox') as HTMLInputElement - expect(checkbox).toHaveAttribute('aria-checked', 'false') + // Non-indeterminate checkboxes rely on native checked state for accessibility + expect(checkbox.checked).toEqual(false) rerender() - expect(checkbox).toHaveAttribute('aria-checked', 'true') + expect(checkbox.checked).toEqual(true) rerender() diff --git a/packages/react/src/Checkbox/Checkbox.tsx b/packages/react/src/Checkbox/Checkbox.tsx index c992f840ebb..a921ac54c32 100644 --- a/packages/react/src/Checkbox/Checkbox.tsx +++ b/packages/react/src/Checkbox/Checkbox.tsx @@ -1,7 +1,6 @@ import {clsx} from 'clsx' -import {useProvidedRefOrCreate} from '../hooks' -import React, {useContext, useEffect, type ChangeEventHandler, type InputHTMLAttributes, type ReactElement} from 'react' -import useLayoutEffect from '../utils/useIsomorphicLayoutEffect' +import {useMergedRefs} from '../hooks' +import React, {useContext, type ChangeEventHandler, type InputHTMLAttributes, type ReactElement} from 'react' import type {FormValidationStatus} from '../utils/types/FormValidationStatus' import {CheckboxGroupContext} from '../CheckboxGroup/CheckboxGroupContext' import classes from './Checkbox.module.css' @@ -45,7 +44,19 @@ const Checkbox = React.forwardRef( ref, // eslint-disable-next-line @typescript-eslint/no-explicit-any ): ReactElement => { - const checkboxRef = useProvidedRefOrCreate(ref as React.RefObject) + const setIndeterminate = React.useCallback( + (node: HTMLInputElement | null) => { + if (node) { + node.indeterminate = indeterminate || false + } + }, + // `checked` is intentionally included: browsers clear the indeterminate state + // when checked changes, so we need the callback to re-run to restore it. + // eslint-disable-next-line react-hooks/exhaustive-deps + [indeterminate, checked], + ) + const mergedRef = useMergedRefs(ref, setIndeterminate) + const checkboxGroupContext = useContext(CheckboxGroupContext) const handleOnChange: ChangeEventHandler = e => { checkboxGroupContext.onChange && checkboxGroupContext.onChange(e) @@ -54,37 +65,19 @@ const Checkbox = React.forwardRef( const inputProps = { type: 'checkbox', disabled, - ref: checkboxRef, + ref: mergedRef, checked: indeterminate ? false : checked, defaultChecked, required, ['aria-required']: required ? ('true' as const) : ('false' as const), ['aria-invalid']: validationStatus === 'error' ? ('true' as const) : ('false' as const), + ['aria-checked']: indeterminate ? ('mixed' as const) : undefined, onChange: handleOnChange, value, name: value, ...rest, } - useLayoutEffect(() => { - if (checkboxRef.current) { - checkboxRef.current.indeterminate = indeterminate || false - } - }, [indeterminate, checked, checkboxRef]) - - useEffect(() => { - const {current: checkbox} = checkboxRef - if (!checkbox) { - return - } - - if (indeterminate) { - checkbox.setAttribute('aria-checked', 'mixed') - } else { - checkbox.setAttribute('aria-checked', checkbox.checked ? 'true' : 'false') - } - }) - // @ts-expect-error inputProp needs a non nullable ref return }, )