-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
fix(material/datepicker): fix duplicate nav stop with Voiceover #24085
fix(material/datepicker): fix duplicate nav stop with Voiceover #24085
Conversation
Fixes double nav stops when navigating the calendar-body with voiceover . Fixes this by putting `aria-hidden="true"` on the `.mat-calendar-body-cell-preview`, since that element is only visual, and is not for giving semantic info to the a11y tree. fixes angular#24082
@@ -60,6 +60,6 @@ | |||
[class.mat-calendar-body-today]="todayValue === item.compareValue"> | |||
{{item.displayValue}} | |||
</div> | |||
<div class="mat-calendar-body-cell-preview"></div> | |||
<div class="mat-calendar-body-cell-preview" aria-hidden="true"></div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this behavior alternatively go away if we remove the content
css property from .mat-calendar-body-cell-preview
?
@crisbeto do you remember why this rule has the content
property?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm guessing it's because I wanted to reuse the same styles with some ::before
and ::after
selectors and I didn't want to split it out only due to the content
: https://github.com/angular/components/blob/master/src/material/datepicker/calendar-body.scss#L54.
This is a little bit surprising to me, I assumed that content
only works on pseudo elements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting note from MDN on the content
property:
CSS-generated content is not included in the DOM. Because of this, it will not be represented in the accessibility tree and certain assistive technology/browser combinations will not announce it. If the content conveys information that is critical to understanding the page's purpose, it is better to include it in the main document.
https://developer.mozilla.org/en-US/docs/Web/CSS/content#accessibility_concerns
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I tried removing the CSS content in #24095, but that did not fix the duplicate nav stop with VoiceOver
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Weird, I wouldn't have expected an empty div
to be picked up by a screen reader. I don't mind going with the aria-hidden
here, but it would be good to understand why it happens.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To look for an explanation, narrowed it down to a minimally reproducing case. For some reason, the empty div must be absolutely positioned and the gridcell needs to have at least 2px of padding to reproduce.
<html lang="en">
<head>
<meta charset="utf-8" />
<title>Basic datepicker</title>
<style>
.mat-calendar-body-cell-preview {
position: absolute;
}
[role="gridcell"] {
padding: 2px;
}
</style>
</head>
<body>
<table role="grid">
<tbody>
<tr role="row">
<td role="gridcell" tabindex="-1" aria-label="December 12, 2021">
<div>12</div>
<div class="mat-calendar-body-cell-preview"></div>
</td>
<td role="gridcell" tabindex="-1" aria-label="December 13, 2021">
<div>13</div>
<div class="mat-calendar-body-cell-preview"></div>
</td>
<td role="gridcell" tabindex="-1" aria-label="December 18, 2021">
<div>18</div>
<div class="mat-calendar-body-cell-preview"></div>
</td>
</tr>
</tbody>
</table>
</body>
</html>
Chrome has an unnecessary navigation stop with VoiceOver, but Firefox and Safari only have a single navigation stop.
Firefox a11y tree does not include a node for the empty div
The best explanation I can offer at this time is that we hit an edge case regarding empty a11y tree nodes being produced. I recommend going with this solution to add aria-hidden="true"
.
OS: macos 12.0.1 (21A559)
Chrome Version 96.0.4664.93 (Official Build) (x86_64)
Firefox 95.0 (64-bit)
Safari Version 15.1 (17612.2.9.1.20)
Removes the `content` declaration from the tab chevrons. Usually it doesn't have an effect, but according to the discussion on angular#24085, it may cause an unnecessary tab stop.
@jelbourn , I took another pass at investigating why the unnecessary nav stop was happening in the first place, and the best explanation I can come up with at this time is that we hit a weird browser edge case. More details at #24085 (comment). this is ready for you to review again pls. |
Removes the `content` declaration from the tab chevrons. Usually it doesn't have an effect, but according to the discussion on #24085, it may cause an unnecessary tab stop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, good work creating a minimal reproduction
Can you file this as a Chrome bug with your repro?
https://bugs.chromium.org/p/chromium/issues/entry?components=UI%3EAccessibility&labels=Type-Bug,Pri-2
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Fixes double nav stops when navigating the calendar-body with voiceover
. Fixes this by putting
aria-hidden="true"
on the.mat-calendar-body-cell-preview
, since that element is only visual,and is not for giving semantic info to the a11y tree.
fixes #24082