Skip to content

Conversation

@acstll
Copy link
Contributor

@acstll acstll commented Oct 31, 2025

Summary

EuiButtonGroup is currently meant "for indicating selection only" (docs). But button groups could be used to handle independent intent (clicks), as in toolbars e.g. upload a file.

Important

This PR does not affect EuiButtonGroup, only the internal EuiButtonGroupButton.

The PR makes the isSelected prop optional, so that the aria-pressed attribute is not always present as to indicate selection. Allowing different usages of buttons within button groups.

Why are we making this change?

To enable other usages for EuiButtonGroupButton other than selection.

Needed to improve accessibility in #9151.

Impact to users

🟢 No impact, internal change.

QA

Remove or strikethrough items that do not apply to your PR.

General checklist

  • Browser QA
    • Checked in both light and dark modes
    • Checked in both MacOS and Windows high contrast modes
    • Checked in mobile
    • Checked in Chrome, Safari, Edge, and Firefox
    • Checked for accessibility including keyboard-only and screenreader modes
  • Docs site QA
  • Code quality checklist
  • Release checklist
    • A changelog entry exists and is marked appropriately
    • If applicable, added the breaking change issue label (and filled out the breaking change checklist)
    • If the changes unblock an issue in a different repo, smoke tested carefully (see Testing EUI features in Kibana ahead of time)
  • Designer checklist
    • If applicable, file an issue to update EUI's Figma library with any corresponding UI changes. (This is an internal repo, if you are external to Elastic, ask a maintainer to submit this request)

@acstll acstll self-assigned this Oct 31, 2025
@acstll acstll added skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation) and removed skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation) labels Oct 31, 2025
@acstll acstll marked this pull request as ready for review October 31, 2025 11:48
@acstll acstll requested a review from a team as a code owner October 31, 2025 11:48
so that aria-pressed can be omitted if needed, which enables button group usage different than selection-only
@acstll acstll force-pushed the button-group-aria-pressed branch from ea1fdde to 47ea284 Compare October 31, 2025 11:49
@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @acstll

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @acstll

@weronikaolejniczak weronikaolejniczak self-requested a review November 3, 2025 09:39
Copy link
Contributor

@weronikaolejniczak weronikaolejniczak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that EuiButtonGroup is already passing a boolean and EuiButtonGroupButton now supports both standard and toggle behavior, the changes are correct 👍🏻

I verified that EuiButtonGroup works as expected:

Before After
Windows + Edge + NVDA
Kapture.2025-11-03.at.12.07.52.mp4
Kapture.2025-11-03.at.12.09.02.mp4
MacOS + Safari + VoiceOver
Kapture.2025-11-03.at.12.32.28.mp4
Kapture.2025-11-03.at.12.33.50.mp4

I also went ahead and used EuiButtonGroupButton as a regular button without isSelected and can verify aria-pressed is not present:

Image

Thanks for the improvement, Arturo! 🙏🏻 🟢

Note: In the case were a component relied on EuiButtonGroupButton providing the false default, there could be an a11y regression. aria-pressed will work only if it's a boolean value, otherwise the screen reader gets confused and doesn't pronounce undefined as an "off" state of the toggle button. BUT EuiButtonGroupButton is only used internally in EuiButtonGroup and EuiFilterGroup and it works well.

@acstll
Copy link
Contributor Author

acstll commented Nov 4, 2025

@weronikaolejniczak thanks for taking the time to do the testing, much appreciated!

@acstll acstll merged commit e84a491 into elastic:main Nov 4, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants