Skip to content

Commit

Permalink
fix(menu): detach lazily-rendered content when the menu is closed
Browse files Browse the repository at this point in the history
Currently the lazily-rendered content is maintained in the background while a menu is open. These changes switch to detaching it once the user closes the menu.

Fixes #9915.
  • Loading branch information
crisbeto committed Feb 25, 2018
1 parent 64ef3a8 commit b3f9677
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 12 deletions.
14 changes: 12 additions & 2 deletions src/lib/menu/menu-content.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ export class MatMenuContent implements OnDestroy {
attach(context: any = {}) {
if (!this._portal) {
this._portal = new TemplatePortal(this._template, this._viewContainerRef);
} else if (this._portal.isAttached) {
this._portal.detach();
}

this.detach();

if (!this._outlet) {
this._outlet = new DomPortalOutlet(this._document.createElement('div'),
this._componentFactoryResolver, this._appRef, this._injector);
Expand All @@ -62,6 +62,16 @@ export class MatMenuContent implements OnDestroy {
this._portal.attach(this._outlet, context);
}

/**
* Detaches the content.
* @docs-private
*/
detach() {
if (this._portal.isAttached) {
this._portal.detach();
}
}

ngOnDestroy() {
if (this._outlet) {
this._outlet.dispose();
Expand Down
14 changes: 12 additions & 2 deletions src/lib/menu/menu-directive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
OnInit,
} from '@angular/core';
import {Observable} from 'rxjs/Observable';
import {Subject} from 'rxjs/Subject';
import {merge} from 'rxjs/observable/merge';
import {Subscription} from 'rxjs/Subscription';
import {matMenuAnimations} from './menu-animations';
Expand All @@ -43,6 +44,7 @@ import {MatMenuContent} from './menu-content';
import {MenuPositionX, MenuPositionY} from './menu-positions';
import {coerceBooleanProperty} from '@angular/cdk/coercion';
import {FocusOrigin} from '@angular/cdk/a11y';
import {AnimationEvent} from '@angular/animations';


/** Default `mat-menu` options that can be overridden. */
Expand Down Expand Up @@ -97,6 +99,9 @@ export class MatMenu implements OnInit, AfterContentInit, MatMenuPanel, OnDestro
/** Current state of the panel animation. */
_panelAnimationState: 'void' | 'enter' = 'void';

/** Emits whenever an animation on the menu completes. */
_animationDone = new Subject<void>();

/** Parent menu of the current menu panel. */
parentMenu: MatMenuPanel | undefined;

Expand Down Expand Up @@ -307,7 +312,12 @@ export class MatMenu implements OnInit, AfterContentInit, MatMenuPanel, OnDestro
}

/** Callback that is invoked when the panel animation completes. */
_onAnimationDone(_event: AnimationEvent) {
// @deletion-target 6.0.0 Not being used anymore. To be removed.
_onAnimationDone(event: AnimationEvent) {
// After the initial expansion is done, trigger the second phase of the enter animation.
if (event.toState === 'enter-start') {
this._panelAnimationState = 'enter';
}

this._animationDone.next();
}
}
26 changes: 20 additions & 6 deletions src/lib/menu/menu-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
} from '@angular/cdk/overlay';
import {TemplatePortal} from '@angular/cdk/portal';
import {filter} from 'rxjs/operators/filter';
import {take} from 'rxjs/operators/take';
import {
AfterContentInit,
Directive,
Expand Down Expand Up @@ -238,14 +239,27 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {

/** Closes the menu and does the necessary cleanup. */
private _destroyMenu() {
if (this._overlayRef && this.menuOpen) {
this._resetMenu();
this._closeSubscription.unsubscribe();
this._overlayRef.detach();
if (!this._overlayRef || !this.menuOpen) {
return;
}

const menu = this.menu;

this._resetMenu();
this._closeSubscription.unsubscribe();
this._overlayRef.detach();

if (menu instanceof MatMenu) {
menu._resetAnimation();

if (this.menu instanceof MatMenu) {
this.menu._resetAnimation();
if (menu.lazyContent) {
// Wait for the exit animation to finish before detaching the content.
menu._animationDone
.pipe(take(1))
.subscribe(() => menu.lazyContent!.detach());
}
} else if (menu.lazyContent) {
menu.lazyContent.detach();
}
}

Expand Down
23 changes: 21 additions & 2 deletions src/lib/menu/menu.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -327,8 +327,8 @@ describe('MatMenu', () => {
describe('lazy rendering', () => {
it('should be able to render the menu content lazily', fakeAsync(() => {
const fixture = TestBed.createComponent(SimpleLazyMenu);
fixture.detectChanges();

fixture.detectChanges();
fixture.componentInstance.triggerEl.nativeElement.click();
fixture.detectChanges();
tick(500);
Expand All @@ -340,6 +340,24 @@ describe('MatMenu', () => {
expect(fixture.componentInstance.trigger.menuOpen).toBe(true, 'Expected menu to be open');
}));

it('should detach the lazy content when the menu is closed', fakeAsync(() => {
const fixture = TestBed.createComponent(SimpleLazyMenu);

fixture.detectChanges();
fixture.componentInstance.trigger.openMenu();
fixture.detectChanges();
tick(500);

expect(fixture.componentInstance.items.length).toBeGreaterThan(0);

fixture.componentInstance.trigger.closeMenu();
fixture.detectChanges();
tick(500);
fixture.detectChanges();

expect(fixture.componentInstance.items.length).toBe(0);
}));

it('should focus the first menu item when opening a lazy menu via keyboard', fakeAsync(() => {
let zone: MockNgZone;

Expand Down Expand Up @@ -372,8 +390,8 @@ describe('MatMenu', () => {

it('should be able to open the same menu with a different context', fakeAsync(() => {
const fixture = TestBed.createComponent(LazyMenuWithContext);
fixture.detectChanges();

fixture.detectChanges();
fixture.componentInstance.triggerOne.openMenu();
fixture.detectChanges();
tick(500);
Expand Down Expand Up @@ -1557,6 +1575,7 @@ class FakeIcon {}
class SimpleLazyMenu {
@ViewChild(MatMenuTrigger) trigger: MatMenuTrigger;
@ViewChild('triggerEl') triggerEl: ElementRef;
@ViewChildren(MatMenuItem) items: QueryList<MatMenuItem>;
}


Expand Down

0 comments on commit b3f9677

Please sign in to comment.