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 18, 2018
1 parent f38bcd9 commit 06d03ed
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 11 deletions.
2 changes: 1 addition & 1 deletion src/lib/chips/chip-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ export class MatChipList extends _MatChipListMixinBase implements MatFormFieldCo
*/
@Input()
get selectable(): boolean { return this._selectable; }
set selectable(value: boolean) {
set selectable(value: boolean) {
this._selectable = coerceBooleanProperty(value);
if (this.chips) {
this.chips.forEach(chip => chip.chipListSelectable = this._selectable);
Expand Down
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
6 changes: 6 additions & 0 deletions src/lib/menu/menu-directive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
NgZone,
} 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 Down Expand Up @@ -97,6 +98,9 @@ export class MatMenu implements AfterContentInit, MatMenuPanel, OnDestroy {
/** Current state of the panel animation. */
_panelAnimationState: 'void' | 'enter-start' | '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 @@ -306,5 +310,7 @@ export class MatMenu implements AfterContentInit, MatMenuPanel, OnDestroy {
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 06d03ed

Please sign in to comment.