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

[a11y] EuiButtonGroup should not be a radio group per recent industry discussion #7101

Closed
cee-chen opened this issue Aug 20, 2023 · 4 comments · Fixed by #7325
Closed

[a11y] EuiButtonGroup should not be a radio group per recent industry discussion #7101

cee-chen opened this issue Aug 20, 2023 · 4 comments · Fixed by #7325
Assignees
Labels
accessibility - screen reader accessibility good first issue low hanging fruit An issue, often a bug, that is lower effort and clearly ought to be fixed

Comments

@cee-chen
Copy link
Member

cee-chen commented Aug 20, 2023

Link to component: https://elastic.github.io/eui/#/navigation/button#button-groups

When EuiButtonGroup is a single selection component (vs multi-select), it becomes an <input type="radio"> group under the hood. However, a recent Twitter discussion by Lea Verou and Léonie Watson (both prominent members of the frontend/accessibility community) have come to the conclusion that this is in fact unintuitive and should not be done (emphasis in quotes is mine):

Unsurprisingly, most people thought the best solution is radio buttons and labels. After all, it works without CSS, right? Progressive enhancement and everything?

That’s what I thought too. I had contorted my component to generate labels and radios in the Shadow DOM from buttons in the light DOM, which resulted in awkward code and awkward CSS, but I felt I was fighting the good fight and doing the best thing for accessibility.

All this was challenged when the actual accessibility expert, Léonie Watson chimed in. For those of you who don’t know her, she is pretty much the expert when it comes to web accessibility and standards. She is also visually impaired herself, giving her a firsthand experience many other a11y aficionados lack. Her recommendation was contrary to what most others were saying: https://twitter.com/LeonieWatson/status/1545745436740313089

She went on to make the point that if a design looks like buttons, it should act like buttons, otherwise there are mismatched expectations and poor UX for AT users:

Copying a Tweet from Leonie with implementation details:

If the grouping is also important, putting each group of buttons inside a <section> element with aria-label to give it an accessible name should do the trick. Has the added bonus of turning each section into a navigable landmark.

Full article and writeup: https://lea.verou.me/blog/2022/07/button-group/

@cee-chen
Copy link
Member Author

CC @1Copenut

@cee-chen
Copy link
Member Author

Changing our approach will greatly simplify our component DOM markup and CSS - I'm in favor of this but would love to hear what others think.

@barlowm
Copy link

barlowm commented Aug 20, 2023

Interesting discussion. and I like her response to how keyboard navigation makes a difference when looking at radio buttons that LOOK like radio buttons vs buttons that act like toggles.

Léonie @[email protected]; @LeonieWatson
Jul 9, 2022
Quite a few differences: Using the Tab key you expect to hit every button in sequence, but only one radio button (and after that to use the arrow keys to navigate between them).

So I'd say that her suggestion should definitely be followed and I know I'm going to use it in my future designs and recommendations as well

Thanks @cee-chen for pointing that out.

@JasonStoltz JasonStoltz added good first issue low hanging fruit An issue, often a bug, that is lower effort and clearly ought to be fixed labels Aug 21, 2023
@1Copenut
Copy link
Contributor

I like this approach a lot. Having a section landmark with an aria-label and buttons inside it makes for easy landmark navigation, and the controls show up in screen reader form controls menus. I quickly mocked this snippet based on Lea Verou's custom component in the linked document.

<section aria-label="EUI controls" role="region">
  <button aria-pressed="true">Button one</button
  <button>Button two</button>
  <button>Button three</button>
</section>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility - screen reader accessibility good first issue low hanging fruit An issue, often a bug, that is lower effort and clearly ought to be fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants