Skip to content

Commit 4d8c712

Browse files
crisbetoandrewseguin
authored andcommitted
fix(menu): not closing when overlay is detached externally (#8868)
Fixes the menu panel not being closed when its `OverlayRef` is detached externally using `detach`, for example when using the `CloseScrollStrategy`.
1 parent 0b05a1f commit 4d8c712

File tree

3 files changed

+42
-8
lines changed

3 files changed

+42
-8
lines changed

src/lib/menu/menu-directive.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,6 @@ export class MatMenu implements AfterContentInit, MatMenuPanel, OnDestroy {
188188

189189
ngOnDestroy() {
190190
this._tabSubscription.unsubscribe();
191-
this.closed.emit();
192191
this.closed.complete();
193192
}
194193

src/lib/menu/menu-trigger.ts

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -193,9 +193,7 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
193193
openMenu(): void {
194194
if (!this._menuOpen) {
195195
this._createOverlay().attach(this._portal);
196-
this._closeSubscription = this._menuClosingActions().subscribe(() => {
197-
this.menu.close.emit();
198-
});
196+
this._closeSubscription = this._menuClosingActions().subscribe(() => this.closeMenu());
199197
this._initMenu();
200198

201199
if (this.menu instanceof MatMenu) {
@@ -218,8 +216,8 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
218216
private _destroyMenu() {
219217
if (this._overlayRef && this.menuOpen) {
220218
this._resetMenu();
221-
this._overlayRef.detach();
222219
this._closeSubscription.unsubscribe();
220+
this._overlayRef.detach();
223221

224222
if (this.menu instanceof MatMenu) {
225223
this.menu._resetAnimation();
@@ -400,13 +398,14 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
400398
/** Returns a stream that emits whenever an action that should close the menu occurs. */
401399
private _menuClosingActions() {
402400
const backdrop = this._overlayRef!.backdropClick();
401+
const detachments = this._overlayRef!.detachments();
403402
const parentClose = this._parentMenu ? this._parentMenu.close : observableOf();
404403
const hover = this._parentMenu ? this._parentMenu._hovered().pipe(
405404
filter(active => active !== this._menuItemInstance),
406405
filter(() => this._menuOpen)
407406
) : observableOf();
408407

409-
return merge(backdrop, parentClose, hover);
408+
return merge(backdrop, parentClose, hover, detachments);
410409
}
411410

412411
/** Handles mouse presses on the trigger. */

src/lib/menu/menu.spec.ts

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {
1313
QueryList,
1414
} from '@angular/core';
1515
import {Direction, Directionality} from '@angular/cdk/bidi';
16-
import {OverlayContainer} from '@angular/cdk/overlay';
16+
import {OverlayContainer, Overlay} from '@angular/cdk/overlay';
1717
import {ESCAPE, LEFT_ARROW, RIGHT_ARROW} from '@angular/cdk/keycodes';
1818
import {
1919
MAT_MENU_DEFAULT_OPTIONS,
@@ -25,7 +25,7 @@ import {
2525
MenuPositionY,
2626
MatMenuItem,
2727
} from './index';
28-
import {MENU_PANEL_TOP_PADDING} from './menu-trigger';
28+
import {MENU_PANEL_TOP_PADDING, MAT_MENU_SCROLL_STRATEGY} from './menu-trigger';
2929
import {MatRipple} from '@angular/material/core';
3030
import {
3131
dispatchKeyboardEvent,
@@ -35,6 +35,8 @@ import {
3535
createMouseEvent,
3636
dispatchFakeEvent,
3737
} from '@angular/cdk/testing';
38+
import {Subject} from 'rxjs/Subject';
39+
import {ScrollDispatcher} from '@angular/cdk/scrolling';
3840

3941

4042
describe('MatMenu', () => {
@@ -242,6 +244,40 @@ describe('MatMenu', () => {
242244
expect(document.activeElement).toBe(panel, 'Expected the panel to be focused.');
243245
});
244246

247+
it('should close the menu when using the CloseScrollStrategy', fakeAsync(() => {
248+
const scrolledSubject = new Subject();
249+
250+
TestBed
251+
.resetTestingModule()
252+
.configureTestingModule({
253+
imports: [MatMenuModule, NoopAnimationsModule],
254+
declarations: [SimpleMenu, FakeIcon],
255+
providers: [
256+
{provide: ScrollDispatcher, useFactory: () => ({scrolled: () => scrolledSubject})},
257+
{
258+
provide: MAT_MENU_SCROLL_STRATEGY,
259+
deps: [Overlay],
260+
useFactory: (overlay: Overlay) => () => overlay.scrollStrategies.close()
261+
}
262+
]
263+
})
264+
.compileComponents();
265+
266+
const fixture = TestBed.createComponent(SimpleMenu);
267+
const trigger = fixture.componentInstance.trigger;
268+
269+
fixture.detectChanges();
270+
trigger.openMenu();
271+
fixture.detectChanges();
272+
273+
expect(trigger.menuOpen).toBe(true);
274+
275+
scrolledSubject.next();
276+
tick(500);
277+
278+
expect(trigger.menuOpen).toBe(false);
279+
}));
280+
245281
describe('positions', () => {
246282
let fixture: ComponentFixture<PositionedMenu>;
247283
let panel: HTMLElement;

0 commit comments

Comments
 (0)