Closed tellthemachines closed 3 years ago
aria-label
is redundant, unless there's a good reason to keep it I can't think of
We added aria-label
after testing with Dragon and verifying that the button would not be activated unless it had the aria-label
matching the visible text. It's still open for debate whether it's more intuitive to activate the button by speaking out the label above it or the actual button content though. Perhaps making it equal to the button content would solve the problem?
I'd also like this issue and the implementation from #17926 to be discussed in the next accessibility team meeting.
It would be great to have some more opinions on this! Unfortunately I can't join because the meeting takes place in the middle of the night for me, but more than happy to discuss asynchronously 🙂
Got it. The root problem is:
Overall, even if this component tries to use ARIA techniques it is still a non-native implementation that reduces the accessibility of this control and as such it's actually an accessibility regression compared to the implementation with a native <select>
element. A properly labeled <select>
element would work for everyone and also with Dragon.
I'd also like to remind the previous implementation with the custom font size selector was flagged as a WCAG failure by the WPCampus audit in https://github.com/WordPress/gutenberg/issues/15319. That's the reason why it was changed to a native <select>
following the remediation guidance provided on the issue. There are many non-standard things in this component that it can't be really called "WAI-ARIA compliant custom select" and it will likely be flagged again as a failure by any future accessibility audit.
That said, I do realize others in the team strongly feel in favour of this component because they want a "preview" of the font size. For what is worth, I can't really support this implementation as it comes to a cost of reduced accessibility. I'd like to point out there are other ways to show a preview that haven't been even considered.
@afercia is right, the button being triggered by the label click is an unfortunate side effect.
htmlFor
can be removed from the label but the click on the label will not do anything.button
element can be replaced with something else but I don't know what other element can be used to just accept the focus on label click and not be triggered.Of course a native select
will have its accessibility covered out of the box. You will have to spend effort on styling the thing though. Maybe on other things as well.
Do you think of any improvements that can be done here in terms of accessibility?
Do you think of any improvements that can be done here in terms of accessibility?
I'd start strictly following the pattern described on the ARIA Authoring Practices. The patterns described there are the ones assistive technologies are supposed to support. https://www.w3.org/TR/wai-aria-practices-1.2/#Listbox https://www.w3.org/TR/wai-aria-practices-1.2/examples/listbox/listbox-collapsible.html
<span id="exp_elem">Choose an element:</span>
<div id="exp_wrapper">
<button aria-haspopup="listbox" aria-labelledby="exp_elem exp_button" id="exp_button">
Neptunium
</button>
<ul ...
<label>
elementaria-labelledby
which references two elements:
This way, also the selected value within the button will be announced by screen readers e.g.: Choose an element: Neptunium
Then, this should be tested again with Dragon and other speech recognition software, e.g. Voice Control / Dictation on macOS.
Still, the different behaviors and interactions across operating systems and browsers can't be covered by a custom component. That's my main concern as they greatly differ and users are used to them.
As I understand the aria-label
was added to make it work with Dragon Speech. Originally it should have been only an aria-labelledby
with the 2 IDs you correctly mentioned since that's the default useSelect
behavior. We can try to remove the aria-label
again and dig deeper into why it's not workign.
About the htmlFor
I am OK with removing it since it causes this toggle issue. However a native <select>
works with a <label>
by default. It will just move focus on the <select>
without opening it. Do you think it's a good common ground? Removing the htmlFor
and adding a onClick
on the getLabelProps
that will focus()
the toggleButton?
About the htmlFor I am OK with removing it since it causes this toggle issue
Removing only the htmlFor
would produce an orphaned label, which should be avoided. The whole <label>
element should be removed instead.
Yes the aria-labelledby with two IDs should be tested again with speech recognition software but that's the only way I can think of to make assistive technologies announce also the currently selected "option" as they would do with a native <select>
element.
The label will not be orphaned, as it will still have the id
used in aria-labelledby
on the button
and menu
. So I think we can prop the getLabelProps
with {htmFor: undefined}
and test it from there. I am pushing for a speech recognition in my org so I can test with that.
I'm closing this issue as we now have both CustomSelectControl
and ComboboxControl
in our components package. Any specific a11y issues with these components should be reported separately. Thanks everyone for the input!
Is your feature request related to a problem? Please describe.
HTML
select
elements are not visually customisable, and in some circumstances we need to customise the options in a dropdown. When showing available text styles, for example:We also need these dropdowns to be fully accessible when used with keyboard, screen readers, voice commands, and any assistive technology.
Describe the solution you'd like
Investigate how to best achieve full accessibility while using non-semantic markup, with ARIA and custom keyboard interaction. Previous discussion for reference: https://github.com/WordPress/gutenberg/pull/16148
Deque's custom select might be a good solution: https://pattern-library.dequelabs.com/components/selects
React-select is another possibility; there's ongoing work/discussion on accessibility improvements over there: https://github.com/JedWatson/react-select/issues/2456