[checkbox group] Fix parent checkbox ignoring custom id prop
#2896
Conversation
edmolima
requested review from
atomiks,
colmtuite and
michaldudak
as code owners
commit: |
✅ Deploy Preview for base-ui ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
mj12albert
added
the
component: checkbox group
|
@edmolima I tested this PR using the original repro: https://codesandbox.io/p/sandbox/base-ui-checkbox-nested-bug-forked-zkhqtl A custom |
mj12albert
changed the title
id prop (#2691)id prop
…=20(staff) groups=20(staff),12(everyone),61(localaccounts),79(_appserverusr),80(admin),81(_appserveradm),33(_appstore),98(_lpadmin),100(_lpoperator),204(_developer),250(_analyticsusers),395(com.apple.access_ftp),398(com.apple.access_screensharing),399(com.apple.access_ssh),400(com.apple.access_remote_ae),701(com.apple.sharepoint.group.1) prop
|
Hey @mj12albert! Thanks for testing this and catching the issue! I added a new test, and right now everything looks good. The playground is here to easily the test again: |
| indeterminate, | ||
| checked, | ||
| 'aria-controls': allValues.map((v) => `${id}-${v}`).join(' '), | ||
| 'aria-controls': allValues.map((v) => `${parentId ?? id}-${v}`).join(' '), |
There was a problem hiding this comment.
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
github-actions
bot
added
the
PR: out-of-date
Fixes #2691
Context
In the "parent checkbox" use case of CheckboxGroup, the consumer-specified
idonCheckbox.Rootwas being ignored. This made it impossible to associate a label to the checkbox usinghtmlFor.The issue occurred because
useCheckboxGroupParentwas returning its own generatedidingetParentProps(), which overrode the user-providedidprop.Summary
getParentProps()to accept an optionalparentIdparameterCheckboxRootnow passes itsidtogetParentProps()for parent checkboxesidprops are honored andaria-controlsuses the correct idTest plan
idis respected on parent checkboxhtmlForworks correctlyI have followed (at least) the PR section of the contributing guide.