Skip to content

fix(web-components): expose toggle checked state to screen readers - #4603

Open
sylvesterkaczmarek wants to merge 1 commit into
mi6:developfrom
sylvesterkaczmarek:fix/4518-popover-toggle-aria-checked
Open

fix(web-components): expose toggle checked state to screen readers#4603
sylvesterkaczmarek wants to merge 1 commit into
mi6:developfrom
sylvesterkaczmarek:fix/4518-popover-toggle-aria-checked

Conversation

@sylvesterkaczmarek

@sylvesterkaczmarek sylvesterkaczmarek commented Aug 16, 2026

Copy link
Copy Markdown

Summary of the changes

Fixes the accessibility semantics for toggle items in ic-popover-menu.

The toggle role and aria-checked state were applied to the ic-button host while keyboard focus and screen-reader interaction occur on its native button. This change forwards those semantics to the native focus target and keeps aria-checked synchronized as the item is toggled.

Regression coverage now verifies both unchecked and checked states on the focusable native button, with the snapshot updated for the corrected accessible DOM.

Related issue

Closes #4518

Validation

  • Acceptance criteria reviewed against the implementation
  • Focused unit regression coverage added
  • Existing component API remains unchanged
  • ARIA role and checked state are placed on the focusable control

@CLAassistant

CLAassistant commented Aug 16, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Welcome 👋

Welcome to the ic-ui-kit repo, thank you for submitting a pull request!

How to contribute

Please read our CONTRIBUTING.md, which explains our ways of working and guidelines for contributions.

Code of Conduct

We'd appreciate it if you could read and abide by our Code of Conduct, as we wish to foster an inclusive and respectful community.

Targeting your pull request

We use develop rather than main as the base for contributions - please make sure your PR is targeting develop.

Signing the CLA

We require all contributors to sign our Contributor License Agreement (CLA) before we can accept a contribution. If you are contributing on behalf of an organization please follow your organization's policies in signing CLAs.

Associated issue

Please make sure that your pull request has an issue open - this allows us to keep track of changes made and offer support where needed.

@GCHQ-Developer-530

Copy link
Copy Markdown
Contributor

Hi @sylvesterkaczmarek, looks like this pull request is just failing prettier checks, please could you run npm run prettier:fix?

@sylvesterkaczmarek

Copy link
Copy Markdown
Author

Thanks for flagging this. I've applied the formatting fix and pushed the update.

@GCHQ-Developer-530

Copy link
Copy Markdown
Contributor

Hi, sorry it's now failing some unit tests. One of them appears to be a snapshot that can be regenerated, but another looks like an unexpected change to functionality. Would you be able to take a look into it?

@sylvesterkaczmarek

Copy link
Copy Markdown
Author

Thanks for flagging this. I’ve pushed the Prettier-only formatting fix. The new workflow run is currently marked “action required”, so it looks like it now just needs maintainer approval before the checks can run.

@GCHQ-Developer-299
GCHQ-Developer-299 force-pushed the develop branch 2 times, most recently from 923f9cd to a8b47fb Compare August 24, 2026 11:55
@GCHQ-Developer-530

Copy link
Copy Markdown
Contributor

Please rebase this branch

@GCHQ-Developer-530
GCHQ-Developer-530 force-pushed the fix/4518-popover-toggle-aria-checked branch from 475a74e to 5393091 Compare August 25, 2026 12:49
Propagate role and aria-checked from the ic-button host to its native focus target so screen readers can announce toggle state changes.
@GCHQ-Developer-530
GCHQ-Developer-530 force-pushed the fix/4518-popover-toggle-aria-checked branch from 5393091 to 089095c Compare August 25, 2026 12:50
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