Refactor PluginMenuContributionHandler

CHANGELOG.md

@@ -7,6 +7,11 @@
 ## v1.28.0 - Unreleased
 - [plugin] added support for property `SourceControlInputBox#visible` [#11412](https://github.com/eclipse-theia/theia/pull/11412) - Contributed on behalf of STMicroelectronics
 
+<a name="breaking_changes_1.28.0">[Breaking Changes:](#breaking_changes_1.28.0)</a>
+
+- [core] `handleDefault`, `handleElectronDefault` method no longer called in `BrowserMainMenuFactory.registerMenu()`, `DynamicMenuWidget.buildSubMenus()` or `ElectronMainMenuFactory.fillSubmenus()`. Override the respective calling function rather than `handleDefault`. The argument to each of the three methods listed above is now `MenuNode` and not `CompositeMenuNode`, and the methods are truly recursive and called on entire menu tree. `ActionMenuNode.action` removed; access relevant field on `ActionMenuNode.command`, `.when` etc. [#11290](https://github.com/eclipse-theia/theia/pull/11290)
+- [plugin-ext] `CodeEditorWidgetUtil` moved to `packages/plugin-ext/src/main/browser/menus/vscode-theia-menu-mappings.ts`. `MenusContributionPointHandler` extensively refactored. See PR description for details. [#11290](https://github.com/eclipse-theia/theia/pull/11290)
+
 ## v1.27.0 - 6/30/2022
 
 - [core] added better styling for active sidepanel borders [#11330](https://github.com/eclipse-theia/theia/pull/11330)

examples/api-samples/src/browser/menu/sample-browser-menu-module.ts

@@ -17,8 +17,8 @@
 import { injectable, ContainerModule } from '@theia/core/shared/inversify';
 import { Menu as MenuWidget } from '@theia/core/shared/@phosphor/widgets';
 import { Disposable } from '@theia/core/lib/common/disposable';
-import { MenuNode, CompositeMenuNode } from '@theia/core/lib/common/menu';
-import { BrowserMainMenuFactory, MenuCommandRegistry, DynamicMenuWidget } from '@theia/core/lib/browser/menu/browser-menu-plugin';
+import { MenuNode, CompositeMenuNode, MenuPath } from '@theia/core/lib/common/menu';
+import { BrowserMainMenuFactory, MenuCommandRegistry, DynamicMenuWidget, BrowserMenuOptions } from '@theia/core/lib/browser/menu/browser-menu-plugin';
 import { PlaceholderMenuNode } from './sample-menu-contribution';
 
 export default new ContainerModule((bind, unbind, isBound, rebind) => {
@@ -28,20 +28,21 @@ export default new ContainerModule((bind, unbind, isBound, rebind) => {
 @injectable()
 class SampleBrowserMainMenuFactory extends BrowserMainMenuFactory {
 
-    protected override handleDefault(menuCommandRegistry: MenuCommandRegistry, menuNode: MenuNode): void {
-        if (menuNode instanceof PlaceholderMenuNode && menuCommandRegistry instanceof SampleMenuCommandRegistry) {
-            menuCommandRegistry.registerPlaceholderMenu(menuNode);
+    protected override registerMenu(menuCommandRegistry: MenuCommandRegistry, menu: MenuNode, args: unknown[]): void {
+        if (menu instanceof PlaceholderMenuNode && menuCommandRegistry instanceof SampleMenuCommandRegistry) {
+            menuCommandRegistry.registerPlaceholderMenu(menu);
+        } else {
+            super.registerMenu(menuCommandRegistry, menu, args);
         }
     }
 
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    protected override createMenuCommandRegistry(menu: CompositeMenuNode, args: any[] = []): MenuCommandRegistry {
+    protected override createMenuCommandRegistry(menu: CompositeMenuNode, args: unknown[] = []): MenuCommandRegistry {
         const menuCommandRegistry = new SampleMenuCommandRegistry(this.services);
         this.registerMenu(menuCommandRegistry, menu, args);
         return menuCommandRegistry;
     }
 
-    override createMenuWidget(menu: CompositeMenuNode, options: MenuWidget.IOptions & { commands: MenuCommandRegistry }): DynamicMenuWidget {
+    override createMenuWidget(menu: CompositeMenuNode, options: BrowserMenuOptions): DynamicMenuWidget {
         return new SampleDynamicMenuWidget(menu, options, this.services);
     }
 
@@ -60,8 +61,8 @@ class SampleMenuCommandRegistry extends MenuCommandRegistry {
         this.placeholders.set(id, menu);
     }
 
-    override snapshot(): this {
-        super.snapshot();
+    override snapshot(menuPath: MenuPath): this {
+        super.snapshot(menuPath);
         for (const menu of this.placeholders.values()) {
             this.toDispose.push(this.registerPlaceholder(menu));
         }
@@ -70,28 +71,28 @@ class SampleMenuCommandRegistry extends MenuCommandRegistry {
 
     protected registerPlaceholder(menu: PlaceholderMenuNode): Disposable {
         const { id } = menu;
-        const unregisterCommand = this.addCommand(id, {
+        return this.addCommand(id, {
             execute: () => { /* NOOP */ },
             label: menu.label,
             icon: menu.icon,
             isEnabled: () => false,
             isVisible: () => true
         });
-        return Disposable.create(() => unregisterCommand.dispose());
     }
 
 }
 
 class SampleDynamicMenuWidget extends DynamicMenuWidget {
 
-    protected override handleDefault(menuNode: MenuNode): MenuWidget.IItemOptions[] {
-        if (menuNode instanceof PlaceholderMenuNode) {
-            return [{
-                command: menuNode.id,
-                type: 'command'
-            }];
+    protected override buildSubMenus(parentItems: MenuWidget.IItemOptions[], menu: MenuNode, commands: MenuCommandRegistry): MenuWidget.IItemOptions[] {
+        if (menu instanceof PlaceholderMenuNode) {
+            parentItems.push({
+                command: menu.id,
+                type: 'command',
+            });
+        } else {
+            super.buildSubMenus(parentItems, menu, commands);
         }
-        return [];
+        return parentItems;
     }
-
 }

examples/api-samples/src/electron-browser/menu/sample-electron-menu-module.ts

@@ -15,7 +15,7 @@
 // *****************************************************************************
 
 import { injectable, ContainerModule } from '@theia/core/shared/inversify';
-import { CompositeMenuNode } from '@theia/core/lib/common/menu';
+import { CompoundMenuNode, MenuNode } from '@theia/core/lib/common/menu';
 import { ElectronMainMenuFactory, ElectronMenuOptions } from '@theia/core/lib/electron-browser/menu/electron-main-menu-factory';
 import { PlaceholderMenuNode } from '../../browser/menu/sample-menu-contribution';
 
@@ -25,17 +25,14 @@ export default new ContainerModule((bind, unbind, isBound, rebind) => {
 
 @injectable()
 class SampleElectronMainMenuFactory extends ElectronMainMenuFactory {
-
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    protected override handleElectronDefault(menuNode: CompositeMenuNode, args: any[] = [], options?: ElectronMenuOptions): Electron.MenuItemConstructorOptions[] {
-        if (menuNode instanceof PlaceholderMenuNode) {
-            return [{
-                label: menuNode.label,
-                enabled: false,
-                visible: true
-            }];
+    protected override fillMenuTemplate(
+        parentItems: Electron.MenuItemConstructorOptions[], menuModel: MenuNode & CompoundMenuNode, args: unknown[] = [], options: ElectronMenuOptions
+    ): Electron.MenuItemConstructorOptions[] {
+        if (menuModel instanceof PlaceholderMenuNode) {
+            parentItems.push({ label: menuModel.label, enabled: false, visible: true });
+        } else {
+            super.fillMenuTemplate(parentItems, menuModel, args, options);
         }
-        return [];
+        return parentItems;
     }
-
 }

examples/playwright/src/tests/theia-main-menu.test.ts

@@ -57,7 +57,7 @@ test.describe('Theia Main Menu', () => {
         expect(label).toBe('New File');
 
         const shortCut = await menuItem?.shortCut();
-        expect(shortCut).toBe(OSUtil.isMacOS ? 'N' : 'Alt+N');
+        expect(shortCut).toBe(OSUtil.isMacOS ? '⌥ N' : 'Alt+N');
 
         const hasSubmenu = await menuItem?.hasSubmenu();
         expect(hasSubmenu).toBe(false);

examples/playwright/src/theia-menu-item.ts

@@ -16,7 +16,7 @@
 
 import { ElementHandle } from '@playwright/test';
 
-import { textContent } from './util';
+import { elementContainsClass, textContent } from './util';
 
 export class TheiaMenuItem {
 
@@ -30,15 +30,28 @@ export class TheiaMenuItem {
         return this.element.waitForSelector('.p-Menu-itemShortcut');
     }
 
+    protected isHidden(): Promise<boolean> {
+        return elementContainsClass(this.element, 'p-mod-collapsed');
+    }
+
     async label(): Promise<string | undefined> {
+        if (await this.isHidden()) {
+            return undefined;
+        }
         return textContent(this.labelElementHandle());
     }
 
     async shortCut(): Promise<string | undefined> {
+        if (await this.isHidden()) {
+            return undefined;
+        }
         return textContent(this.shortCutElementHandle());
     }
 
     async hasSubmenu(): Promise<boolean> {
+        if (await this.isHidden()) {
+            return false;
+        }
         return (await this.element.getAttribute('data-type')) === 'submenu';
     }
 
@@ -47,7 +60,7 @@ export class TheiaMenuItem {
         if (classAttribute === undefined || classAttribute === null) {
             return false;
         }
-        return !classAttribute.includes('p-mod-disabled');
+        return !classAttribute.includes('p-mod-disabled') && !classAttribute.includes('p-mod-collapsed');
     }
 
     async click(): Promise<void> {

packages/core/src/browser/context-key-service.ts

@@ -34,7 +34,7 @@ export namespace ContextKey {
 }
 
 export interface ContextKeyChangeEvent {
-    affects(keys: Set<string>): boolean;
+    affects(keys: { has(key: string): boolean }): boolean;
 }
 
 export const ContextKeyService = Symbol('ContextKeyService');

packages/core/src/browser/context-menu-renderer.ts

@@ -112,5 +112,10 @@ export interface RenderContextMenuOptions {
      * Default is `true`.
      */
     includeAnchorArg?: boolean;
+    /**
+     * A DOM context to use when evaluating any `when` clauses
+     * of menu items registered for this item.
+     */
+    context?: HTMLElement;
     onHide?: () => void;
 }

packages/core/src/browser/frontend-application-module.ts

@@ -31,7 +31,11 @@ import {
     InMemoryResources,
     messageServicePath,
     InMemoryTextResourceResolver,
-    UntitledResourceResolver
+    UntitledResourceResolver,
+    MenuCommandAdapterRegistry,
+    MenuCommandExecutor,
+    MenuCommandAdapterRegistryImpl,
+    MenuCommandExecutorImpl
 } from '../common';
 import { KeybindingRegistry, KeybindingContext, KeybindingContribution } from './keybinding';
 import { FrontendApplication, FrontendApplicationContribution, DefaultFrontendApplicationContribution } from './frontend-application';
@@ -241,6 +245,8 @@ export const frontendApplicationModule = new ContainerModule((bind, _unbind, _is
 
     bind(MenuModelRegistry).toSelf().inSingletonScope();
     bindContributionProvider(bind, MenuContribution);
+    bind(MenuCommandAdapterRegistry).to(MenuCommandAdapterRegistryImpl).inSingletonScope();
+    bind(MenuCommandExecutor).to(MenuCommandExecutorImpl).inSingletonScope();
 
     bind(KeyboardLayoutService).toSelf().inSingletonScope();
     bind(KeybindingRegistry).toSelf().inSingletonScope();

packages/core/src/browser/menu/browser-context-menu-renderer.ts

@@ -36,8 +36,8 @@ export class BrowserContextMenuRenderer extends ContextMenuRenderer {
         super();
     }
 
-    protected doRender({ menuPath, anchor, args, onHide }: RenderContextMenuOptions): ContextMenuAccess {
-        const contextMenu = this.menuFactory.createContextMenu(menuPath, args);
+    protected doRender({ menuPath, anchor, args, onHide, context }: RenderContextMenuOptions): ContextMenuAccess {
+        const contextMenu = this.menuFactory.createContextMenu(menuPath, args, context);
         const { x, y } = coordinateFromAnchor(anchor);
         if (onHide) {
             contextMenu.aboutToClose.connect(() => onHide!());