fix(material/menu): focus lost if active item is removed (#14039)

Fixes the user's focus being lost if the active item is destroyed while a menu is open.

Fixes #14028.

Author: crisbeto

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

@@ -259,6 +259,28 @@ describe('MDC-based MatMenu', () => {
     tick(500);
   }));
 
+  it('should move focus to another item if the active item is destroyed', fakeAsync(() => {
+    const fixture = createComponent(MenuWithRepeatedItems, [], [FakeIcon]);
+    fixture.detectChanges();
+    const triggerEl = fixture.componentInstance.triggerEl.nativeElement;
+
+    triggerEl.click();
+    fixture.detectChanges();
+    tick(500);
+
+    const items = overlayContainerElement.querySelectorAll(
+      '.mat-mdc-menu-panel .mat-mdc-menu-item',
+    );
+
+    expect(document.activeElement).toBe(items[0]);
+
+    fixture.componentInstance.items.shift();
+    fixture.detectChanges();
+    tick(500);
+
+    expect(document.activeElement).toBe(items[1]);
+  }));
+
   it('should be able to set a custom class on the backdrop', fakeAsync(() => {
     const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
 
@@ -3091,3 +3113,18 @@ class StaticAriaLabelledByMenu {}
   template: '<mat-menu aria-describedby="some-element"></mat-menu>',
 })
 class StaticAriaDescribedbyMenu {}
+
+@Component({
+  template: `
+    <button [matMenuTriggerFor]="menu" #triggerEl>Toggle menu</button>
+    <mat-menu #menu="matMenu">
+      <button *ngFor="let item of items" mat-menu-item>{{item}}</button>
+    </mat-menu>
+  `,
+})
+class MenuWithRepeatedItems {
+  @ViewChild(MatMenuTrigger, {static: false}) trigger: MatMenuTrigger;
+  @ViewChild('triggerEl', {static: false}) triggerEl: ElementRef<HTMLElement>;
+  @ViewChild(MatMenu, {static: false}) menu: MatMenu;
+  items = ['One', 'Two', 'Three'];
+}

src/material/menu/menu-item.ts

@@ -76,8 +76,7 @@ export class MatMenuItem
   _triggersSubmenu: boolean = false;
 
   /**
-   * @deprecated `document` parameter to be removed, `changeDetectorRef` and
-   * `focusMonitor` to become required.
+   * @deprecated `_document`, `changeDetectorRef` and `focusMonitor` to become required.
    * @breaking-change 12.0.0
    */
   constructor(
@@ -90,7 +89,7 @@ export class MatMenuItem
 
   constructor(
     private _elementRef: ElementRef<HTMLElement>,
-    @Inject(DOCUMENT) _document?: any,
+    @Inject(DOCUMENT) private _document?: any,
     private _focusMonitor?: FocusMonitor,
     @Inject(MAT_MENU_PANEL) @Optional() public _parentMenu?: MatMenuPanel<MatMenuItem>,
     private _changeDetectorRef?: ChangeDetectorRef,
@@ -176,4 +175,8 @@ export class MatMenuItem
     this._highlighted = isHighlighted;
     this._changeDetectorRef?.markForCheck();
   }
+
+  _hasFocus(): boolean {
+    return this._document && this._document.activeElement === this._getHostElement();
+  }
 }

src/material/menu/menu.spec.ts

@@ -261,6 +261,26 @@ describe('MatMenu', () => {
     tick(500);
   }));
 
+  it('should move focus to another item if the active item is destroyed', fakeAsync(() => {
+    const fixture = createComponent(MenuWithRepeatedItems, [], [FakeIcon]);
+    fixture.detectChanges();
+    const triggerEl = fixture.componentInstance.triggerEl.nativeElement;
+
+    triggerEl.click();
+    fixture.detectChanges();
+    tick(500);
+
+    const items = overlayContainerElement.querySelectorAll('.mat-menu-panel .mat-menu-item');
+
+    expect(document.activeElement).toBe(items[0]);
+
+    fixture.componentInstance.items.shift();
+    fixture.detectChanges();
+    tick(500);
+
+    expect(document.activeElement).toBe(items[1]);
+  }));
+
   it('should be able to set a custom class on the backdrop', fakeAsync(() => {
     const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
 
@@ -353,6 +373,7 @@ describe('MatMenu', () => {
     // Add 50 items to make the menu scrollable
     fixture.componentInstance.extraItems = new Array(50).fill('Hello there');
     fixture.detectChanges();
+    tick(50);
 
     const triggerEl = fixture.componentInstance.triggerEl.nativeElement;
     dispatchFakeEvent(triggerEl, 'mousedown');
@@ -3075,3 +3096,18 @@ class StaticAriaLabelledByMenu {}
   template: '<mat-menu aria-describedby="some-element"></mat-menu>',
 })
 class StaticAriaDescribedbyMenu {}
+
+@Component({
+  template: `
+    <button [matMenuTriggerFor]="menu" #triggerEl>Toggle menu</button>
+    <mat-menu #menu="matMenu">
+      <button *ngFor="let item of items" mat-menu-item>{{item}}</button>
+    </mat-menu>
+  `,
+})
+class MenuWithRepeatedItems {
+  @ViewChild(MatMenuTrigger, {static: false}) trigger: MatMenuTrigger;
+  @ViewChild('triggerEl', {static: false}) triggerEl: ElementRef<HTMLElement>;
+  @ViewChild(MatMenu, {static: false}) menu: MatMenu;
+  items = ['One', 'Two', 'Three'];
+}

src/material/menu/menu.ts

@@ -298,6 +298,24 @@ export class _MatMenuBase
         switchMap(items => merge(...items.map((item: MatMenuItem) => item._focused))),
       )
       .subscribe(focusedItem => this._keyManager.updateActiveItem(focusedItem as MatMenuItem));
+
+    this._directDescendantItems.changes.subscribe((itemsList: QueryList<MatMenuItem>) => {
+      // Move focus to another item, if the active item is removed from the list.
+      // We need to debounce the callback, because multiple items might be removed
+      // in quick succession.
+      const manager = this._keyManager;
+
+      if (this._panelAnimationState === 'enter' && manager.activeItem?._hasFocus()) {
+        const items = itemsList.toArray();
+        const index = Math.max(0, Math.min(items.length - 1, manager.activeItemIndex || 0));
+
+        if (items[index] && !items[index].disabled) {
+          manager.setActiveItem(index);
+        } else {
+          manager.setNextItemActive();
+        }
+      }
+    });
   }
 
   ngOnDestroy() {

tools/public_api_guard/material/menu.md

@@ -203,6 +203,8 @@ export class MatMenuItem extends _MatMenuItemBase implements FocusableOption, Ca
     getLabel(): string;
     _getTabIndex(): string;
     _handleMouseEnter(): void;
+    // (undocumented)
+    _hasFocus(): boolean;
     _highlighted: boolean;
     readonly _hovered: Subject<MatMenuItem>;
     // (undocumented)