Skip to content
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

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/material/datepicker/calendar-body.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Copy link
Member

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?

Copy link
Member

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.

Copy link
Member

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

Copy link
Contributor Author

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

Copy link
Member

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.

Copy link
Contributor Author

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
image

but chrome does
image

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)

</td>
</tr>