From 1b06d1a7319ffc91ba08c72a76da51905e3cff8c Mon Sep 17 00:00:00 2001 From: crisbeto Date: Tue, 15 May 2018 21:42:06 +0200 Subject: [PATCH] fix(menu): lazy-rendered content being duplicated when toggling quickly Fixes the lazily-rendered content of a menu being duplicated, if it user toggles the panel too quickly. The issue comes from the fact that we wait for the animation to finish before we detach the content, however we allow the panel to be reopened as soon the closing sequence is kicked off. Fixes #11331. --- src/lib/menu/menu-directive.ts | 1 + src/lib/menu/menu-trigger.ts | 18 +++++++++---- src/lib/menu/menu.spec.ts | 25 +++++++++++++++++++ .../stepper-vertical-example.ts | 4 +-- 4 files changed, 41 insertions(+), 7 deletions(-) diff --git a/src/lib/menu/menu-directive.ts b/src/lib/menu/menu-directive.ts index e1a8a63b2cef..b70d203a8d9c 100644 --- a/src/lib/menu/menu-directive.ts +++ b/src/lib/menu/menu-directive.ts @@ -37,6 +37,7 @@ import {throwMatMenuInvalidPositionX, throwMatMenuInvalidPositionY} from './menu import {MatMenuItem} from './menu-item'; import {MAT_MENU_PANEL, MatMenuPanel} from './menu-panel'; import {MenuPositionX, MenuPositionY} from './menu-positions'; +import {AnimationEvent} from '@angular/animations'; /** Default `mat-menu` options that can be overridden. */ diff --git a/src/lib/menu/menu-trigger.ts b/src/lib/menu/menu-trigger.ts index 885c828834ed..e914669b7fc4 100644 --- a/src/lib/menu/menu-trigger.ts +++ b/src/lib/menu/menu-trigger.ts @@ -233,7 +233,6 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy { const menu = this.menu; - this._resetMenu(); this._closeSubscription.unsubscribe(); this._overlayRef.detach(); @@ -243,11 +242,20 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy { if (menu.lazyContent) { // Wait for the exit animation to finish before detaching the content. menu._animationDone - .pipe(take(1)) - .subscribe(() => menu.lazyContent!.detach()); + .pipe(filter(event => event.toState === 'void'), take(1)) + .subscribe(() => { + menu.lazyContent!.detach(); + this._resetMenu(); + }); + } else { + this._resetMenu(); + } + } else { + this._resetMenu(); + + if (menu.lazyContent) { + menu.lazyContent.detach(); } - } else if (menu.lazyContent) { - menu.lazyContent.detach(); } } diff --git a/src/lib/menu/menu.spec.ts b/src/lib/menu/menu.spec.ts index ca4be2315829..574c1c08ebcb 100644 --- a/src/lib/menu/menu.spec.ts +++ b/src/lib/menu/menu.spec.ts @@ -418,6 +418,31 @@ describe('MatMenu', () => { expect(fixture.componentInstance.items.length).toBe(0); })); + it('should wait for the close animation to finish before considering the panel as closed', + fakeAsync(() => { + const fixture = createComponent(SimpleLazyMenu); + fixture.detectChanges(); + const trigger = fixture.componentInstance.trigger; + + expect(trigger.menuOpen).toBe(false, 'Expected menu to start off closed'); + + trigger.openMenu(); + fixture.detectChanges(); + tick(500); + + expect(trigger.menuOpen).toBe(true, 'Expected menu to be open'); + + trigger.closeMenu(); + fixture.detectChanges(); + + expect(trigger.menuOpen) + .toBe(true, 'Expected menu to be considered open while the close animation is running'); + tick(500); + fixture.detectChanges(); + + expect(trigger.menuOpen).toBe(false, 'Expected menu to be closed'); + })); + it('should focus the first menu item when opening a lazy menu via keyboard', fakeAsync(() => { let zone: MockNgZone; let fixture = createComponent(SimpleLazyMenu, [{ diff --git a/src/material-examples/stepper-vertical/stepper-vertical-example.ts b/src/material-examples/stepper-vertical/stepper-vertical-example.ts index ba958383b4f5..13229872f1fd 100644 --- a/src/material-examples/stepper-vertical/stepper-vertical-example.ts +++ b/src/material-examples/stepper-vertical/stepper-vertical-example.ts @@ -1,4 +1,4 @@ -import {Component} from '@angular/core'; +import {Component, OnInit} from '@angular/core'; import {FormBuilder, FormGroup, Validators} from '@angular/forms'; /** @@ -9,7 +9,7 @@ import {FormBuilder, FormGroup, Validators} from '@angular/forms'; templateUrl: 'stepper-vertical-example.html', styleUrls: ['stepper-vertical-example.css'] }) -export class StepperVerticalExample { +export class StepperVerticalExample implements OnInit { isLinear = false; firstFormGroup: FormGroup; secondFormGroup: FormGroup;