Skip to content

Commit 37b1a09

Browse files
crisbetotinayuangao
authored andcommitted
fix(menu): detach lazily-rendered content when the menu is closed (#10005)
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.
1 parent c7f89de commit 37b1a09

File tree

5 files changed

+60
-16
lines changed

5 files changed

+60
-16
lines changed

src/lib/menu/menu-content.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,10 @@ export class MatMenuContent implements OnDestroy {
4444
attach(context: any = {}) {
4545
if (!this._portal) {
4646
this._portal = new TemplatePortal(this._template, this._viewContainerRef);
47-
} else if (this._portal.isAttached) {
48-
this._portal.detach();
4947
}
5048

49+
this.detach();
50+
5151
if (!this._outlet) {
5252
this._outlet = new DomPortalOutlet(this._document.createElement('div'),
5353
this._componentFactoryResolver, this._appRef, this._injector);
@@ -62,6 +62,16 @@ export class MatMenuContent implements OnDestroy {
6262
this._portal.attach(this._outlet, context);
6363
}
6464

65+
/**
66+
* Detaches the content.
67+
* @docs-private
68+
*/
69+
detach() {
70+
if (this._portal.isAttached) {
71+
this._portal.detach();
72+
}
73+
}
74+
6575
ngOnDestroy() {
6676
if (this._outlet) {
6777
this._outlet.dispose();

src/lib/menu/menu-directive.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import {
3333
NgZone,
3434
} from '@angular/core';
3535
import {Observable} from 'rxjs/Observable';
36+
import {Subject} from 'rxjs/Subject';
3637
import {merge} from 'rxjs/observable/merge';
3738
import {Subscription} from 'rxjs/Subscription';
3839
import {matMenuAnimations} from './menu-animations';
@@ -97,6 +98,9 @@ export class MatMenu implements AfterContentInit, MatMenuPanel, OnDestroy {
9798
/** Current state of the panel animation. */
9899
_panelAnimationState: 'void' | 'enter-start' | 'enter' = 'void';
99100

101+
/** Emits whenever an animation on the menu completes. */
102+
_animationDone = new Subject<void>();
103+
100104
/** Parent menu of the current menu panel. */
101105
parentMenu: MatMenuPanel | undefined;
102106

@@ -301,10 +305,7 @@ export class MatMenu implements AfterContentInit, MatMenuPanel, OnDestroy {
301305
}
302306

303307
/** Callback that is invoked when the panel animation completes. */
304-
_onAnimationDone(event: AnimationEvent) {
305-
// After the initial expansion is done, trigger the second phase of the enter animation.
306-
if (event.toState === 'enter-start') {
307-
this._panelAnimationState = 'enter';
308-
}
308+
_onAnimationDone() {
309+
this._animationDone.next();
309310
}
310311
}

src/lib/menu/menu-trigger.ts

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {
2121
} from '@angular/cdk/overlay';
2222
import {TemplatePortal} from '@angular/cdk/portal';
2323
import {filter} from 'rxjs/operators/filter';
24+
import {take} from 'rxjs/operators/take';
2425
import {
2526
AfterContentInit,
2627
Directive,
@@ -238,14 +239,27 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
238239

239240
/** Closes the menu and does the necessary cleanup. */
240241
private _destroyMenu() {
241-
if (this._overlayRef && this.menuOpen) {
242-
this._resetMenu();
243-
this._closeSubscription.unsubscribe();
244-
this._overlayRef.detach();
242+
if (!this._overlayRef || !this.menuOpen) {
243+
return;
244+
}
245+
246+
const menu = this.menu;
247+
248+
this._resetMenu();
249+
this._closeSubscription.unsubscribe();
250+
this._overlayRef.detach();
251+
252+
if (menu instanceof MatMenu) {
253+
menu._resetAnimation();
245254

246-
if (this.menu instanceof MatMenu) {
247-
this.menu._resetAnimation();
255+
if (menu.lazyContent) {
256+
// Wait for the exit animation to finish before detaching the content.
257+
menu._animationDone
258+
.pipe(take(1))
259+
.subscribe(() => menu.lazyContent!.detach());
248260
}
261+
} else if (menu.lazyContent) {
262+
menu.lazyContent.detach();
249263
}
250264
}
251265

src/lib/menu/menu.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
(keydown)="_handleKeydown($event)"
66
(click)="closed.emit('click')"
77
[@transformMenu]="_panelAnimationState"
8-
(@transformMenu.done)="_onAnimationDone($event)"
8+
(@transformMenu.done)="_onAnimationDone()"
99
tabindex="-1"
1010
role="menu">
1111
<div class="mat-menu-content" [@fadeInItems]="'showing'">

src/lib/menu/menu.spec.ts

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -327,8 +327,8 @@ describe('MatMenu', () => {
327327
describe('lazy rendering', () => {
328328
it('should be able to render the menu content lazily', fakeAsync(() => {
329329
const fixture = TestBed.createComponent(SimpleLazyMenu);
330-
fixture.detectChanges();
331330

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

343+
it('should detach the lazy content when the menu is closed', fakeAsync(() => {
344+
const fixture = TestBed.createComponent(SimpleLazyMenu);
345+
346+
fixture.detectChanges();
347+
fixture.componentInstance.trigger.openMenu();
348+
fixture.detectChanges();
349+
tick(500);
350+
351+
expect(fixture.componentInstance.items.length).toBeGreaterThan(0);
352+
353+
fixture.componentInstance.trigger.closeMenu();
354+
fixture.detectChanges();
355+
tick(500);
356+
fixture.detectChanges();
357+
358+
expect(fixture.componentInstance.items.length).toBe(0);
359+
}));
360+
343361
it('should focus the first menu item when opening a lazy menu via keyboard', fakeAsync(() => {
344362
let zone: MockNgZone;
345363

@@ -372,8 +390,8 @@ describe('MatMenu', () => {
372390

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

394+
fixture.detectChanges();
377395
fixture.componentInstance.triggerOne.openMenu();
378396
fixture.detectChanges();
379397
tick(500);
@@ -1557,6 +1575,7 @@ class FakeIcon {}
15571575
class SimpleLazyMenu {
15581576
@ViewChild(MatMenuTrigger) trigger: MatMenuTrigger;
15591577
@ViewChild('triggerEl') triggerEl: ElementRef;
1578+
@ViewChildren(MatMenuItem) items: QueryList<MatMenuItem>;
15601579
}
15611580

15621581

0 commit comments

Comments
 (0)