Listen for focus event on calendar cells #24279

zarend · 2022-01-26T17:13:34Z

Description

Please see commit message for description.

Guide to reviewing this

Would probably make the most sense to reviewers to look at code changes in this order:

MatCalendarBody._scheduleFocusActiveCellAfterViewChecked (used for refactoring handing keydown events without changing existing behavior)
Tests added to src/material/datepicker/month-view.spec.ts (the behavior change this PR creates)
MatCalendarBody._cellFocused
MatMonthView._dateBecomesActive

github-actions · 2022-01-26T21:56:27Z

Deployed dev-app to: https://ng-comp-dev--pr-24279-5b3f379a508e0e1f56b26f1428b4b379-dva346t7.web.app

mmalerba

LGTM

src/material/datepicker/month-view.spec.ts

src/material/datepicker/month-view.ts

jelbourn · 2022-02-01T22:01:52Z

src/material/datepicker/multi-year-view.spec.ts

+            '[data-mat-row="1"][data-mat-col="2"] button',
+          ) as HTMLElement;
+
+          dispatchFakeEvent(year2022El, 'focus');


Would the test worked if you actually focused the element with .focus()?

well, not sure exactly what the question is here. If we changed this line to year2022El.focus() then the test would fail when we check that the focusSpy was not called.

An alternative would be to actually focus the element with .focus(), but assert that the focusSpy was called exactly one.

I went with dispathFakeEvent because that's what we use in all the other tests, but I'm open to suggestions.

src/material/datepicker/month-view.ts

zarend · 2022-02-04T22:58:01Z

@jelbourn , I've address the PR comments, and this is ready for your eyes again 👀

jelbourn

lgtm

When a a date cell on the calendar recieves focus, set the active date to that cell. This ensures that the active date matches the date with browser focus. Previously, we set the active date on keydown and click, but that was problematic for screenreaders. That's because many screenreaders trigger a focus event instead of a keydown event when using screenreader specific navigation (VoiceOver, Chromevox, NVDA). Fixes angular#23483

