Refactor menu nodes

.vscode/launch.json

@@ -4,6 +4,17 @@
   // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
   "version": "0.2.0",
   "configurations": [
+    {
+      "name": "Playwright Tests",
+      "type": "node",
+      "request": "launch",
+      "program": "${workspaceFolder}/node_modules/@playwright/test/cli.js",
+      "cwd": "${workspaceFolder}/examples/playwright",
+      "args": [
+        "test",
+        "--config=./configs/playwright.config.ts"
+      ]
+    },
     {
       "type": "node",
       "request": "attach",

CHANGELOG.md

@@ -11,6 +11,7 @@
 - [core] fixed version `@types/express` to `^4.17.21` and `@types/express-serve-static-core` to `5.0.4`. This might be required for adopters as well if they run into typing issues. [#15147](https://github.com/eclipse-theia/theia/pull/15147)
 - [core] migration from deprecated `phosphorJs` to actively maintained fork `Lumino` [#14320](https://github.com/eclipse-theia/theia/pull/14320) - Contributed on behalf of STMicroelectronics
   Adopters importing `@phosphor` packages now need to import from `@lumino`. CSS selectors refering to `.p-` classes now need to refer to `.lm-` classes. There are also minor code adaptations, for example now using `iconClass` instead of `icon` in Lumino commands.
+- [core] Refactor menu nodes [#14676](https://github.com/eclipse-theia/theia/pull/14676) - Contributed on behalf of STMicroelectronics
 
 <a name="breaking_changes_1.60.0">[Breaking Changes:](#breaking_changes_1.60.0)</a>
 

doc/Migration.md

@@ -59,6 +59,14 @@ For example:
 }
 ```
 
+### v1.59.0
+
+#### Refactor menu nodes [#14676](https://github.com/eclipse-theia/theia/pull/14676)
+
+This PR makes menu nodes and tab toolbar items into active object instead of pure data descriptors. This means they can polymorphically handle concerns like enablement, visibility, command execution and rendering. This keeps concerns like conversion of parameters out of the general tool bar and menu handling code. In this way, we could get rid of the MenuCommandExecutor and MenuCommandAdapter infrastructure.
+If you are simply registering toolbar items and menus, little will change for you as a Theia adopter. Mainly, some of the paremeter types have changed in menu-model-registry.ts. Menu registration has been simplified in that an independent submenu is simply a menu that is registered under a path that does not start with the MAIN_MENU_BAR prefix.
+If you override any of the toolbar or menu related implementations in your product, the biggest change will be that some functionality is now delegated to the menu and too bar item implementations. If this breaks your use case, please let us know.
+
 ### v1.38.0
 
 #### Inversify 6.0

examples/api-samples/package.json

@@ -25,10 +25,6 @@
       "frontend": "lib/browser/api-samples-frontend-module",
       "backend": "lib/node/api-samples-backend-module"
     },
-    {
-      "frontend": "lib/browser/menu/sample-browser-menu-module",
-      "frontendElectron": "lib/electron-browser/menu/sample-electron-menu-module"
-    },
     {
       "electronMain": "lib/electron-main/update/sample-updater-main-module",
       "frontendElectron": "lib/electron-browser/updater/sample-updater-frontend-module"
@@ -65,4 +61,4 @@
   "devDependencies": {
     "@theia/ext-scripts": "1.59.0"
   }
-}
+}

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

@@ -18,8 +18,8 @@ import { ConfirmDialog, Dialog, QuickInputService } from '@theia/core/lib/browse
 import { ReactDialog } from '@theia/core/lib/browser/dialogs/react-dialog';
 import { SelectComponent } from '@theia/core/lib/browser/widgets/select-component';
 import {
-    Command, CommandContribution, CommandRegistry, MAIN_MENU_BAR,
-    MenuContribution, MenuModelRegistry, MenuNode, MessageService, SubMenuOptions
+    Command, CommandContribution, CommandMenu, CommandRegistry, ContextExpressionMatcher, MAIN_MENU_BAR,
+    MenuContribution, MenuModelRegistry, MenuPath, MessageService
 } from '@theia/core/lib/common';
 import { inject, injectable, interfaces } from '@theia/core/shared/inversify';
 import * as React from '@theia/core/shared/react';
@@ -227,9 +227,8 @@ export class SampleMenuContribution implements MenuContribution {
     registerMenus(menus: MenuModelRegistry): void {
         setTimeout(() => {
             const subMenuPath = [...MAIN_MENU_BAR, 'sample-menu'];
-            menus.registerSubmenu(subMenuPath, 'Sample Menu', {
-                order: '2' // that should put the menu right next to the File menu
-            });
+            menus.registerSubmenu(subMenuPath, 'Sample Menu', { sortString: '2' }); // that should put the menu right next to the File menu
+
             menus.registerMenuAction(subMenuPath, {
                 commandId: SampleCommand.id,
                 order: '0'
@@ -239,7 +238,7 @@ export class SampleMenuContribution implements MenuContribution {
                 order: '2'
             });
             const subSubMenuPath = [...subMenuPath, 'sample-sub-menu'];
-            menus.registerSubmenu(subSubMenuPath, 'Sample sub menu', { order: '2' });
+            menus.registerSubmenu(subSubMenuPath, 'Sample sub menu', { sortString: '2' });
             menus.registerMenuAction(subSubMenuPath, {
                 commandId: SampleCommand.id,
                 order: '1'
@@ -248,8 +247,8 @@ export class SampleMenuContribution implements MenuContribution {
                 commandId: SampleCommand2.id,
                 order: '3'
             });
-            const placeholder = new PlaceholderMenuNode([...subSubMenuPath, 'placeholder'].join('-'), 'Placeholder', { order: '0' });
-            menus.registerMenuNode(subSubMenuPath, placeholder);
+            const placeholder = new PlaceholderMenuNode([...subSubMenuPath, 'placeholder'].join('-'), 'Placeholder', '0');
+            menus.registerCommandMenu(subSubMenuPath, placeholder);
 
             /**
              * Register an action menu with an invalid command (un-registered and without a label) in order
@@ -258,22 +257,35 @@ export class SampleMenuContribution implements MenuContribution {
             menus.registerMenuAction(subMenuPath, { commandId: 'invalid-command' });
         }, 10000);
     }
-
 }
 
 /**
  * Special menu node that is not backed by any commands and is always disabled.
  */
-export class PlaceholderMenuNode implements MenuNode {
+export class PlaceholderMenuNode implements CommandMenu {
 
-    constructor(readonly id: string, public readonly label: string, protected options?: SubMenuOptions) { }
+    constructor(readonly id: string, public readonly label: string, readonly order?: string, readonly icon?: string) { }
 
-    get icon(): string | undefined {
-        return this.options?.iconClass;
+    isEnabled(effectiveMenuPath: MenuPath, ...args: unknown[]): boolean {
+        return false;
+    }
+
+    isToggled(effectiveMenuPath: MenuPath): boolean {
+        return false;
+    }
+    run(effectiveMenuPath: MenuPath, ...args: unknown[]): Promise<void> {
+        throw new Error('Should never happen');
+    }
+    getAccelerator(context: HTMLElement | undefined): string[] {
+        return [];
     }
 
     get sortString(): string {
-        return this.options?.order || this.label;
+        return this.order || this.label;
+    }
+
+    isVisible<T>(effectiveMenuPath: MenuPath, contextMatcher: ContextExpressionMatcher<T>, context: T | undefined, ...args: unknown[]): boolean {
+        return true;
     }
 
 }

examples/playwright/src/theia-notebook-editor.ts

@@ -49,7 +49,7 @@ export class TheiaNotebookEditor extends TheiaEditor {
     }
 
     tabLocator(): Locator {
-        return this.page.locator(this.data.viewSelector);
+        return this.page.locator(this.data.tabSelector);
     }
 
     override async waitForVisible(): Promise<void> {

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

@@ -28,7 +28,8 @@ export class TheiaToolbarItem extends TheiaPageObject {
     }
 
     async isEnabled(): Promise<boolean> {
-        const classAttribute = await this.element.getAttribute('class');
+        const child = await this.element.$(':first-child');
+        const classAttribute = child && await child.getAttribute('class');
         if (classAttribute === undefined || classAttribute === null) {
             return false;
         }

packages/core/src/browser/common-frontend-contribution.ts

@@ -18,7 +18,7 @@
 
 import debounce = require('lodash.debounce');
 import { injectable, inject, optional } from 'inversify';
-import { MAIN_MENU_BAR, MANAGE_MENU, MenuContribution, MenuModelRegistry, ACCOUNTS_MENU } from '../common/menu';
+import { MAIN_MENU_BAR, MANAGE_MENU, MenuContribution, MenuModelRegistry, ACCOUNTS_MENU, CompoundMenuNode, CommandMenu, Group, Submenu } from '../common/menu';
 import { KeybindingContribution, KeybindingRegistry } from './keybinding';
 import { FrontendApplication } from './frontend-application';
 import { FrontendApplicationContribution, OnWillStopAction } from './frontend-application-contribution';
@@ -84,7 +84,7 @@ export namespace CommonMenus {
     export const FILE_SETTINGS_SUBMENU_THEME = [...FILE_SETTINGS_SUBMENU, '2_settings_submenu_theme'];
     export const FILE_CLOSE = [...FILE, '6_close'];
 
-    export const FILE_NEW_CONTRIBUTIONS = 'file/newFile';
+    export const FILE_NEW_CONTRIBUTIONS = ['file', 'newFile'];
 
     export const EDIT = [...MAIN_MENU_BAR, '2_edit'];
     export const EDIT_UNDO = [...EDIT, '1_undo'];
@@ -622,7 +622,7 @@ export class CommonFrontendContribution implements FrontendApplicationContributi
         registry.registerSubmenu(CommonMenus.HELP, nls.localizeByDefault('Help'));
 
         // For plugins contributing create new file commands/menu-actions
-        registry.registerIndependentSubmenu(CommonMenus.FILE_NEW_CONTRIBUTIONS, nls.localizeByDefault('New File...'));
+        registry.registerSubmenu(CommonMenus.FILE_NEW_CONTRIBUTIONS, nls.localizeByDefault('New File...'));
 
         registry.registerMenuAction(CommonMenus.FILE_SAVE, {
             commandId: CommonCommands.SAVE.id
@@ -763,7 +763,7 @@ export class CommonFrontendContribution implements FrontendApplicationContributi
             commandId: CommonCommands.SELECT_ICON_THEME.id
         });
 
-        registry.registerSubmenu(CommonMenus.MANAGE_SETTINGS_THEMES, nls.localizeByDefault('Themes'), { order: 'a50' });
+        registry.registerSubmenu(CommonMenus.MANAGE_SETTINGS_THEMES, nls.localizeByDefault('Themes'), { sortString: 'a50' });
         registry.registerMenuAction(CommonMenus.MANAGE_SETTINGS_THEMES, {
             commandId: CommonCommands.SELECT_COLOR_THEME.id,
             order: '0'
@@ -1499,7 +1499,7 @@ export class CommonFrontendContribution implements FrontendApplicationContributi
      * @todo https://github.com/eclipse-theia/theia/issues/12824
      */
     protected async showNewFilePicker(): Promise<void> {
-        const newFileContributions = this.menuRegistry.getMenuNode(CommonMenus.FILE_NEW_CONTRIBUTIONS); // Add menus
+        const newFileContributions = this.menuRegistry.getMenuNode(CommonMenus.FILE_NEW_CONTRIBUTIONS) as Submenu; // Add menus
         const items: QuickPickItemOrSeparator[] = [
             {
                 label: nls.localizeByDefault('New Text File'),
@@ -1508,22 +1508,22 @@ export class CommonFrontendContribution implements FrontendApplicationContributi
             },
             ...newFileContributions.children
                 .flatMap(node => {
-                    if (node.children && node.children.length > 0) {
+                    if (CompoundMenuNode.is(node) && node.children.length > 0) {
                         return node.children;
                     }
                     return node;
                 })
-                .filter(node => node.role || node.command)
+                .filter(node => Group.is(node) || CommandMenu.is(node))
                 .map(node => {
-                    if (node.role) {
+                    if (Group.is(node)) {
                         return { type: 'separator' } as QuickPickSeparator;
+                    } else {
+                        const item = node as CommandMenu;
+                        return {
+                            label: item.label,
+                            execute: () => item.run(CommonMenus.FILE_NEW_CONTRIBUTIONS)
+                        };
                     }
-                    const command = this.commandRegistry.getCommand(node.command!);
-                    return {
-                        label: command!.label!,
-                        execute: async () => this.commandRegistry.executeCommand(command!.id!)
-                    };
-
                 })
         ];