Exposed onChanged event on the menu registry.

From now on, both the electron and the browser
based main menu factories listen `onChanged`
events and updates the main menu by recreating
it from scratch.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>

CHANGELOG.md

@@ -2,10 +2,13 @@
 
 ## v0.17.0
 
+- [core] From now on, one can dynamically create and remove menu actions and arbitrary submenu items at runtime. [#7276](https://github.com/eclipse-theia/theia/pull/7276)
 - [preferences] add a new preference to silence notifications [#7195](https://github.com/eclipse-theia/theia/pull/7195)
 
 Breaking changes:
 
+- [core] Renamed the `browser-menu-plugin.ts` module to `browser-main-menu-factory.ts`. From now on, both the `BrowserMainMenuFactory` and the `ElectronMainMenuFactory`
+have a no-args `constructor`. Renamed `BrowserMenuBarContribution` to `BrowserMenuContribution`. `BrowserMenuContribution` was moved to its own `browser-menu-contribution.ts` module. Renamed the `browserMenuBarContribution` field to `browserMenuContribution` in the `MonacoQuickOpenService`.
 - [scm][git] the History view (GitHistoryWidget) has moved from the git package to a new   package, scm-extra, and
   renamed to ScmHistoryWidget.  GitNavigableListWidget has also moved.
   CSS classes have been moved renamed accordingly.  [6381](https://github.com/eclipse-theia/theia/pull/6381)

examples/api-tests/src/menus.spec.js

@@ -17,7 +17,7 @@
 // @ts-check
 describe('Menus', function () {
 
-    const { BrowserMenuBarContribution } = require('@theia/core/lib/browser/menu/browser-menu-plugin');
+    const { BrowserMenuContribution } = require('@theia/core/lib/browser/menu/browser-menu-contribution');
     const { ApplicationShell } = require('@theia/core/lib/browser/shell/application-shell');
     const { CallHierarchyContribution } = require('@theia/callhierarchy/lib/browser/callhierarchy-contribution');
     const { FileNavigatorContribution } = require('@theia/navigator/lib/browser/navigator-contribution');
@@ -32,8 +32,8 @@ describe('Menus', function () {
     /** @type {import('inversify').Container} */
     const container = window['theia'].container;
     const shell = container.get(ApplicationShell);
-    const menuBarContribution = container.get(BrowserMenuBarContribution);
-    const menuBar = menuBarContribution.menuBar;
+    const menuContribution = container.get(BrowserMenuContribution);
+    const menuBar = menuContribution.menuBar;
 
     for (const contribution of [
         container.get(CallHierarchyContribution),

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

@@ -19,7 +19,7 @@
 import { inject, injectable } from 'inversify';
 import { MenuPath } from '../../common/menu';
 import { ContextMenuRenderer, Anchor, RenderContextMenuOptions } from '../context-menu-renderer';
-import { BrowserMainMenuFactory } from './browser-menu-plugin';
+import { BrowserMainMenuFactory } from './browser-main-menu-factory';
 
 @injectable()
 export class BrowserContextMenuRenderer implements ContextMenuRenderer {

packages/core/src/browser/menu/browser-menu-plugin.ts → packages/core/src/browser/menu/browser-main-menu-factory.ts

@@ -17,18 +17,16 @@
 /* eslint-disable @typescript-eslint/no-explicit-any */
 
 import { injectable, inject } from 'inversify';
-import { MenuBar, Menu as MenuWidget, Widget } from '@phosphor/widgets';
+import { MenuBar, Menu as MenuWidget } from '@phosphor/widgets';
 import { CommandRegistry as PhosphorCommandRegistry } from '@phosphor/commands';
 import {
     CommandRegistry, ActionMenuNode, CompositeMenuNode,
-    MenuModelRegistry, MAIN_MENU_BAR, MenuPath, ILogger
+    MenuModelRegistry, MAIN_MENU_BAR, MenuPath
 } from '../../common';
 import { KeybindingRegistry } from '../keybinding';
-import { FrontendApplicationContribution, FrontendApplication } from '../frontend-application';
 import { ContextKeyService } from '../context-key-service';
 import { ContextMenuContext } from './context-menu-context';
 import { waitForRevealed } from '../widgets';
-import { ApplicationShell } from '../shell';
 
 export abstract class MenuBarWidget extends MenuBar {
     abstract activateMenu(label: string, ...labels: string[]): Promise<MenuWidget>;
@@ -38,20 +36,20 @@ export abstract class MenuBarWidget extends MenuBar {
 @injectable()
 export class BrowserMainMenuFactory {
 
-    @inject(ILogger)
-    protected readonly logger: ILogger;
-
     @inject(ContextKeyService)
     protected readonly contextKeyService: ContextKeyService;
 
     @inject(ContextMenuContext)
     protected readonly context: ContextMenuContext;
 
-    constructor(
-        @inject(CommandRegistry) protected readonly commandRegistry: CommandRegistry,
-        @inject(KeybindingRegistry) protected readonly keybindingRegistry: KeybindingRegistry,
-        @inject(MenuModelRegistry) protected readonly menuProvider: MenuModelRegistry
-    ) { }
+    @inject(CommandRegistry)
+    protected readonly commandRegistry: CommandRegistry;
+
+    @inject(KeybindingRegistry)
+    protected readonly keybindingRegistry: KeybindingRegistry;
+
+    @inject(MenuModelRegistry)
+    protected readonly menuRegistry: MenuModelRegistry;
 
     createMenuBar(): MenuBarWidget {
         const menuBar = new DynamicMenuBarWidget();
@@ -65,8 +63,8 @@ export class BrowserMainMenuFactory {
         return menuBar;
     }
 
-    protected fillMenuBar(menuBar: MenuBarWidget): void {
-        const menuModel = this.menuProvider.getMenu(MAIN_MENU_BAR);
+    fillMenuBar(menuBar: MenuBarWidget): void {
+        const menuModel = this.menuRegistry.getMenu(MAIN_MENU_BAR);
         const phosphorCommands = this.createPhosphorCommands(menuModel);
         // for the main menu we want all items to be visible.
         phosphorCommands.isVisible = () => true;
@@ -80,7 +78,7 @@ export class BrowserMainMenuFactory {
     }
 
     createContextMenu(path: MenuPath, args?: any[]): MenuWidget {
-        const menuModel = this.menuProvider.getMenu(path);
+        const menuModel = this.menuRegistry.getMenu(path);
         const phosphorCommands = this.createPhosphorCommands(menuModel, args);
 
         const contextMenu = new DynamicMenuWidget(menuModel, { commands: phosphorCommands }, this.contextKeyService, this.context);
@@ -298,32 +296,3 @@ class DynamicMenuWidget extends MenuWidget {
     }
 
 }
-
-@injectable()
-export class BrowserMenuBarContribution implements FrontendApplicationContribution {
-
-    @inject(ApplicationShell)
-    protected readonly shell: ApplicationShell;
-
-    constructor(
-        @inject(BrowserMainMenuFactory) protected readonly factory: BrowserMainMenuFactory
-    ) { }
-
-    onStart(app: FrontendApplication): void {
-        const logo = this.createLogo();
-        app.shell.addWidget(logo, { area: 'top' });
-        const menu = this.factory.createMenuBar();
-        app.shell.addWidget(menu, { area: 'top' });
-    }
-
-    get menuBar(): MenuBarWidget | undefined {
-        return this.shell.topPanel.widgets.find(w => w instanceof MenuBarWidget) as MenuBarWidget | undefined;
-    }
-
-    protected createLogo(): Widget {
-        const logo = new Widget();
-        logo.id = 'theia:icon';
-        logo.addClass('theia-icon');
-        return logo;
-    }
-}

packages/core/src/browser/menu/browser-menu-contribution.ts

@@ -0,0 +1,64 @@
+/********************************************************************************
+ * Copyright (C) 2017 TypeFox and others.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v. 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0.
+ *
+ * This Source Code may also be made available under the following Secondary
+ * Licenses when the conditions for such availability set forth in the Eclipse
+ * Public License v. 2.0 are satisfied: GNU General Public License, version 2
+ * with the GNU Classpath Exception which is available at
+ * https://www.gnu.org/software/classpath/license.html.
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
+ ********************************************************************************/
+
+import { injectable, inject } from 'inversify';
+import { Widget } from '@phosphor/widgets';
+import debounce = require('lodash.debounce');
+import { ApplicationShell } from '../shell';
+import { MenuModelRegistry } from '../../common';
+import { FrontendApplicationContribution, FrontendApplication } from '../frontend-application';
+import { BrowserMainMenuFactory, MenuBarWidget } from './browser-main-menu-factory';
+
+@injectable()
+export class BrowserMenuContribution implements FrontendApplicationContribution {
+
+    @inject(ApplicationShell)
+    protected readonly shell: ApplicationShell;
+
+    @inject(BrowserMainMenuFactory)
+    protected readonly factory: BrowserMainMenuFactory;
+
+    @inject(MenuModelRegistry)
+    protected readonly menuRegistry: MenuModelRegistry;
+
+    onStart(app: FrontendApplication): void {
+        const logo = this.createLogo();
+        app.shell.addWidget(logo, { area: 'top' });
+        const menuBar = this.factory.createMenuBar();
+        app.shell.addWidget(menuBar, { area: 'top' });
+        const update = debounce(() => this.update(), 100);
+        this.menuRegistry.onChanged(update);
+    }
+
+    get menuBar(): MenuBarWidget | undefined {
+        return this.shell.topPanel.widgets.find(w => w instanceof MenuBarWidget) as MenuBarWidget | undefined;
+    }
+
+    protected createLogo(): Widget {
+        const logo = new Widget();
+        logo.id = 'theia:icon';
+        logo.addClass('theia-icon');
+        return logo;
+    }
+
+    protected update(): void {
+        if (this.menuBar) {
+            this.menuBar.clearMenus();
+            this.factory.fillMenuBar(this.menuBar);
+        }
+    }
+
+}

packages/core/src/browser/menu/browser-menu-module.ts

@@ -17,12 +17,13 @@
 import { ContainerModule } from 'inversify';
 import { FrontendApplicationContribution } from '../frontend-application';
 import { ContextMenuRenderer } from '../context-menu-renderer';
-import { BrowserMenuBarContribution, BrowserMainMenuFactory } from './browser-menu-plugin';
+import { BrowserMainMenuFactory } from './browser-main-menu-factory';
+import { BrowserMenuContribution } from './browser-menu-contribution';
 import { BrowserContextMenuRenderer } from './browser-context-menu-renderer';
 
 export default new ContainerModule(bind => {
     bind(BrowserMainMenuFactory).toSelf().inSingletonScope();
     bind(ContextMenuRenderer).to(BrowserContextMenuRenderer).inSingletonScope();
-    bind(BrowserMenuBarContribution).toSelf().inSingletonScope();
-    bind(FrontendApplicationContribution).toService(BrowserMenuBarContribution);
+    bind(BrowserMenuContribution).toSelf().inSingletonScope();
+    bind(FrontendApplicationContribution).toService(BrowserMenuContribution);
 });

packages/core/src/common/menu.ts

@@ -16,6 +16,7 @@
 
 import { injectable, inject, named } from 'inversify';
 import { Disposable } from './disposable';
+import { Event, Emitter } from './event';
 import { CommandRegistry, Command } from './command';
 import { ContributionProvider } from './contribution-provider';
 
@@ -41,7 +42,7 @@ export namespace MenuAction {
 }
 
 export interface SubMenuOptions {
-    iconClass: string
+    iconClass: string | undefined;
 }
 
 export type MenuPath = string[];
@@ -64,6 +65,7 @@ export interface MenuContribution {
 @injectable()
 export class MenuModelRegistry {
     protected readonly root = new CompositeMenuNode('');
+    protected readonly onChangedEmitter = new Emitter<void>();
 
     constructor(
         @inject(ContributionProvider) @named(MenuContribution)
@@ -80,7 +82,9 @@ export class MenuModelRegistry {
     registerMenuAction(menuPath: MenuPath, item: MenuAction): Disposable {
         const parent = this.findGroup(menuPath);
         const actionNode = new ActionMenuNode(item, this.commands);
-        return parent.addNode(actionNode);
+        const toDispose = parent.addNode(actionNode);
+        this.fireChanged();
+        return toDispose;
     }
 
     registerSubmenu(menuPath: MenuPath, label: string, options?: SubMenuOptions): Disposable {
@@ -94,15 +98,25 @@ export class MenuModelRegistry {
         let groupNode = this.findSubMenu(parent, menuId);
         if (!groupNode) {
             groupNode = new CompositeMenuNode(menuId, label, options ? options.iconClass : undefined);
-            return parent.addNode(groupNode);
+            const toDispose = parent.addNode(groupNode);
+            this.fireChanged();
+            return toDispose;
         } else {
+            let hasChanged = false;
             if (!groupNode.label) {
+                hasChanged = true;
                 groupNode.label = label;
             } else if (groupNode.label !== label) {
                 throw new Error("The group '" + menuPath.join('/') + "' already has a different label.");
             }
-            if (!groupNode.iconClass && options) {
-                groupNode.iconClass = options.iconClass;
+            if (options) {
+                if (groupNode.iconClass !== options.iconClass) {
+                    groupNode.iconClass = options.iconClass;
+                    hasChanged = true;
+                }
+            }
+            if (hasChanged) {
+                this.fireChanged();
             }
             return { dispose: () => { } };
         }
@@ -133,15 +147,21 @@ export class MenuModelRegistry {
 
         if (menuPath) {
             const parent = this.findGroup(menuPath);
-            parent.removeNode(id);
+            const hasChanged = parent.removeNode(id);
+            if (hasChanged) {
+                this.fireChanged();
+            }
             return;
         }
 
         // Recurse all menus, removing any menus matching the id
         const recurse = (root: CompositeMenuNode) => {
             root.children.forEach(node => {
                 if (node instanceof CompositeMenuNode) {
-                    node.removeNode(id);
+                    const hasChanged = node.removeNode(id);
+                    if (hasChanged) {
+                        this.fireChanged();
+                    }
                     recurse(node);
                 }
             });
@@ -173,6 +193,18 @@ export class MenuModelRegistry {
     getMenu(menuPath: MenuPath = []): CompositeMenuNode {
         return this.findGroup(menuPath);
     }
+
+    protected fireChanged(): void {
+        this.onChangedEmitter.fire();
+    }
+
+    /**
+     * An `onChanged` event is emitted when either the menu parentage or one of the menu node's property has changed.
+     */
+    get onChanged(): Event<void> {
+        return this.onChangedEmitter.event;
+    }
+
 }
 
 export interface MenuNode {
@@ -225,14 +257,19 @@ export class CompositeMenuNode implements MenuNode {
         };
     }
 
-    public removeNode(id: string): void {
+    /**
+     * `true` if the composite node contained a child node with this `id`. Otherwise, `false`.
+     */
+    public removeNode(id: string): boolean {
         const node = this._children.find(n => n.id === id);
         if (node) {
             const idx = this._children.indexOf(node);
             if (idx >= 0) {
                 this._children.splice(idx, 1);
+                return true;
             }
         }
+        return false;
     }
 
     get sortString(): string {