Skip to content

Commit 1dee741

Browse files
committed
fix(cdk-experimental/menu): ensure context menu is closed out with other 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.
1 parent 25ce323 commit 1dee741

File tree

5 files changed

+147
-3
lines changed

5 files changed

+147
-3
lines changed

src/cdk-experimental/menu/context-menu.spec.ts

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {dispatchMouseEvent} from '@angular/cdk/testing/private';
77
import {By} from '@angular/platform-browser';
88
import {CdkMenuItem} from './menu-item';
99
import {CdkMenuItemTrigger} from './menu-item-trigger';
10+
import {CdkMenuBar} from './menu-bar';
1011

1112
describe('CdkContextMenuTrigger', () => {
1213
describe('with simple context menu trigger', () => {
@@ -246,6 +247,110 @@ describe('CdkContextMenuTrigger', () => {
246247
expect(instance.copyMenu).toBeDefined();
247248
});
248249
});
250+
251+
describe('with menubar and inline menu on page', () => {
252+
let fixture: ComponentFixture<ContextMenuWithMenuBarAndInlineMenu>;
253+
let nativeMenuBar: HTMLElement;
254+
let nativeMenuBarTrigger: HTMLElement;
255+
256+
beforeEach(async(() => {
257+
TestBed.configureTestingModule({
258+
imports: [CdkMenuModule],
259+
declarations: [ContextMenuWithMenuBarAndInlineMenu],
260+
}).compileComponents();
261+
}));
262+
263+
beforeEach(() => {
264+
fixture = TestBed.createComponent(ContextMenuWithMenuBarAndInlineMenu);
265+
fixture.detectChanges();
266+
267+
nativeMenuBar = fixture.componentInstance.nativeMenuBar.nativeElement;
268+
nativeMenuBarTrigger = fixture.componentInstance.nativeMenuBarTrigger.nativeElement;
269+
});
270+
271+
/** Get the menu opened by the context menu trigger. */
272+
function getContextMenu() {
273+
return fixture.componentInstance.contextMenu;
274+
}
275+
276+
/** Get the menu opened by the menu bar item trigger. */
277+
function getFileMenu() {
278+
return fixture.componentInstance.fileMenu;
279+
}
280+
281+
/** Get the context in which the context menu should trigger. */
282+
function getMenuContext() {
283+
return fixture.componentInstance.trigger.nativeElement;
284+
}
285+
286+
/** Get the inline menus trigger element. */
287+
function getInlineMenuTrigger() {
288+
return fixture.componentInstance.nativeInlineMenuButton.nativeElement;
289+
}
290+
291+
/** Return the native element for the inline menu. */
292+
function getInlineMenuElement() {
293+
return fixture.componentInstance.nativeInlineMenu.nativeElement;
294+
}
295+
296+
/** Open up the context menu and run change detection. */
297+
function openContextMenu() {
298+
// right click triggers a context menu event
299+
dispatchMouseEvent(getMenuContext(), 'contextmenu');
300+
dispatchMouseEvent(getMenuContext(), 'mousedown');
301+
fixture.detectChanges();
302+
}
303+
304+
/** Open up the file menu from the menu bar. */
305+
function openFileMenu() {
306+
nativeMenuBarTrigger.click();
307+
fixture.detectChanges();
308+
}
309+
310+
it('should close the open context menu when clicking on the menubar element', () => {
311+
openContextMenu();
312+
313+
dispatchMouseEvent(nativeMenuBar, 'click');
314+
fixture.detectChanges();
315+
316+
expect(getContextMenu()).not.toBeDefined();
317+
});
318+
319+
it('should close the open context menu when clicking on the menubar menu item', () => {
320+
openContextMenu();
321+
322+
nativeMenuBarTrigger.click();
323+
fixture.detectChanges();
324+
325+
expect(getContextMenu()).not.toBeDefined();
326+
});
327+
328+
it('should close the open context menu when clicking on the inline menu element', () => {
329+
openContextMenu();
330+
331+
getInlineMenuElement().click();
332+
fixture.detectChanges();
333+
334+
expect(getContextMenu()).not.toBeDefined();
335+
});
336+
337+
it('should close the open context menu when clicking on an inline menu item', () => {
338+
openContextMenu();
339+
340+
getInlineMenuTrigger().click();
341+
fixture.detectChanges();
342+
343+
expect(getContextMenu()).not.toBeDefined();
344+
});
345+
346+
it('should close the open menu when opening a context menu', () => {
347+
openFileMenu();
348+
349+
openContextMenu();
350+
351+
expect(getFileMenu()).not.toBeDefined();
352+
});
353+
});
249354
});
250355

