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): remove abbr from day of week header #24106

Merged
merged 1 commit into from
Feb 4, 2022

Conversation

zarend
Copy link
Contributor

@zarend zarend commented Dec 15, 2021

Removes the abbr tag that is nested inside the cells of the day of the
week header. This is to fix #23477 where VoiceOver reads day of week
three times: 'Sunday, Sunday, Sunday...'

I tested this change on VoiceOverAfter this change and sometimes it read
the day of week twice and sometimes it read it once. When navigating
from a day on the calendar to the header it reads: "row 1 of 7 Sunday
S". When navigating from Sunday to Monday, it reads "Monday Monday M
Column 2 of 7".

Tested on macos 12.0.1 (21A559) with chrome Version 96.0.4664.110 (Official Build) (x86_64).

I'll leave this as a draft until I can at least test it on other
screenreaders.

fixes #23477

@zarend zarend requested a review from jelbourn December 15, 2021 23:03
@zarend zarend force-pushed the datepicker-sunday-sunday-sunday branch from 8f631fe to 7fd0b94 Compare December 15, 2021 23:04
@jelbourn
Copy link
Member

jelbourn commented Dec 16, 2021

FYI I added the <abbr> element in #23022 in order to get something that worked (to some degree) in both VoiceOver and ChromeVox, which did very much not want to be mutually happy.

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Dec 29, 2021
@zarend
Copy link
Contributor Author

zarend commented Jan 15, 2022

Hmm.. I tested on ChromeOS, and this works. Let's compare this to what's on material.angular.io right now. Here's what each screenreader reads navigating from left to right along the day-of-week header.

On material.angular.io:

  • ChromeVox reads: "S row 1 column 1 Sunday Sunday Sunday Abbreviation"
  • Voiceover oreads "Monday Monday Monday Column 2 of 7
  • JAWS reads "M Column 1"

With this change:

  • ChromeVox reads: "S row 1 column 1 Sunday Sunday"
  • Voiceover reads "Tuesday Tuesday T Column 2 of 7"
  • JAWS reads "M Column 1" (no behavior change)

Voiceover and Chromevox are still reading the day-of-week twice because it's the header cell. It reads "Sunday" once for the name of the column, then again for the text content of the cell. IN other words, it announces the name of the column the same as for any other cell in the table (e.g. Voiceover reads"Sunday January 16 16 Column 1 of 7" ).

I wasn't able to navigate to the day-of-week header on NVDA.

@jelbourn
Copy link
Member

Can you push up a preview build? I'll give it a spin in VO and ChromeVox

@zarend zarend force-pushed the datepicker-sunday-sunday-sunday branch from 7fd0b94 to 75cb058 Compare January 19, 2022 04:23
@zarend zarend added Accessibility This issue is related to accessibility (a11y) area: material/datepicker dev-app preview When applied, previews of the dev-app are deployed to Firebase target: patch This PR is targeted for the next patch release labels Jan 19, 2022
@github-actions
Copy link

github-actions bot commented Jan 19, 2022

@zarend
Copy link
Contributor Author

zarend commented Jan 19, 2022

Good idea, a deploy is ready for you to look at https://ng-comp-dev--pr-24106-75cb058b9fb5ebe218a0f8a3bb260a47-h91vbocv.web.app

Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

Tested w/ ChomeVox and VoiceOver, looks good to move forward

@zarend zarend marked this pull request as ready for review January 26, 2022 22:24
@zarend zarend requested a review from mmalerba as a code owner January 26, 2022 22:24
@zarend zarend force-pushed the datepicker-sunday-sunday-sunday branch from 75cb058 to 3bfa3fe Compare January 26, 2022 22:27
@zarend zarend added the action: merge The PR is ready for merge by the caretaker label Jan 26, 2022
Removes the `abbr` tag that is nested inside the cells of the day of the
week header. This is to fix angular#23477 where VoiceOver reads day of week
three times: 'Sunday, Sunday, Sunday...'

I tested this change on VoiceOverAfter this change and sometimes it read
the day of week twice and sometimes it read it once. When navigating
from a day on the calendar to the header it reads: "row 1 of 7 Sunday
S". When navigating from Sunday to Monday, it reads "Monday Monday M
Column 2 of 7".

Tested on macos 12.0.1 (21A559) with chrome Version 96.0.4664.110 (Official Build) (x86_64).

I'll leave this as a draft until I can at least test it on other
screenreaders.

