Skip to content
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

Add option to remove anchor arg from context menus #10626

Merged
merged 2 commits into from
Jan 21, 2022

Conversation

colin-grant-work
Copy link
Contributor

@colin-grant-work colin-grant-work commented Jan 12, 2022

What it does

Closes #10540
Fixes #10571
This PR introduces an additional option for the context menu renderer to remove the anchor argument included by default, and applies that option in the handlers for menus from the sidebar. The anchor argument is used by a number of commands for identifying a specific widget when several could trigger the same context menu, but it is not intended to be used with either of the side panel menus.

See #9931 for another example of the problem

How to test

  1. Set the window.menuBarVisibility preference to compact.
  2. Run the Run --> Start Debugging command from the main menu.
  3. If a default launch configuration is available, a debug session should start.

  1. Set the window.menuBarVisibility preference to compact.
  2. Open an editor and select some text.
  3. Use the menu to run the Edit --> Cut command.
  4. The selected text should be cut and be available in the clipboard.

Review checklist

Reminder for reviewers

@colin-grant-work colin-grant-work marked this pull request as draft January 12, 2022 17:55
@colin-grant-work colin-grant-work added the menus issues related to the menu label Jan 17, 2022
@colin-grant-work colin-grant-work marked this pull request as ready for review January 18, 2022 20:00
@colin-grant-work
Copy link
Contributor Author

@shuyaqian, would you mind testing this to see if it solves the problems you encountered with the compact-style menu?

Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

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

The changes look good to me!

  • Using the Cut command from the main menu (classic/collapsed) works correctly for different contexts (monaco/html input elements)
  • I could not notice any regressions related to other menu entries/context menus

@shuyaqian
Copy link
Contributor

@shuyaqian, would you mind testing this to see if it solves the problems you encountered with the compact-style menu?

Tested and the problem has been fixed.

@colin-grant-work colin-grant-work merged commit 4bf7a93 into master Jan 21, 2022
@github-actions github-actions bot added this to the 1.22.0 milestone Jan 21, 2022
@msujew msujew deleted the bugfix/anchors-out-of-place branch January 21, 2022 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
menus issues related to the menu
Projects
None yet
Development

Successfully merging this pull request may close these issues.

'CUT' not work in context menu
3 participants