From bb553ae17af2bde2d71eae2cac824e7851ee4fc7 Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Wed, 28 Feb 2024 23:12:02 +0000 Subject: [PATCH] refactor(material/menu): Remove use of zone onStable to focus items --- src/dev-app/menu/menu-demo.html | 4 +- src/material/menu/menu.ts | 71 +++++++++++++++++++-------------- 2 files changed, 44 insertions(+), 31 deletions(-) diff --git a/src/dev-app/menu/menu-demo.html b/src/dev-app/menu/menu-demo.html index 69338bb190e8..eaa55e4366b0 100644 --- a/src/dev-app/menu/menu-demo.html +++ b/src/dev-app/menu/menu-demo.html @@ -15,7 +15,7 @@ } - +
This div is for testing scrolled menus.
diff --git a/src/material/menu/menu.ts b/src/material/menu/menu.ts index 943552867401..666226b7ddb2 100644 --- a/src/material/menu/menu.ts +++ b/src/material/menu/menu.ts @@ -27,6 +27,10 @@ import { OnInit, ChangeDetectorRef, booleanAttribute, + afterNextRender, + AfterRenderRef, + inject, + Injector, } from '@angular/core'; import {AnimationEvent} from '@angular/animations'; import {FocusKeyManager, FocusOrigin} from '@angular/cdk/a11y'; @@ -39,8 +43,8 @@ import { UP_ARROW, hasModifierKey, } from '@angular/cdk/keycodes'; -import {merge, Observable, Subject, Subscription} from 'rxjs'; -import {startWith, switchMap, take} from 'rxjs/operators'; +import {merge, Observable, Subject} from 'rxjs'; +import {startWith, switchMap} from 'rxjs/operators'; import {MatMenuItem} from './menu-item'; import {MatMenuPanel, MAT_MENU_PANEL} from './menu-panel'; import {MenuPositionX, MenuPositionY} from './menu-positions'; @@ -115,7 +119,7 @@ export class MatMenu implements AfterContentInit, MatMenuPanel, OnI private _keyManager: FocusKeyManager; private _xPosition: MenuPositionX; private _yPosition: MenuPositionY; - private _firstItemFocusSubscription?: Subscription; + private _firstItemFocusRef?: AfterRenderRef; private _previousElevation: string; private _elevationPrefix = 'mat-elevation-z'; private _baseElevation = 8; @@ -267,6 +271,8 @@ export class MatMenu implements AfterContentInit, MatMenuPanel, OnI readonly panelId = `mat-menu-panel-${menuPanelUid++}`; + private _injector = inject(Injector); + constructor( elementRef: ElementRef, ngZone: NgZone, @@ -287,7 +293,11 @@ export class MatMenu implements AfterContentInit, MatMenuPanel, OnI constructor( private _elementRef: ElementRef, - private _ngZone: NgZone, + /** + * @deprecated Unused param, will be removed. + * @breaking-change 19.0.0 + */ + _unusedNgZone: NgZone, @Inject(MAT_MENU_DEFAULT_OPTIONS) defaultOptions: MatMenuDefaultOptions, // @breaking-change 15.0.0 `_changeDetectorRef` to become a required parameter. private _changeDetectorRef?: ChangeDetectorRef, @@ -345,7 +355,7 @@ export class MatMenu implements AfterContentInit, MatMenuPanel, OnI this._keyManager?.destroy(); this._directDescendantItems.destroy(); this.closed.complete(); - this._firstItemFocusSubscription?.unsubscribe(); + this._firstItemFocusRef?.destroy(); } /** Stream that emits whenever the hovered menu item changes. */ @@ -415,32 +425,35 @@ export class MatMenu implements AfterContentInit, MatMenuPanel, OnI * @param origin Action from which the focus originated. Used to set the correct styling. */ focusFirstItem(origin: FocusOrigin = 'program'): void { - // Wait for `onStable` to ensure iOS VoiceOver screen reader focuses the first item (#24735). - this._firstItemFocusSubscription?.unsubscribe(); - this._firstItemFocusSubscription = this._ngZone.onStable.pipe(take(1)).subscribe(() => { - let menuPanel: HTMLElement | null = null; - - if (this._directDescendantItems.length) { - // Because the `mat-menuPanel` is at the DOM insertion point, not inside the overlay, we don't - // have a nice way of getting a hold of the menuPanel panel. We can't use a `ViewChild` either - // because the panel is inside an `ng-template`. We work around it by starting from one of - // the items and walking up the DOM. - menuPanel = this._directDescendantItems.first!._getHostElement().closest('[role="menu"]'); - } - - // If an item in the menuPanel is already focused, avoid overriding the focus. - if (!menuPanel || !menuPanel.contains(document.activeElement)) { - const manager = this._keyManager; - manager.setFocusOrigin(origin).setFirstItemActive(); + // Wait for `afterNextRender` to ensure iOS VoiceOver screen reader focuses the first item (#24735). + this._firstItemFocusRef?.destroy(); + this._firstItemFocusRef = afterNextRender( + () => { + let menuPanel: HTMLElement | null = null; + + if (this._directDescendantItems.length) { + // Because the `mat-menuPanel` is at the DOM insertion point, not inside the overlay, we don't + // have a nice way of getting a hold of the menuPanel panel. We can't use a `ViewChild` either + // because the panel is inside an `ng-template`. We work around it by starting from one of + // the items and walking up the DOM. + menuPanel = this._directDescendantItems.first!._getHostElement().closest('[role="menu"]'); + } - // If there's no active item at this point, it means that all the items are disabled. - // Move focus to the menuPanel panel so keyboard events like Escape still work. Also this will - // give _some_ feedback to screen readers. - if (!manager.activeItem && menuPanel) { - menuPanel.focus(); + // If an item in the menuPanel is already focused, avoid overriding the focus. + if (!menuPanel || !menuPanel.contains(document.activeElement)) { + const manager = this._keyManager; + manager.setFocusOrigin(origin).setFirstItemActive(); + + // If there's no active item at this point, it means that all the items are disabled. + // Move focus to the menuPanel panel so keyboard events like Escape still work. Also this will + // give _some_ feedback to screen readers. + if (!manager.activeItem && menuPanel) { + menuPanel.focus(); + } } - } - }); + }, + {injector: this._injector}, + ); } /**