Skip to content

Commit 0c51b2c

Browse files
committed
fix(menu): not closed correctly if opened by different trigger while visible
Fixes the `MatMenu` overlay not being detached if the menu is opened by a different trigger while the panel is still open. Fixes #15354.
1 parent 92448bc commit 0c51b2c

File tree

5 files changed

+62
-8
lines changed

5 files changed

+62
-8
lines changed

src/material/menu/menu-panel.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ export interface MatMenuPanel<T = any> {
3333
focusFirstItem: (origin?: FocusOrigin) => void;
3434
resetActiveItem: () => void;
3535
setPositionClasses?: (x: MenuPositionX, y: MenuPositionY) => void;
36+
openedBy?: EventEmitter<any>;
3637
setElevation?(depth: number): void;
3738
lazyContent?: MatMenuContent;
3839
backdropClass?: string;

src/material/menu/menu-trigger.ts

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ import {
3434
ViewContainerRef,
3535
} from '@angular/core';
3636
import {normalizePassiveListenerOptions} from '@angular/cdk/platform';
37-
import {asapScheduler, merge, of as observableOf, Subscription} from 'rxjs';
37+
import {asapScheduler, merge, of as observableOf, Subscription, Subject} from 'rxjs';
3838
import {delay, filter, take, takeUntil} from 'rxjs/operators';
3939
import {MatMenu} from './menu';
4040
import {throwMatMenuMissingError} from './menu-errors';
@@ -87,7 +87,7 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
8787
private _menuOpen: boolean = false;
8888
private _closingActionsSubscription = Subscription.EMPTY;
8989
private _hoverSubscription = Subscription.EMPTY;
90-
private _menuCloseSubscription = Subscription.EMPTY;
90+
private _menuChanged = new Subject<void>();
9191
private _scrollStrategy: () => ScrollStrategy;
9292

9393
/**
@@ -119,10 +119,18 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
119119
}
120120

121121
this._menu = menu;
122-
this._menuCloseSubscription.unsubscribe();
122+
this._menuChanged.next();
123123

124124
if (menu) {
125-
this._menuCloseSubscription = menu.close.asObservable().subscribe(reason => {
125+
if (menu.openedBy) {
126+
menu.openedBy.pipe(takeUntil(this._menuChanged)).subscribe((reason?: MatMenuTrigger) => {
127+
if (reason && reason !== this) {
128+
this._destroyMenu();
129+
}
130+
});
131+
}
132+
133+
menu.close.pipe(takeUntil(this._menuChanged)).subscribe(reason => {
126134
this._destroyMenu();
127135

128136
// If a click closed the menu, we should close the entire chain of nested menus.
@@ -201,9 +209,9 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
201209
this._element.nativeElement.removeEventListener('touchstart', this._handleTouchStart,
202210
passiveEventListenerOptions);
203211

204-
this._menuCloseSubscription.unsubscribe();
205212
this._closingActionsSubscription.unsubscribe();
206213
this._hoverSubscription.unsubscribe();
214+
this._menuChanged.complete();
207215
}
208216

209217
/** Whether the menu is open. */
@@ -315,11 +323,16 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
315323
* the menu was opened via the keyboard.
316324
*/
317325
private _initMenu(): void {
318-
this.menu.parentMenu = this.triggersSubmenu() ? this._parentMenu : undefined;
319-
this.menu.direction = this.dir;
326+
const menu = this.menu;
327+
menu.parentMenu = this.triggersSubmenu() ? this._parentMenu : undefined;
328+
menu.direction = this.dir;
320329
this._setMenuElevation();
321330
this._setIsMenuOpen(true);
322-
this.menu.focusFirstItem(this._openedBy || 'program');
331+
menu.focusFirstItem(this._openedBy || 'program');
332+
333+
if (menu.openedBy) {
334+
menu.openedBy.emit(this);
335+
}
323336
}
324337

325338
/** Updates the menu elevation based on the amount of parent menus that it has. */

src/material/menu/menu.spec.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -743,6 +743,21 @@ describe('MatMenu', () => {
743743
flush();
744744
}));
745745

746+
it('should close the menu if it is opened by a different trigger', fakeAsync(() => {
747+
const fixture = createComponent(MenuWithMultipleTriggers);
748+
fixture.detectChanges();
749+
750+
fixture.componentInstance.triggers.first.openMenu();
751+
fixture.detectChanges();
752+
flush();
753+
expect(overlayContainerElement.querySelectorAll('.mat-menu-panel').length).toBe(1);
754+
755+
fixture.componentInstance.triggers.last.openMenu();
756+
fixture.detectChanges();
757+
flush();
758+
expect(overlayContainerElement.querySelectorAll('.mat-menu-panel').length).toBe(1);
759+
}));
760+
746761
describe('lazy rendering', () => {
747762
it('should be able to render the menu content lazily', fakeAsync(() => {
748763
const fixture = createComponent(SimpleLazyMenu);
@@ -2313,3 +2328,18 @@ class DynamicPanelMenu {
23132328
class MenuWithCheckboxItems {
23142329
@ViewChild(MatMenuTrigger, {static: false}) trigger: MatMenuTrigger;
23152330
}
2331+
2332+
2333+
@Component({
2334+
template: `
2335+
<button [matMenuTriggerFor]="menu">First trigger</button>
2336+
<button [matMenuTriggerFor]="menu">Second trigger</button>
2337+
<mat-menu #menu="matMenu">
2338+
<button mat-menu-item>Item one</button>
2339+
<button mat-menu-item>Item two</button>
2340+
</mat-menu>
2341+
`
2342+
})
2343+
class MenuWithMultipleTriggers {
2344+
@ViewChildren(MatMenuTrigger) triggers: QueryList<MatMenuTrigger>;
2345+
}

src/material/menu/menu.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,13 @@ export class _MatMenuBase implements AfterContentInit, MatMenuPanel<MatMenuItem>
225225
@Output() readonly closed: EventEmitter<void | 'click' | 'keydown' | 'tab'> =
226226
new EventEmitter<void | 'click' | 'keydown' | 'tab'>();
227227

228+
229+
/**
230+
* Stream that emits the trigger that the menu was opened by.
231+
* @docs-private
232+
*/
233+
@Output() openedBy = new EventEmitter<any>();
234+
228235
/**
229236
* Event emitted when the menu is closed.
230237
* @deprecated Switch to `closed` instead
@@ -249,6 +256,7 @@ export class _MatMenuBase implements AfterContentInit, MatMenuPanel<MatMenuItem>
249256
ngOnDestroy() {
250257
this._tabSubscription.unsubscribe();
251258
this.closed.complete();
259+
this.openedBy.complete();
252260
}
253261

254262
/** Stream that emits whenever the hovered menu item changes. */

tools/public_api_guard/material/menu.d.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ export declare class _MatMenuBase implements AfterContentInit, MatMenuPanel<MatM
1717
hasBackdrop: boolean | undefined;
1818
items: QueryList<MatMenuItem>;
1919
lazyContent: MatMenuContent;
20+
openedBy: EventEmitter<any>;
2021
overlapTrigger: boolean;
2122
panelClass: string;
2223
parentMenu: MatMenuPanel | undefined;
@@ -102,6 +103,7 @@ export interface MatMenuPanel<T = any> {
102103
focusFirstItem: (origin?: FocusOrigin) => void;
103104
hasBackdrop?: boolean;
104105
lazyContent?: MatMenuContent;
106+
openedBy?: EventEmitter<any>;
105107
overlapTrigger: boolean;
106108
parentMenu?: MatMenuPanel | undefined;
107109
removeItem?: (item: T) => void;

0 commit comments

Comments
 (0)