Skip to content

Commit dcfe515

Browse files
authored
Revert "fix(menu): multiple close events for a single close" (#7036)
* Revert "feat(datepicker): Add Moment.js adapter (#6860)" This reverts commit 9545427. * Revert "fix(menu): multiple close events for a single close (#6961)" This reverts commit 1cccd4b.
1 parent 9545427 commit dcfe515

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
@@ -98,7 +98,7 @@ describe('MdMenu', () => {
9898
expect(overlayContainerElement.textContent).toBe('');
9999
}));
100100

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

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

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

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

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

522509
it('should complete the callback when the menu is destroyed', () => {
523-
const emitCallback = jasmine.createSpy('emit callback');
524-
const completeCallback = jasmine.createSpy('complete callback');
510+
let emitCallback = jasmine.createSpy('emit callback');
511+
let completeCallback = jasmine.createSpy('complete callback');
525512

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

529-
expect(emitCallback).toHaveBeenCalledWith(undefined);
530-
expect(emitCallback).toHaveBeenCalledTimes(1);
516+
expect(emitCallback).toHaveBeenCalled();
531517
expect(completeCallback).toHaveBeenCalled();
532518
});
533519
});
@@ -1009,9 +995,6 @@ describe('MdMenu', () => {
1009995
tick(500);
1010996

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

10171000
it('should toggle a nested menu when its trigger is added after init', fakeAsync(() => {
@@ -1076,7 +1059,7 @@ describe('MdMenu default overrides', () => {
10761059
@Component({
10771060
template: `
10781061
<button [mdMenuTriggerFor]="menu" #triggerEl>Toggle menu</button>
1079-
<md-menu class="custom-one custom-two" #menu="mdMenu" (close)="closeCallback($event)">
1062+
<md-menu class="custom-one custom-two" #menu="mdMenu" (close)="closeCallback()">
10801063
<button md-menu-item> Item </button>
10811064
<button md-menu-item disabled> Disabled </button>
10821065
</md-menu>
@@ -1169,7 +1152,7 @@ class CustomMenu {
11691152
[mdMenuTriggerFor]="levelTwo"
11701153
#alternateTrigger="mdMenuTrigger">Toggle alternate menu</button>
11711154
1172-
<md-menu #root="mdMenu" (close)="rootCloseCallback($event)">
1155+
<md-menu #root="mdMenu">
11731156
<button md-menu-item
11741157
id="level-one-trigger"
11751158
[mdMenuTriggerFor]="levelOne"
@@ -1182,7 +1165,7 @@ class CustomMenu {
11821165
#lazyTrigger="mdMenuTrigger">Three</button>
11831166
</md-menu>
11841167
1185-
<md-menu #levelOne="mdMenu" (close)="levelOneCloseCallback($event)">
1168+
<md-menu #levelOne="mdMenu">
11861169
<button md-menu-item>Four</button>
11871170
<button md-menu-item
11881171
id="level-two-trigger"
@@ -1191,7 +1174,7 @@ class CustomMenu {
11911174
<button md-menu-item>Six</button>
11921175
</md-menu>
11931176
1194-
<md-menu #levelTwo="mdMenu" (close)="levelTwoCloseCallback($event)">
1177+
<md-menu #levelTwo="mdMenu">
11951178
<button md-menu-item>Seven</button>
11961179
<button md-menu-item>Eight</button>
11971180
<button md-menu-item>Nine</button>
@@ -1209,15 +1192,12 @@ class NestedMenu {
12091192
@ViewChild('rootTrigger') rootTrigger: MdMenuTrigger;
12101193
@ViewChild('rootTriggerEl') rootTriggerEl: ElementRef;
12111194
@ViewChild('alternateTrigger') alternateTrigger: MdMenuTrigger;
1212-
readonly rootCloseCallback = jasmine.createSpy('root menu closed callback');
12131195

12141196
@ViewChild('levelOne') levelOneMenu: MdMenu;
12151197
@ViewChild('levelOneTrigger') levelOneTrigger: MdMenuTrigger;
1216-
readonly levelOneCloseCallback = jasmine.createSpy('level one menu closed callback');
12171198

12181199
@ViewChild('levelTwo') levelTwoMenu: MdMenu;
12191200
@ViewChild('levelTwoTrigger') levelTwoTrigger: MdMenuTrigger;
1220-
readonly levelTwoCloseCallback = jasmine.createSpy('level one menu closed callback');
12211201

12221202
@ViewChild('lazy') lazyMenu: MdMenu;
12231203
@ViewChild('lazyTrigger') lazyTrigger: MdMenuTrigger;

0 commit comments

Comments
 (0)