From fc5f5d3c774cdfe9386cf31e9ae45d9a3f4c3109 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Thu, 25 Oct 2018 22:23:27 +0200 Subject: [PATCH] fix(menu): unable to swap menu panel after first open Fixes the menu trigger being locked into the first menu panel that is passed in, after it has been opened at least once. Relates to #13812. --- src/lib/menu/menu-trigger.ts | 54 ++++++++++++++++++++++++------------ src/lib/menu/menu.spec.ts | 52 ++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 17 deletions(-) diff --git a/src/lib/menu/menu-trigger.ts b/src/lib/menu/menu-trigger.ts index 784903c3bd4c..5c7ff283dcd7 100644 --- a/src/lib/menu/menu-trigger.ts +++ b/src/lib/menu/menu-trigger.ts @@ -84,6 +84,7 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy { private _menuOpen: boolean = false; private _closeSubscription = Subscription.EMPTY; private _hoverSubscription = Subscription.EMPTY; + private _menuCloseSubscription = Subscription.EMPTY; private _scrollStrategy: () => ScrollStrategy; // Tracking input type is necessary so it's possible to only auto-focus @@ -95,16 +96,34 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy { * @breaking-change 8.0.0 */ @Input('mat-menu-trigger-for') - get _deprecatedMatMenuTriggerFor(): MatMenuPanel { - return this.menu; - } - + get _deprecatedMatMenuTriggerFor(): MatMenuPanel { return this.menu; } set _deprecatedMatMenuTriggerFor(v: MatMenuPanel) { this.menu = v; } /** References the menu instance that the trigger is associated with. */ - @Input('matMenuTriggerFor') menu: MatMenuPanel; + @Input('matMenuTriggerFor') + get menu() { return this._menu; } + set menu(menu: MatMenuPanel) { + if (menu === this._menu) { + return; + } + + this._menu = menu; + this._menuCloseSubscription.unsubscribe(); + + if (menu) { + this._menuCloseSubscription = menu.close.asObservable().subscribe(reason => { + this._destroyMenu(); + + // If a click closed the menu, we should close the entire chain of nested menus. + if ((reason === 'click' || reason === 'tab') && this._parentMenu) { + this._parentMenu.closed.emit(reason); + } + }); + } + } + private _menu: MatMenuPanel; /** Data to be passed along to any lazily-rendered content. */ @Input('matMenuTriggerData') menuData: any; @@ -151,16 +170,6 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy { ngAfterContentInit() { this._checkMenu(); - - this.menu.close.asObservable().subscribe(reason => { - this._destroyMenu(); - - // If a click closed the menu, we should close the entire chain of nested menus. - if ((reason === 'click' || reason === 'tab') && this._parentMenu) { - this._parentMenu.closed.emit(reason); - } - }); - this._handleHover(); } @@ -203,7 +212,7 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy { const overlayRef = this._createOverlay(); this._setPosition(overlayRef.getConfig().positionStrategy as FlexibleConnectedPositionStrategy); - overlayRef.attach(this._portal); + overlayRef.attach(this._getPortal()); if (this.menu.lazyContent) { this.menu.lazyContent.attach(this.menuData); @@ -347,7 +356,6 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy { */ private _createOverlay(): OverlayRef { if (!this._overlayRef) { - this._portal = new TemplatePortal(this.menu.templateRef, this._viewContainerRef); const config = this._getOverlayConfig(); this._subscribeToPositions(config.positionStrategy as FlexibleConnectedPositionStrategy); this._overlayRef = this._overlay.create(config); @@ -531,4 +539,16 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy { }); } + /** Gets the portal that should be attached to the overlay. */ + private _getPortal(): TemplatePortal { + // Note that we can avoid this check by keeping the portal on the menu panel. + // While it would be cleaner, we'd have to introduce another required method on + // `MatMenuPanel`, making it harder to consume. + if (!this._portal || this._portal.templateRef !== this.menu.templateRef) { + this._portal = new TemplatePortal(this.menu.templateRef, this._viewContainerRef); + } + + return this._portal; + } + } diff --git a/src/lib/menu/menu.spec.ts b/src/lib/menu/menu.spec.ts index 9bb994c25b5b..87c25446cbee 100644 --- a/src/lib/menu/menu.spec.ts +++ b/src/lib/menu/menu.spec.ts @@ -517,6 +517,39 @@ describe('MatMenu', () => { }).toThrowError(/must pass in an mat-menu instance/); }); + it('should be able to swap out a menu after the first time it is opened', fakeAsync(() => { + const fixture = createComponent(DynamicPanelMenu); + fixture.detectChanges(); + expect(overlayContainerElement.textContent).toBe(''); + + fixture.componentInstance.trigger.openMenu(); + fixture.detectChanges(); + + expect(overlayContainerElement.textContent).toContain('One'); + expect(overlayContainerElement.textContent).not.toContain('Two'); + + fixture.componentInstance.trigger.closeMenu(); + fixture.detectChanges(); + tick(500); + fixture.detectChanges(); + + expect(overlayContainerElement.textContent).toBe(''); + + fixture.componentInstance.trigger.menu = fixture.componentInstance.secondMenu; + fixture.componentInstance.trigger.openMenu(); + fixture.detectChanges(); + + expect(overlayContainerElement.textContent).not.toContain('One'); + expect(overlayContainerElement.textContent).toContain('Two'); + + fixture.componentInstance.trigger.closeMenu(); + fixture.detectChanges(); + tick(500); + fixture.detectChanges(); + + expect(overlayContainerElement.textContent).toBe(''); + })); + describe('lazy rendering', () => { it('should be able to render the menu content lazily', fakeAsync(() => { const fixture = createComponent(SimpleLazyMenu); @@ -2048,3 +2081,22 @@ class LazyMenuWithContext { @ViewChild('triggerTwo') triggerTwo: MatMenuTrigger; } + + +@Component({ + template: ` + + + + + + + + + ` +}) +class DynamicPanelMenu { + @ViewChild(MatMenuTrigger) trigger: MatMenuTrigger; + @ViewChild('one') firstMenu: MatMenu; + @ViewChild('two') secondMenu: MatMenu; +}