skills/angular__angular
1
0
Fork 0
mirror of https://github.com/angular/angular.git synced 2026-09-14 13:54:52 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(menu): multiple close events for a single close (#7037)

Browse Source 
Don't emit a closed event when another event will be emitted. Previously, if one clicked on a menu item, one would get two events: `undefined` and `click` in that order. One would see similar behavior for `keydown` or clicking the backdrop. Unit tests were updated to prevent a regression.
This commit is contained in:
Philip Pham
2017-09-29 13:14:22 -07:00
committed by Andrew Seguin
parent b0b91f4307
commit 2dcb76c8f9
2 changed files with 51 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/lib/menu/menu-trigger.ts
+20 -15
View File
@@ -44,7 +44,6 @@ import {MatMenuItem} from './menu-item';
import {MatMenuPanel} from './menu-panel';
import {MenuPositionX, MenuPositionY} from './menu-positions';
/** Injection token that determines the scroll handling while the menu is open. */
export const MAT_MENU_SCROLL_STRATEGY =
new InjectionToken<() => ScrollStrategy>('mat-menu-scroll-strategy');
@@ -130,7 +129,7 @@ export class MatMenuTrigger implements AfterViewInit, OnDestroy {
this._checkMenu();
this.menu.close.subscribe(reason => {
this.closeMenu();
this._destroyMenu();
// If a click closed the menu, we should close the entire chain of nested menus.
if (reason === 'click' && this._parentMenu) {
@@ -182,7 +181,9 @@ export class MatMenuTrigger 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 MatMenu) {
@@ -193,16 +194,7 @@ export class MatMenuTrigger implements AfterViewInit, OnDestroy {
/** Closes the menu. */
closeMenu(): void {
if (this._overlayRef && this.menuOpen) {
this._resetMenu();
this._overlayRef.detach();
this._closeSubscription.unsubscribe();
this.menu.close.emit();
if (this.menu instanceof MatMenu) {
this.menu._resetAnimation();
}
}
this.menu.close.emit();
}
/** Focuses the menu trigger. */
@@ -210,6 +202,19 @@ export class MatMenuTrigger implements AfterViewInit, OnDestroy {
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();
if (this.menu instanceof MatMenu) {
this.menu._resetAnimation();
}
}
}
/**
* This method sets the menu state to open and focuses the first item if
* the menu was opened via the keyboard.
@@ -377,11 +382,11 @@ export class MatMenuTrigger 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(null);
const parentClose = this._parentMenu ? this._parentMenu.close : observableOf();
const hover = this._parentMenu ? RxChain.from(this._parentMenu.hover())
.call(filter, active => active !== this._menuItemInstance)
.call(filter, () => this._menuOpen)
.result() : observableOf(null);
.result() : observableOf();
return merge(backdrop, parentClose, hover);
}
src/lib/menu/menu.spec.ts
+31 -11
View File
@@ -98,7 +98,7 @@ describe('MatMenu', () => {
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();
@@ -494,26 +494,40 @@ describe('MatMenu', () => {
menuItem.click();
fixture.detectChanges();
expect(fixture.componentInstance.closeCallback).toHaveBeenCalled();
expect(fixture.componentInstance.closeCallback).toHaveBeenCalledWith('click');
expect(fixture.componentInstance.closeCallback).toHaveBeenCalledTimes(1);
});
it('should emit a close event when the backdrop is clicked', () => {
const backdrop = <HTMLElement>overlayContainerElement.querySelector('.cdk-overlay-backdrop');
const backdrop = overlayContainerElement
.querySelector('.cdk-overlay-backdrop') as HTMLElement;
backdrop.click();
fixture.detectChanges();
expect(fixture.componentInstance.closeCallback).toHaveBeenCalled();
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);
});
it('should complete the callback when the menu is destroyed', () => {
let emitCallback = jasmine.createSpy('emit callback');
let completeCallback = jasmine.createSpy('complete callback');
const emitCallback = jasmine.createSpy('emit callback');
const completeCallback = jasmine.createSpy('complete callback');
fixture.componentInstance.menu.close.subscribe(emitCallback, null, completeCallback);
fixture.destroy();
expect(emitCallback).toHaveBeenCalled();
expect(emitCallback).toHaveBeenCalledWith(undefined);
expect(emitCallback).toHaveBeenCalledTimes(1);
expect(completeCallback).toHaveBeenCalled();
});
});
@@ -995,6 +1009,9 @@ describe('MatMenu', () => {
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(() => {
@@ -1059,7 +1076,7 @@ describe('MatMenu default overrides', () => {
@Component({
template: `
<button [matMenuTriggerFor]="menu" #triggerEl>Toggle menu</button>
<mat-menu class="custom-one custom-two" #menu="matMenu" (close)="closeCallback()">
<mat-menu class="custom-one custom-two" #menu="matMenu" (close)="closeCallback($event)">
<button mat-menu-item> Item </button>
<button mat-menu-item disabled> Disabled </button>
</mat-menu>
@@ -1152,7 +1169,7 @@ class CustomMenu {
[matMenuTriggerFor]="levelTwo"
#alternateTrigger="matMenuTrigger">Toggle alternate menu</button>
<mat-menu #root="matMenu">
<mat-menu #root="matMenu" (close)="rootCloseCallback($event)">
<button mat-menu-item
id="level-one-trigger"
[matMenuTriggerFor]="levelOne"
@@ -1165,7 +1182,7 @@ class CustomMenu {
#lazyTrigger="matMenuTrigger">Three</button>
</mat-menu>
<mat-menu #levelOne="matMenu">
<mat-menu #levelOne="matMenu" (close)="levelOneCloseCallback($event)">
<button mat-menu-item>Four</button>
<button mat-menu-item
id="level-two-trigger"
@@ -1174,7 +1191,7 @@ class CustomMenu {
<button mat-menu-item>Six</button>
</mat-menu>
<mat-menu #levelTwo="matMenu">
<mat-menu #levelTwo="matMenu" (close)="levelTwoCloseCallback($event)">
<button mat-menu-item>Seven</button>
<button mat-menu-item>Eight</button>
<button mat-menu-item>Nine</button>
@@ -1192,12 +1209,15 @@ class NestedMenu {
@ViewChild('rootTrigger') rootTrigger: MatMenuTrigger;
@ViewChild('rootTriggerEl') rootTriggerEl: ElementRef;
@ViewChild('alternateTrigger') alternateTrigger: MatMenuTrigger;
readonly rootCloseCallback = jasmine.createSpy('root menu closed callback');
@ViewChild('levelOne') levelOneMenu: MatMenu;
@ViewChild('levelOneTrigger') levelOneTrigger: MatMenuTrigger;
readonly levelOneCloseCallback = jasmine.createSpy('level one menu closed callback');
@ViewChild('levelTwo') levelTwoMenu: MatMenu;
@ViewChild('levelTwoTrigger') levelTwoTrigger: MatMenuTrigger;
readonly levelTwoCloseCallback = jasmine.createSpy('level one menu closed callback');
@ViewChild('lazy') lazyMenu: MatMenu;
@ViewChild('lazyTrigger') lazyTrigger: MatMenuTrigger;