Skip to content

[checkbox group] Fix parent checkbox ignoring custom id prop #2896

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

[checkbox group] Fix parent checkbox ignoring custom id prop #2896

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e17d5b3
[checkbox group] Fix parent checkbox ignoring custom `id` prop (#2691)
edmolima Oct 2, 2025
e75beb2
[checkbox group] Fix child checkbox ignoring custom uid=501(edmo) gid…
edmolima Oct 3, 2025
8780294
Merge branch 'master' into checkbox-group/fix-parent-id-2691
edmolima Oct 3, 2025
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
78 changes: 78 additions & 0 deletions packages/react/src/checkbox-group/useCheckboxGroupParent.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,84 @@ describe('useCheckboxGroupParent', () => {
expect(parent).to.have.attribute('aria-controls', allValues.map((v) => `${id}-${v}`).join(' '));
});

it('should honor custom id on parent checkbox', () => {
function App() {
const [value, setValue] = React.useState<string[]>([]);
return (
<CheckboxGroup value={value} onValueChange={setValue} allValues={allValues}>
<Checkbox.Root parent id="custom-parent-id" data-testid="parent" />
<Checkbox.Root value="a" />
<Checkbox.Root value="b" />
<Checkbox.Root value="c" />
</CheckboxGroup>
);
}

render(<App />);

const parent = screen.getByTestId('parent');

expect(parent).to.have.attribute('id', 'custom-parent-id');
expect(parent).to.have.attribute(
'aria-controls',
allValues.map((v) => `custom-parent-id-${v}`).join(' '),
);
});

it('should allow label association via htmlFor with custom id', () => {
function App() {
const [value, setValue] = React.useState<string[]>([]);
return (
<CheckboxGroup value={value} onValueChange={setValue} allValues={allValues}>
<label htmlFor="parent-checkbox-id">Select All</label>
<Checkbox.Root parent id="parent-checkbox-id" data-testid="parent" />
<Checkbox.Root value="a" />
<Checkbox.Root value="b" />
<Checkbox.Root value="c" />
</CheckboxGroup>
);
}

render(<App />);

const parent = screen.getByTestId('parent');
const label = screen.getByText('Select All');

expect(parent).to.have.attribute('id', 'parent-checkbox-id');
expect(label).to.have.attribute('for', 'parent-checkbox-id');

// Click the label should toggle the parent checkbox
fireEvent.click(label);
expect(parent).to.have.attribute('aria-checked', 'true');
});

it('should allow label association via htmlFor with custom id on child checkbox', () => {
function App() {
const [value, setValue] = React.useState<string[]>([]);
return (
<CheckboxGroup value={value} onValueChange={setValue} allValues={allValues}>
<Checkbox.Root parent data-testid="parent" />
<label htmlFor="child-checkbox-a">Option A</label>
<Checkbox.Root value="a" id="child-checkbox-a" data-testid="child-a" />
<Checkbox.Root value="b" />
<Checkbox.Root value="c" />
</CheckboxGroup>
);
}

render(<App />);

const childA = screen.getByTestId('child-a');
const label = screen.getByText('Option A');

expect(childA).to.have.attribute('id', 'child-checkbox-a');
expect(label).to.have.attribute('for', 'child-checkbox-a');

// Click the label should toggle the child checkbox
fireEvent.click(label);
expect(childA).to.have.attribute('aria-checked', 'true');
});

it('preserves initial state if mixed when parent is clicked', () => {
function App() {
const [value, setValue] = React.useState<string[]>([]);
Expand Down
14 changes: 6 additions & 8 deletions packages/react/src/checkbox-group/useCheckboxGroupParent.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,10 @@ export function useCheckboxGroupParent(
const onValueChange = useEventCallback(onValueChangeProp);

const getParentProps: useCheckboxGroupParent.ReturnValue['getParentProps'] = React.useCallback(
() => ({
id,
(parentId?: string) => ({
indeterminate,
checked,
'aria-controls': allValues.map((v) => `${id}-${v}`).join(' '),
'aria-controls': allValues.map((v) => `${parentId ?? id}-${v}`).join(' '),
Copy link
Member

Choose a reason for hiding this comment

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

aria-controls here also needs to reference custom child ids when present

This is where it gets complicated, I think for this to work all the children need to register their ids with the parent, currently the parent is just deriving them from allValues

onCheckedChange(_, eventDetails) {
const uncontrolledState = uncontrolledStateRef.current;

Expand Down Expand Up @@ -72,9 +71,9 @@ export function useCheckboxGroupParent(
);

const getChildProps: useCheckboxGroupParent.ReturnValue['getChildProps'] = React.useCallback(
(name: string) => ({
(name: string, childId?: string) => ({
name,
id: `${id}-${name}`,
id: childId ?? `${id}-${name}`,
checked: value.includes(name),
onCheckedChange(nextChecked, eventDetails) {
const newValue = value.slice();
Expand Down Expand Up @@ -114,14 +113,13 @@ export namespace useCheckboxGroupParent {
id: string | undefined;
indeterminate: boolean;
disabledStatesRef: React.RefObject<Map<string, boolean>>;
getParentProps: () => {
id: string | undefined;
getParentProps: (parentId?: string) => {
indeterminate: boolean;
checked: boolean;
'aria-controls': string;
onCheckedChange: (checked: boolean, eventDetails: BaseUIChangeEventDetails<'none'>) => void;
};
getChildProps: (name: string) => {
getChildProps: (name: string, childId?: string) => {
name: string;
id: string;
checked: boolean;
Expand Down
8 changes: 4 additions & 4 deletions packages/react/src/checkbox/root/CheckboxRoot.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,14 @@ export const CheckboxRoot = React.forwardRef(function CheckboxRoot(
const name = fieldName ?? nameProp;
const value = valueProp ?? name;

const id = useBaseUiId(idProp);

let groupProps: Partial<Omit<CheckboxRoot.Props, 'className'>> = {};
if (isGrouped) {
if (parent) {
groupProps = groupContext.parent.getParentProps();
groupProps = groupContext.parent.getParentProps(id);
} else if (value) {
groupProps = groupContext.parent.getChildProps(value);
groupProps = groupContext.parent.getChildProps(value, id);
}
}

Expand Down Expand Up @@ -119,8 +121,6 @@ export const CheckboxRoot = React.forwardRef(function CheckboxRoot(
state: 'checked',
});

const id = useBaseUiId(idProp);

useIsoLayoutEffect(() => {
const element = controlRef?.current;
if (!element) {
Expand Down
Loading