Skip to content

Commit

Permalink
fix(menu): unable to swap menu panel after first open (#13819)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
crisbeto authored and jelbourn committed Nov 6, 2018
1 parent b88b79d commit cbb76ec
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 17 deletions.
54 changes: 37 additions & 17 deletions src/lib/menu/menu-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}

}
52 changes: 52 additions & 0 deletions src/lib/menu/menu.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -2048,3 +2081,22 @@ class LazyMenuWithContext {
@ViewChild('triggerTwo') triggerTwo: MatMenuTrigger;
}



@Component({
template: `
<button [matMenuTriggerFor]="one">Toggle menu</button>
<mat-menu #one="matMenu">
<button mat-menu-item>One</button>
</mat-menu>
<mat-menu #two="matMenu">
<button mat-menu-item>Two</button>
</mat-menu>
`
})
class DynamicPanelMenu {
@ViewChild(MatMenuTrigger) trigger: MatMenuTrigger;
@ViewChild('one') firstMenu: MatMenu;
@ViewChild('two') secondMenu: MatMenu;
}

0 comments on commit cbb76ec

Please sign in to comment.