-
Notifications
You must be signed in to change notification settings - Fork 6.8k
feat(cdk-experimental/menu): enable keyboard handling for context menu #20171
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
feat(cdk-experimental/menu): enable keyboard handling for context menu #20171
Conversation
@@ -216,6 +216,7 @@ export class CdkContextMenuTrigger implements OnDestroy { | |||
|
|||
this._contextMenuTracker.update(this); | |||
this.open({x: event.clientX, y: event.clientY}); | |||
this._menuPanel._menu?.focusFirstItem('mouse'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know that this should be mouse, since one could trigger this event with the keyboard.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Referring to the fact that this method handles the keyboard shortcut to trigger a context menu??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, exactly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok. As discussed, fixed to consider to clicked mouse button when setting focus
93f226b
to
a16f8f3
Compare
By default on both Mac and Windows the context menu keyboard trigger is emitted as a contextmenu mouse event automatically, eliminating the need to handle keyboard events specifically for context menus. Therefore, this simply places focus on the first menu item in the opened context menu relying on the existing menu and menu item keyboard handling logic.
a16f8f3
to
9c18aec
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
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. |
By default on both Mac and Windows the context menu keyboard trigger is emitted as a contextmenu
mouse event automatically, eliminating the need to handle keyboard events specifically for context
menus. Therefore, this simply places focus on the first menu item in the opened context menu
relying on the existing menu and menu item keyboard handling logic.