feat(cdk-experimental/menu): add ability to close menus when clicking outside the menu tree #20064
Conversation
googlebot
added
the
cla: yes
| * @return true if the given element is part of the menu module. | ||
| */ | ||
| function isCdkMenuComponent(target: Element) { | ||
| const classList = target.classList; |
There was a problem hiding this comment.
I think you could get away with target.className.indexOf('cdk-menu') !== -1
There was a problem hiding this comment.
Only check for cdk-menu and skip the others?
There was a problem hiding this comment.
They all start with cdk-menu, so as long as that prefix is there you're detecting them all
There was a problem hiding this comment.
My mistake, didn't see that it was className not classList
| * @param target the element to check. | ||
| * @return true if the given element is part of the menu module. | ||
| */ | ||
| function isCdkMenuComponent(target: Element) { |
There was a problem hiding this comment.
How about isElementWithinMenu?
There was a problem hiding this comment.
It's not really checking if the element is within a menu, but rather if the element is a menu its self. If we go with the below suggestions it could be isMenu. Thoughts?
There was a problem hiding this comment.
Yeah, good point, maybe just isMenuElement
(adding "element" here to differentiate between a menu instance)
andy9775
force-pushed
the
cdk-menu-close-outside-click
branch
from
3c37ce9 to
de4745d
Compare
|
@jelbourn feedback should be addressed. |
jelbourn
added
lgtm
action: merge
| let triggers: CdkMenuItemTrigger[]; | ||
|
|
||
| /** open the attached menu. */ | ||
| const openMenu = () => { |
There was a problem hiding this comment.
Nit: our internal style guide recommends using 'function openMenu() {' instead of assigning lambda functions to variables.
|
|
||
| const menuGroups = fixture.debugElement.queryAll(By.directive(CdkMenuGroup)); | ||
| menuGroups[0].nativeElement.click(); | ||
| menuGroups[1].nativeElement.click(); |
There was a problem hiding this comment.
Just one may be fine. (With multiple clicks, future readers might wonder if there's a particular reason you clicked multiple times.)
| * @param target the element to check. | ||
| * @return true if the given element is part of the menu module. | ||
| */ | ||
| function isMenuElement(target: Element) { |
There was a problem hiding this comment.
The name is a little ambiguous. Maybe a more verbose name, 'isElementAMenu', would be clearer?
There was a problem hiding this comment.
@jelbourn thoughts?
… outside the menu tree Add functionality to close any open menus (and submenus) when a user clicks on an element outside the open menu tree and the menu bar components.
andy9775
force-pushed
the
cdk-menu-close-outside-click
branch
from
de4745d to
bf1ffb8
Compare
|
@teflonwaffles feedback should be addressed. |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Add functionality to close any open menus (and submenus) when a user clicks on an element outside
the open menu tree and the menu bar components.