-
Notifications
You must be signed in to change notification settings - Fork 13
fix: Fix when the context menu is allowed to open #550
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
fix: Fix when the context menu is allowed to open #550
Conversation
|
Assigning @BenHenning to review because I don't understand the workspace state changes. |
|
Also fixes (?) #276 |
|
It partly fixes 276 by allowing the context menu to open in the flyout, but making copy/paste work correctly requires fixing some things in core or undoing Maribeth's change to depend on the core callbacks. |
BenHenning
left a comment
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.
Thanks @RoboErikG. I think the solution makes sense. Is it possible add tests for these cases?
BenHenning
left a comment
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.
Thanks @RoboErikG! Approving to unblock, but I did have some thoughts on the tests. Feel free to re-add me if you'd like another review pass.
Fixes #434
Fixes #276
#546 is partly fixed by this change but may still need some work for shortcuts other than menu opening.
This fixes the precondition checks for showing the action menu and also enables the action menu to be shown in the flyout so that it's consistent with right click.