🐛 [BUG] - `FormControl.Validation` ignores `id` attribute, blocking `aria-describedby` linkage

sergioalvz commented 1 month ago

Describe the bug

Hey folks,

We’re working with Primer Brand form components and think we’ve found an accessibility issue with the FormControl.Validation component. We’re trying to connect the TextInput with the corresponding FormControl.Validation component using aria-describedby and id attributes on both components. The issue is that FormControl.Validation seems to ignore the id attribute we pass as a prop, so we’re unable to link the two components together (even though the documentation states it’s a valid prop for this component).

<FormControl fullWidth required>
  <FormControl.Label>Your email</FormControl.Label>

  <TextInput aria-describedby="email-validation-msg" type="email" />

  <FormControl.Validation id="email-validation-msg"> {/* <- this id gets ignored */}
    {error}
  </FormControl.Validation>
</FormControl>

Additionally, I wonder if this is something FormControl could handle by default, so we wouldn’t need to manually connect both components, and Primer Brand could provide an accessible experience out of the box 🤔

Please forgive me if there are other ways to achieve this same accessibility that I’m not aware of but are already supported in Primer Brand!

Reproduction steps

https://codepen.io/sergioalvz/pen/poMarxY

Expected behavior

The FormControl.Validation component does forward the id prop to the actual DOM element.

Screenshots

No response

Browsers

No response

OS

No response

joshfarrant commented 1 month ago

Thanks for this @sergioalvz, great spot!

I think this is related to https://github.com/github/primer/issues/3780, which I'm hoping to get to this week, so hopefully this will be a quick resolution for you 🙂

Ideally, this will be taken care of automatically by the FormControl, but you probably should be able to override the id if you really want to.

joshfarrant commented 1 week ago

@sergioalvz This should be resolved by https://github.com/primer/brand/pull/818

If you include a FormControl.Validation alongside your input inside a FormControl then the aria-describedby will be handled automatically.

This will go out in the next Primer Brand release, let me know if you have any issues or questions 🙂

sergioalvz commented 1 week ago

Thanks, @joshfarrant! That's exciting :-) Do you know when we can expect the next release to happen? 👀

primer / brand