Skip to content

Commit a3b0260

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. Fixes #13812.
1 parent d2d8a1f commit a3b0260

File tree

2 files changed

+86
-17
lines changed

2 files changed

+86
-17
lines changed

src/lib/menu/menu-trigger.ts

Lines changed: 34 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,31 @@ 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 || menu === this._menu) {
109+
return;
110+
}
111+
112+
this._menu = menu;
113+
this._menuCloseSubscription.unsubscribe();
114+
this._menuCloseSubscription = menu.close.asObservable().subscribe(reason => {
115+
this._destroyMenu();
116+
117+
// If a click closed the menu, we should close the entire chain of nested menus.
118+
if ((reason === 'click' || reason === 'tab') && this._parentMenu) {
119+
this._parentMenu.closed.emit(reason);
120+
}
121+
});
122+
}
123+
private _menu: MatMenuPanel;
108124

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

152168
ngAfterContentInit() {
153169
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-
164170
this._handleHover();
165171
}
166172

@@ -203,7 +209,7 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
203209

204210
const overlayRef = this._createOverlay();
205211
this._setPosition(overlayRef.getConfig().positionStrategy as FlexibleConnectedPositionStrategy);
206-
overlayRef.attach(this._portal);
212+
overlayRef.attach(this._getPortal());
207213

208214
if (this.menu.lazyContent) {
209215
this.menu.lazyContent.attach(this.menuData);
@@ -347,7 +353,6 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
347353
*/
348354
private _createOverlay(): OverlayRef {
349355
if (!this._overlayRef) {
350-
this._portal = new TemplatePortal(this.menu.templateRef, this._viewContainerRef);
351356
const config = this._getOverlayConfig();
352357
this._subscribeToPositions(config.positionStrategy as FlexibleConnectedPositionStrategy);
353358
this._overlayRef = this._overlay.create(config);
@@ -531,4 +536,16 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
531536
});
532537
}
533538

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

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)