…ell (#24279) When a a date cell on the calendar recieves focus, set the active date to that cell. This ensures that the active date matches the date with browser focus. Previously, we set the active date on keydown and click, but that was problematic for screenreaders. That's because many screenreaders trigger a focus event instead of a keydown event when using screenreader specific navigation (VoiceOver, Chromevox, NVDA). Fixes #23483 (cherry picked from commit 052b97d)

…lendar cell (#24279)" This reverts commit 052b97d.

…lendar cell (#24279)" (#24380) This reverts commit 052b97d.

…lendar cell (#24279)" (#24380) This reverts commit 052b97d. (cherry picked from commit e04e4a7)

When a a date cell on the calendar recieves focus, set the active date to that cell. This ensures that the active date matches the date with browser focus. Previously, we set the active date on keydown and click, but that was problematic for screenreaders. That's because many screenreaders trigger a focus event instead of a keydown event when using screenreader specific navigation (VoiceOver, Chromevox, NVDA). Note that this is the second attempt at this. First attempt was angular#24279, which was reverted in angular#24380 due to an internal issue. Fixes angular#23483

When a a date cell on the calendar recieves focus, set the active date to that cell. This ensures that the active date matches the date with browser focus. Previously, we set the active date on keydown and click, but that was problematic for screenreaders. That's because many screenreaders trigger a focus event instead of a keydown event when using screenreader specific navigation (VoiceOver, Chromevox, NVDA). Note that this is the second attempt at this. First attempt was #24279, which was reverted in #24380 due to an internal issue. Fixes #23483

…ell (angular#24279) When a a date cell on the calendar recieves focus, set the active date to that cell. This ensures that the active date matches the date with browser focus. Previously, we set the active date on keydown and click, but that was problematic for screenreaders. That's because many screenreaders trigger a focus event instead of a keydown event when using screenreader specific navigation (VoiceOver, Chromevox, NVDA). Fixes angular#23483

…lendar cell (angular#24279)" (angular#24380) This reverts commit 052b97d.

…lar#24384) When a a date cell on the calendar recieves focus, set the active date to that cell. This ensures that the active date matches the date with browser focus. Previously, we set the active date on keydown and click, but that was problematic for screenreaders. That's because many screenreaders trigger a focus event instead of a keydown event when using screenreader specific navigation (VoiceOver, Chromevox, NVDA). Note that this is the second attempt at this. First attempt was angular#24279, which was reverted in angular#24380 due to an internal issue. Fixes angular#23483

angular-automatic-lock-bot · 2022-03-10T00:03:38Z

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.}

zarend force-pushed the listen-for-focus-event-on-calendar-cells branch from 90e63ed to 0ab736c Compare January 26, 2022 21:46

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 26, 2022

zarend force-pushed the listen-for-focus-event-on-calendar-cells branch from 0ab736c to 5b3f379 Compare January 26, 2022 21:50

zarend marked this pull request as ready for review January 26, 2022 22:19

zarend requested review from mmalerba and a team as code owners January 26, 2022 22:19

zarend requested review from crisbeto, andrewseguin and jelbourn January 26, 2022 22:19

josephperrott removed the request for review from a team January 27, 2022 16:56

mmalerba approved these changes Jan 27, 2022

View reviewed changes

mmalerba added the action: merge The PR is ready for merge by the caretaker label Jan 27, 2022

zarend added the merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note label Jan 28, 2022

zarend mentioned this pull request Jan 28, 2022

fix(material/datepicker): update active date on focus #24172

Closed

zarend removed the merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note label Jan 31, 2022

jelbourn reviewed Feb 1, 2022

View reviewed changes

jelbourn removed the action: merge The PR is ready for merge by the caretaker label Feb 1, 2022

zarend force-pushed the listen-for-focus-event-on-calendar-cells branch 2 times, most recently from e35faa8 to 8152864 Compare February 4, 2022 22:57

zarend force-pushed the listen-for-focus-event-on-calendar-cells branch 2 times, most recently from cb9b201 to 782f893 Compare February 5, 2022 00:18

jelbourn approved these changes Feb 5, 2022

View reviewed changes

jelbourn added the action: merge The PR is ready for merge by the caretaker label Feb 5, 2022

zarend force-pushed the listen-for-focus-event-on-calendar-cells branch from 782f893 to 0805774 Compare February 5, 2022 01:37

zarend force-pushed the listen-for-focus-event-on-calendar-cells branch from 0805774 to e82f7a6 Compare February 5, 2022 01:39

zarend merged commit 052b97d into angular:master Feb 7, 2022

zarend added a commit that referenced this pull request Feb 8, 2022

Revert "fix(material/datepicker): update active date on focusing a ca…

a0f93f7

…lendar cell (#24279)" This reverts commit 052b97d.

zarend mentioned this pull request Feb 8, 2022

Revert "Listen for focus event on calendar cells" #24380

Merged

zarend added a commit that referenced this pull request Feb 8, 2022

Revert "fix(material/datepicker): update active date on focusing a ca…

e04e4a7

…lendar cell (#24279)" (#24380) This reverts commit 052b97d.

zarend added a commit that referenced this pull request Feb 8, 2022

Revert "fix(material/datepicker): update active date on focusing a ca…

60abc41

…lendar cell (#24279)" (#24380) This reverts commit 052b97d. (cherry picked from commit e04e4a7)

zarend mentioned this pull request Feb 9, 2022

fix(material/datepicker): update active date on focusing a cell #24384

Merged

amysorto pushed a commit to amysorto/components that referenced this pull request Feb 15, 2022

Revert "fix(material/datepicker): update active date on focusing a ca…

7b23c19

…lendar cell (angular#24279)" (angular#24380) This reverts commit 052b97d.

angular-automatic-lock-bot bot locked and limited conversation to collaborators Mar 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Listen for focus event on calendar cells #24279

Listen for focus event on calendar cells #24279

zarend commented Jan 26, 2022 •

edited

Loading

github-actions bot commented Jan 26, 2022

mmalerba left a comment

jelbourn Feb 1, 2022

zarend Feb 4, 2022

zarend commented Feb 4, 2022

jelbourn left a comment

angular-automatic-lock-bot bot commented Mar 10, 2022

Listen for focus event on calendar cells #24279

Listen for focus event on calendar cells #24279

Conversation

zarend commented Jan 26, 2022 • edited Loading

Description

Guide to reviewing this

github-actions bot commented Jan 26, 2022

mmalerba left a comment

Choose a reason for hiding this comment

jelbourn Feb 1, 2022

Choose a reason for hiding this comment

zarend Feb 4, 2022

Choose a reason for hiding this comment

zarend commented Feb 4, 2022

jelbourn left a comment

Choose a reason for hiding this comment

angular-automatic-lock-bot bot commented Mar 10, 2022

zarend commented Jan 26, 2022 •

edited

Loading