Refactor menu nodes (#14676)

Fixes #14217

Makes menu nodes active object that can decide on visibility,
enablement, etc. themselves.

Contributed on behalf of STMicroelectronics

Signed-off-by: Thomas Mäder <[email protected]>
Co-authored-by: Dennis Huebner <[email protected]>

Author: tsmaeder

.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

@@ -4,6 +4,14 @@
 
 - [Previous Changelogs](https://github.com/eclipse-theia/theia/tree/master/doc/changelogs/)
 
+## 1.62.0 - 
+
+
+<a name="breaking_changes_1.62.0">[Breaking Changes:](#breaking_changes_1.62.0)</a>
+
+- [core] Refactor menu nodes [#14676](https://github.com/eclipse-theia/theia/pull/14676) - Contributed on behalf of STMicroelectronics
+
+
 ## 1.61.0 - 4/29/2025
 
 - [ai-anthropic] fix: do not set anthropic tool choice without tools [#15329](https://github.com/eclipse-theia/theia/pull/15329)
@@ -199,11 +207,14 @@
 <a name="breaking_changes_1.60.0">[Breaking Changes:](#breaking_changes_1.60.0)</a>
 
 - [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] migrated from deprecated `phosphorJs` to `Lumino`; adopters must update imports from `@phosphor` to `@lumino`, adjust CSS selectors from `.p-` to `.lm-`, and make minor code changes such as replacing `icon` with `iconClass` in commands [#14320](https://github.com/eclipse-theia/theia/pull/14320) - Contributed on behalf of STMicroelectronics
-- [core] corrected typing of `addKeyListener` and `Widget.addKeyListener` to reflect events for `additionalEventTypes`; adopters explicitly expecting `KeyboardEvent` may need to update type declarations [#15210](https://github.com/eclipse-theia/theia/pull/15210)
-- [ai] changed the format of `ai-features.modelSettings.requestSettings`; also improved message types and updated the request object for LLMs [#15092](https://github.com/eclipse-theia/theia/pull/15092)
-- [ai-chat] changed `ParsedChatRequest.variables` to be `ResolvedAIVariable[]` instead of `Map<string, AIVariable>` [#15196](https://github.com/eclipse-theia/theia/pull/15196)
-- [ai-chat] made `ChatRequestParser.parseChatRequest` asynchronous and added a `ChatContext` parameter [#15196](https://github.com/eclipse-theia/theia/pull/15196)
+- [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>
+
+- [ai-chat] `ParsedChatRequest.variables` is now `ResolvedAIVariable[]` instead of a `Map<string, AIVariable>` [#15196](https://github.com/eclipse-theia/theia/pull/15196)
+- [ai-chat] `ChatRequestParser.parseChatRequest` is now asynchronous and expects an additional `ChatContext` parameter [#15196](https://github.com/eclipse-theia/theia/pull/15196)
 
 ## 1.59.0 - 02/27/2025
 

doc/Migration.md

@@ -59,6 +59,14 @@ For example:
 }
 ```
 
+### v1.62.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.61.0"
   }
-}
+}

examples/api-samples/src/browser/menu/sample-browser-menu-module.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/api-samples/src/browser/menu/sample-menu-contribution.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/api-samples/src/electron-browser/menu/sample-electron-menu-module.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;
         }