Skip to content

Commit fc5f5d3

Browse files
committed
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.
1 parent d2d8a1f commit fc5f5d3

File tree

2 files changed

+89
-17
lines changed

2 files changed

+89
-17
lines changed

src/lib/menu/menu-trigger.ts

Lines changed: 37 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
8484
private _menuOpen: boolean = false;
8585
private _closeSubscription = Subscription.EMPTY;
8686
private _hoverSubscription = Subscription.EMPTY;
87+
private _menuCloseSubscription = Subscription.EMPTY;
8788
private _scrollStrategy: () => ScrollStrategy;
8889

8990
// Tracking input type is necessary so it's possible to only auto-focus
@@ -95,16 +96,34 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
9596
* @breaking-change 8.0.0
9697
*/
9798
@Input('mat-menu-trigger-for')
98-
get _deprecatedMatMenuTriggerFor(): MatMenuPanel {
99-
return this.menu;
100-
}
101-
99+
get _deprecatedMatMenuTriggerFor(): MatMenuPanel { return this.menu; }
102100
set _deprecatedMatMenuTriggerFor(v: MatMenuPanel) {
103101
this.menu = v;
104102
}
105103

106104
/** References the menu instance that the trigger is associated with. */
107-
@Input('matMenuTriggerFor') menu: MatMenuPanel;
105+
@Input('matMenuTriggerFor')
106+
get menu() { return this._menu; }
107+
set menu(menu: MatMenuPanel) {
108+
if (menu === this._menu) {
109+
return;
110+
}
111+
112+
this._menu = menu;
113+
this._menuCloseSubscription.unsubscribe();
114+
115+
if (menu) {
116+
this._menuCloseSubscription = menu.close.asObservable().subscribe(reason => {
117+
this._destroyMenu();
118+
119+
// If a click closed the menu, we should close the entire chain of nested menus.
120+
if ((reason === 'click' || reason === 'tab') && this._parentMenu) {
121+
this._parentMenu.closed.emit(reason);
122+
}
123+
});
124+
}
125+
}
126+
private _menu: MatMenuPanel;
108127

109128
/** Data to be passed along to any lazily-rendered content. */
110129
@Input('matMenuTriggerData') menuData: any;
@@ -151,16 +170,6 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
151170

152171
ngAfterContentInit() {
153172
this._checkMenu();
154-
155-
this.menu.close.asObservable().subscribe(reason => {
156-
this._destroyMenu();
157-
158-
// If a click closed the menu, we should close the entire chain of nested menus.
159-
if ((reason === 'click' || reason === 'tab') && this._parentMenu) {
160-
this._parentMenu.closed.emit(reason);
161-
}
162-
});
163-
164173
this._handleHover();
165174
}
166175

@@ -203,7 +212,7 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
203212

204213
const overlayRef = this._createOverlay();
205214
this._setPosition(overlayRef.getConfig().positionStrategy as FlexibleConnectedPositionStrategy);
206-
overlayRef.attach(this._portal);
215+
overlayRef.attach(this._getPortal());
207216

208217
if (this.menu.lazyContent) {
209218
this.menu.lazyContent.attach(this.menuData);
@@ -347,7 +356,6 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
347356
*/
348357
private _createOverlay(): OverlayRef {
349358
if (!this._overlayRef) {
350-
this._portal = new TemplatePortal(this.menu.templateRef, this._viewContainerRef);
351359
const config = this._getOverlayConfig();
352360
this._subscribeToPositions(config.positionStrategy as FlexibleConnectedPositionStrategy);
353361
this._overlayRef = this._overlay.create(config);
@@ -531,4 +539,16 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
531539
});
532540
}
533541

542+
/** Gets the portal that should be attached to the overlay. */
543+
private _getPortal(): TemplatePortal {
544+
// Note that we can avoid this check by keeping the portal on the menu panel.
545+
// While it would be cleaner, we'd have to introduce another required method on
546+
// `MatMenuPanel`, making it harder to consume.
547+
if (!this._portal || this._portal.templateRef !== this.menu.templateRef) {
548+
this._portal = new TemplatePortal(this.menu.templateRef, this._viewContainerRef);
549+
}
550+
551+
return this._portal;
552+
}
553+
534554
}

src/lib/menu/menu.spec.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -517,6 +517,39 @@ describe('MatMenu', () => {
517517
}).toThrowError(/must pass in an mat-menu instance/);
518518
});
519519

520+
it('should be able to swap out a menu after the first time it is opened', fakeAsync(() => {
521+
const fixture = createComponent(DynamicPanelMenu);
522+
fixture.detectChanges();
523+
expect(overlayContainerElement.textContent).toBe('');
524+
525+
fixture.componentInstance.trigger.openMenu();
526+
fixture.detectChanges();
527+
528+
expect(overlayContainerElement.textContent).toContain('One');
529+
expect(overlayContainerElement.textContent).not.toContain('Two');
530+
531+
fixture.componentInstance.trigger.closeMenu();
532+
fixture.detectChanges();
533+
tick(500);
534+
fixture.detectChanges();
535+
536+
expect(overlayContainerElement.textContent).toBe('');
537+
538+
fixture.componentInstance.trigger.menu = fixture.componentInstance.secondMenu;
539+
fixture.componentInstance.trigger.openMenu();
540+
fixture.detectChanges();
541+
542+
expect(overlayContainerElement.textContent).not.toContain('One');
543+
expect(overlayContainerElement.textContent).toContain('Two');
544+
545+
fixture.componentInstance.trigger.closeMenu();
546+
fixture.detectChanges();
547+
tick(500);
548+
fixture.detectChanges();
549+
550+
expect(overlayContainerElement.textContent).toBe('');
551+
}));
552+
520553
describe('lazy rendering', () => {
521554
it('should be able to render the menu content lazily', fakeAsync(() => {
522555
const fixture = createComponent(SimpleLazyMenu);
@@ -2048,3 +2081,22 @@ class LazyMenuWithContext {
20482081
@ViewChild('triggerTwo') triggerTwo: MatMenuTrigger;
20492082
}
20502083

2084+
2085+
2086+
@Component({
2087+
template: `
2088+
<button [matMenuTriggerFor]="one">Toggle menu</button>
2089+
<mat-menu #one="matMenu">
2090+
<button mat-menu-item>One</button>
2091+
</mat-menu>
2092+
2093+
<mat-menu #two="matMenu">
2094+
<button mat-menu-item>Two</button>
2095+
</mat-menu>
2096+
`
2097+
})
2098+
class DynamicPanelMenu {
2099+
@ViewChild(MatMenuTrigger) trigger: MatMenuTrigger;
2100+
@ViewChild('one') firstMenu: MatMenu;
2101+
@ViewChild('two') secondMenu: MatMenu;
2102+
}

0 commit comments

Comments
 (0)