Skip to content

Commit

Permalink
fix(cdk-experimental/menu): ensure context menu is closed out with ot…
Browse files Browse the repository at this point in the history
…her menu elements on page

Fixes an issue which prevents an open context menu from closing out if clicked on a menu bar
element or on an inline menu. Further fixes an issue where opening a context menu does not close
out any open menu.
  • Loading branch information
andy9775 committed Aug 10, 2020
1 parent 25ce323 commit 1dee741
Show file tree
Hide file tree
Showing 5 changed files with 147 additions and 3 deletions.
140 changes: 140 additions & 0 deletions src/cdk-experimental/menu/context-menu.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {dispatchMouseEvent} from '@angular/cdk/testing/private';
import {By} from '@angular/platform-browser';
import {CdkMenuItem} from './menu-item';
import {CdkMenuItemTrigger} from './menu-item-trigger';
import {CdkMenuBar} from './menu-bar';

describe('CdkContextMenuTrigger', () => {
describe('with simple context menu trigger', () => {
Expand Down Expand Up @@ -246,6 +247,110 @@ describe('CdkContextMenuTrigger', () => {
expect(instance.copyMenu).toBeDefined();
});
});

describe('with menubar and inline menu on page', () => {
let fixture: ComponentFixture<ContextMenuWithMenuBarAndInlineMenu>;
let nativeMenuBar: HTMLElement;
let nativeMenuBarTrigger: HTMLElement;

beforeEach(async(() => {
TestBed.configureTestingModule({
imports: [CdkMenuModule],
declarations: [ContextMenuWithMenuBarAndInlineMenu],
}).compileComponents();
}));

beforeEach(() => {
fixture = TestBed.createComponent(ContextMenuWithMenuBarAndInlineMenu);
fixture.detectChanges();

nativeMenuBar = fixture.componentInstance.nativeMenuBar.nativeElement;
nativeMenuBarTrigger = fixture.componentInstance.nativeMenuBarTrigger.nativeElement;
});

/** Get the menu opened by the context menu trigger. */
function getContextMenu() {
return fixture.componentInstance.contextMenu;
}

/** Get the menu opened by the menu bar item trigger. */
function getFileMenu() {
return fixture.componentInstance.fileMenu;
}

/** Get the context in which the context menu should trigger. */
function getMenuContext() {
return fixture.componentInstance.trigger.nativeElement;
}

/** Get the inline menus trigger element. */
function getInlineMenuTrigger() {
return fixture.componentInstance.nativeInlineMenuButton.nativeElement;
}

/** Return the native element for the inline menu. */
function getInlineMenuElement() {
return fixture.componentInstance.nativeInlineMenu.nativeElement;
}

/** Open up the context menu and run change detection. */
function openContextMenu() {
// right click triggers a context menu event
dispatchMouseEvent(getMenuContext(), 'contextmenu');
dispatchMouseEvent(getMenuContext(), 'mousedown');
fixture.detectChanges();
}

/** Open up the file menu from the menu bar. */
function openFileMenu() {
nativeMenuBarTrigger.click();
fixture.detectChanges();
}

it('should close the open context menu when clicking on the menubar element', () => {
openContextMenu();

dispatchMouseEvent(nativeMenuBar, 'click');
fixture.detectChanges();

expect(getContextMenu()).not.toBeDefined();
});

it('should close the open context menu when clicking on the menubar menu item', () => {
openContextMenu();

nativeMenuBarTrigger.click();
fixture.detectChanges();

expect(getContextMenu()).not.toBeDefined();
});

it('should close the open context menu when clicking on the inline menu element', () => {
openContextMenu();

getInlineMenuElement().click();
fixture.detectChanges();

expect(getContextMenu()).not.toBeDefined();
});

it('should close the open context menu when clicking on an inline menu item', () => {
openContextMenu();

getInlineMenuTrigger().click();
fixture.detectChanges();

expect(getContextMenu()).not.toBeDefined();
});

it('should close the open menu when opening a context menu', () => {
openFileMenu();

openContextMenu();

expect(getFileMenu()).not.toBeDefined();
});
});
});

