Skip to content

fix(material/core): mat-optgroup labels are read twice during screen-reader page navigation #21858

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 18, 2021
Merged

fix(material/core): mat-optgroup labels are read twice during screen-reader page navigation #21858

merged 1 commit into from
Feb 18, 2021

Conversation

axverma
Copy link
Contributor

@axverma axverma commented Feb 10, 2021 •
edited
Loading

Fixed by adding aria-hidden="true" to the elements, so that while screenreaders will still announce the label text for the group (due to the aria-labelledby property on mat-optgroup), they will skip over the actual label element. Additionally, changed the label to a span to make it clear that the text is visual only and not carrying any special semantic meaning for the a11y tree.

Fixes #21857

@axverma axverma requested a review from crisbeto as a code owner February 10, 2021 03:23
@google-cla google-cla bot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Feb 10, 2021
@axverma axverma changed the title fix(material/core): mat-optgroup labels can be are read twice during screen-reader page navigation fix(material/core): mat-optgroup labels are read twice during screen-reader page navigation Feb 10, 2021
crisbeto
crisbeto reviewed Feb 10, 2021
View reviewed changes
Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

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

Also spent some time testing it with a screen reader and didn't notice any issues.

src/material/core/option/optgroup.html
@@ -1,2 +1,2 @@
<label class="mat-optgroup-label" [id]="_labelId">{{ label }} <ng-content></ng-content></label>
<span class="mat-optgroup-label" aria-hidden="true" [id]="_labelId">{{ label }} <ng-content></ng-content></span>
Copy link
Member

Choose a reason for hiding this comment

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

The same fix will have to be applied to the MDC-based option group in src/material-experimental/mdc-core/option/optgroup.html.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added the changes to src/material-experimental/mdc-core/option/optgroup.html.

I described the a11y issues I saw in #21847 (with ChromeVox and with VoiceOver + Chrome). I believe removing the group labels from the a11y tree would stop the double-reading in these cases without impacting browsers that do not have this issue. I could provide some screen recordings if that would help.

fix(material/core): mat-optgroup labels are read twice during screen-…
cce9af7 
…reader page navigation.

Fixed by adding aria-hidden="true" to the <label> elements.
While screenreaders will still announce the label text for the group they will skip over the actual label element.
Additionally, changed the <label> to a <span> to make it clear that the text is visual only.
crisbeto
crisbeto approved these changes Feb 11, 2021
View reviewed changes
Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

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

LGTM

@crisbeto crisbeto added Accessibility This issue is related to accessibility (a11y) G This is is related to a Google internal issue action: merge The PR is ready for merge by the caretaker P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent target: patch This PR is targeted for the next patch release labels Feb 11, 2021
@andrewseguin andrewseguin merged commit ca37709 into angular:master Feb 18, 2021
andrewseguin pushed a commit that referenced this pull request Feb 18, 2021
@axverma @andrewseguin
fix(material/core): mat-optgroup labels are read twice during screen-…
fad83cb 
…reader page navigation. (#21858)

Fixed by adding aria-hidden="true" to the <label> elements.
While screenreaders will still announce the label text for the group they will skip over the actual label element.
Additionally, changed the <label> to a <span> to make it clear that the text is visual only.

Co-authored-by: Akshay Verma <[email protected]>
(cherry picked from commit ca37709)
@axverma axverma deleted the optgroup-sr branch February 18, 2021 20:07
@dependabot dependabot bot mentioned this pull request Mar 6, 2021
Closed
@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 Mar 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@crisbeto crisbeto crisbeto approved these changes

Assignees
No one assigned
Labels
Accessibility This issue is related to accessibility (a11y) action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement G This is is related to a Google internal issue P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent target: patch This PR is targeted for the next patch release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

bug(mat-optgroup): <label> element should be skipped during screen-reader page navigation
3 participants
@axverma @crisbeto @andrewseguin