From c7979009a3259e5af5de5c8642f3b3e92600a53a Mon Sep 17 00:00:00 2001
From: crisbeto <crisbeto@abv.bg>
Date: Thu, 8 Nov 2018 11:12:14 +0000
Subject: [PATCH] fix(menu): focus lost if active item is removed

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

Fixes #14028.
---
 src/lib/menu/menu-directive.ts | 29 +++++++++++++++++++++++---
 src/lib/menu/menu.spec.ts      | 37 ++++++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+), 3 deletions(-)

diff --git a/src/lib/menu/menu-directive.ts b/src/lib/menu/menu-directive.ts
index 2798b138d93d..5eccf440e8f1 100644
--- a/src/lib/menu/menu-directive.ts
+++ b/src/lib/menu/menu-directive.ts
@@ -30,8 +30,8 @@ import {
   ViewEncapsulation,
   OnInit,
 } from '@angular/core';
-import {merge, Observable, Subject, Subscription} from 'rxjs';
-import {startWith, switchMap, take} from 'rxjs/operators';
+import {merge, Observable, Subject, Subscription, asapScheduler} from 'rxjs';
+import {startWith, switchMap, take, debounceTime} from 'rxjs/operators';
 import {matMenuAnimations} from './menu-animations';
 import {MatMenuContent} from './menu-content';
 import {throwMatMenuInvalidPositionX, throwMatMenuInvalidPositionY} from './menu-errors';
@@ -240,11 +240,34 @@ export class MatMenu implements AfterContentInit, MatMenuPanel<MatMenuItem>, OnI
   ngAfterContentInit() {
     this._keyManager = new FocusKeyManager<MatMenuItem>(this._items).withWrap().withTypeAhead();
     this._tabSubscription = this._keyManager.tabOut.subscribe(() => this.closed.emit('tab'));
+
+    // TODO(crisbeto): the `debounce` here should be removed since it's something
+    // that people have to flush in their tests. It'll be possible once we switch back
+    // to using a QueryList in #11720.
+    this._ngZone.runOutsideAngular(() => {
+      // 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.
+      this._itemChanges.pipe(debounceTime(50)).subscribe(items => {
+        const manager = this._keyManager;
+
+        if (manager.activeItem && items.indexOf(manager.activeItem) === -1) {
+          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() {
     this._tabSubscription.unsubscribe();
     this.closed.complete();
+    this._itemChanges.complete();
   }
 
   /** Stream that emits whenever the hovered menu item changes. */
@@ -347,7 +370,7 @@ export class MatMenu implements AfterContentInit, MatMenuPanel<MatMenuItem>, OnI
   removeItem(item: MatMenuItem) {
     const index = this._items.indexOf(item);
 
-    if (this._items.indexOf(item) > -1) {
+    if (index > -1) {
       this._items.splice(index, 1);
       this._itemChanges.next(this._items);
     }
diff --git a/src/lib/menu/menu.spec.ts b/src/lib/menu/menu.spec.ts
index 87c25446cbee..dcfef3e3cc4f 100644
--- a/src/lib/menu/menu.spec.ts
+++ b/src/lib/menu/menu.spec.ts
@@ -132,6 +132,26 @@ describe('MatMenu', () => {
     expect(document.activeElement).toBe(triggerEl);
   }));
 
+  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]);
 
@@ -189,6 +209,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');
@@ -2100,3 +2121,19 @@ class DynamicPanelMenu {
   @ViewChild('one') firstMenu: MatMenu;
   @ViewChild('two') secondMenu: MatMenu;
 }
+
+
+@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) trigger: MatMenuTrigger;
+  @ViewChild('triggerEl') triggerEl: ElementRef<HTMLElement>;
+  @ViewChild(MatMenu) menu: MatMenu;
+  items = ['One', 'Two', 'Three'];
+}