@Component({
Expand Down Expand Up @@ -317,3 +422,38 @@ class ContextMenuWithSubmenu {
@ViewChild('cut_menu', {read: CdkMenu}) cutMenu: CdkMenu;
@ViewChild('copy_menu', {read: CdkMenu}) copyMenu: CdkMenu;
}

@Component({
template: `
<div cdkMenuBar id="menu_bar">
<button #trigger cdkMenuItem [cdkMenuTriggerFor]="file">File</button>
</div>
<ng-template cdkMenuPanel #file="cdkMenuPanel">
<div cdkMenu #file_menu id="file_menu" [cdkMenuPanel]="file"></div>
</ng-template>
<div [cdkContextMenuTriggerFor]="context"></div>
<ng-template cdkMenuPanel #context="cdkMenuPanel">
<div cdkMenu #context_menu [cdkMenuPanel]="context">
<button cdkMenuItem></button>
</div>
</ng-template>
<div #inline_menu cdkMenu>
<button #inline_menu_button cdkMenuItem></button>
</div>
`,
})
class ContextMenuWithMenuBarAndInlineMenu {
@ViewChild(CdkMenuBar, {read: ElementRef}) nativeMenuBar: ElementRef;
@ViewChild('trigger', {read: ElementRef}) nativeMenuBarTrigger: ElementRef;

@ViewChild('context_menu') contextMenu?: CdkMenu;
@ViewChild(CdkContextMenuTrigger, {read: ElementRef}) trigger: ElementRef<HTMLElement>;

@ViewChild('file_menu') fileMenu?: CdkMenu;

@ViewChild('inline_menu') nativeInlineMenu: ElementRef;
@ViewChild('inline_menu_button') nativeInlineMenuButton: ElementRef;
}
2 changes: 1 addition & 1 deletion src/cdk-experimental/menu/context-menu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import {MenuStack, MenuStackItem} from './menu-stack';
*/
function isWithinMenuElement(target: Element | null) {
while (target instanceof Element) {
if (target.className.indexOf('cdk-menu') !== -1) {
if (target.classList.contains('cdk-menu') && !target.classList.contains('cdk-menu-inline')) {
return true;
}
target = target.parentElement;
Expand Down
5 changes: 4 additions & 1 deletion src/cdk-experimental/menu/menu-bar.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -779,7 +779,10 @@ describe('MenuBar', () => {
openMenu();
expect(menus.length).toBe(1);

fixture.debugElement.query(By.css('#container')).nativeElement.click();
dispatchMouseEvent(
fixture.debugElement.query(By.css('#container')).nativeElement,
'mousedown'
);
detectChanges();

expect(menus.length).toBe(0);
Expand Down
2 changes: 1 addition & 1 deletion src/cdk-experimental/menu/menu-bar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ export class CdkMenuBar extends CdkMenuGroup implements Menu, AfterContentInit,
// to avoid double event listeners, we need to use `HostListener`. Once Ivy is the default, we
// can move this back into `host`.
// tslint:disable:no-host-decorator-in-concrete
@HostListener('document:click', ['$event'])
@HostListener('document:mousedown', ['$event'])
/** Close any open submenu if there was a click event which occurred outside the menu stack. */
_closeOnBackgroundClick(event: MouseEvent) {
if (this._hasOpenSubmenu()) {
Expand Down
1 change: 1 addition & 0 deletions src/cdk-experimental/menu/menu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ import {getItemPointerEntries} from './item-pointer-entries';
'[tabindex]': '_isInline() ? 0 : null',
'role': 'menu',
'class': 'cdk-menu',
'[class.cdk-menu-inline]': '_isInline()',
'[attr.aria-orientation]': 'orientation',
},
providers: [
Expand Down

0 comments on commit 1dee741

Please sign in to comment.