LaunchPadLab / lp-components

Our Components
http://lp-components.herokuapp.com
MIT License
5 stars 1 forks source link

aria-labelled by for LabeledField #436

Open inveterateliterate opened 4 years ago

inveterateliterate commented 4 years ago

from pa11y-ci:

dpikt commented 4 years ago

@chawes13 was the second point what you fixed in https://github.com/LaunchPadLab/lp-components/pull/384? (unreleased)

inveterateliterate commented 4 years ago

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

dpikt commented 4 years ago

Well, it's been open since Jan 2019 😵I can prioritize getting it merged next week. I was trying to finish https://github.com/LaunchPadLab/lp-components/issues/410 first but I don't think that's happening any time soon.

inveterateliterate commented 4 years ago

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

dpikt commented 4 years ago

Here's the v4 branch: https://github.com/LaunchPadLab/lp-components/pull/311

chawes13 commented 4 years ago

@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 commented 4 years ago

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!

jhp0621 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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

jhp0621 commented 1 year ago

@danparnella 💟🙇🏼‍♀️