From d8101384750b74779fc7d11a275fd1be8a5bfe95 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Sun, 9 Jul 2017 03:48:58 +0200 Subject: [PATCH] fix(menu): complete close stream on destroy (#5368) This is something I noticed while working on the nested menus: We're subscribing to the `close` event internally inside the menu trigger, but we're never unsubscribing. These changes complete the observable so we don't have to unsubscribe manually. --- src/lib/menu/menu-directive.ts | 3 +++ src/lib/menu/menu.spec.ts | 15 ++++++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/lib/menu/menu-directive.ts b/src/lib/menu/menu-directive.ts index 52222e503d19..3a09cd57e913 100644 --- a/src/lib/menu/menu-directive.ts +++ b/src/lib/menu/menu-directive.ts @@ -116,6 +116,9 @@ export class MdMenu implements AfterContentInit, MdMenuPanel, OnDestroy { if (this._tabSubscription) { this._tabSubscription.unsubscribe(); } + + this._emitCloseEvent(); + this.close.complete(); } /** Handle a keyboard event from the menu, delegating to the appropriate action. */ diff --git a/src/lib/menu/menu.spec.ts b/src/lib/menu/menu.spec.ts index 9ec4f7684d75..c029c56154bb 100644 --- a/src/lib/menu/menu.spec.ts +++ b/src/lib/menu/menu.spec.ts @@ -15,7 +15,8 @@ import { MdMenuTrigger, MdMenuPanel, MenuPositionX, - MenuPositionY + MenuPositionY, + MdMenu } from './index'; import {OverlayContainer} from '../core/overlay/overlay-container'; import {Directionality, Direction} from '../core/bidi/index'; @@ -477,6 +478,17 @@ describe('MdMenu', () => { expect(fixture.componentInstance.closeCallback).toHaveBeenCalled(); }); + + it('should complete the callback when the menu is destroyed', () => { + let emitCallback = jasmine.createSpy('emit callback'); + let completeCallback = jasmine.createSpy('complete callback'); + + fixture.componentInstance.menu.close.subscribe(emitCallback, null, completeCallback); + fixture.destroy(); + + expect(emitCallback).toHaveBeenCalled(); + expect(completeCallback).toHaveBeenCalled(); + }); }); describe('destroy', () => { @@ -499,6 +511,7 @@ describe('MdMenu', () => { class SimpleMenu { @ViewChild(MdMenuTrigger) trigger: MdMenuTrigger; @ViewChild('triggerEl') triggerEl: ElementRef; + @ViewChild(MdMenu) menu: MdMenu; closeCallback = jasmine.createSpy('menu closed callback'); }