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

Border control: missing labels #42095

Open
afercia opened this issue Jul 1, 2022 · 13 comments
Open

Border control: missing labels #42095

afercia opened this issue Jul 1, 2022 · 13 comments
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Components /packages/components [Type] Bug An existing feature does not function as intended

Comments

@afercia
Copy link
Contributor

afercia commented Jul 1, 2022

Description

In the Border control, some form elements are unlabelled. There's nothing that gives these form elements a name so they will be announced by screen readers only with their type. Users won't have a clue what these form elements are meant for and they will be forced to explore the surrounding UI to figure it out.

Step-by-step reproduction instructions

  • edit a block that has a padding control e.g.: Columns
  • use the Chrome dev tools to inspect the source of the Border control that is displayed in the sidebar
  • select the border input field (type=number)
  • observe there's no associated label element or any aria-label / aria-labelledby
  • in the Chrome dev tools, go to the Accessibility tab
  • observe the accessible 'Name' under the computed properties is empty:

Screenshot 2022-07-01 at 16 02 44

  • repeat for the slider input (type=range) and observe it's unlabelled.

  • additionally: the 'Border color and style picker' button doesn't have a tooltip so its accessible name is not visible for users (think at speech recognition software users)

  • click the 'Unlink sides' button

  • there are now 4 input fields, each one for a border side

  • check their accessible name in the Chrome dev tools is empty

  • check there's actually a visually hidden <label> element but it's not associated with the input field: there's no for'id attributes association so the input fields are actually unlabelled

  • additionally: none of the 'Border color and style picker' buttons specifies whether it's for top, right, bottom, left

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@afercia afercia added [Type] Bug An existing feature does not function as intended [Package] Components /packages/components [a11y] Labelling labels Jul 1, 2022
@mirka
Copy link
Member

mirka commented Jul 8, 2022

See also #42118 (comment)

Here's the breakdown of tasks.

BorderControl

#42348

  • Wrap the whole thing in a fieldset and change the label to a legend
  • Add a label to the UnitControl
  • Add a label to the RangeControl
  • Add a ToolTip to the "Border color and style picker" button
  • Update the getByRole() queries in the tests so they include a name

BorderBoxControl

#42351

  • Wrap the whole thing in a fieldset and change the label to a legend
  • In unlinked mode, each BorderControl should be labelled (visually hidden), and also be wrapped in a ToolTip with the same label
  • Update the getByRole() queries in the tests so they include a name

cc @ciampo @aaronrobertshaw

@aaronrobertshaw
Copy link
Contributor

Thanks for the breakdown @mirka 👍

I've created a couple of quick PRs hoping to address most of the tasks noted.

  • BorderControl: Update labelling, tooltips and wrap with fieldset and legend #42348
    • Should tick most of the boxes quoted above
    • I wasn't too sure what the labelling should be in a couple of places here, so I've left a question on the PR itself
  • BorderBoxControl: Improve accessibility and labelling #42351
    • The component now has a fieldset as its wrapper and legend for its label
    • The split border side controls already get given labels e.g. Top border etc. Given the side border controls are within a fieldset>legend markup now, do they still need tooltips?
    • People using screen readers would already have the field identified via the fieldset, legend, and field label right?
    • Visually, the positioning of each side border control overlaying a border box would also indicate which border the control configures despite that control's label being hidden.

I'm happy to make any further changes we need, I'm just a little unclear on what exactly is required.

@mirka
Copy link
Member

mirka commented Jul 14, 2022

  • The split border side controls already get given labels e.g. Top border etc. Given the side border controls are within a fieldset>legend markup now, do they still need tooltips?
  • People using screen readers would already have the field identified via the fieldset, legend, and field label right?
  • Visually, the positioning of each side border control overlaying a border box would also indicate which border the control configures despite that control's label being hidden.

Good question. What I understood from #42118 (comment) is that, users of speech recognition software (not screen reader) need to know the actual name of the form element so they can use voice commands to navigate to it. So even if they can visually perceive that a control is for the top border, and even if it's accessibly labeled, they don't use a screen reader so they need a way to visually tell what the actual label is. Tooltips aren't ideal because they still require the user to focus on the field first, but at least if they do it once they'll have a chance to learn what the label is and can use voice commands from then on.

@aaronrobertshaw
Copy link
Contributor

need to know the actual name of the form element so they can use voice commands to navigate to it

Thanks for the explanation. Updating the BorderBoxControl PR #42351 with feedback from #42348 is on my todo list today or tomorrow, so I'll take a run at adding the Tooltips as part of those efforts.

@afercia
Copy link
Contributor Author

afercia commented Jul 18, 2022

they need a way to visually tell what the actual label is. Tooltips aren't ideal because they still require the user to focus on the field first, but at least if they do it once they'll have a chance to learn what the label is

@mirka thanks, yes that's exactly the point. Tooltips aren't ideal but without them the only alternative for these users (who generally have some form of motor impairment) would be inspecting the source in the browser dev tools to discover the name of a control 🙁

@aaronrobertshaw
Copy link
Contributor

A draft PR exploring tooltips for inner BorderControl fields within a BorderBoxControl can be found in: #42661

@priethor priethor added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). and removed [a11y] Labelling labels Jul 24, 2023
@andrewhayward
Copy link
Contributor

I've been digging around a bit with the border controls in the editor, and as has been mentioned elsewhere, I'm not entirely convinced that the current set up provides "a good and intuitive UX". Apologies for the following, potentially somewhat off-topic, brain dump!

