[6.x] Fix Checkbox & Radio label click capturing and useId prop binding - #15139
[6.x] Fix Checkbox & Radio label click capturing and useId prop binding#15139ibrokemycomputer wants to merge 1 commit into
Conversation
99988a7 to
41547f9
Compare
jasonvarga
left a comment
There was a problem hiding this comment.
This overlaps with #15172, which fixes a related but distinct bug worth folding in here.
Note
I'm happy to update this PR with appropriate fixes myself. I'm just putting this feedback here so this doesn't get merged without it. You're welcome to do it if you want, though.
The bug: Checkbox/Item.vue calls useId() with no way to override it. Vue's useId() generates ids from a counter scoped to the Vue app instance, not the whole page. Statamic mounts separate Vue app instances for things like widgets and repeated bard/replicator sets, so each one restarts its own id counter — meaning two unrelated checkboxes in two different bard sets/widgets can end up with the same generated id (e.g. both reka-v-52). Since that id backs both <CheckboxRoot :id> and <label :for>, a collision breaks the label↔input association: clicking one checkbox's label can toggle a totally different checkbox, or fail to toggle anything.
Why this matters for this PR: this PR exposes an id prop on Checkbox/Item.vue and Radio/Item.vue, but nothing here actually supplies an override — so the collision bug above isn't fixed by this PR alone, just made fixable. #15172 closes the loop for checkboxes by having CheckboxesFieldtype.vue pass a deterministic id (derived from the field's full path, e.g. field_page_content_2_..._card_rounded_corners_0) into that new prop, so it's unique per-field regardless of how many separate Vue app instances exist.
Since this PR already touches the same lines of Checkbox/Item.vue (and also fixes Radio/Item.vue, which has the identical problem), it'd be good to fold #15172's CheckboxesFieldtype.vue change in here to avoid a merge conflict between the two. And since RadioFieldtype.vue has the same v-for pattern as CheckboxesFieldtype.vue, it'd make sense to give it the same fix (a unique id per radio option) while this PR is already touching Radio/Item.vue.
41547f9 to
0c576bf
Compare
|
@jasonvarga Thank you for the feedback! I added the changes from #15172 and combined them all in one commit. |
Summary
Fixes
useIdandidprop binding inCheckbox/Item.vueandRadio/Item.vue:useId& Prop Binding Flaw:Checkbox/Item.vueandRadio/Item.vuecalleduseId()internally as a fixed local variable (const id = useId()) instead of exposing anidprop.Checkbox/Item.vuealso importeduseIdfromreka-uirather than Vue. Passing a customidprop was ignored, breaking accessibility customization, causing duplicate or mismatched ID target bindings across items, and failing to align with Statamic's standard UI component design pattern (id: { type: String, default: () => useId() }).<label :for="props.id">and<CheckboxRoot :id="props.id">/<RadioGroupItem :id="props.id">, ensuring that the intendedbefore:absolute before:inset-0full-item click overlay (which makes descriptions and chip paddings clickable) targets the correct control without misfiring or triggering adjacent components.Reproduction
<CheckboxItem id="custom-id">or<RadioItem id="custom-id">.props.idand used an internal auto-generated ID. Becausereka-ui'suseId/ missing prop binding generated mismatched or non-unique IDs across fieldsets, clicking a label or its full-item overlay triggered wrong input controls or ignored explicit ID overrides.idprop, falling back to Vue's nativeuseId()when omitted, and cleanly links<label :for>to the input element.Minimal Reproduction Repository:
https://github.com/ibrokemycomputer/statamic-input-error-poc
Root Cause
const id = useId()was defined directly in<script setup>scope without exposing anidprop, preventing overrides. Additionally,Checkbox/Item.vueimporteduseIdfromreka-uiinstead of'vue', creating ID generation inconsistencies across Statamic's component library.Solution
Standardized
useId& AddedidProp:useIdfrom'vue'across both components.id: { type: String, default: () => useId() }todefineProps.props.idfor:idon<CheckboxRoot>/<RadioGroupItem>,:foron<label>, and:idon<p>/<span>description elements.before:absolute before:inset-0 before:content-['']on<label>so the entire item box (including descriptions and chip padding) remains clickable as intended in PR [6.x] Make the whole radio/checkbox item clickable #14821.Changes
resources/js/components/ui/Checkbox/Item.vuereka-uiuseIdimport for Vue's nativeuseId.id: { type: String, default: () => useId() }prop.:id,:for, andaria-describedbytemplate bindings to useprops.id.resources/js/components/ui/Radio/Item.vueuseIdfrom'vue'.id: { type: String, default: () => useId() }prop.:id,:for, andaria-describedbytemplate bindings to useprops.id.