fix(material/menu): nested menu trigger not restoring focus (#21531)

Fixes that navigating back from a nested menu using the keyboard doesn't restore focus
to the previous trigger, breaking the interaction since it ends up going back to the `body`.
This used to work, but seems to have regressed at some point.

Author: crisbeto

src/material-experimental/mdc-menu/menu.spec.ts

@@ -1883,6 +1883,24 @@ describe('MDC-based MatMenu', () => {
           .toBe(true, 'Expected focus to be back inside the root menu');
     });
 
+    it('should restore focus to a nested trigger when navgating via the keyboard', fakeAsync(() => {
+      compileTestComponent();
+      instance.rootTriggerEl.nativeElement.click();
+      fixture.detectChanges();
+
+      const levelOneTrigger = overlay.querySelector('#level-one-trigger')! as HTMLElement;
+      dispatchKeyboardEvent(levelOneTrigger, 'keydown', RIGHT_ARROW);
+      fixture.detectChanges();
+
+      const spy = spyOn(levelOneTrigger, 'focus').and.callThrough();
+      dispatchKeyboardEvent(
+        overlay.querySelectorAll('.mat-mdc-menu-panel')[1], 'keydown', LEFT_ARROW);
+      fixture.detectChanges();
+      tick(500);
+
+      expect(spy).toHaveBeenCalled();
+    }));
+
     it('should position the sub-menu to the right edge of the trigger in ltr', () => {
       compileTestComponent();
       instance.rootTriggerEl.nativeElement.style.position = 'fixed';

src/material/menu/menu-trigger.ts

@@ -36,7 +36,7 @@ import {
 import {normalizePassiveListenerOptions} from '@angular/cdk/platform';
 import {asapScheduler, merge, of as observableOf, Subscription} from 'rxjs';
 import {delay, filter, take, takeUntil} from 'rxjs/operators';
-import {_MatMenuBase} from './menu';
+import {MenuCloseReason, _MatMenuBase} from './menu';
 import {throwMatMenuMissingError, throwMatMenuRecursiveError} from './menu-errors';
 import {MatMenuItem} from './menu-item';
 import {MatMenuPanel, MAT_MENU_PANEL} from './menu-panel';
@@ -103,7 +103,7 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
 
   // Tracking input type is necessary so it's possible to only auto-focus
   // the first item of the list when the menu is opened via the keyboard
-  _openedBy: Exclude<FocusOrigin, 'program'> = null;
+  _openedBy: Exclude<FocusOrigin, 'program' | null> | undefined = undefined;
 
   /**
    * @deprecated
@@ -131,15 +131,14 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
         throwMatMenuRecursiveError();
       }
 
-      this._menuCloseSubscription = menu.close.subscribe(
-        (reason: 'click' | 'tab' | 'keydown' | undefined) => {
-          this._destroyMenu();
+      this._menuCloseSubscription = menu.close.subscribe((reason: MenuCloseReason) => {
+        this._destroyMenu(reason);
 
-          // If a click closed the menu, we should close the entire chain of nested menus.
-          if ((reason === 'click' || reason === 'tab') && this._parentMaterialMenu) {
-            this._parentMaterialMenu.closed.emit(reason);
-          }
-        });
+        // If a click closed the menu, we should close the entire chain of nested menus.
+        if ((reason === 'click' || reason === 'tab') && this._parentMaterialMenu) {
+          this._parentMaterialMenu.closed.emit(reason);
+        }
+      });
     }
   }
   private _menu: MatMenuPanel;
@@ -284,15 +283,24 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
   }
 
   /** Closes the menu and does the necessary cleanup. */
-  private _destroyMenu() {
+  private _destroyMenu(reason: MenuCloseReason) {
     if (!this._overlayRef || !this.menuOpen) {
       return;
     }
 
     const menu = this.menu;
     this._closingActionsSubscription.unsubscribe();
     this._overlayRef.detach();
-    this._restoreFocus();
+
+    // Always restore focus if the user is navigating using the keyboard or the menu was opened
+    // programmatically. We don't restore for non-root triggers, because it can prevent focus
+    // from making it back to the root trigger when closing a long chain of menus by clicking
+    // on the backdrop.
+    if (this.restoreFocus && (reason === 'keydown' || !this._openedBy || !this.triggersSubmenu())) {
+      this.focus(this._openedBy);
+    }
+
+    this._openedBy = undefined;
 
     if (menu instanceof _MatMenuBase) {
       menu._resetAnimation();
@@ -350,24 +358,6 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
     }
   }
 
-  /** Restores focus to the element that was focused before the menu was open. */
-  private _restoreFocus() {
-    // We should reset focus if the user is navigating using a keyboard or
-    // if we have a top-level trigger which might cause focus to be lost
-    // when clicking on the backdrop.
-    if (this.restoreFocus) {
-      if (!this._openedBy) {
-        // Note that the focus style will show up both for `program` and
-        // `keyboard` so we don't have to specify which one it is.
-        this.focus();
-      } else if (!this.triggersSubmenu()) {
-        this.focus(this._openedBy);
-      }
-    }
-
-    this._openedBy = null;
-  }
-
   // set state rather than toggle to support triggers sharing a menu
   private _setIsMenuOpen(isOpen: boolean): void {
     this._menuOpen = isOpen;
@@ -506,7 +496,7 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
     if (!isFakeMousedownFromScreenReader(event)) {
       // Since right or middle button clicks won't trigger the `click` event,
       // we shouldn't consider the menu as opened by mouse in those cases.
-      this._openedBy = event.button === 0 ? 'mouse' : null;
+      this._openedBy = event.button === 0 ? 'mouse' : undefined;
 
       // Since clicking on the trigger won't close the menu if it opens a sub-menu,
       // we should prevent focus from moving onto it via click to avoid the

src/material/menu/menu.spec.ts

@@ -1894,6 +1894,23 @@ describe('MatMenu', () => {
           .toBe(true, 'Expected focus to be back inside the root menu');
     });
 
+    it('should restore focus to a nested trigger when navgating via the keyboard', fakeAsync(() => {
+      compileTestComponent();
+      instance.rootTriggerEl.nativeElement.click();
+      fixture.detectChanges();
+
+      const levelOneTrigger = overlay.querySelector('#level-one-trigger')! as HTMLElement;
+      dispatchKeyboardEvent(levelOneTrigger, 'keydown', RIGHT_ARROW);
+      fixture.detectChanges();
+
+      const spy = spyOn(levelOneTrigger, 'focus').and.callThrough();
+      dispatchKeyboardEvent(overlay.querySelectorAll('.mat-menu-panel')[1], 'keydown', LEFT_ARROW);
+      fixture.detectChanges();
+      tick(500);
+
+      expect(spy).toHaveBeenCalled();
+    }));
+
     it('should position the sub-menu to the right edge of the trigger in ltr', () => {
       compileTestComponent();
       instance.rootTriggerEl.nativeElement.style.position = 'fixed';

src/material/menu/menu.ts

@@ -93,6 +93,11 @@ const MAT_MENU_BASE_ELEVATION = 4;
 
 let menuPanelUid = 0;
 
+
+/** Reason why the menu was closed. */
+export type MenuCloseReason = void | 'click' | 'keydown' | 'tab';
+
+
 /** Base class with all of the `MatMenu` functionality. */
 @Directive()
 export class _MatMenuBase implements AfterContentInit, MatMenuPanel<MatMenuItem>, OnInit,
@@ -239,15 +244,14 @@ export class _MatMenuBase implements AfterContentInit, MatMenuPanel<MatMenuItem>
   set classList(classes: string) { this.panelClass = classes; }
 
   /** Event emitted when the menu is closed. */
-  @Output() readonly closed: EventEmitter<void | 'click' | 'keydown' | 'tab'> =
-      new EventEmitter<void | 'click' | 'keydown' | 'tab'>();
+  @Output() readonly closed: EventEmitter<MenuCloseReason> = new EventEmitter<MenuCloseReason>();
 
   /**
    * Event emitted when the menu is closed.
    * @deprecated Switch to `closed` instead
    * @breaking-change 8.0.0
    */
-  @Output() close: EventEmitter<void | 'click' | 'keydown' | 'tab'> = this.closed;
+  @Output() close: EventEmitter<MenuCloseReason> = this.closed;
 
   readonly panelId = `mat-menu-panel-${menuPanelUid++}`;
 

tools/public_api_guard/material/menu.d.ts

@@ -12,8 +12,8 @@ export declare class _MatMenuBase implements AfterContentInit, MatMenuPanel<MatM
     backdropClass: string;
     get classList(): string;
     set classList(classes: string);
-    close: EventEmitter<void | 'click' | 'keydown' | 'tab'>;
-    readonly closed: EventEmitter<void | 'click' | 'keydown' | 'tab'>;
+    close: EventEmitter<MenuCloseReason>;
+    readonly closed: EventEmitter<MenuCloseReason>;
     direction: Direction;
     get hasBackdrop(): boolean | undefined;
     set hasBackdrop(value: boolean | undefined);
@@ -149,7 +149,7 @@ export interface MatMenuPanel<T = any> {
 export declare class MatMenuTrigger implements AfterContentInit, OnDestroy {
     get _deprecatedMatMenuTriggerFor(): MatMenuPanel;
     set _deprecatedMatMenuTriggerFor(v: MatMenuPanel);
-    _openedBy: Exclude<FocusOrigin, 'program'>;
+    _openedBy: Exclude<FocusOrigin, 'program' | null> | undefined;
     get dir(): Direction;
     get menu(): MatMenuPanel;
     set menu(menu: MatMenuPanel);