diff --git a/src/material/menu/menu-content.ts b/src/material/menu/menu-content.ts index fc4369907ddd..cabacfe4b5a8 100644 --- a/src/material/menu/menu-content.ts +++ b/src/material/menu/menu-content.ts @@ -41,8 +41,8 @@ export class MatMenuContent implements OnDestroy { private _document = inject(DOCUMENT); private _changeDetectorRef = inject(ChangeDetectorRef); - private _portal: TemplatePortal; - private _outlet: DomPortalOutlet; + private _portal: TemplatePortal | undefined; + private _outlet: DomPortalOutlet | undefined; /** Emits when the menu content has been attached. */ readonly _attached = new Subject(); @@ -93,14 +93,13 @@ export class MatMenuContent implements OnDestroy { * @docs-private */ detach() { - if (this._portal.isAttached) { + if (this._portal?.isAttached) { this._portal.detach(); } } ngOnDestroy() { - if (this._outlet) { - this._outlet.dispose(); - } + this.detach(); + this._outlet?.dispose(); } } diff --git a/src/material/menu/menu-trigger.ts b/src/material/menu/menu-trigger.ts index d7f85f2a22d1..42bdbe2b8feb 100644 --- a/src/material/menu/menu-trigger.ts +++ b/src/material/menu/menu-trigger.ts @@ -39,8 +39,8 @@ import { ViewContainerRef, } from '@angular/core'; import {normalizePassiveListenerOptions} from '@angular/cdk/platform'; -import {asapScheduler, merge, Observable, of as observableOf, Subscription} from 'rxjs'; -import {delay, filter, take, takeUntil} from 'rxjs/operators'; +import {merge, Observable, of as observableOf, Subscription} from 'rxjs'; +import {filter, takeUntil} from 'rxjs/operators'; import {MatMenu, MenuCloseReason} from './menu'; import {throwMatMenuRecursiveError} from './menu-errors'; import {MatMenuItem} from './menu-item'; @@ -81,6 +81,9 @@ const passiveEventListenerOptions = normalizePassiveListenerOptions({passive: tr */ export const MENU_PANEL_TOP_PADDING = 8; +/** Mapping between menu panels and the last trigger that opened them. */ +const PANELS_TO_TRIGGERS = new WeakMap(); + /** Directive applied to an element that should trigger a `mat-menu`. */ @Directive({ selector: `[mat-menu-trigger-for], [matMenuTriggerFor]`, @@ -234,9 +237,8 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy { } ngOnDestroy() { - if (this._overlayRef) { - this._overlayRef.dispose(); - this._overlayRef = null; + if (this.menu && this._ownsMenu(this.menu)) { + PANELS_TO_TRIGGERS.delete(this.menu); } this._element.nativeElement.removeEventListener( @@ -248,6 +250,11 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy { this._menuCloseSubscription.unsubscribe(); this._closingActionsSubscription.unsubscribe(); this._hoverSubscription.unsubscribe(); + + if (this._overlayRef) { + this._overlayRef.dispose(); + this._overlayRef = null; + } } /** Whether the menu is open. */ @@ -335,7 +342,6 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy { return; } - const menu = this.menu; this._closingActionsSubscription.unsubscribe(); this._overlayRef.detach(); @@ -348,30 +354,10 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy { } this._openedBy = undefined; + this._setIsMenuOpen(false); - if (menu instanceof MatMenu) { - menu._resetAnimation(); - - if (menu.lazyContent) { - // Wait for the exit animation to finish before detaching the content. - menu._animationDone - .pipe( - filter(event => event.toState === 'void'), - take(1), - // Interrupt if the content got re-attached. - takeUntil(menu.lazyContent._attached), - ) - .subscribe({ - next: () => menu.lazyContent!.detach(), - // No matter whether the content got re-attached, reset the menu. - complete: () => this._setIsMenuOpen(false), - }); - } else { - this._setIsMenuOpen(false); - } - } else { - this._setIsMenuOpen(false); - menu?.lazyContent?.detach(); + if (this.menu && this._ownsMenu(this.menu)) { + PANELS_TO_TRIGGERS.delete(this.menu); } } @@ -380,6 +366,15 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy { * the menu was opened via the keyboard. */ private _initMenu(menu: MatMenuPanel): void { + const previousTrigger = PANELS_TO_TRIGGERS.get(menu); + + // If the same menu is currently attached to another trigger, + // we need to close it so it doesn't end up in a broken state. + if (previousTrigger && previousTrigger !== this) { + previousTrigger.closeMenu(); + } + + PANELS_TO_TRIGGERS.set(menu, this); menu.parentMenu = this.triggersSubmenu() ? this._parentMaterialMenu : undefined; menu.direction = this.dir; menu.focusFirstItem(this._openedBy || 'program'); @@ -520,10 +515,9 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy { const detachments = this._overlayRef!.detachments(); const parentClose = this._parentMaterialMenu ? this._parentMaterialMenu.closed : observableOf(); const hover = this._parentMaterialMenu - ? this._parentMaterialMenu._hovered().pipe( - filter(active => active !== this._menuItemInstance), - filter(() => this._menuOpen), - ) + ? this._parentMaterialMenu + ._hovered() + .pipe(filter(active => this._menuOpen && active !== this._menuItemInstance)) : observableOf(); return merge(backdrop, parentClose as Observable, hover, detachments); @@ -578,35 +572,14 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy { /** Handles the cases where the user hovers over the trigger. */ private _handleHover() { // Subscribe to changes in the hovered item in order to toggle the panel. - if (!this.triggersSubmenu() || !this._parentMaterialMenu) { - return; - } - - this._hoverSubscription = this._parentMaterialMenu - ._hovered() - // Since we might have multiple competing triggers for the same menu (e.g. a sub-menu - // with different data and triggers), we have to delay it by a tick to ensure that - // it won't be closed immediately after it is opened. - .pipe( - filter(active => active === this._menuItemInstance && !active.disabled), - delay(0, asapScheduler), - ) - .subscribe(() => { - this._openedBy = 'mouse'; - - // If the same menu is used between multiple triggers, it might still be animating - // while the new trigger tries to re-open it. Wait for the animation to finish - // before doing so. Also interrupt if the user moves to another item. - if (this.menu instanceof MatMenu && this.menu._isAnimating) { - // We need the `delay(0)` here in order to avoid - // 'changed after checked' errors in some cases. See #12194. - this.menu._animationDone - .pipe(take(1), delay(0, asapScheduler), takeUntil(this._parentMaterialMenu!._hovered())) - .subscribe(() => this.openMenu()); - } else { + if (this.triggersSubmenu() && this._parentMaterialMenu) { + this._hoverSubscription = this._parentMaterialMenu._hovered().subscribe(active => { + if (active === this._menuItemInstance && !active.disabled) { + this._openedBy = 'mouse'; this.openMenu(); } }); + } } /** Gets the portal that should be attached to the overlay. */ @@ -620,4 +593,13 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy { return this._portal; } + + /** + * Determines whether the trigger owns a specific menu panel, at the current point in time. + * This allows us to distinguish the case where the same panel is passed into multiple triggers + * and multiple are open at a time. + */ + private _ownsMenu(menu: MatMenuPanel): boolean { + return PANELS_TO_TRIGGERS.get(menu) === this; + } } diff --git a/src/material/menu/menu.spec.ts b/src/material/menu/menu.spec.ts index fc7de98f0928..d25a6c9713b1 100644 --- a/src/material/menu/menu.spec.ts +++ b/src/material/menu/menu.spec.ts @@ -1219,49 +1219,6 @@ describe('MatMenu', () => { .toBe(true); })); - it('should detach the lazy content when the menu is closed', fakeAsync(() => { - const fixture = 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 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).withContext('Expected menu to start off closed').toBe(false); - - trigger.openMenu(); - fixture.detectChanges(); - tick(500); - - expect(trigger.menuOpen).withContext('Expected menu to be open').toBe(true); - - trigger.closeMenu(); - fixture.detectChanges(); - - expect(trigger.menuOpen) - .withContext('Expected menu to be considered open while the close animation is running') - .toBe(true); - tick(500); - fixture.detectChanges(); - - expect(trigger.menuOpen).withContext('Expected menu to be closed').toBe(false); - })); - it('should focus the first menu item when opening a lazy menu via keyboard', async () => { const fixture = createComponent(SimpleLazyMenu); fixture.autoDetectChanges(); @@ -1741,15 +1698,12 @@ describe('MatMenu', () => { })); it('should complete the callback when the menu is destroyed', fakeAsync(() => { - const emitCallback = jasmine.createSpy('emit callback'); const completeCallback = jasmine.createSpy('complete callback'); - fixture.componentInstance.menu.closed.subscribe(emitCallback, null, completeCallback); + fixture.componentInstance.menu.closed.subscribe(null, null, completeCallback); fixture.destroy(); tick(500); - expect(emitCallback).toHaveBeenCalledWith(undefined); - expect(emitCallback).toHaveBeenCalledTimes(1); expect(completeCallback).toHaveBeenCalled(); })); });