The initially presented view visually implies some sort of 'border' group, with an input labelled 'radius'...

Screenshot of editor control panel, showing a hand-drawn 'border' group, containing an input associated by an arrow with a 'radius' label.

This association breaks down though with the addition of the border size and style control (which itself is somewhat confusingly labelled "border" in the pop-up tools menu). At this point, is 'border' still a group legend, or a label for the new field?

Screenshot of editor control panel, showing an input associated by an arrow with a 'border' label, and another input associated by an arrow with a 'radius' label.

This issue could potentially be resolved by adding a more explicit label to the size and style control, making it clear that "border" still applies to the group as a whole...

Screenshot of editor control panel, showing a hand-drawn 'border' group, containing input associated by arrows with 'size and style' and 'radius' labels.

The whole notion falls apart a bit though, because in reality 'border' is not a group legend but a header, and 'radius' isn't a label but a legend. This issue becomes particularly clear when 'radius' (for example) is unlinked to show four separate inputs. At this point, 'radius' ceases to be a label for a single control, and instead acts as a group heading.

Screenshot of editor control panel, showing a hand-drawn 'border' group, containing an input associated by an arrows with 'size and style' and a 'radius' group with four unlabelled inputs.

Here it is particularly unclear what the separate 'radius' fields are labelled as, and even though it is somewhat discoverable via tooltip, 'top left' doesn't actually mean much when taken out of isolation.

@aaronrobertshaw
Copy link
Contributor

Great breakdown of some of the issues around the border controls @andrewhayward 👍

I wonder if some lessons or inspiration can be taken from the latest iteration of our spacing controls. They now have a dropdown menu beside the property to select the desired side, axis, or "custom" configuration. Could something like that allow better labelling or use of fieldset/legend?

Screenshot 2023-09-01 at 11 45 00 am

After some very quick and dirty hacking in dev tools, it might look something like this:
Screenshot 2023-09-01 at 11 51 03 am
Screenshot 2023-09-01 at 12 08 45 pm
Screenshot 2023-09-01 at 12 10 01 pm

Obviously, there are some issues with the resulting screenshots above but they convey the basic idea.

@afercia
Copy link
Contributor Author

afercia commented Sep 13, 2023

Personally, I'd think the root problem is a bit broader and it relates to the UI inconsistencies between mental models that, from an user perspective, are similar but in the UI are visually and semantically represented in different ways.

For example, border and padding are very similar mental models, as in: they both relate to the four sides of a box. However, the editor UI for these settings is very differnt:

Padding: the expanded UI always shows the controls stacked horizontallu:

padding

Bodrer: the 4 controls are placed in a way to visually represent the top, left, right, and bottom sides:

border

Whether or not such a 'positional' UI is good for users (and I doubt it, given the cognitive load), it's inconsistent with the padding. From a user perspetive, the most intuitive concept is that both paddign and borders apply to the four sides of the box. Why the UI is different?

Radius: the mental model behind this setting is sligthly different, as radius applies to the corners of a box instead of to its sides. However, the UI shows the four controls in a grid with two rows and two columns:

radius

Margin: similar to padding but it only applies to top ad/or bottom sides. It may display one control or two, stacked horizontally.

Overall, I think these 4 settings show too many options to users and they're inconsistent, adding too much cognitiv load for users. Idea;;y, they should be greatly simplified. I'm not sure implementing 'smart' solutions, like the boder 'positional' controls, is ideal. It may seem a smart solution at first but in the longer run it's just confusing. Personally, I'd vote for a gret simplification and for removing the alternative view. Providing just one view, the simpler the better, and consistent design and UX for all these 4 settings would be likely a best option.

@afercia
Copy link
Contributor Author

afercia commented Sep 13, 2023

Additionally: labelling the input fields with icons isn't great from both an accessibility and usability perspective. It would be great to simply have visible label elements.

@ciampo
Copy link
Contributor

ciampo commented Sep 13, 2023

Thank you @andrewhayward and @afercia for the feedback — I also agree that it would be good to have a more consistent and uniform set of controls dealing with the 4 sides of the box model. I'm going to cc @WordPress/gutenberg-design as this conversation may be particularly relevant.

@richtabor
Copy link
Member

Radius: the mental model behind this setting is sligthly different, as radius applies to the corners of a box instead of to its sides. However, the UI shows the four controls in a grid with two rows and two columns

It's the four "corners": top, right, bottom, left.

I wonder if some lessons or inspiration can be taken from the latest iteration of our spacing controls. They now have a dropdown menu beside the property to select the desired side, axis, or "custom" configuration. Could something like that allow better labelling or use of fieldset/legend?

I think leaning into this approach could be interesting.

It's natural as we push forward in some areas to take those and apply them elsewhere. Worth exploring common patterns across these, as long as there's comprehension. We can have varying experiences for different controls, as long as they support the user. But I do think it's worth exploring on this front.

@jameskoster
Copy link
Contributor

I think leaning into this approach could be interesting.

Agreed. Another big benefit is that multiple controls can benefit from subsequent enhancements to the general pattern.

One note: splitting padding/margin into horizontal/vertical by default serves common use cases that I'm not sure are shared by border. IE if we tried this, I'd consider the default control for border to manipulate all sides together, with options to select just one side, or custom for all sides. No linked top/bottom and left/right.

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 [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

8 participants