Skip to content

Commit

Permalink
Revert "fix(menu): multiple close events for a single close" (angular…
Browse files Browse the repository at this point in the history 
…#7036)

* Revert "feat(datepicker): Add Moment.js adapter (angular#6860)"

This reverts commit 9545427.

* Revert "fix(menu): multiple close events for a single close (angular#6961)"

This reverts commit 1cccd4b.
  • Loading branch information
@mmalerba
mmalerba committed Sep 13, 2017
1 parent 9545427 commit dcfe515
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 48 deletions.
28 changes: 11 additions & 17 deletions src/lib/menu/menu-trigger.ts
View file
Expand Up @@ -43,8 +43,8 @@ import {MdMenuItem} from './menu-item';
import {MdMenuPanel} from './menu-panel';
import {MenuPositionX, MenuPositionY} from './menu-positions';
import {throwMdMenuMissingError} from './menu-errors';
import {merge} from 'rxjs/observable/merge';
import {of as observableOf} from 'rxjs/observable/of';
import {merge} from 'rxjs/observable/merge';
import {Subscription} from 'rxjs/Subscription';

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

this.menu.close.subscribe(reason => {
this._destroyMenu();
this.closeMenu();

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

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

/** Closes the menu. */
closeMenu(): void {
this.menu.close.emit();
}

/** Focuses the menu trigger. */
focus() {
this._element.nativeElement.focus();
}

/** Closes the menu and does the necessary cleanup. */
private _destroyMenu() {
if (this._overlayRef && this.menuOpen) {
this._resetMenu();
this._overlayRef.detach();
this._closeSubscription.unsubscribe();
this.menu.close.emit();

if (this.menu instanceof MdMenu) {
this.menu._resetAnimation();
}
}
}

/** Focuses the menu trigger. */
focus() {
this._element.nativeElement.focus();
}

/**
* This method sets the menu state to open and focuses the first item if
* the menu was opened via the keyboard.
Expand Down Expand Up @@ -406,11 +400,11 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy {
/** Returns a stream that emits whenever an action that should close the menu occurs. */
private _menuClosingActions() {
const backdrop = this._overlayRef!.backdropClick();
const parentClose = this._parentMenu ? this._parentMenu.close : observableOf();
const parentClose = this._parentMenu ? this._parentMenu.close : observableOf(null);
const hover = this._parentMenu ? RxChain.from(this._parentMenu.hover())
.call(filter, active => active !== this._menuItemInstance)
.call(filter, () => this._menuOpen)
.result() : observableOf();
.result() : observableOf(null);

return merge(backdrop, parentClose, hover);
}
Expand Down
42 changes: 11 additions & 31 deletions src/lib/menu/menu.spec.ts
View file
Expand Up @@ -98,7 +98,7 @@ describe('MdMenu', () => {
expect(overlayContainerElement.textContent).toBe('');
}));

it('should close the menu when pressing ESCAPE', fakeAsync(() => {
it('should close the menu when pressing escape', fakeAsync(() => {
const fixture = TestBed.createComponent(SimpleMenu);
fixture.detectChanges();
fixture.componentInstance.trigger.openMenu();
Expand Down Expand Up @@ -494,40 +494,26 @@ describe('MdMenu', () => {
menuItem.click();
fixture.detectChanges();

expect(fixture.componentInstance.closeCallback).toHaveBeenCalledWith('click');
expect(fixture.componentInstance.closeCallback).toHaveBeenCalledTimes(1);
expect(fixture.componentInstance.closeCallback).toHaveBeenCalled();
});

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

backdrop.click();
fixture.detectChanges();

expect(fixture.componentInstance.closeCallback).toHaveBeenCalledWith(undefined);
expect(fixture.componentInstance.closeCallback).toHaveBeenCalledTimes(1);
});

it('should emit an event when pressing ESCAPE', () => {
const menu = overlayContainerElement.querySelector('.mat-menu-panel') as HTMLElement;

dispatchKeyboardEvent(menu, 'keydown', ESCAPE);
fixture.detectChanges();

expect(fixture.componentInstance.closeCallback).toHaveBeenCalledWith('keydown');
expect(fixture.componentInstance.closeCallback).toHaveBeenCalledTimes(1);
expect(fixture.componentInstance.closeCallback).toHaveBeenCalled();
});

it('should complete the callback when the menu is destroyed', () => {
const emitCallback = jasmine.createSpy('emit callback');
const completeCallback = jasmine.createSpy('complete callback');
let emitCallback = jasmine.createSpy('emit callback');
let completeCallback = jasmine.createSpy('complete callback');

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

expect(emitCallback).toHaveBeenCalledWith(undefined);
expect(emitCallback).toHaveBeenCalledTimes(1);
expect(emitCallback).toHaveBeenCalled();
expect(completeCallback).toHaveBeenCalled();
});
});
Expand Down Expand Up @@ -1009,9 +995,6 @@ describe('MdMenu', () => {
tick(500);

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

it('should toggle a nested menu when its trigger is added after init', fakeAsync(() => {
Expand Down Expand Up @@ -1076,7 +1059,7 @@ describe('MdMenu default overrides', () => {
@Component({
template: `
<button [mdMenuTriggerFor]="menu" #triggerEl>Toggle menu</button>
<md-menu class="custom-one custom-two" #menu="mdMenu" (close)="closeCallback($event)">
<md-menu class="custom-one custom-two" #menu="mdMenu" (close)="closeCallback()">
<button md-menu-item> Item </button>
<button md-menu-item disabled> Disabled </button>
</md-menu>
Expand Down Expand Up @@ -1169,7 +1152,7 @@ class CustomMenu {
[mdMenuTriggerFor]="levelTwo"
#alternateTrigger="mdMenuTrigger">Toggle alternate menu</button>
<md-menu #root="mdMenu" (close)="rootCloseCallback($event)">
<md-menu #root="mdMenu">
<button md-menu-item
id="level-one-trigger"
[mdMenuTriggerFor]="levelOne"
Expand All @@ -1182,7 +1165,7 @@ class CustomMenu {
#lazyTrigger="mdMenuTrigger">Three</button>
</md-menu>
<md-menu #levelOne="mdMenu" (close)="levelOneCloseCallback($event)">
<md-menu #levelOne="mdMenu">
<button md-menu-item>Four</button>
<button md-menu-item
id="level-two-trigger"
Expand All @@ -1191,7 +1174,7 @@ class CustomMenu {
<button md-menu-item>Six</button>
</md-menu>
<md-menu #levelTwo="mdMenu" (close)="levelTwoCloseCallback($event)">
<md-menu #levelTwo="mdMenu">
<button md-menu-item>Seven</button>
<button md-menu-item>Eight</button>
<button md-menu-item>Nine</button>
Expand All @@ -1209,15 +1192,12 @@ class NestedMenu {
@ViewChild('rootTrigger') rootTrigger: MdMenuTrigger;
@ViewChild('rootTriggerEl') rootTriggerEl: ElementRef;
@ViewChild('alternateTrigger') alternateTrigger: MdMenuTrigger;
readonly rootCloseCallback = jasmine.createSpy('root menu closed callback');

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

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

@ViewChild('lazy') lazyMenu: MdMenu;
@ViewChild('lazyTrigger') lazyTrigger: MdMenuTrigger;
Expand Down

0 comments on commit dcfe515

Please sign in to comment.