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

[EuiAccordion] Should we really be using role="region"? #7316

Closed
cee-chen opened this issue Oct 25, 2023 · 5 comments · Fixed by #7326
Closed

[EuiAccordion] Should we really be using role="region"? #7316

cee-chen opened this issue Oct 25, 2023 · 5 comments · Fixed by #7326

Comments

@cee-chen
Copy link
Member

cee-chen commented Oct 25, 2023

Totally not sure if I'm way off here, feel free to close this issue if so!

We're currently applying role="region" onto all accordion children:

I know that W3's WAI accordion example uses role="region", but based on what I'm reading from MDN's docs I'm skeptical that this should be the case for EUI's accordions. Specifically:

The region role is used to identify document areas the author deems significant
[...]
Use sparingly! Landmark roles are intended to be used sparingly, to identify larger overall sections of the document. Using too many landmark roles can create "noise" in screen readers, making it difficult to understand the overall layout of the page.

I really don't feel like we can say we know enough about the content in our accordions to deem them "significant", and in particularly for our collapsible nav that reuses multiple accordions which contain nothing but links, region makes no sense for those usages.

I'd like to propose switching to a role="group" by default and allowing consumers to override/set this role themselves if they know their own content well enough.

@cee-chen
Copy link
Member Author

CCing @1Copenut and @barlowm for their thoughts/expertise!

@cee-chen cee-chen changed the title [EuiAccordion] Should we really be using role="region" for this? [EuiAccordion] Should we really be using role="region"? Oct 25, 2023
@1Copenut
Copy link
Contributor

@cee-chen This is a great question. I didn't have a strong feeling either way, and could see both points. So in true a11y "it depends" fashion, I mocked up a simple example. It uses the same behavior as our EuiAccordion by moving focus on click/keypress.

The interesting thing was how VoiceOver treated "region" and "group" basically the same by themselves. They were both announced as "group" and neither showed up in the VO Landmarks menu. It wasn't until I applied an aria-labelledby that things got interesting.

By using "region" and adding aria-labelledby that referenced the sibling button ID, the content block was announced as a region, and appeared in the Landmarks menu. This seems like the idea scenario to me, but I'm happy to discuss further and hear what Mike thinks.

@cee-chen
Copy link
Member Author

cee-chen commented Oct 27, 2023

By using "region" and adding aria-labelledby that referenced the sibling button ID, the content block was announced as a region, and appeared in the Landmarks menu. This seems like the idea scenario to me

Apply worst case scenario thinking to this however - is this really an ideal scenario if a consumer has, e.g. 500 accordions/"regions" on the page? Or even in the non-worst-case scenario of the new EuiCollapsibleNav component which can have 10+ accordions in a single nav - is using region within nav even valid?

@1Copenut
Copy link
Contributor

@cee-chen and I discussed this further in a Zoom call on 27 October. We opted to default to role="group" for our EuiAccordion for two reasons:

  1. This lines up with the native <summary> && <details> UX in VoiceOver, which announces the detail content as "group". NVDA and JAWS only announce the summary as "button, collapsed | expanded". This default is the closest to native behavior we're going to get.
  2. Content that is contained in a role="group" will be read by screen readers immediately. If consumers opt to raise the content's importance / significance, they can do so by changing the role to region. This will cause the region to appear in screen readers' Landmarks menu and feels a good UX balance and content strategy.

@cee-chen
Copy link
Member Author

Along the same lines of "region roles should be used more carefully", I'd also like to propose switching EuiGlobalToastList's role="region" default to role="log" - while not 100% exactly what we're going for, I think the log role is far closer than region, which is essentially a <section>.

role="alert" could also qualify but I don't fully trust consumers to not use toasts for non-immediate notifications, and I'd rather the default be less aggressive rather than more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants