refactor(material/menu): Remove use of zone onStable to focus items (#28663)

Author: mmalerba

src/material/menu/menu.spec.ts

@@ -1,55 +1,53 @@
-import {ComponentFixture, fakeAsync, flush, TestBed, tick} from '@angular/core/testing';
-import {By} from '@angular/platform-browser';
-import {NoopAnimationsModule} from '@angular/platform-browser/animations';
+import {FocusMonitor} from '@angular/cdk/a11y';
+import {Direction, Directionality} from '@angular/cdk/bidi';
+import {
+  DOWN_ARROW,
+  END,
+  ENTER,
+  ESCAPE,
+  HOME,
+  LEFT_ARROW,
+  RIGHT_ARROW,
+  TAB,
+} from '@angular/cdk/keycodes';
+import {Overlay, OverlayContainer} from '@angular/cdk/overlay';
+import {ScrollDispatcher, ViewportRuler} from '@angular/cdk/scrolling';
 import {
+  ChangeDetectionStrategy,
   Component,
   ElementRef,
   EventEmitter,
   Input,
   Output,
-  NgZone,
+  Provider,
+  QueryList,
   TemplateRef,
+  Type,
   ViewChild,
   ViewChildren,
-  QueryList,
-  Type,
-  Provider,
-  ChangeDetectionStrategy,
 } from '@angular/core';
-import {Direction, Directionality} from '@angular/cdk/bidi';
-import {OverlayContainer, Overlay} from '@angular/cdk/overlay';
-import {
-  ESCAPE,
-  LEFT_ARROW,
-  RIGHT_ARROW,
-  DOWN_ARROW,
-  TAB,
-  HOME,
-  END,
-  ENTER,
-} from '@angular/cdk/keycodes';
-import {MatMenu, MatMenuModule, MatMenuItem} from './index';
+import {ComponentFixture, fakeAsync, flush, TestBed, tick} from '@angular/core/testing';
 import {MatRipple} from '@angular/material/core';
+import {By} from '@angular/platform-browser';
+import {NoopAnimationsModule} from '@angular/platform-browser/animations';
+import {Subject} from 'rxjs';
 import {
-  dispatchKeyboardEvent,
-  dispatchMouseEvent,
-  dispatchEvent,
   createKeyboardEvent,
   createMouseEvent,
+  dispatchEvent,
   dispatchFakeEvent,
+  dispatchKeyboardEvent,
+  dispatchMouseEvent,
   patchElementFocus,
-  MockNgZone,
 } from '../../cdk/testing/private';
-import {Subject} from 'rxjs';
-import {ScrollDispatcher, ViewportRuler} from '@angular/cdk/scrolling';
-import {FocusMonitor} from '@angular/cdk/a11y';
+import {MatMenu, MatMenuItem, MatMenuModule} from './index';
 import {
+  MAT_MENU_DEFAULT_OPTIONS,
   MAT_MENU_SCROLL_STRATEGY,
+  MatMenuPanel,
   MatMenuTrigger,
   MenuPositionX,
   MenuPositionY,
-  MatMenuPanel,
-  MAT_MENU_DEFAULT_OPTIONS,
 } from './public-api';
 
 const MENU_PANEL_TOP_PADDING = 8;
@@ -772,27 +770,14 @@ describe('MDC-based MatMenu', () => {
   }));
 
   it('should set the proper origin when calling focusFirstItem after the opening sequence has started', () => {
-    let zone: MockNgZone;
-    const fixture = createComponent(
-      SimpleMenu,
-      [
-        {
-          provide: NgZone,
-          useFactory: () => (zone = new MockNgZone()),
-        },
-      ],
-      [FakeIcon],
-    );
+    const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
     fixture.detectChanges();
     spyOn(fixture.componentInstance.items.first, 'focus').and.callThrough();
 
-    const triggerEl = fixture.componentInstance.triggerEl.nativeElement;
-
-    dispatchMouseEvent(triggerEl, 'mousedown');
-    triggerEl.click();
-    fixture.detectChanges();
+    fixture.componentInstance.trigger.openMenu();
+    fixture.componentInstance.menu.focusFirstItem('mouse');
     fixture.componentInstance.menu.focusFirstItem('touch');
-    zone!.onStable.next();
+    fixture.detectChanges();
 
     expect(fixture.componentInstance.items.first.focus).toHaveBeenCalledOnceWith('touch');
   });
@@ -1304,30 +1289,18 @@ describe('MDC-based MatMenu', () => {
       expect(trigger.menuOpen).withContext('Expected menu to be closed').toBe(false);
     }));
 
-    it('should focus the first menu item when opening a lazy menu via keyboard', fakeAsync(() => {
-      let zone: MockNgZone;
-      let fixture = createComponent(SimpleLazyMenu, [
-        {
-          provide: NgZone,
-          useFactory: () => (zone = new MockNgZone()),
-        },
-      ]);
-
-      fixture.detectChanges();
+    it('should focus the first menu item when opening a lazy menu via keyboard', async () => {
+      const fixture = createComponent(SimpleLazyMenu);
+      fixture.autoDetectChanges();
 
       // A click without a mousedown before it is considered a keyboard open.
       fixture.componentInstance.triggerEl.nativeElement.click();
-      fixture.detectChanges();
-      tick(500);
-      zone!.simulateZoneExit();
-
-      // Flush due to the additional tick that is necessary for the FocusMonitor.
-      flush();
+      await fixture.whenStable();
 
       const item = document.querySelector('.mat-mdc-menu-panel [mat-menu-item]')!;
 
       expect(document.activeElement).withContext('Expected first item to be focused').toBe(item);
-    }));
+    });
 
     it('should be able to open the same menu with a different context', fakeAsync(() => {
       const fixture = createComponent(LazyMenuWithContext);

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<MatMenuItem>, OnI
   private _keyManager: FocusKeyManager<MatMenuItem>;
   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<MatMenuItem>, OnI
 
   readonly panelId = `mat-menu-panel-${menuPanelUid++}`;
 
+  private _injector = inject(Injector);
+
   constructor(
     elementRef: ElementRef<HTMLElement>,
     ngZone: NgZone,
@@ -287,7 +293,11 @@ export class MatMenu implements AfterContentInit, MatMenuPanel<MatMenuItem>, OnI
 
   constructor(
     private _elementRef: ElementRef<HTMLElement>,
-    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<MatMenuItem>, 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<MatMenuItem>, 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},
+    );
   }
 
   /**