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

tabbar: fix command contributions regression #12869

Merged
merged 1 commit into from
Aug 29, 2023

Conversation

cdamus
Copy link
Contributor

@cdamus cdamus commented Aug 28, 2023

What it does

The merge of #12799 introduced a regression in the rendering of MenuToolbarItems that aren't pop-up menus but instead just commands that
happen to have a menu path: they were rendered as pop-up menus instead of simple buttons.

This fix refines the condition for the new pop-up menu rendering to check that the menu to be rendered is not a command to be executed directly on click.

Fixes #12687

How to test

  1. Re-verify the original test scenario from vscode: independent editor/title/run menu #12799, the original fix for the issue.
  2. Quit Theia.
  3. Switch your clone of the OP's Theia Sandbox repository to the menu_bug_1 branch.
  4. Launch Theia again.
  5. Observe that in an editor now the "Hello World" and "Goodbye World" actions contributed by the myext extension to the toolbar are rendered as separate buttons that don't show pop-up menus when clicked but just execute their commands, as is correct. Note that the action without the icon will not be rendered correctly, which is the problem to be addressed in Icon-less VS Code “editor/title” menu contribution not rendered properly #12686. It should look like this, exactly as in the description of 12686:

CleanShot 2023-08-28 at 17 37 30

Review checklist

Reminder for reviewers

The merge of eclipse-theia#12799 introduced a regression in
the rendering of MenuToolbarItems that aren't
pop-up menus but instead just commands that
happen to have a menu path: they were rendered
as pop-up menus instead of simple buttons.

This fix refines the condition for the new pop-up
menu rendering to check that the menu to be
rendered is not a command to be executed
directly on click.

Fixes eclipse-theia#12687

Signed-off-by: Christian W. Damus <[email protected]>
@cdamus
Copy link
Contributor Author

cdamus commented Aug 28, 2023

@martin-fleck-at this is a follow-up to fix a regression caused by a PR that you reviewed, yesterday. Would you mind having a look? TIA!

@cdamus cdamus mentioned this pull request Aug 28, 2023
1 task
@martin-fleck-at martin-fleck-at self-requested a review August 29, 2023 06:04
Copy link
Contributor

@martin-fleck-at martin-fleck-at left a comment

Choose a reason for hiding this comment

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

Thank you for finding this! I can confirm that it works as expected again, thank you very much @cdamus!

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 toolbar issues related to the toolbar vscode issues related to VSCode compatibility
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

VS Code menu contributions to "editor/title/run" seem to behave no different than to "editor/title"
3 participants