Skip to content
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.

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

aria-labelled by for LabeledField #436

Open
inveterateliterate opened this issue Jul 17, 2020 · 12 comments
Open

aria-labelled by for LabeledField #436

inveterateliterate opened this issue Jul 17, 2020 · 12 comments
Assignees
Labels
a11y Accessibility help wanted
Milestone

Comments

@inveterateliterate
Copy link
Contributor

inveterateliterate commented Jul 17, 2020

from pa11y-ci:

  • This fieldset element does not have a name available to an accessibility API. Valid names are: legend element, aria-label undefined, aria-labelledby undefined.
  • Fieldset does not contain a legend element. All fieldsets should contain a legend element that describes a description of the field group.
@dpikt
Copy link
Contributor

dpikt commented Jul 17, 2020

@chawes13 was the second point what you fixed in #384? (unreleased)

@inveterateliterate
Copy link
Contributor Author

and if so @dpikt what's the release plan for v4? we can ignore this in our CI until then

@dpikt
Copy link
Contributor

dpikt commented Jul 17, 2020

Well, it's been open since Jan 2019 😵I can prioritize getting it merged next week. I was trying to finish #410 first but I don't think that's happening any time soon.

@inveterateliterate
Copy link
Contributor Author

it's not urgent! I know you're swamped. I will put a flag to ignore in CI for now

@dpikt
Copy link
Contributor

dpikt commented Jul 17, 2020

Here's the v4 branch: #311

@chawes13
Copy link
Contributor

chawes13 commented Jul 20, 2020

@dpikt This issue would only be partially addressed by #384 (for radio and checkbox groups). While it does appear that a fieldset with a single input is valid HTML, it's not recommended according to WebAIM (excerpt below).

Fieldset and legend should only be used when a higher-level label is necessary. Single checkboxes or basic radio buttons that make sense from their labels alone do not require fieldset and legend.

Another source (UK Gov A11y blog), states:

You should not use the <fieldset> and <legend> when: You have a single form field that asks for a single piece of information.

I believe that this indicates that <fieldset> is not the appropriate element to be wrapping inputs except for Radio and Checkbox groups. Since v4 is unreleased, it's possible that we could include a change that swaps out the element for a div. I wonder how other design systems handle this - it might be worth doing more design research to figure out the most appropriate element.

cc: @inveterateliterate

@inveterateliterate
Copy link
Contributor Author

thanks @chawes13 and @dpikt for now I just made a version of LabeledField in prosci-components and changed fieldset to div (won't link here because the repo is limited). I will revert back once v4 is published!

@dpikt dpikt added the a11y Accessibility label Sep 1, 2020
@chawes13 chawes13 added this to the v6 milestone Aug 31, 2021
@danparnella danparnella self-assigned this Oct 19, 2021
@danparnella danparnella linked a pull request Nov 19, 2021 that will close this issue
5 tasks
@chawes13 chawes13 modified the milestones: v7, v8 Jun 16, 2022
@jhp0621
Copy link
Contributor

jhp0621 commented Mar 9, 2023

@danparnella any progress on this..? 😅 If we change fieldset to div in LabeledField like you have in your draft PR, how would this impact RadioGroup and CheckboxGroup? Should those still have fieldset at the top level? (like in these examples)

context: I'm working on prosci EA tasks (https://share.getcloudapp.com/8LuqZJ4E)

@danparnella
Copy link
Contributor

Ugh, yeah I need to wrap this up, sorry. Yes, I have that in my list of things I'm attempting to accomplish in that PR (to make sure RadioGroup and CheckboxGroup have a wrapping fieldset, but not the other input types).

@danparnella
Copy link
Contributor

Made some progress today, hopefully can get something up tomorrow.

@jhp0621
Copy link
Contributor

jhp0621 commented Mar 10, 2023

@danparnella 💟🙇🏼‍♀️

@jhp0621 jhp0621 mentioned this issue Sep 25, 2023
5 tasks
@danparnella
Copy link
Contributor

@jhp0621 Are we able to close this one out because of your changes in #595?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a11y Accessibility help wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants