Skip to content

Commit 1fb1c55

Browse files
crisbetojelbourn
authored andcommitted
fix(menu): unable to swap menu panel after first open (#13819)
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 077f68e commit 1fb1c55

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)