[browser] allow multiple menu items for the same command but in diffe…

…rent contexts Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
eclipse-theia · Jan 29, 2019 · 265c0e0 · 265c0e0
1 parent 4f5e0ef
commit 265c0e0
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 15 deletions.
diff --git a/packages/core/src/browser/menu/browser-menu-plugin.ts b/packages/core/src/browser/menu/browser-menu-plugin.ts
@@ -53,7 +53,7 @@ export class BrowserMainMenuFactory {
 
         for (const menu of menuModel.children) {
             if (menu instanceof CompositeMenuNode) {
-                const menuWidget = new DynamicMenuWidget(menu, { commands: phosphorCommands });
+                const menuWidget = new DynamicMenuWidget(menu, { commands: phosphorCommands }, this.contextKeyService);
                 menuBar.addMenu(menuWidget);
             }
         }
@@ -64,7 +64,7 @@ export class BrowserMainMenuFactory {
         const menuModel = this.menuProvider.getMenu(path);
         const phosphorCommands = this.createPhosphorCommands(menuModel);
 
-        const contextMenu = new DynamicMenuWidget(menuModel, { commands: phosphorCommands });
+        const contextMenu = new DynamicMenuWidget(menuModel, { commands: phosphorCommands }, this.contextKeyService);
         return contextMenu;
     }
 
@@ -92,17 +92,16 @@ export class BrowserMainMenuFactory {
             return;
         }
         if (commands.hasCommand(command.id)) {
-            this.logger.warn(`Command with ID ${command.id} is already registered`);
+            // several menu items can be registered for the same command in different contexts
             return;
         }
-        const { when } = menu.action;
         const args = options.args || [];
         commands.addCommand(command.id, {
             execute: () => this.commandRegistry.executeCommand(command.id, ...args),
             label: menu.label,
             icon: menu.icon,
             isEnabled: () => this.commandRegistry.isEnabled(command.id, ...args),
-            isVisible: () => this.commandRegistry.isVisible(command.id, ...args) && (!when || this.contextKeyService.match(when)),
+            isVisible: () => this.commandRegistry.isVisible(command.id, ...args),
             isToggled: () => this.commandRegistry.isToggled(command.id, ...args)
         });
 
@@ -145,7 +144,11 @@ class DynamicMenuBarWidget extends MenuBarWidget {
  */
 class DynamicMenuWidget extends MenuWidget {
 
-    constructor(protected menu: CompositeMenuNode, protected options: MenuWidget.IOptions) {
+    constructor(
+        protected menu: CompositeMenuNode,
+        protected options: MenuWidget.IOptions,
+        protected contextKeyService: ContextKeyService
+    ) {
         super(options);
         if (menu.label) {
             this.title.label = menu.label;
@@ -192,7 +195,7 @@ class DynamicMenuWidget extends MenuWidget {
 
                     if (item.isSubmenu) { // submenu node
 
-                        const submenu = new DynamicMenuWidget(item, this.options);
+                        const submenu = new DynamicMenuWidget(item, this.options, this.contextKeyService);
                         if (submenu.items.length === 0) {
                             continue;
                         }
@@ -223,8 +226,8 @@ class DynamicMenuWidget extends MenuWidget {
                 }
 
             } else if (item instanceof ActionMenuNode) {
-
-                if (!commands.isVisible(item.action.commandId)) {
+                const { when } = item.action;
+                if (!(commands.isVisible(item.action.commandId) && (!when || this.contextKeyService.match(when)))) {
                     continue;
                 }
 

diff --git a/packages/plugin-ext/src/main/browser/menus/menus-contribution-handler.ts b/packages/plugin-ext/src/main/browser/menus/menus-contribution-handler.ts
@@ -49,12 +49,6 @@ export class MenusContributionPointHandler {
     @inject(PluginSharedStyle)
     protected readonly style: PluginSharedStyle;
 
-    /**
-     * Handles the `menus` contribution point.
-     * In VSCode, a menu can have more than one item for the same command. Each item may have it's own visibility rules.
-     * In Theia, a menu can't have more than one item for the same command.
-     * So, several handlers for the same command are registered to support different visibility rules for a menu item in different contexts.
-     */
     handle(contributions: PluginContribution): void {
         const allMenus = contributions.menus;
         if (!allMenus) {