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

Voiceover pagedown #24399

Merged
merged 1 commit into from
Mar 14, 2022
Merged

Voiceover pagedown #24399

merged 1 commit into from
Mar 14, 2022

Conversation

zarend
Copy link
Contributor

@zarend zarend commented Feb 11, 2022

Please see commit message for description.

@zarend zarend added Accessibility This issue is related to accessibility (a11y) target: patch This PR is targeted for the next patch release area: material/datepicker labels Feb 11, 2022
@zarend zarend requested review from jelbourn and crisbeto February 11, 2022 00:41
@zarend zarend requested a review from mmalerba as a code owner February 11, 2022 00:41
src/material/datepicker/calendar-body.ts Outdated Show resolved Hide resolved
src/material/datepicker/calendar-body.ts Outdated Show resolved Hide resolved
src/material/datepicker/calendar-header.html Outdated Show resolved Hide resolved
@zarend zarend force-pushed the voiceover-pagedown branch from 9ef8e59 to 5939b36 Compare February 15, 2022 17:32
@zarend zarend marked this pull request as draft February 15, 2022 17:34
@zarend
Copy link
Contributor Author

zarend commented Feb 15, 2022

Changing this to a draft, as I need to fix many tests. Adding the 20ms timeout means many tests need to be updated to have a tick(20). I'll change it to a real PR this this is ready.

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.

Any thoughts on whether we could add a unit test here?

src/material/datepicker/calendar-body.ts Outdated Show resolved Hide resolved
src/material/datepicker/calendar-body.ts Outdated Show resolved Hide resolved
@zarend zarend force-pushed the voiceover-pagedown branch 4 times, most recently from 5188119 to 94605e5 Compare March 2, 2022 23:53
@zarend zarend force-pushed the voiceover-pagedown branch from 94605e5 to 353a900 Compare March 3, 2022 23:37
Add a timeout before programatically focusing cells on the calendar. This seems
to fix an issue where Voiceover loses focus when pressing the
PageDown/PageUp keys.

Fixes angular#24330.
@zarend zarend force-pushed the voiceover-pagedown branch from 353a900 to bd9d4e1 Compare March 4, 2022 18:59
@zarend zarend marked this pull request as ready for review March 4, 2022 19:08
@zarend zarend requested review from jelbourn and crisbeto March 4, 2022 19:08
@zarend
Copy link
Contributor Author

zarend commented Mar 4, 2022

@jelbourn and @crisbeto , I address the comments and cleaned up this PR. It's ready for your eyes again 👀 .

After further testing, I determined 0ms would be best for the timeout duration since longer could interfere with repeated keys and cause the entire page to scrolling while pressing and holding pagedown.

I added a code comment with more details on my testing methodology and how I determined 0ms would be best. The code comment is there so we can consider why it was set to 0ms if we ever want to change the timeout value later.

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

@zarend zarend added the action: merge The PR is ready for merge by the caretaker label Mar 7, 2022
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 6b4f2bf into angular:master Mar 14, 2022
zarend added a commit that referenced this pull request Mar 14, 2022
)

Add a timeout before programatically focusing cells on the calendar. This seems
to fix an issue where Voiceover loses focus when pressing the
PageDown/PageUp keys.

Fixes #24330.

(cherry picked from commit 6b4f2bf)
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request Mar 23, 2022
This PR contains the following updates:

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

---

### Release Notes

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

### [`v13.3.0`](https://github.com/angular/components/blob/HEAD/CHANGELOG.md#&#8203;1330-aluminum-armadillo-2022-03-16)

[Compare Source](angular/components@13.2.6...13.3.0)

##### cdk

| Commit | Type | Description |
| -- | -- | -- |
| [e4c64dd56](angular/components@e4c64dd) | fix | **drag-drop:** only block dragstart event on event targets ([#&#8203;24581](angular/components#24581)) |

##### material

| Commit | Type | Description |
| -- | -- | -- |
| [33d07df95](angular/components@33d07df) | fix | **badge:** ensure overflow visible ([#&#8203;24602](angular/components#24602)) |
| [dfef17351](angular/components@dfef173) | fix | **datepicker:** fix Voiceover losing focus on PageDown ([#&#8203;24399](angular/components#24399)) |
| [1703b83ae](angular/components@1703b83) | fix | **datepicker:** use cdk-visually-hidden on calendar header ([#&#8203;24523](angular/components#24523)) |
| [41320d07e](angular/components@41320d0) | fix | **tabs:** avoid timeouts in background tabs ([#&#8203;24000](angular/components#24000)) |

##### material-experimental

| Commit | Type | Description |
| -- | -- | -- |
| [097ec0d11](angular/components@097ec0d) | fix | **mdc-core:** add app background color ([#&#8203;22992](angular/components#22992)) |
| [15a0676d5](angular/components@15a0676) | fix | **mdc-radio:** add hover indication ([#&#8203;24595](angular/components#24595)) |

#### Special Thanks

Andrew Seguin, Kristiyan Kostadinov, Paul Gschwendtner and Zach Arend

<!-- 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/1229
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>
forsti0506 pushed a commit to forsti0506/components that referenced this pull request Apr 3, 2022
…ular#24399)

Add a timeout before programatically focusing cells on the calendar. This seems
to fix an issue where Voiceover loses focus when pressing the
PageDown/PageUp keys.

Fixes angular#24330.
@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 Apr 14, 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 target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants