fix(material/menu): focus the first item when opening menu on iOS VoiceOver #24733

zarend · 2022-04-04T20:57:17Z

When opening the menu using the iOS VoiceOver screen reader, focus the first item in the menu.
Previously, the first menu item would focus on other screen readers like desktop VoiceOver but
not with iOS VoiceOver.

Waiting until onStable seems to fix this.

Fixes #24735

github-actions · 2022-04-04T21:04:26Z

Deployed dev-app to: https://ng-comp-dev--pr-24733-3df39ff52d92ef2afd89186d51460fa6-how8d5o9.web.app

crisbeto

LGTM

zarend · 2022-04-19T23:19:52Z

Hmm, it seems like this doesn't work since the menu emits the "opened" event before calling .focus() on the first menu item.

I'll look into another solution for this and send out a new PR when I have it working.

zarend · 2022-04-28T01:41:31Z

Hmm, it seems like this doesn't work since the menu emits the "opened" event before calling .focus() on the first menu item.

I'll look into another solution for this and send out a new PR when I have it working.

Re-opening because because there is no requirement when focus is called, just that it has to be called some time after openMenu is called.

zarend · 2022-04-28T01:52:42Z

src/material/menu/menu.spec.ts

@@ -1147,6 +1147,7 @@ describe('MatMenu', () => {
    fixture.detectChanges();

    fixture.componentInstance.trigger.menuOpened.subscribe(() => {
+      flush();


@crisbeto I'm confused about this test. I understand that for the openMenu method, there is no requirement on when the first element of the list is focused. It has to be focused in some time after calling openMenu.

For the menuOpened event, is there a requirement that the first element must be focused by the time the menuOpened event fires?

To avoid a race condition with the _focusFirstItem method, this test adds a flush. In application code, the developer would have to wait for onStable or do a setTimeout. There is still a way for developers to move focus inside the open event callback, but not at the time the open event is fired.

Does this PR meet the requirements for the behavior of menuOpened?

This is checking that we don't override the consumer's focus, if they moved it into the menu themselves before we tried to focus the first item. That's why I was proposing the if (!menu.contains(document.activeElement) check.

zarend · 2022-04-28T23:17:27Z

Hi @crisbeto , I fixed this PR to use the !menu.contains(document.activeElement), and it's ready for your eyes again.

crisbeto

LGTM

src/material/menu/menu.ts

…ceOver When opening the menu using the iOS VoiceOver screen reader, focus the first item in the menu. Previously, the first menu item would focus on other screen readers like desktop VoiceOver but not with iOS VoiceOver. Waiting until `onStable` seems to fix this. Fixes angular#24735

zarend · 2022-05-06T00:13:39Z

caretaker: please approve this again. Since @crisbeto approved it, I made the fix for this comment.

You can get rid of this whole loop with the following:
menuPanel = this._directDescendantItems.first!._getHostElement().closest('[role="menu"]');

…ceOver (#24733) When opening the menu using the iOS VoiceOver screen reader, focus the first item in the menu. Previously, the first menu item would focus on other screen readers like desktop VoiceOver but not with iOS VoiceOver. Waiting until `onStable` seems to fix this. Fixes #24735 (cherry picked from commit f76103d)

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@angular/cdk](https://github.com/angular/components) | dependencies | patch | [`13.3.6` -> `13.3.7`](https://renovatebot.com/diffs/npm/@angular%2fcdk/13.3.6/13.3.7) | | [@angular/material](https://github.com/angular/components) | dependencies | patch | [`13.3.6` -> `13.3.7`](https://renovatebot.com/diffs/npm/@angular%2fmaterial/13.3.6/13.3.7) | --- ### Release Notes <details> <summary>angular/components</summary> ### [`v13.3.7`](https://github.com/angular/components/blob/HEAD/CHANGELOG.md#1337-chiffon-carambola-2022-05-11) [Compare Source](angular/components@13.3.6...13.3.7) ##### material | Commit | Type | Description | | -- | -- | -- | | [0bede63d33](angular/components@0bede63) | fix | **datepicker:** add ability to have numeric zero value in input ([#24813](angular/components#24813)) | | [7a122f7f03](angular/components@7a122f7) | fix | **expansion:** inconsistent spacing for anchor buttons ([#24882](angular/components#24882)) | | [e486ed93e4](angular/components@e486ed9) | fix | **menu:** focus the first item when opening menu on iOS VoiceOver ([#24733](angular/components#24733)) | #### Special Thanks Dmytro Prokhorov, Kristiyan Kostadinov and Zach Arend  </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. --- - [ ] 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/1349 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>

angular-automatic-lock-bot · 2022-06-06T00:03:47Z

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 added Accessibility This issue is related to accessibility (a11y) area: material/menu dev-app preview When applied, previews of the dev-app are deployed to Firebase labels Apr 4, 2022

zarend added the target: patch This PR is targeted for the next patch release label Apr 4, 2022

zarend force-pushed the menu-ios-voiceover branch 2 times, most recently from 06fa24d to 2ce0153 Compare April 4, 2022 23:44

zarend changed the title ~~fix(material/menu): debugging focusing the first item with iOS Voiceover~~ fix(material/menu): focus the first item when opening menu on iOS VoiceOver Apr 4, 2022

zarend force-pushed the menu-ios-voiceover branch 4 times, most recently from f1a439c to 4b508e7 Compare April 13, 2022 21:19

zarend marked this pull request as ready for review April 13, 2022 21:21

zarend requested a review from crisbeto as a code owner April 13, 2022 21:21

zarend force-pushed the menu-ios-voiceover branch from 4b508e7 to 0396c73 Compare April 13, 2022 21:24

crisbeto previously approved these changes Apr 16, 2022

View reviewed changes

crisbeto added the action: merge The PR is ready for merge by the caretaker label Apr 16, 2022

zarend closed this Apr 19, 2022

zarend reopened this Apr 28, 2022

zarend commented Apr 28, 2022

View reviewed changes

zarend force-pushed the menu-ios-voiceover branch from 0396c73 to 708cb12 Compare April 28, 2022 19:59

zarend requested a review from crisbeto April 28, 2022 23:16

andrewseguin dismissed crisbeto’s stale review via 708cb12 May 2, 2022 20:32

zarend removed the action: merge The PR is ready for merge by the caretaker label May 3, 2022

crisbeto previously approved these changes May 3, 2022

View reviewed changes

src/material/menu/menu.ts Outdated Show resolved Hide resolved

zarend dismissed crisbeto’s stale review via 3df39ff May 3, 2022 17:55

zarend force-pushed the menu-ios-voiceover branch from 708cb12 to 3df39ff Compare May 3, 2022 17:55

zarend added the action: merge The PR is ready for merge by the caretaker label May 4, 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 May 6, 2022

andrewseguin approved these changes May 6, 2022

View reviewed changes

zarend merged commit f76103d into angular:main May 6, 2022

sync-by-unito bot mentioned this pull request May 12, 2022

build(deps): bump @angular/cdk from 12.2.4 to 13.3.7 numbersprotocol/capture-lite#1610

Closed

zarend mentioned this pull request May 17, 2022

Bug: Mat-menu wont set focus to mat-menu-items when using Voice over #24883

Closed

1 task

This was referenced May 29, 2022

build(deps): bump @angular/cdk from 12.2.4 to 13.3.8 numbersprotocol/capture-lite#1650

Closed

build(deps): bump @angular/material from 12.2.4 to 13.3.8 numbersprotocol/capture-lite#1676

Closed

angular-automatic-lock-bot bot locked and limited conversation to collaborators Jun 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(material/menu): focus the first item when opening menu on iOS VoiceOver #24733

fix(material/menu): focus the first item when opening menu on iOS VoiceOver #24733

zarend commented Apr 4, 2022 •

edited

Loading

github-actions bot commented Apr 4, 2022 •

edited

Loading

crisbeto left a comment

zarend commented Apr 19, 2022

zarend commented Apr 28, 2022

zarend Apr 28, 2022

crisbeto Apr 28, 2022

zarend commented Apr 28, 2022

crisbeto left a comment

zarend commented May 6, 2022

angular-automatic-lock-bot bot commented Jun 6, 2022

fix(material/menu): focus the first item when opening menu on iOS VoiceOver #24733

fix(material/menu): focus the first item when opening menu on iOS VoiceOver #24733

Conversation

zarend commented Apr 4, 2022 • edited Loading

github-actions bot commented Apr 4, 2022 • edited Loading

crisbeto left a comment

Choose a reason for hiding this comment

zarend commented Apr 19, 2022

zarend commented Apr 28, 2022

zarend Apr 28, 2022

Choose a reason for hiding this comment

crisbeto Apr 28, 2022

Choose a reason for hiding this comment

zarend commented Apr 28, 2022

crisbeto left a comment

Choose a reason for hiding this comment

zarend commented May 6, 2022

angular-automatic-lock-bot bot commented Jun 6, 2022

zarend commented Apr 4, 2022 •

edited

Loading

github-actions bot commented Apr 4, 2022 •

edited

Loading