fix(menu): unable to open same sub-menu from different triggers and not picking up indirect descendant items

* Reworks the relationship between the menu items and the menu panel to allow for the items to be indirect descendants of the menu, while also allowing for `mat-menu` instances to be declared inside of other `mat-menu` instances without having the root `mat-menu` pick up all of the descendant items.
* Adds the ability to pass in an array to the `ListKeyManager`, in addition to a `QueryList`.
* Fixes not being able to re-use the same sub-menu between multiple sub-menu triggers.

Fixes #9969.
Fixes #9987.

Author: crisbeto

src/cdk/a11y/key-manager/list-key-manager.ts

@@ -52,17 +52,22 @@ export class ListKeyManager<T extends ListKeyManagerOption> {
   // Buffer for the letters that the user has pressed when the typeahead option is turned on.
   private _pressedLetters: string[] = [];
 
-  constructor(private _items: QueryList<T>) {
-    _items.changes.subscribe((newItems: QueryList<T>) => {
-      if (this._activeItem) {
-        const itemArray = newItems.toArray();
-        const newIndex = itemArray.indexOf(this._activeItem);
-
-        if (newIndex > -1 && newIndex !== this._activeItemIndex) {
-          this._activeItemIndex = newIndex;
+  constructor(private _items: QueryList<T> | T[]) {
+    // We allow for the items to be an array because, in some cases, the consumer may
+    // not have access to a QueryList of the items they want to manage (e.g. when the
+    // items aren't being collected via `ViewChildren` or `ContentChildren`).
+    if (_items instanceof QueryList) {
+      _items.changes.subscribe((newItems: QueryList<T>) => {
+        if (this._activeItem) {
+          const itemArray = newItems.toArray();
+          const newIndex = itemArray.indexOf(this._activeItem);
+
+          if (newIndex > -1 && newIndex !== this._activeItemIndex) {
+            this._activeItemIndex = newIndex;
+          }
         }
-      }
-    });
+      });
+    }
   }
 
   /**
@@ -132,7 +137,7 @@ export class ListKeyManager<T extends ListKeyManagerOption> {
       filter(() => this._pressedLetters.length > 0),
       map(() => this._pressedLetters.join(''))
     ).subscribe(inputString => {
-      const items = this._items.toArray();
+      const items = this._getItemsArray();
 
       // Start at 1 because we want to start searching at the item immediately
       // following the current active item.
@@ -288,7 +293,7 @@ export class ListKeyManager<T extends ListKeyManagerOption> {
   updateActiveItem(item: T): void;
 
   updateActiveItem(item: any): void {
-    const itemArray = this._items.toArray();
+    const itemArray = this._getItemsArray();
     const index = typeof item === 'number' ? item : itemArray.indexOf(item);
 
     this._activeItemIndex = index;
@@ -310,17 +315,18 @@ export class ListKeyManager<T extends ListKeyManagerOption> {
    * currently active item and the new active item. It will calculate differently
    * depending on whether wrap mode is turned on.
    */
-  private _setActiveItemByDelta(delta: -1 | 1, items = this._items.toArray()): void {
-    this._wrap ? this._setActiveInWrapMode(delta, items)
-               : this._setActiveInDefaultMode(delta, items);
+  private _setActiveItemByDelta(delta: -1 | 1): void {
+    this._wrap ? this._setActiveInWrapMode(delta) : this._setActiveInDefaultMode(delta);
   }
 
   /**
    * Sets the active item properly given "wrap" mode. In other words, it will continue to move
    * down the list until it finds an item that is not disabled, and it will wrap if it
    * encounters either end of the list.
    */
-  private _setActiveInWrapMode(delta: -1 | 1, items: T[]): void {
+  private _setActiveInWrapMode(delta: -1 | 1): void {
+    const items = this._getItemsArray();
+
     for (let i = 1; i <= items.length; i++) {
       const index = (this._activeItemIndex + (delta * i) + items.length) % items.length;
       const item = items[index];
@@ -337,17 +343,18 @@ export class ListKeyManager<T extends ListKeyManagerOption> {
    * continue to move down the list until it finds an item that is not disabled. If
    * it encounters either end of the list, it will stop and not wrap.
    */
-  private _setActiveInDefaultMode(delta: -1 | 1, items: T[]): void {
-    this._setActiveItemByIndex(this._activeItemIndex + delta, delta, items);
+  private _setActiveInDefaultMode(delta: -1 | 1): void {
+    this._setActiveItemByIndex(this._activeItemIndex + delta, delta);
   }
 
   /**
    * Sets the active item to the first enabled item starting at the index specified. If the
    * item is disabled, it will move in the fallbackDelta direction until it either
    * finds an enabled item or encounters the end of the list.
    */
-  private _setActiveItemByIndex(index: number, fallbackDelta: -1 | 1,
-                                items = this._items.toArray()): void {
+  private _setActiveItemByIndex(index: number, fallbackDelta: -1 | 1): void {
+    const items = this._getItemsArray();
+
     if (!items[index]) {
       return;
     }
@@ -362,4 +369,9 @@ export class ListKeyManager<T extends ListKeyManagerOption> {
 
     this.setActiveItem(index);
   }
+
+  /** Returns the items as an array. */
+  private _getItemsArray(): T[] {
+    return this._items instanceof QueryList ? this._items.toArray() : this._items;
+  }
 }

src/lib/menu/menu-directive.ts

@@ -25,8 +25,8 @@ import {
   OnDestroy,
   OnInit,
   Output,
-  QueryList,
   TemplateRef,
+  QueryList,
   ViewChild,
   ViewEncapsulation,
 } from '@angular/core';
@@ -36,7 +36,7 @@ import {matMenuAnimations} from './menu-animations';
 import {MatMenuContent} from './menu-content';
 import {throwMatMenuInvalidPositionX, throwMatMenuInvalidPositionY} from './menu-errors';
 import {MatMenuItem} from './menu-item';
-import {MatMenuPanel} from './menu-panel';
+import {MAT_MENU_PANEL, MatMenuPanel} from './menu-panel';
 import {MenuPositionX, MenuPositionY} from './menu-positions';
 
 
@@ -84,18 +84,28 @@ const MAT_MENU_BASE_ELEVATION = 2;
   styleUrls: ['menu.css'],
   changeDetection: ChangeDetectionStrategy.OnPush,
   encapsulation: ViewEncapsulation.None,
+  preserveWhitespaces: false,
+  exportAs: 'matMenu',
   animations: [
     matMenuAnimations.transformMenu,
     matMenuAnimations.fadeInItems
   ],
-  exportAs: 'matMenu'
+  providers: [
+    {provide: MAT_MENU_PANEL, useExisting: MatMenu}
+  ]
 })
-export class MatMenu implements OnInit, AfterContentInit, MatMenuPanel, OnDestroy {
+export class MatMenu implements OnInit, AfterContentInit, MatMenuPanel<MatMenuItem>, OnDestroy {
   private _keyManager: FocusKeyManager<MatMenuItem>;
   private _xPosition: MenuPositionX = this._defaultOptions.xPosition;
   private _yPosition: MenuPositionY = this._defaultOptions.yPosition;
   private _previousElevation: string;
 
+  /** Menu items inside the current menu. */
+  private _items: MatMenuItem[] = [];
+
+  /** Emits whenever the amount of menu items changes. */
+  private _itemChanges = new Subject<MatMenuItem[]>();
+
   /** Subscription to tab events on the menu panel */
   private _tabSubscription = Subscription.EMPTY;
 
@@ -106,7 +116,10 @@ export class MatMenu implements OnInit, AfterContentInit, MatMenuPanel, OnDestro
   _panelAnimationState: 'void' | 'enter' = 'void';
 
   /** Emits whenever an animation on the menu completes. */
-  _animationDone = new Subject<void>();
+  _animationDone = new Subject<AnimationEvent>();
+
+  /** Whether the menu is animating. */
+  _isAnimating: boolean;
 
   /** Parent menu of the current menu panel. */
   parentMenu: MatMenuPanel | undefined;
@@ -142,7 +155,11 @@ export class MatMenu implements OnInit, AfterContentInit, MatMenuPanel, OnDestro
   /** @docs-private */
   @ViewChild(TemplateRef) templateRef: TemplateRef<any>;
 
-  /** List of the items inside of a menu. */
+  /**
+   * List of the items inside of a menu.
+   * @deprecated
+   * @deletion-target 7.0.0
+   */
   @ContentChildren(MatMenuItem) items: QueryList<MatMenuItem>;
 
   /**
@@ -218,7 +235,7 @@ export class MatMenu implements OnInit, AfterContentInit, MatMenuPanel, OnDestro
   }
 
   ngAfterContentInit() {
-    this._keyManager = new FocusKeyManager<MatMenuItem>(this.items).withWrap().withTypeAhead();
+    this._keyManager = new FocusKeyManager<MatMenuItem>(this._items).withWrap().withTypeAhead();
     this._tabSubscription = this._keyManager.tabOut.subscribe(() => this.close.emit('tab'));
   }
 
@@ -229,16 +246,10 @@ export class MatMenu implements OnInit, AfterContentInit, MatMenuPanel, OnDestro
 
   /** Stream that emits whenever the hovered menu item changes. */
   _hovered(): Observable<MatMenuItem> {
-    if (this.items) {
-      return this.items.changes.pipe(
-        startWith(this.items),
-        switchMap(items => merge(...items.map(item => item._hovered)))
-      );
-    }
-
-    return this._ngZone.onStable
-      .asObservable()
-      .pipe(take(1), switchMap(() => this._hovered()));
+    return this._itemChanges.pipe(
+      startWith(this._items),
+      switchMap(items => merge(...items.map(item => item._hovered)))
+    );
   }
 
   /** Handle a keyboard event from the menu, delegating to the appropriate action. */
@@ -316,6 +327,35 @@ export class MatMenu implements OnInit, AfterContentInit, MatMenuPanel, OnDestro
     }
   }
 
+  /**
+   * Registers a menu item with the menu.
+   * @docs-private
+   */
+  addItem(item: MatMenuItem) {
+    // We register the items through this method, rather than picking them up through
+    // `ContentChildren`, because we need the items to be picked up by their closest
+    // `mat-menu` ancestor. If we used `@ContentChildren(MatMenuItem, {descendants: true})`,
+    // all descendant items will bleed into the top-level menu in the case where the consumer
+    // has `mat-menu` instances nested inside each other.
+    if (this._items.indexOf(item) === -1) {
+      this._items.push(item);
+      this._itemChanges.next(this._items);
+    }
+  }
+
+  /**
+   * Removes an item from the menu.
+   * @docs-private
+   */
+  removeItem(item: MatMenuItem) {
+    const index = this._items.indexOf(item);
+
+    if (this._items.indexOf(item) > -1) {
+      this._items.splice(index, 1);
+      this._itemChanges.next(this._items);
+    }
+  }
+
   /** Starts the enter animation. */
   _startAnimation() {
     // @deletion-target 7.0.0 Combine with _resetAnimation.
@@ -329,7 +369,8 @@ export class MatMenu implements OnInit, AfterContentInit, MatMenuPanel, OnDestro
   }
 
   /** Callback that is invoked when the panel animation completes. */
-  _onAnimationDone() {
-    this._animationDone.next();
+  _onAnimationDone(event: AnimationEvent) {
+    this._animationDone.next(event);
+    this._isAnimating = false;
   }
 }

src/lib/menu/menu-item.ts

@@ -14,6 +14,7 @@ import {
   OnDestroy,
   ViewEncapsulation,
   Inject,
+  Optional,
 } from '@angular/core';
 import {
   CanDisable,
@@ -23,6 +24,7 @@ import {
 } from '@angular/material/core';
 import {Subject} from 'rxjs';
 import {DOCUMENT} from '@angular/common';
+import {MAT_MENU_PANEL, MatMenuPanel} from './menu-panel';
 
 // Boilerplate for applying mixins to MatMenuItem.
 /** @docs-private */
@@ -70,7 +72,8 @@ export class MatMenuItem extends _MatMenuItemMixinBase
   constructor(
     private _elementRef: ElementRef,
     @Inject(DOCUMENT) document?: any,
-    private _focusMonitor?: FocusMonitor) {
+    private _focusMonitor?: FocusMonitor,
+    @Inject(MAT_MENU_PANEL) @Optional() private _parentMenu?: MatMenuPanel<MatMenuItem>) {
 
     // @deletion-target 7.0.0 make `_focusMonitor` and `document` required params.
     super();
@@ -82,6 +85,10 @@ export class MatMenuItem extends _MatMenuItemMixinBase
       _focusMonitor.monitor(this._getHostElement(), false);
     }
 
+    if (_parentMenu && _parentMenu.addItem) {
+      _parentMenu.addItem(this);
+    }
+
     this._document = document;
   }
 
@@ -99,6 +106,10 @@ export class MatMenuItem extends _MatMenuItemMixinBase
       this._focusMonitor.stopMonitoring(this._getHostElement());
     }
 
+    if (this._parentMenu && this._parentMenu.removeItem) {
+      this._parentMenu.removeItem(this);
+    }
+
     this._hovered.complete();
   }
 

src/lib/menu/menu-panel.ts

@@ -6,17 +6,23 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {EventEmitter, TemplateRef} from '@angular/core';
+import {EventEmitter, TemplateRef, InjectionToken} from '@angular/core';
 import {MenuPositionX, MenuPositionY} from './menu-positions';
 import {Direction} from '@angular/cdk/bidi';
 import {FocusOrigin} from '@angular/cdk/a11y';
 import {MatMenuContent} from './menu-content';
 
+/**
+ * Injection token used to provide the parent menu to menu-specific components.
+ * @docs-private
+ */
+export const MAT_MENU_PANEL = new InjectionToken<MatMenuPanel>('MAT_MENU_PANEL');
+
 /**
  * Interface for a custom menu panel that can be used with `matMenuTriggerFor`.
  * @docs-private
  */
-export interface MatMenuPanel {
+export interface MatMenuPanel<T = any> {
   xPosition: MenuPositionX;
   yPosition: MenuPositionY;
   overlapTrigger: boolean;
@@ -31,4 +37,6 @@ export interface MatMenuPanel {
   lazyContent?: MatMenuContent;
   backdropClass?: string;
   hasBackdrop?: boolean;
+  addItem?: (item: T) => void;
+  removeItem?: (item: T) => void;
 }

src/lib/menu/menu-trigger.ts

@@ -19,7 +19,7 @@ import {
   VerticalConnectionPos,
 } from '@angular/cdk/overlay';
 import {TemplatePortal} from '@angular/cdk/portal';
-import {filter, take} from 'rxjs/operators';
+import {filter, take, delay, takeUntil} from 'rxjs/operators';
 import {
   AfterContentInit,
   Directive,
@@ -149,15 +149,7 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
       }
     });
 
-    if (this.triggersSubmenu()) {
-      // Subscribe to changes in the hovered item in order to toggle the panel.
-      this._hoverSubscription = this._parentMenu._hovered()
-          .pipe(filter(active => active === this._menuItemInstance))
-          .subscribe(() => {
-            this._openedByMouse = true;
-            this.openMenu();
-          });
-    }
+    this._handleHover();
   }
 
   ngOnDestroy() {
@@ -467,4 +459,32 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
     }
   }
 
+  /** Handles the cases where the user hovers over the trigger. */
+  private _handleHover() {
+    // Subscribe to changes in the hovered item in order to toggle the panel.
+    if (!this.triggersSubmenu()) {
+      return;
+    }
+
+    this._hoverSubscription = this._parentMenu._hovered()
+      // Since we might have multiple competing triggers for the same menu (e.g. a sub-menu
+      // with different data and triggers), we have to delay it by a tick to ensure that
+      // it won't be closed immediately after it is opened.
+      .pipe(filter(active => active === this._menuItemInstance), delay(0))
+      .subscribe(() => {
+        this._openedByMouse = true;
+
+        // If the same menu is used between multiple triggers, it might still be animating
+        // while the new trigger tries to re-open it. Wait for the animation to finish
+        // before doing so. Also interrupt if the user moves to another item.
+        if (this.menu instanceof MatMenu && this.menu._isAnimating) {
+          this.menu._animationDone
+            .pipe(take(1), takeUntil(this._parentMenu._hovered()))
+            .subscribe(() => this.openMenu());
+        } else {
+          this.openMenu();
+        }
+      });
+  }
+
 }