251356
@Component({
@@ -317,3 +422,38 @@ class ContextMenuWithSubmenu {
317422
@ViewChild('cut_menu', {read: CdkMenu}) cutMenu: CdkMenu;
318423
@ViewChild('copy_menu', {read: CdkMenu}) copyMenu: CdkMenu;
319424
}
425+
426+
@Component({
427+
template: `
428+
<div cdkMenuBar id="menu_bar">
429+
<button #trigger cdkMenuItem [cdkMenuTriggerFor]="file">File</button>
430+
</div>
431+
432+
<ng-template cdkMenuPanel #file="cdkMenuPanel">
433+
<div cdkMenu #file_menu id="file_menu" [cdkMenuPanel]="file"></div>
434+
</ng-template>
435+
436+
<div [cdkContextMenuTriggerFor]="context"></div>
437+
<ng-template cdkMenuPanel #context="cdkMenuPanel">
438+
<div cdkMenu #context_menu [cdkMenuPanel]="context">
439+
<button cdkMenuItem></button>
440+
</div>
441+
</ng-template>
442+
443+
<div #inline_menu cdkMenu>
444+
<button #inline_menu_button cdkMenuItem></button>
445+
</div>
446+
`,
447+
})
448+
class ContextMenuWithMenuBarAndInlineMenu {
449+
@ViewChild(CdkMenuBar, {read: ElementRef}) nativeMenuBar: ElementRef;
450+
@ViewChild('trigger', {read: ElementRef}) nativeMenuBarTrigger: ElementRef;
451+
452+
@ViewChild('context_menu') contextMenu?: CdkMenu;
453+
@ViewChild(CdkContextMenuTrigger, {read: ElementRef}) trigger: ElementRef<HTMLElement>;
454+
455+
@ViewChild('file_menu') fileMenu?: CdkMenu;
456+
457+
@ViewChild('inline_menu') nativeInlineMenu: ElementRef;
458+
@ViewChild('inline_menu_button') nativeInlineMenuButton: ElementRef;
459+
}

src/cdk-experimental/menu/context-menu.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ import {MenuStack, MenuStackItem} from './menu-stack';
4141
*/
4242
function isWithinMenuElement(target: Element | null) {
4343
while (target instanceof Element) {
44-
if (target.className.indexOf('cdk-menu') !== -1) {
44+
if (target.classList.contains('cdk-menu') && !target.classList.contains('cdk-menu-inline')) {
4545
return true;
4646
}
4747
target = target.parentElement;

src/cdk-experimental/menu/menu-bar.spec.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -779,7 +779,10 @@ describe('MenuBar', () => {
779779
openMenu();
780780
expect(menus.length).toBe(1);
781781

782-
fixture.debugElement.query(By.css('#container')).nativeElement.click();
782+
dispatchMouseEvent(
783+
fixture.debugElement.query(By.css('#container')).nativeElement,
784+
'mousedown'
785+
);
783786
detectChanges();
784787

785788
expect(menus.length).toBe(0);

src/cdk-experimental/menu/menu-bar.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ export class CdkMenuBar extends CdkMenuGroup implements Menu, AfterContentInit,
260260
// to avoid double event listeners, we need to use `HostListener`. Once Ivy is the default, we
261261
// can move this back into `host`.
262262
// tslint:disable:no-host-decorator-in-concrete
263-
@HostListener('document:click', ['$event'])
263+
@HostListener('document:mousedown', ['$event'])
264264
/** Close any open submenu if there was a click event which occurred outside the menu stack. */
265265
_closeOnBackgroundClick(event: MouseEvent) {
266266
if (this._hasOpenSubmenu()) {

src/cdk-experimental/menu/menu.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ import {getItemPointerEntries} from './item-pointer-entries';
5454
'[tabindex]': '_isInline() ? 0 : null',
5555
'role': 'menu',
5656
'class': 'cdk-menu',
57+
'[class.cdk-menu-inline]': '_isInline()',
5758
'[attr.aria-orientation]': 'orientation',
5859
},
5960
providers: [

0 commit comments

Comments
 (0)