Skip to content

Commit 4011b40

Browse files
authored
Revert "fix(menu): multiple close events for a single close (#6961)"
This reverts commit 1cccd4b.
1 parent 4bddcee commit 4011b40

File tree

2 files changed

+22
-48
lines changed

2 files changed

+22
-48
lines changed

src/lib/menu/menu-trigger.ts

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@ import {MdMenuItem} from './menu-item';
4343
import {MdMenuPanel} from './menu-panel';
4444
import {MenuPositionX, MenuPositionY} from './menu-positions';
4545
import {throwMdMenuMissingError} from './menu-errors';
46-
import {merge} from 'rxjs/observable/merge';
4746
import {of as observableOf} from 'rxjs/observable/of';
47+
import {merge} from 'rxjs/observable/merge';
4848
import {Subscription} from 'rxjs/Subscription';
4949

5050
/** Injection token that determines the scroll handling while the menu is open. */
@@ -153,7 +153,7 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy {
153153
this._checkMenu();
154154

155155
this.menu.close.subscribe(reason => {
156-
this._destroyMenu();
156+
this.closeMenu();
157157

158158
// If a click closed the menu, we should close the entire chain of nested menus.
159159
if (reason === 'click' && this._parentMenu) {
@@ -205,9 +205,7 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy {
205205
openMenu(): void {
206206
if (!this._menuOpen) {
207207
this._createOverlay().attach(this._portal);
208-
this._closeSubscription = this._menuClosingActions().subscribe(() => {
209-
this.menu.close.emit();
210-
});
208+
this._closeSubscription = this._menuClosingActions().subscribe(() => this.menu.close.emit());
211209
this._initMenu();
212210

213211
if (this.menu instanceof MdMenu) {
@@ -218,27 +216,23 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy {
218216

219217
/** Closes the menu. */
220218
closeMenu(): void {
221-
this.menu.close.emit();
222-
}
223-
224-
/** Focuses the menu trigger. */
225-
focus() {
226-
this._element.nativeElement.focus();
227-
}
228-
229-
/** Closes the menu and does the necessary cleanup. */
230-
private _destroyMenu() {
231219
if (this._overlayRef && this.menuOpen) {
232220
this._resetMenu();
233221
this._overlayRef.detach();
234222
this._closeSubscription.unsubscribe();
223+
this.menu.close.emit();
235224

236225
if (this.menu instanceof MdMenu) {
237226
this.menu._resetAnimation();
238227
}
239228
}
240229
}
241230

231+
/** Focuses the menu trigger. */
232+
focus() {
233+
this._element.nativeElement.focus();
234+
}
235+
242236
/**
243237
* This method sets the menu state to open and focuses the first item if
244238
* the menu was opened via the keyboard.
@@ -406,11 +400,11 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy {
406400
/** Returns a stream that emits whenever an action that should close the menu occurs. */
407401
private _menuClosingActions() {
408402
const backdrop = this._overlayRef!.backdropClick();
409-
const parentClose = this._parentMenu ? this._parentMenu.close : observableOf();
403+
const parentClose = this._parentMenu ? this._parentMenu.close : observableOf(null);
410404
const hover = this._parentMenu ? RxChain.from(this._parentMenu.hover())
411405
.call(filter, active => active !== this._menuItemInstance)
412406
.call(filter, () => this._menuOpen)
413-
.result() : observableOf();
407+
.result() : observableOf(null);
414408

415409
return merge(backdrop, parentClose, hover);
416410
}

src/lib/menu/menu.spec.ts

Lines changed: 11 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ describe('MdMenu', () => {
9797
expect(overlayContainerElement.textContent).toBe('');
9898
}));
9999

100-
it('should close the menu when pressing ESCAPE', fakeAsync(() => {
100+
it('should close the menu when pressing escape', fakeAsync(() => {
101101
const fixture = TestBed.createComponent(SimpleMenu);
102102
fixture.detectChanges();
103103
fixture.componentInstance.trigger.openMenu();
@@ -493,40 +493,26 @@ describe('MdMenu', () => {
493493
menuItem.click();
494494
fixture.detectChanges();
495495

496-
expect(fixture.componentInstance.closeCallback).toHaveBeenCalledWith('click');
497-
expect(fixture.componentInstance.closeCallback).toHaveBeenCalledTimes(1);
496+
expect(fixture.componentInstance.closeCallback).toHaveBeenCalled();
498497
});
499498

500499
it('should emit a close event when the backdrop is clicked', () => {
501-
const backdrop = overlayContainerElement
502-
.querySelector('.cdk-overlay-backdrop') as HTMLElement;
500+
const backdrop = <HTMLElement>overlayContainerElement.querySelector('.cdk-overlay-backdrop');
503501

504502
backdrop.click();
505503
fixture.detectChanges();
506504

507-
expect(fixture.componentInstance.closeCallback).toHaveBeenCalledWith(undefined);
508-
expect(fixture.componentInstance.closeCallback).toHaveBeenCalledTimes(1);
509-
});
510-
511-
it('should emit an event when pressing ESCAPE', () => {
512-
const menu = overlayContainerElement.querySelector('.mat-menu-panel') as HTMLElement;
513-
514-
dispatchKeyboardEvent(menu, 'keydown', ESCAPE);
515-
fixture.detectChanges();
516-
517-
expect(fixture.componentInstance.closeCallback).toHaveBeenCalledWith('keydown');
518-
expect(fixture.componentInstance.closeCallback).toHaveBeenCalledTimes(1);
505+
expect(fixture.componentInstance.closeCallback).toHaveBeenCalled();
519506
});
520507

521508
it('should complete the callback when the menu is destroyed', () => {
522-
const emitCallback = jasmine.createSpy('emit callback');
523-
const completeCallback = jasmine.createSpy('complete callback');
509+
let emitCallback = jasmine.createSpy('emit callback');
510+
let completeCallback = jasmine.createSpy('complete callback');
524511

525512
fixture.componentInstance.menu.close.subscribe(emitCallback, null, completeCallback);
526513
fixture.destroy();
527514

528-
expect(emitCallback).toHaveBeenCalledWith(undefined);
529-
expect(emitCallback).toHaveBeenCalledTimes(1);
515+
expect(emitCallback).toHaveBeenCalled();
530516
expect(completeCallback).toHaveBeenCalled();
531517
});
532518
});
@@ -1008,9 +994,6 @@ describe('MdMenu', () => {
1008994
tick(500);
1009995

1010996
expect(overlay.querySelectorAll('.mat-menu-panel').length).toBe(0, 'Expected no open menus');
1011-
expect(instance.rootCloseCallback).toHaveBeenCalledTimes(1);
1012-
expect(instance.levelOneCloseCallback).toHaveBeenCalledTimes(1);
1013-
expect(instance.levelTwoCloseCallback).toHaveBeenCalledTimes(1);
1014997
}));
1015998

1016999
it('should toggle a nested menu when its trigger is added after init', fakeAsync(() => {
@@ -1061,7 +1044,7 @@ describe('MdMenu default overrides', () => {
10611044
@Component({
10621045
template: `
10631046
<button [mdMenuTriggerFor]="menu" #triggerEl>Toggle menu</button>
1064-
<md-menu class="custom-one custom-two" #menu="mdMenu" (close)="closeCallback($event)">
1047+
<md-menu class="custom-one custom-two" #menu="mdMenu" (close)="closeCallback()">
10651048
<button md-menu-item> Item </button>
10661049
<button md-menu-item disabled> Disabled </button>
10671050
</md-menu>
@@ -1154,7 +1137,7 @@ class CustomMenu {
11541137
[mdMenuTriggerFor]="levelTwo"
11551138
#alternateTrigger="mdMenuTrigger">Toggle alternate menu</button>
11561139
1157-
<md-menu #root="mdMenu" (close)="rootCloseCallback($event)">
1140+
<md-menu #root="mdMenu">
11581141
<button md-menu-item
11591142
id="level-one-trigger"
11601143
[mdMenuTriggerFor]="levelOne"
@@ -1167,7 +1150,7 @@ class CustomMenu {
11671150
#lazyTrigger="mdMenuTrigger">Three</button>
11681151
</md-menu>
11691152
1170-
<md-menu #levelOne="mdMenu" (close)="levelOneCloseCallback($event)">
1153+
<md-menu #levelOne="mdMenu">
11711154
<button md-menu-item>Four</button>
11721155
<button md-menu-item
11731156
id="level-two-trigger"
@@ -1176,7 +1159,7 @@ class CustomMenu {
11761159
<button md-menu-item>Six</button>
11771160
</md-menu>
11781161
1179-
<md-menu #levelTwo="mdMenu" (close)="levelTwoCloseCallback($event)">
1162+
<md-menu #levelTwo="mdMenu">
11801163
<button md-menu-item>Seven</button>
11811164
<button md-menu-item>Eight</button>
11821165
<button md-menu-item>Nine</button>
@@ -1194,15 +1177,12 @@ class NestedMenu {
11941177
@ViewChild('rootTrigger') rootTrigger: MdMenuTrigger;
11951178
@ViewChild('rootTriggerEl') rootTriggerEl: ElementRef;
11961179
@ViewChild('alternateTrigger') alternateTrigger: MdMenuTrigger;
1197-
readonly rootCloseCallback = jasmine.createSpy('root menu closed callback');
11981180

11991181
@ViewChild('levelOne') levelOneMenu: MdMenu;
12001182
@ViewChild('levelOneTrigger') levelOneTrigger: MdMenuTrigger;
1201-
readonly levelOneCloseCallback = jasmine.createSpy('level one menu closed callback');
12021183

12031184
@ViewChild('levelTwo') levelTwoMenu: MdMenu;
12041185
@ViewChild('levelTwoTrigger') levelTwoTrigger: MdMenuTrigger;
1205-
readonly levelTwoCloseCallback = jasmine.createSpy('level one menu closed callback');
12061186

12071187
@ViewChild('lazy') lazyMenu: MdMenu;
12081188
@ViewChild('lazyTrigger') lazyTrigger: MdMenuTrigger;

0 commit comments

Comments
 (0)