diff --git a/examples/api-tests/src/menus.spec.js b/examples/api-tests/src/menus.spec.js index 542c533bc6ff9..905e60422dfaa 100644 --- a/examples/api-tests/src/menus.spec.js +++ b/examples/api-tests/src/menus.spec.js @@ -44,8 +44,8 @@ describe('Menus', function () { const container = window.theia.container; const shell = container.get(ApplicationShell); + /** @type {BrowserMenuBarContribution} */ const menuBarContribution = container.get(BrowserMenuBarContribution); - const menuBar = /** @type {import('@theia/core/lib/browser/menu/browser-menu-plugin').MenuBarWidget} */ (menuBarContribution.menuBar); const pluginService = container.get(HostedPluginSupport); const menus = container.get(MenuModelRegistry); const commands = container.get(CommandRegistry); @@ -54,6 +54,9 @@ describe('Menus', function () { before(async function () { await pluginService.didStart; await pluginService.activateByViewContainer('explorer'); + // Updating the menu interferes with our ability to programmatically test it + // We simply disable the menu updating + menus.isReady = false; }); const toTearDown = new DisposableCollection(); @@ -73,7 +76,7 @@ describe('Menus', function () { ]) { it(`should toggle '${contribution.viewLabel}' view`, async () => { await contribution.closeView(); - await menuBar.triggerMenuItem('View', contribution.viewLabel); + await menuBarContribution.menuBar.triggerMenuItem('View', contribution.viewLabel); await shell.waitForActivation(contribution.viewId); }); } diff --git a/packages/core/src/browser/menu/browser-menu-plugin.ts b/packages/core/src/browser/menu/browser-menu-plugin.ts index 392c2003eac26..d1ceac06b8220 100644 --- a/packages/core/src/browser/menu/browser-menu-plugin.ts +++ b/packages/core/src/browser/menu/browser-menu-plugin.ts @@ -71,25 +71,30 @@ export class BrowserMainMenuFactory implements MenuWidgetFactory { const menuBar = new DynamicMenuBarWidget(); menuBar.id = 'theia:menubar'; this.corePreferences.ready.then(() => { - this.showMenuBar(menuBar, this.corePreferences.get('window.menuBarVisibility', 'classic')); - }); - const preferenceListener = this.corePreferences.onPreferenceChanged(preference => { - if (preference.preferenceName === 'window.menuBarVisibility') { - this.showMenuBar(menuBar, preference.newValue); - } - }); - const keybindingListener = this.keybindingRegistry.onKeybindingsChanged(() => { - const preference = this.corePreferences['window.menuBarVisibility']; - this.showMenuBar(menuBar, preference); - }); - menuBar.disposed.connect(() => { - preferenceListener.dispose(); - keybindingListener.dispose(); + this.showMenuBar(menuBar); }); + const disposable = new DisposableCollection( + this.corePreferences.onPreferenceChanged(change => { + if (change.preferenceName === 'window.menuBarVisibility') { + this.showMenuBar(menuBar, change.newValue); + } + }), + this.keybindingRegistry.onKeybindingsChanged(() => { + this.showMenuBar(menuBar); + }), + this.menuProvider.onDidChange(() => { + this.showMenuBar(menuBar); + }) + ); + menuBar.disposed.connect(() => disposable.dispose()); return menuBar; } - protected showMenuBar(menuBar: DynamicMenuBarWidget, preference: string | undefined): void { + protected getMenuBarVisibility(): string { + return this.corePreferences.get('window.menuBarVisibility', 'classic'); + } + + protected showMenuBar(menuBar: DynamicMenuBarWidget, preference = this.getMenuBarVisibility()): void { if (preference && ['classic', 'visible'].includes(preference)) { menuBar.clearMenus(); this.fillMenuBar(menuBar); @@ -187,13 +192,13 @@ export class DynamicMenuBarWidget extends MenuBarWidget { this.openActiveMenu(); await waitForRevealed(menu); - const menuPath = [label]; + const menuPath = [label, ...labels]; let current = menu; for (const itemLabel of labels) { const item = current.items.find(i => i.label === itemLabel); if (!item || !item.submenu) { - throw new Error(`could not find '${label}' submenu in ${menuPath.map(l => "'" + l + "'").join(' -> ')} menu`); + throw new Error(`could not find '${itemLabel}' submenu in ${menuPath.map(l => "'" + l + "'").join(' -> ')} menu`); } current.activeItem = item; current.triggerActiveItem(); @@ -211,7 +216,7 @@ export class DynamicMenuBarWidget extends MenuBarWidget { const menu = await this.activateMenu(menuPath[0], ...menuPath.slice(1)); const item = menu.items.find(i => i.label === labels[labels.length - 1]); if (!item) { - throw new Error(`could not find '${label}' item in ${menuPath.map(l => "'" + l + "'").join(' -> ')} menu`); + throw new Error(`could not find '${labels[labels.length - 1]}' item in ${menuPath.map(l => "'" + l + "'").join(' -> ')} menu`); } menu.activeItem = item; menu.triggerActiveItem(); diff --git a/packages/core/src/common/menu/menu-model-registry.ts b/packages/core/src/common/menu/menu-model-registry.ts index 42b4a93ac9b96..e321440c45170 100644 --- a/packages/core/src/common/menu/menu-model-registry.ts +++ b/packages/core/src/common/menu/menu-model-registry.ts @@ -18,6 +18,7 @@ import { inject, injectable, named } from 'inversify'; import { Command, CommandRegistry } from '../command'; import { ContributionProvider } from '../contribution-provider'; import { Disposable } from '../disposable'; +import { Emitter, Event } from '../event'; import { ActionMenuNode } from './action-menu-node'; import { CompositeMenuNode, CompositeMenuNodeWrapper } from './composite-menu-node'; import { CompoundMenuNode, MenuAction, MenuNode, MenuNodeMetadata, MenuPath, MutableCompoundMenuNode, SubMenuOptions } from './menu-types'; @@ -68,6 +69,14 @@ export class MenuModelRegistry { protected readonly root = new CompositeMenuNode(''); protected readonly independentSubmenus = new Map(); + protected readonly onDidChangeEmitter = new Emitter(); + + get onDidChange(): Event { + return this.onDidChangeEmitter.event; + } + + protected isReady = false; + constructor( @inject(ContributionProvider) @named(MenuContribution) protected readonly contributions: ContributionProvider, @@ -78,6 +87,7 @@ export class MenuModelRegistry { for (const contrib of this.contributions.getContributions()) { contrib.registerMenus(this); } + this.isReady = true; } /** @@ -97,7 +107,9 @@ export class MenuModelRegistry { */ registerMenuNode(menuPath: MenuPath | string, menuNode: MenuNode, group?: string): Disposable { const parent = this.getMenuNode(menuPath, group); - return parent.addNode(menuNode); + const disposable = parent.addNode(menuNode); + this.fireChangeEvent(); + return this.changeEventOnDispose(disposable); } getMenuNode(menuPath: MenuPath | string, group?: string): MutableCompoundMenuNode { @@ -137,13 +149,15 @@ export class MenuModelRegistry { const groupPath = index === 0 ? [] : menuPath.slice(0, index); const parent = this.findGroup(groupPath, options); let groupNode = this.findSubMenu(parent, menuId, options); + let disposable = Disposable.NULL; if (!groupNode) { groupNode = new CompositeMenuNode(menuId, label, options, parent); - return parent.addNode(groupNode); + disposable = this.changeEventOnDispose(parent.addNode(groupNode)); } else { groupNode.updateOptions({ ...options, label }); - return Disposable.NULL; } + this.fireChangeEvent(); + return disposable; } registerIndependentSubmenu(id: string, label: string, options?: SubMenuOptions): Disposable { @@ -151,7 +165,7 @@ export class MenuModelRegistry { console.debug(`Independent submenu with path ${id} registered, but given ID already exists.`); } this.independentSubmenus.set(id, new CompositeMenuNode(id, label, options)); - return { dispose: () => this.independentSubmenus.delete(id) }; + return this.changeEventOnDispose(Disposable.create(() => this.independentSubmenus.delete(id))); } linkSubmenu(parentPath: MenuPath | string, childId: string | MenuPath, options?: SubMenuOptions, group?: string): Disposable { @@ -175,7 +189,9 @@ export class MenuModelRegistry { } const wrapper = new CompositeMenuNodeWrapper(child, parent, options); - return parent.addNode(wrapper); + const disposable = parent.addNode(wrapper); + this.fireChangeEvent(); + return this.changeEventOnDispose(disposable); } /** @@ -207,6 +223,7 @@ export class MenuModelRegistry { if (menuPath) { const parent = this.findGroup(menuPath); parent.removeNode(id); + this.fireChangeEvent(); return; } @@ -228,6 +245,7 @@ export class MenuModelRegistry { }); }; recurse(this.root); + this.fireChangeEvent(); } /** @@ -321,6 +339,19 @@ export class MenuModelRegistry { return true; } + protected changeEventOnDispose(disposable: Disposable): Disposable { + return Disposable.create(() => { + disposable.dispose(); + this.fireChangeEvent(); + }); + } + + protected fireChangeEvent(): void { + if (this.isReady) { + this.onDidChangeEmitter.fire(); + } + } + /** * Returns the {@link MenuPath path} at which a given menu node can be accessed from this registry, if it can be determined. * Returns `undefined` if the `parent` of any node in the chain is unknown. diff --git a/packages/core/src/electron-browser/menu/electron-main-menu-factory.ts b/packages/core/src/electron-browser/menu/electron-main-menu-factory.ts index 162b4dca9235c..68a305c29b094 100644 --- a/packages/core/src/electron-browser/menu/electron-main-menu-factory.ts +++ b/packages/core/src/electron-browser/menu/electron-main-menu-factory.ts @@ -101,6 +101,9 @@ export class ElectronMainMenuFactory extends BrowserMainMenuFactory { this.keybindingRegistry.onKeybindingsChanged(() => { this.setMenuBar(); }); + this.menuProvider.onDidChange(() => { + this.setMenuBar(); + }); } async setMenuBar(): Promise {