zendeskgarden/react-containers

useSelection: default aria attributes and roles can cause a11y issues

Closed this issue · 2 comments

hzhu commented

Problem

Prop getters from the useSelection hook includes certain aria attributes such as aria-orientation, role, and aria-selected by default. This poses a challenge for consumers of useSelection and its prop getters because it may require consumers to override the default aria attributes and roles that come with useSelection.

Consumers might need to explicitly override certain aria attributes and roles, otherwise, the default aria attributes and roles from useSelection might not be correctly mapped to accessibility APIs. Incorrect application of aria attributes and roles also lowers the score on accessibility auditing tools such as Chrome Lighthouse and Axe. Below outlines problems for two Storybook examples that use the useSelection hook in its implementation:

ButtonGroup

In the Storybook for ButtonGroup, the useSelection hook’s container prop getter returns properties which are spread onto a div with the role="group". One of these properties is aria-orientation. This is problematic because according to the W3, aria-orientation is not recommended for widgets with a role="group".

The Chrome Lighthouse and Axe tools provide warnings for these problems:

Screenshot of Lighthouse & Axe for <ButtonGroup>

🕵️Lighthouse & Axe audits can be run on the official Storybook example.
🕵️Lighthouse & Axe audits can also be run on the <ButtonGroup> component from React Components.
⭐️Lighthouse score can be improved from 91 to 100 by addressing the issue(s) outlined above.

Pagination

In the Storybook for Pagination, the useSelection hook’s container prop getter returns properties which are spread onto a div with the role="nav" or onto a <nav> element. One of these properties is aria-orientation. This is problematic because according to the W3, aria-orientation is not recommended for widgets with a role="nav" or <nav> elements.

Also, each page nav item has a role="option". This is problematic because W3 recommends that elements with role option are contained in, or owned by, an element with the role listbox. Options not associated with a listbox might not be correctly mapped to an accessibility API.

Unless the <Pagination> component uses the role="listbox", the attribute aria-selected should also be removed. Like aria-orientation, aria-selected usage is recommended only for a specific set of roles.

Screenshot of Lighthouse & Axe for <Pagination>

🕵️Lighthouse & Axe audits can be run on the official Storybook example.
🕵️Lighthouse & Axe audits can also be run the <Pagination> component from React Components.
⭐️Lighthouse score can be improved from 77 to 100 by addressing the issue(s) outlined above.

Solutions

Approach 1

Manually override aria attributes for affected widgets (buttongroup & pagination).

With this approach, the core useSelection API is not changed. Instead, it will be the responsibility of the consumer to override aria attributes and roles if they want to satisfy accessibility concerns. This works but has the drawback that the consumer must know to override certain attributes and roles.

Importantly, it seems that the API was intentionally designed this way.

Approach 2

Update the useSelection hook so that prop getters do not declare default aria-attributes such as aria-orientation, aria-selected, role, etc. This would change the current API design.

In this approach, consumers of the useSelection hook don’t have to worry about defensively satisfying accessibility concerns. In other words, when someone uses useSelection hook, they are by default not breaking any accessibility recommendations. If the consumer needs a specific attribute (for example, they are implementing <Tabs> widget and need both aria-orientation and aria-selected) it will be their responsibility to add those attributes into their implementation.

Please chime in your thoughts, feedback, and alternative approaches!

FWIW the original implementation of the useSelection didn't include getContainerProps or any specific roles, they got added in over time. So I think this makes sense.

Let's break this down:

  • aria-orientation is only valid in very specific scenarios (it also looks like the both value has been removed from the spec 😠 )
    • Solution: Remove this value. Vertical usages are rare, a component using this hook should be able to handle it.
  • role values are invalid for some usages
    • Solution: Remove all role values and ensure that the composing containers apply these themselves. Depending on the required roles for the individual items this may be required.
  • aria-selected
    • This is a unique one since we using the "roving tab index" model for our single-focus widgets.

Any explicit assignment of aria-selected takes precedence over the implicit selection based on focus. If no DOM element in the widget is explicitly marked as selected, assistive technologies MAY convey implicit selection which follows the keyboard focus of the managed focus widget. If any DOM element in the widget is explicitly marked as selected, the user agent MUST NOT convey implicit selection for the widget.

  • From the guidance above I think we should work to keep this around.

The Pagination example is interesting that some of these aria failures are specific to the example and not the values provided by the hook. Rather than apply getContainerProps() to the nav element we could apply it to the <ul>.

Approach

I vote we remove aria-orientation since it's a rare occurrence. We just need to implement this in any react-components that have a vertical styling (tabs only I think).

I also believe aria-selected is an important aspect to denote selection state since we are using roving tab index. Let's find examples where we are not overriding these values correctly (i.e. pagination) and fix these examples. I think these failures are actually less common than we think, container-tabs is currently at 100% in AXE.

hzhu commented

@Austin94 thanks for the thoughts! I've incorporated them in #129.