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

Components: label multiple / composed inputs correctly #42630

Open
ciampo opened this issue Jul 22, 2022 · 5 comments
Open

Components: label multiple / composed inputs correctly #42630

ciampo opened this issue Jul 22, 2022 · 5 comments
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Components /packages/components

Comments

@ciampo
Copy link
Contributor

ciampo commented Jul 22, 2022

What problem does this address?

For more context: this topic was first pointed out in #42118 (comment) and #42095, and recently discussed in #42351 (review)

There are a few instances in the @wordpress/components package where multiple input elements are combined into higher-level components without appropriate labelling.

As established in previous conversations (see links above) and in w3c's official documentation, the most correct way would be to associate related inputs by wrapping them in a fieldset elements, while keeping individual labels for each single input.

The challenge in doing so is to avoid nested fieldsets when composing components which already internally group multiple inputs. For example:

What is your proposed solution?

We should come up with a solution that can be applied systematically across all *Control components.

@ciampo ciampo added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Components /packages/components labels Jul 22, 2022
@ciampo
Copy link
Contributor Author

ciampo commented Jul 22, 2022

One idea that I though we could explore, is to:

  • make sure all components in need of grouping multiple inputs are wrapped with the same component — could be the existing BaseControl, or a new ControlGroup (name TBD) component
  • add functionality to BaseControl (or the new component) to optionally render a fieldset around the control
  • leveraging the context system, BaseControl (or the new component) could ready its context to look for a disallowFieldset (name TBD) prop that would disallow rendering the fieldset
  • therefore, each BaseControl (or the new component) that is supposed to render a fieldset would set the disallowFieldset flag via context, which will be consumed by all nested BaseControl components

Also, an alternative approach, according to the official W3C's guidelines, would be to use a div role=group and the aria-labelledby attribute. Given that we established that it's not the best approach, would it still be a good fallback for when we can't render a fieldset ?

Thoughts?

@ciampo
Copy link
Contributor Author

ciampo commented Jul 25, 2022

The same approach could be used to avoid displaying nested Tooltips (as discussed in #42661)

@afercia
Copy link
Contributor

afercia commented Jul 26, 2022

Thanks for creating this issue.

A few points I'd like to highlight:

I'm not sure 'base' components like BorderControl should have the responsibility to render a fieldset + legend. We should not assume that all base controls need to be grouped. That actually depends on the nature of the control and quality of the labels. For example, a set of inputs like the following:

  • First name
  • Last name
  • Email
  • Phone

doesn't need to be grouped in a fieldset, as the purpose of this set of inputs is clear. Instead, a set of inputs like:

  • Red
  • Green
  • Blue

do need to be grouped in a fieldset with a legend, otherwise their purpose isn't clear at all.

That's to say that in most of the cases it's up to the developer to make a decision on whether to use a fieldset + legend. Instead, having this logic built-in in the components doesn't seem ideal to me.

Also, for educational purposes, I'd consider to make the usage of fieldset + legend more explicit. Thinking at a simple Fieldset component with a required legend. From an educational perspective, this would make developers more aware of what a named group is and when to use it.

@ciampo
Copy link
Contributor Author

ciampo commented Jul 26, 2022

We should not assume that all base controls need to be grouped. That actually depends on the nature of the control and quality of the labels

A more concrete scenario (apart from BorderBoxControl) for which we need to agree on an approach is the following:

  • NumberControl renders an input[type="number"]
  • UnitControl renders a NumberControl (for the quantity) and a select dropdown (for the unit)
  • RangeControl can render a NumberControl next to an input[type="slider"] as two alternative ways to change the same value
  • We are planning to build a SliderUnitControl component (name TBD) that puts together a UnitControl (which in itself combines NumberControl and a select) and an input[type="slider"].

In which instances should we render a fieldset, since these components conceptually offer multiple inputs to change the same resulting value ?

If we agree that UnitControl would normally render a fieldset, do we agree that, when embedded inside a SliderUnitControl , it is SliderUnitControl that should render a fieldset instead (while UnitControl should not render a fieldset) ?

Also, for educational purposes, I'd consider to make the usage of fieldset + legend more explicit. Thinking at a simple Fieldset component with a required legend. From an educational perspective, this would make developers more aware of what a named group is and when to use it.

I see your point, but calling components (and props) with names that explicitly refer to the implementation details has the drawback of making the component tied to that particular implementation — hence why I prefer more semantic names.

@afercia
Copy link
Contributor

afercia commented Sep 1, 2022

Thanks @ciampo for the clarification. Conceptually, I'm not sure we can always determine in advance whether a control needs a fieldset + legend or not. It always depends on the nature of the control, the context, and the quality of the labels.

That's why i'd tend to think education and training are more effective rather than 'smart' components that inevitably are based on assumptions.

To go through your examples:

NumberControl
It controls a single value. I'm not sure it needs a fieldset, because there's nothing to 'group' together. Of course, multiple NumberControl(s) would need to be grouped.

UnitControl
It really depends on the actual usage and quality of the labels, For example, it could be grouped in a fieldset:

  • Top margin (fieldset legend)
    • Size (NumberControl label)
    • Unit (select label)

OR, not use a fieldset and use more explicit labels instead:

  • Top margin size (NumberControl label)
  • Top margin unit (select label)

The same consideration applies to the planned SliderUnitControl.

RangeControl
The NumberControl and the slider change the same value. They are basically the same thing. I'm not sure a fieldset would be appropriate, in this case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Components /packages/components
Projects
None yet
Development

No branches or pull requests

7 participants