base-ui icon indicating copy to clipboard operation
base-ui copied to clipboard

[checkbox group] Fix `onCheckedChange` not running when parent checkbox is present

Open mj12albert opened this issue 5 months ago • 4 comments

Repro: https://codesandbox.io/p/sandbox/runtime-moon-m4snqz

In this case although checking the parent also changes the checked state of the child, the child checkbox's change handler does not run

<CheckboxGroup>
  <Checkbox parent onCheckedChange={handleParentChange} />
  <Checkbox onCheckedChange={handleChildChange} />
</CheckboxGroup>

mj12albert avatar Jun 24 '25 09:06 mj12albert

Open in StackBlitz

npm i https://pkg.pr.new/@base-ui-components/react@2155

commit: fd08ec8

pkg-pr-new[bot] avatar Jun 24 '25 09:06 pkg-pr-new[bot]

Bundle size report

Bundle Parsed Size Gzip Size
@base-ui-components/react 🔺+19B(+0.01%) 🔺+3B(0.00%)

Details of bundle changes

Generated by :no_entry_sign: dangerJS against fd08ec8e3501e13e5b02c4082735648e5b78031b

mui-bot avatar Jun 24 '25 09:06 mui-bot

Deploy Preview for base-ui ready!

Name Link
Latest commit fd08ec8e3501e13e5b02c4082735648e5b78031b
Latest deploy log https://app.netlify.com/projects/base-ui/deploys/685cb58e3eb7910008a5f84c
Deploy Preview https://deploy-preview-2155--base-ui.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

netlify[bot] avatar Jun 24 '25 09:06 netlify[bot]

Currently onCheckedChange doesn't run when the controlled value changes externally, and Checkbox.Root itself can't differentiate between a external change caused by a parent or this:

<button onClick={() => setChecked(!checked)}>external</button>

<Checkbox.Root
  checked={checked}
  onCheckedChange={(nextChecked) => {
    console.log('checked change');
    setChecked(nextChecked);
  }}
/>

Another thing is if a child checkbox's onCheckedChange is caused by a parent checkbox change, there's no relevant event to pass to the 2nd arg of onCheckedChange. It would need to be undefined for this scenario, or provide the event from the parent checkbox which doesn't seem right

UPDATE: For consistency with other components controlled behavior, checking the parent should not fire the child's onCheckedChange

mj12albert avatar Jun 24 '25 12:06 mj12albert