vscode: independent editor/title/run menu

[cdamus]

What it does

In VS Code, contributions to the "editor/title/run" menu contribution point are not added to the ellipsis menu but to a dedicated item on the toolbar. This commit makes Theia do the same, except that unlike VS Code, a pop-up menu is always presented, even if there is only one action available.

In my opinion, the inconsistency of the VS Code implementation in not showing the Run/Debug pop-up menu but just the one contributed action on the toolbar, which looks quite different to the Run/Debug pop-up menu action, is confusing. If an action is contributed to an extension point that is a menu of Run/Debug actions, then that should show a menu of Run/Debug actions even if there is only one item on the menu.

If it really is necessary to emulate this detail of the VS Code behaviour, then that should be a follow-up issue to discuss with the user community.

Note also that this PR uses the debug-alt icon from the VS Code codicons set as the best available alternative from that set to the icon that VS Code uses for its "Run/Debug" menu, which is not in the codicons set. If somebody would like to contribute a better license-compatible open-source icon for the purpose, please do!

Fixes #12687

How to test

1. Clone the menu_bug_3 branch of the OP's sample repository used for other recent bug reports: Theia Sandbox.
2. Change the menu contribution in the myext extension to the "editor/title/run" point (it should be checked out as "editor/lineNumber/context" for another issue report) and remove the "group" property that isn't needed for this use case.
3. Because there probably wouldn't be any other actions on the "more" menu, I recommend adding the "myext.goodbyeWorld" command from this extension to the "editor/title" menu so that we can see that this menu exists and doesn't get contributions intended for the "editor/title/run" menu.
4. Build the myext extension and link it into your plugins/ directory.
5. Launch Theia and open some editor (e.g., TS or JSON file).
6. Observe that the "Run/Debug" menu shows up something like the screen recording below, with the "Hello World" action from the extension. Additionally, hovering the pointer over it shows a "Run or Debug..." tooltip, similar to the "More Items..." tooltip on the ... "more" menu.
7. Verify that the "Hello World" action is not in the ... menu.

Also worth verifying are some related use cases:

• check that a Theia instance without any contributions to the "editor/title/run" menu does not show that menu in the toolbar.
• also that in the case there are contributions to this menu, but all of them are suppressed by their "when" clauses not matching the current context, then in this case also the menu does not appear in the toolbar.

Review checklist

• As an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• As a reviewer, I agree to behave in accordance with the review guidelines

[jcortell68]

If an action is contributed to an extension point that is a menu of Run/Debug actions, then that should show a menu of Run/Debug actions even if there is only one item on the menu.

@cdamus I agree. IMO, doing things better than VS Code in this case is better than emulating VS Code.

[martin-fleck-at]

Hi @cdamus,

Thank you very much for your change! I tested it with commands and submenus and everything seems to work mostly as expected. I do believe we currently render the menu also on views which should not happen. Could you please have a look at that? (In case you cannot reproduce it, I actually rebased your change on the master branch, maybe there were some changes inbetween.)

Thank you!

[martin-fleck-at]

I think this overall condition is flawed as I can see the menu also on views, not only editors. I believe the problem is that we do not evaluate the conditions on menu toolbar items (only if they have content) but we do register our menu in menus-contribution-handler.ts with a visibility condition. Since this already a rather complex condition, I'd suggest to pull that out into something that is more readable and easier to overwrite, e.g.,:

    protected isVisible(item: TabBarToolbarItem | ReactTabBarToolbarItem, widget: Widget): boolean {
        if (AnyToolbarItem.isConditional(item)) {
            if (item.isVisible && !item.isVisible(widget)) {
                return false;
            }
            if (item.when && !this.contextKeyService.match(item.when, widget.node)) {
                return false;
            }
        }
        if (TabBarToolbarItem.is(item) && item.command && !this.commandRegistry.isVisible(item.command, widget)) {
            return false;
        }
        if (MenuToolbarItem.is(item) && !this.hasMenuContent(item, widget)) {
            return false;
        }
        return true;
    }

[cdamus]

A welcome refactoring suggestion.

[martin-fleck-at]

Could you please add the onClick listener to the parent? It feels a bit odd that you cannot open the menu by clicking the chevron.

[cdamus]

The bigger target area for clicking is important for accessibility in any case!

[cdamus]

Thanks for the review, @martin-fleck-at ! Indeed, intervening changes merged to master did have the effect of leaking the item contribution that you observed. Commit 4f0c438 should address that and also the mouse-click target problem.

[martin-fleck-at]

I re-tested the changes and everything looks great to me now, thank you very much Christian! As soon as the CI is happy we can merge this.

[cdamus]

Commit 0e6186f adds the missing return type annotation found by ESLint.