Skip to content

feat(cdk-experimental/menu): add ability to close menus when clicking outside the menu tree #20064

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 126 additions & 0 deletions src/cdk-experimental/menu/menu-bar.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ import {CdkMenuItemRadio} from './menu-item-radio';
import {CdkMenu} from './menu';
import {CdkMenuItem} from './menu-item';
import {CdkMenuItemCheckbox} from './menu-item-checkbox';
import {CdkMenuItemTrigger} from './menu-item-trigger';
import {CdkMenuGroup} from './menu-group';

describe('MenuBar', () => {
describe('as radio group', () => {
Expand Down Expand Up @@ -735,6 +737,106 @@ describe('MenuBar', () => {
);
});
});

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

let menus: CdkMenu[];
let triggers: CdkMenuItemTrigger[];

/** open the attached menu. */
function openMenu() {
triggers[0].toggle();
detectChanges();
}

/** set the menus and triggers arrays. */
function grabElementsForTesting() {
menus = fixture.componentInstance.menus.toArray();
triggers = fixture.componentInstance.triggers.toArray();
}

/** run change detection and, extract and set the rendered elements. */
function detectChanges() {
fixture.detectChanges();
grabElementsForTesting();
}

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

@Component({
Expand Down Expand Up @@ -847,3 +949,27 @@ class MenuWithRadioButtons {

@ViewChildren(CdkMenuItemRadio) radioItems: QueryList<CdkMenuItemRadio>;
}

@Component({
template: `
<div id="container">
<div cdkMenuBar>
<button cdkMenuItem [cdkMenuTriggerFor]="sub1">Trigger</button>
</div>

<ng-template cdkMenuPanel #sub1="cdkMenuPanel">
<div cdkMenu [cdkMenuPanel]="sub1">
<div cdkMenuGroup>
<button cdkMenuItemCheckbox>Trigger</button>
<span id="inner-element">A nested non-menuitem element</span>
</div>
</div>
</ng-template>
</div>
`,
})
class MenuBarWithMenus {
@ViewChildren(CdkMenu) menus: QueryList<CdkMenu>;

@ViewChildren(CdkMenuItemTrigger) triggers: QueryList<CdkMenuItemTrigger>;
}
32 changes: 32 additions & 0 deletions src/cdk-experimental/menu/menu-bar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,15 @@ import {CDK_MENU, Menu} from './menu-interface';
import {CdkMenuItem} from './menu-item';
import {MenuStack, MenuStackItem, FocusNext} from './menu-stack';

/**
* Check if the given element is part of the cdk menu module.
* @param target the element to check.
* @return true if the given element is part of the menu module.
*/
function isMenuElement(target: Element) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name is a little ambiguous. Maybe a more verbose name, 'isElementAMenu', would be clearer?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jelbourn thoughts?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer isMenuElement

return target.className.indexOf('cdk-menu') !== -1;
}

/**
* Directive applied to an element which configures it as a MenuBar by setting the appropriate
* role, aria attributes, and accessible keyboard and mouse handling logic. The component that
Expand All @@ -36,8 +45,10 @@ import {MenuStack, MenuStackItem, FocusNext} from './menu-stack';
exportAs: 'cdkMenuBar',
host: {
'(keydown)': '_handleKeyEvent($event)',
'(document:click)': '_closeOnBackgroundClick($event)',
'(focus)': 'focusFirstItem()',
'role': 'menubar',
'class': 'cdk-menu-bar',
'tabindex': '0',
'[attr.aria-orientation]': 'orientation',
},
Expand Down Expand Up @@ -212,6 +223,22 @@ export class CdkMenuBar extends CdkMenuGroup implements Menu, AfterContentInit,
return this.orientation === 'horizontal';
}

/** Close any open submenu if there was a click event which occurred outside the menu stack. */
_closeOnBackgroundClick(event: MouseEvent) {
if (this._hasOpenSubmenu()) {
// get target from composed path to account for shadow dom
let target = event.composedPath ? event.composedPath()[0] : event.target;
while (target instanceof Element) {
if (isMenuElement(target)) {
return;
}
target = target.parentElement;
}

this._openItem?.getMenuTrigger()?.toggle();
}
}

/**
* Subscribe to the menu trigger's open events in order to track the trigger which opened the menu
* and stop tracking it when the menu is closed.
Expand All @@ -236,6 +263,11 @@ export class CdkMenuBar extends CdkMenuGroup implements Menu, AfterContentInit,
.subscribe(() => (this._openItem = undefined));
}

/** Return true if the MenuBar has an open submenu. */
private _hasOpenSubmenu() {
return !!this._openItem;
}

ngOnDestroy() {
super.ngOnDestroy();
this._destroyed.next();
Expand Down
1 change: 1 addition & 0 deletions src/cdk-experimental/menu/menu-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {CdkMenuItem} from './menu-item';
exportAs: 'cdkMenuGroup',
host: {
'role': 'group',
'class': 'cdk-menu-group',
},
providers: [{provide: UniqueSelectionDispatcher, useClass: UniqueSelectionDispatcher}],
})
Expand Down
1 change: 1 addition & 0 deletions src/cdk-experimental/menu/menu-item.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ function removeIcons(element: Element) {
'tabindex': '-1',
'type': 'button',
'role': 'menuitem',
'class': 'cdk-menu-item',
'[attr.aria-disabled]': 'disabled || null',
},
})
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 @@ -51,6 +51,7 @@ import {MenuStack, MenuStackItem, FocusNext} from './menu-stack';
host: {
'(keydown)': '_handleKeyEvent($event)',
'role': 'menu',
'class': 'cdk-menu',
'[attr.aria-orientation]': 'orientation',
},
providers: [
Expand Down