fixes angular#23477
@zarend zarend force-pushed the datepicker-sunday-sunday-sunday branch from 3bfa3fe to aaac56f Compare January 26, 2022 22:34
Copy link
Contributor

@mmalerba mmalerba left a comment

Choose a reason for hiding this comment

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

LGTM

@zarend zarend merged commit 7a6549f into angular:master Feb 4, 2022
zarend added a commit that referenced this pull request Feb 4, 2022
Removes the `abbr` tag that is nested inside the cells of the day of the
week header. This is to fix #23477 where VoiceOver reads day of week
three times: 'Sunday, Sunday, Sunday...'

I tested this change on VoiceOverAfter this change and sometimes it read
the day of week twice and sometimes it read it once. When navigating
from a day on the calendar to the header it reads: "row 1 of 7 Sunday
S". When navigating from Sunday to Monday, it reads "Monday Monday M
Column 2 of 7".

Tested on macos 12.0.1 (21A559) with chrome Version 96.0.4664.110 (Official Build) (x86_64).

I'll leave this as a draft until I can at least test it on other
screenreaders.

fixes #23477

(cherry picked from commit 7a6549f)
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request Feb 15, 2022
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [@angular/cdk](https://github.com/angular/components) | dependencies | patch | [`13.2.1` -> `13.2.2`](https://renovatebot.com/diffs/npm/@angular%2fcdk/13.2.1/13.2.2) |
| [@angular/material](https://github.com/angular/components) | dependencies | patch | [`13.2.1` -> `13.2.2`](https://renovatebot.com/diffs/npm/@angular%2fmaterial/13.2.1/13.2.2) |

---

### Release Notes

<details>
<summary>angular/components</summary>

### [`v13.2.2`](https://github.com/angular/components/blob/HEAD/CHANGELOG.md#&#8203;1322-enamel-eagle-2022-02-09)

[Compare Source](angular/components@13.2.1...13.2.2)

##### cdk

| Commit | Type | Description |
| -- | -- | -- |
| [4b6e83274](angular/components@4b6e832) | fix | **scrolling:** fix scrolling in appendOnly mode ([#&#8203;24153](angular/components#24153)) |

##### material

| Commit | Type | Description |
| -- | -- | -- |
| [f5199eeeb](angular/components@f5199ee) | fix | **datepicker:** fix improper focus trapping with VoiceOver and ChromeVox ([#&#8203;24300](angular/components#24300)) |
| [a72bcbe50](angular/components@a72bcbe) | fix | **datepicker:** remove abbr from day of week header ([#&#8203;24106](angular/components#24106)) |

##### material-experimental

| Commit | Type | Description |
| -- | -- | -- |
| [3bbcb444f](angular/components@3bbcb44) | fix | **mdc-checkbox:** add missing classes for checked ([#&#8203;24350](angular/components#24350)) |

##### material-experiental

| Commit | Type | Description |
| -- | -- | -- |
| [70bec6054](angular/components@70bec60) | fix | **mdc-list:** update material-components-web to pick up multi-select list keyboard support ([#&#8203;24354](angular/components#24354)) |

#### Special Thanks

Alan Agius, Artur Androsovych, Chabbey François, Joey Perrott, Kristiyan Kostadinov, Miles Malerba, Paul Gschwendtner, Zach Arend and renovate\[bot]

<!-- CHANGELOG SPLIT MARKER -->

</details>

---

### Configuration

📅 **Schedule**: At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these updates again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1161
Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
amysorto pushed a commit to amysorto/components that referenced this pull request Feb 15, 2022
…r#24106)

Removes the `abbr` tag that is nested inside the cells of the day of the
week header. This is to fix angular#23477 where VoiceOver reads day of week
three times: 'Sunday, Sunday, Sunday...'

I tested this change on VoiceOverAfter this change and sometimes it read
the day of week twice and sometimes it read it once. When navigating
from a day on the calendar to the header it reads: "row 1 of 7 Sunday
S". When navigating from Sunday to Monday, it reads "Monday Monday M
Column 2 of 7".

Tested on macos 12.0.1 (21A559) with chrome Version 96.0.4664.110 (Official Build) (x86_64).

I'll leave this as a draft until I can at least test it on other
screenreaders.

fixes angular#23477
@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 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) action: merge The PR is ready for merge by the caretaker area: material/datepicker cla: yes PR author has agreed to Google's Contributor License Agreement dev-app preview When applied, previews of the dev-app are deployed to Firebase 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(datepicker): Redundant announcement of header row cell values
4 participants