Skip to content

Commit

Permalink
fix(cdk-experimental/menu): ensure menu is closed out when clicking o…
Browse files Browse the repository at this point in the history
…n an inline menu item

Fixes an issue where clicking an item in an inline menu prevents any open popout menus from closing
out.
  • Loading branch information
andy9775 committed Aug 10, 2020
1 parent 25ce323 commit dedf046
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 20 deletions.
53 changes: 35 additions & 18 deletions src/cdk-experimental/menu/menu-bar.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -740,10 +740,11 @@ describe('MenuBar', () => {
});

describe('background click closeout', () => {
let fixture: ComponentFixture<MenuBarWithMenus>;
let fixture: ComponentFixture<MenuBarWithMenusAndInlineMenu>;

let menus: CdkMenu[];
let popoutMenus: CdkMenu[];
let triggers: CdkMenuItemTrigger[];
let nativeInlineMenuItem: HTMLElement;

/** open the attached menu. */
function openMenu() {
Expand All @@ -753,8 +754,9 @@ describe('MenuBar', () => {

/** set the menus and triggers arrays. */
function grabElementsForTesting() {
menus = fixture.componentInstance.menus.toArray();
popoutMenus = fixture.componentInstance.menus.toArray().filter(el => !el._isInline());
triggers = fixture.componentInstance.triggers.toArray();
nativeInlineMenuItem = fixture.componentInstance.nativeInlineMenuItem.nativeElement;
}

/** run change detection and, extract and set the rendered elements. */
Expand All @@ -766,77 +768,86 @@ describe('MenuBar', () => {
beforeEach(async(() => {
TestBed.configureTestingModule({
imports: [CdkMenuModule],
declarations: [MenuBarWithMenus],
declarations: [MenuBarWithMenusAndInlineMenu],
}).compileComponents();
}));

beforeEach(() => {
fixture = TestBed.createComponent(MenuBarWithMenus);
fixture = TestBed.createComponent(MenuBarWithMenusAndInlineMenu);
detectChanges();
});

it('should close out all open menus when clicked outside the menu tree', () => {
openMenu();
expect(menus.length).toBe(1);
expect(popoutMenus.length).toBe(1);

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

expect(menus.length).toBe(0);
expect(popoutMenus.length).toBe(0);
});

it('should not close open menus when clicking on a menu group', () => {
openMenu();
expect(menus.length).toBe(1);
expect(popoutMenus.length).toBe(1);

const menuGroups = fixture.debugElement.queryAll(By.directive(CdkMenuGroup));
menuGroups[2].nativeElement.click();
detectChanges();

expect(menus.length).toBe(1);
expect(popoutMenus.length).toBe(1);
});

it('should not close open menus when clicking on a menu', () => {
openMenu();
expect(menus.length).toBe(1);
expect(popoutMenus.length).toBe(1);

fixture.debugElement.query(By.directive(CdkMenu)).nativeElement.click();
detectChanges();

expect(menus.length).toBe(1);
expect(popoutMenus.length).toBe(1);
});

it('should not close open menus when clicking on a menu bar', () => {
openMenu();
expect(menus.length).toBe(1);
expect(popoutMenus.length).toBe(1);

fixture.debugElement.query(By.directive(CdkMenuBar)).nativeElement.click();
detectChanges();

expect(menus.length).toBe(1);
expect(popoutMenus.length).toBe(1);
});

it('should not close when clicking on a CdkMenuItemCheckbox element', () => {
openMenu();
expect(menus.length).toBe(1);
expect(popoutMenus.length).toBe(1);

fixture.debugElement.query(By.directive(CdkMenuItemCheckbox)).nativeElement.click();
fixture.detectChanges();

expect(menus.length).toBe(1);
expect(popoutMenus.length).toBe(1);
});

it('should not close when clicking on a non-menu element inside menu', () => {
openMenu();
expect(menus.length).toBe(1);
expect(popoutMenus.length).toBe(1);

fixture.debugElement.query(By.css('#inner-element')).nativeElement.click();
detectChanges();

expect(menus.length)
expect(popoutMenus.length)
.withContext('menu should stay open if clicking on an inner span element')
.toBe(1);
});

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

nativeInlineMenuItem.click();
detectChanges();

expect(popoutMenus.length).toBe(0);
});
});

describe('Mouse handling', () => {
Expand Down Expand Up @@ -1171,10 +1182,16 @@ class MenuWithRadioButtons {
</div>
</ng-template>
</div>
<div cdkMenu>
<button #inline_menu_item cdkMenuItem></button>
</div>
`,
})
class MenuBarWithMenus {
class MenuBarWithMenusAndInlineMenu {
@ViewChildren(CdkMenu) menus: QueryList<CdkMenu>;

@ViewChildren(CdkMenuItemTrigger) triggers: QueryList<CdkMenuItemTrigger>;

@ViewChild('inline_menu_item') nativeInlineMenuItem: ElementRef;
}
7 changes: 5 additions & 2 deletions src/cdk-experimental/menu/menu-bar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,15 @@ import {MenuStack, MenuStackItem, FocusNext} from './menu-stack';
import {getItemPointerEntries} from './item-pointer-entries';

/**
* Check if the given element is part of the cdk menu module.
* Whether the element is a menu bar or a popup menu.
* @param target the element to check.
* @return true if the given element is part of the menu module.
*/
function isMenuElement(target: Element) {
return target.className.indexOf('cdk-menu') !== -1;
return (
target.classList.contains('cdk-menu-bar') ||
(target.classList.contains('cdk-menu') && !target.classList.contains('cdk-menu-inline'))
);
}

/**
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 dedf046

Please sign in to comment.