fix(menu): not unsubscribing from close stream if trigger is destroyed

Fixes the menu trigger not unsubscribing from the menu panel's `close` stream. In most cases it isn't an issue, because `close` gets completed when the panel is destroyed, however the user can still run into it if the trigger is destroyed, but the panel stays in place.

Author: crisbeto

src/lib/menu/menu-trigger.ts

@@ -34,7 +34,7 @@ import {
   ViewContainerRef,
 } from '@angular/core';
 import {normalizePassiveListenerOptions} from '@angular/cdk/platform';
-import {asapScheduler, merge, of as observableOf, Subscription} from 'rxjs';
+import {asapScheduler, merge, of as observableOf, Subscription, Subject} from 'rxjs';
 import {delay, filter, take, takeUntil} from 'rxjs/operators';
 import {MatMenu} from './menu-directive';
 import {throwMatMenuMissingError} from './menu-errors';
@@ -85,8 +85,7 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
   private _portal: TemplatePortal;
   private _overlayRef: OverlayRef | null = null;
   private _menuOpen: boolean = false;
-  private _closeSubscription = Subscription.EMPTY;
-  private _hoverSubscription = Subscription.EMPTY;
+  private _destroyed = new Subject();
   private _menuCloseSubscription = Subscription.EMPTY;
   private _scrollStrategy: () => ScrollStrategy;
 
@@ -194,7 +193,9 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
     this._element.nativeElement.removeEventListener('touchstart', this._handleTouchStart,
         passiveEventListenerOptions);
 
-    this._cleanUpSubscriptions();
+    this._menuCloseSubscription.unsubscribe();
+    this._destroyed.next();
+    this._destroyed.complete();
   }
 
   /** Whether the menu is open. */
@@ -233,7 +234,9 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
       this.menu.lazyContent.attach(this.menuData);
     }
 
-    this._closeSubscription = this._menuClosingActions().subscribe(() => this.closeMenu());
+    this._menuClosingActions()
+      .pipe(takeUntil(merge(this._destroyed, overlayRef.detachments())))
+      .subscribe(() => this.closeMenu());
     this._initMenu();
 
     if (this.menu instanceof MatMenu) {
@@ -266,7 +269,6 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
 
     const menu = this.menu;
 
-    this._closeSubscription.unsubscribe();
     this._overlayRef.detach();
 
     if (menu instanceof MatMenu) {
@@ -464,12 +466,6 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
     ]);
   }
 
-  /** Cleans up the active subscriptions. */
-  private _cleanUpSubscriptions(): void {
-    this._closeSubscription.unsubscribe();
-    this._hoverSubscription.unsubscribe();
-  }
-
   /** Returns a stream that emits whenever an action that should close the menu occurs. */
   private _menuClosingActions() {
     const backdrop = this._overlayRef!.backdropClick();
@@ -528,13 +524,14 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
       return;
     }
 
-    this._hoverSubscription = this._parentMenu._hovered()
+    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 && !active.disabled),
-        delay(0, asapScheduler)
+        delay(0, asapScheduler),
+        takeUntil(this._destroyed)
       )
       .subscribe(() => {
         this._openedBy = 'mouse';