Skip to content

fix(core/mat-option): don't remove aria-selected from deselected options #25749

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

Closed
wants to merge 1 commit into from

Conversation

zarend
Copy link
Contributor

@zarend zarend commented Oct 3, 2022

For mat-option, set aria-selected="false" on deselected options that are selectable. Conforms with WAI ARIA Listbox authoring practices guide, which says to always include aria-selected attribute on options that are selectable. Fix issue where voiceover reads every option as "selected" (#25736).

Fix #25736

@zarend zarend added Accessibility This issue is related to accessibility (a11y) target: patch This PR is targeted for the next patch release area: material/autocomplete area: material/select labels Oct 3, 2022
@zarend zarend requested a review from mmalerba October 3, 2022 17:17
@zarend zarend requested a review from crisbeto as a code owner October 3, 2022 17:17
@zarend zarend force-pushed the mat-option-aria-selected branch from 3368086 to 95bd6f5 Compare October 3, 2022 17:35
@zarend
Copy link
Contributor Author

zarend commented Oct 3, 2022

Draft. Here is remaining work before this converts to a real PR

  • validate on NVDA, VoiceOver and Chromeos
  • fix tests

@zarend zarend marked this pull request as draft October 3, 2022 17:35
@zarend zarend force-pushed the mat-option-aria-selected branch 4 times, most recently from 16d67d7 to f198ec2 Compare October 5, 2022 18:36
@zarend zarend marked this pull request as ready for review October 5, 2022 20:20
@zarend
Copy link
Contributor Author

zarend commented Oct 5, 2022

@mmalerba @crisbeto this PR is ready for your eyes 👀 . I've tested that it works on NVDA, VoiceOver and ChromeVox

@zarend zarend force-pushed the mat-option-aria-selected branch 2 times, most recently from 494cc1e to c5ec0e3 Compare October 6, 2022 18:53
@zarend zarend requested a review from andrewseguin as a code owner October 6, 2022 18:53
@zarend zarend requested a review from crisbeto October 6, 2022 18:53
@zarend
Copy link
Contributor Author

zarend commented Oct 6, 2022

responded to comments, and this is ready for review again 👀

@@ -243,6 +233,19 @@ export class _MatOptionBase<T = any> implements FocusableOption, AfterViewChecke

/**
* Single option inside of a `<mat-select>` element.
*
* The aria-selected attribute applied to the option conforms to WAI ARIA best practices for listbox
Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't put the aria-selected explanation here, because it's going to show up in the docs site. It can be put right before the '[attr.aria-selected]': 'selected', on line 261.

@zarend zarend force-pushed the mat-option-aria-selected branch from c5ec0e3 to c31f13a Compare October 7, 2022 17:10
For mat-option, set `aria-selected="false"` on deselected options that
are selectable. Conforms with [WAI ARIA Listbox authoring practices guide](
https://www.w3.org/WAI/ARIA/apg/patterns/listbox/), which says to always
include aria-selected attribute on options that are selectable. Fix issue where
voiceover reads every option as "selected" (angular#25736).

Fix angular#25736
@zarend zarend force-pushed the mat-option-aria-selected branch from c31f13a to ac9e95d Compare October 7, 2022 17:41
@zarend
Copy link
Contributor Author

zarend commented Oct 7, 2022

Closing as this appears to make VoiceOver issue #24390 worse. This improves how VoiceOver announces selected state, but it appears to worsen the navigation problems with VoiceOver. I recommend going through with this change (putting aria-selected="false", but only after fixing #24390 first.

@zarend zarend closed this Oct 7, 2022
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Nov 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Accessibility This issue is related to accessibility (a11y) area: material/autocomplete area: material/core area: material/select target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug(material/chips): for single-select chips VoiceOver reads every option as selected
2 participants