Skip to content

Commit e572c4e

Browse files
committed
Pass the menu node parent chain to menu items as execution context
Fixes eclipse-theia#16148 Contributed on behalf of STMicroelectronics Signed-off-by: Thomas Mäder <[email protected]>
1 parent dad8fae commit e572c4e

25 files changed

+304
-208
lines changed

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ import { ConfirmDialog, Dialog, QuickInputService } from '@theia/core/lib/browse
1818
import { ReactDialog } from '@theia/core/lib/browser/dialogs/react-dialog';
1919
import { SelectComponent } from '@theia/core/lib/browser/widgets/select-component';
2020
import {
21-
Command, CommandContribution, CommandMenu, CommandRegistry, ContextExpressionMatcher, MAIN_MENU_BAR,
22-
MenuContribution, MenuModelRegistry, MenuPath, MessageService
21+
Command, CommandContribution, CommandMenu, CommandRegistry, CompoundMenuNode, ContextExpressionMatcher, MAIN_MENU_BAR,
22+
MenuContribution, MenuModelRegistry, MessageService
2323
} from '@theia/core/lib/common';
2424
import { inject, injectable, interfaces } from '@theia/core/shared/inversify';
2525
import * as React from '@theia/core/shared/react';
@@ -266,14 +266,14 @@ export class PlaceholderMenuNode implements CommandMenu {
266266

267267
constructor(readonly id: string, public readonly label: string, readonly order?: string, readonly icon?: string) { }
268268

269-
isEnabled(effectiveMenuPath: MenuPath, ...args: unknown[]): boolean {
269+
isEnabled(parentChain: CompoundMenuNode[], ...args: unknown[]): boolean {
270270
return false;
271271
}
272272

273-
isToggled(effectiveMenuPath: MenuPath): boolean {
273+
isToggled(parentChain: CompoundMenuNode[]): boolean {
274274
return false;
275275
}
276-
run(effectiveMenuPath: MenuPath, ...args: unknown[]): Promise<void> {
276+
run(parentChain: CompoundMenuNode[], ...args: unknown[]): Promise<void> {
277277
throw new Error('Should never happen');
278278
}
279279
getAccelerator(context: HTMLElement | undefined): string[] {
@@ -284,7 +284,7 @@ export class PlaceholderMenuNode implements CommandMenu {
284284
return this.order || this.label;
285285
}
286286

287-
isVisible<T>(effectiveMenuPath: MenuPath, contextMatcher: ContextExpressionMatcher<T>, context: T | undefined, ...args: unknown[]): boolean {
287+
isVisible<T>(parentChain: CompoundMenuNode[], contextMatcher: ContextExpressionMatcher<T>, context: T | undefined, ...args: unknown[]): boolean {
288288
return true;
289289
}
290290

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1521,7 +1521,7 @@ export class CommonFrontendContribution implements FrontendApplicationContributi
15211521
const item = node as CommandMenu;
15221522
return {
15231523
label: item.label,
1524-
execute: () => item.run(CommonMenus.FILE_NEW_CONTRIBUTIONS)
1524+
execute: () => item.run([])
15251525
};
15261526
}
15271527
})

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ export abstract class ContextMenuRenderer {
8686
render(options: RenderContextMenuOptions): ContextMenuAccess {
8787
let menu = options.menu;
8888
if (!menu) {
89-
menu = this.menuRegistry.getMenu(options.menuPath) || new GroupImpl('emtpyContextMenu');
89+
menu = this.menuRegistry.getMenu(options.menuPath!) || new GroupImpl('emtpyContextMenu');
9090
}
9191

9292
const resolvedOptions = this.resolve(options);
@@ -96,7 +96,6 @@ export abstract class ContextMenuRenderer {
9696
}
9797

9898
const access = this.doRender({
99-
menuPath: options.menuPath,
10099
menu,
101100
anchor: resolvedOptions.anchor,
102101
contextMatcher: options.contextKeyService || this.contextKeyService,
@@ -109,7 +108,6 @@ export abstract class ContextMenuRenderer {
109108
}
110109

111110
protected abstract doRender(params: {
112-
menuPath: MenuPath,
113111
menu: CompoundMenuNode,
114112
anchor: Anchor,
115113
contextMatcher: ContextMatcher,
@@ -133,7 +131,7 @@ export abstract class ContextMenuRenderer {
133131

134132
export interface RenderContextMenuOptions {
135133
menu?: CompoundMenuNode,
136-
menuPath: MenuPath;
134+
menuPath?: MenuPath;
137135
anchor: Anchor;
138136
args?: any[];
139137
/**

packages/core/src/browser/menu/action-menu-node.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
import { KeybindingRegistry } from '../keybinding';
1818
import { ContextKeyService } from '../context-key-service';
1919
import { DisposableCollection, isObject, CommandRegistry, Emitter } from '../../common';
20-
import { CommandMenu, ContextExpressionMatcher, MenuAction, MenuPath } from '../../common/menu/menu-types';
20+
import { CommandMenu, CompoundMenuNode, ContextExpressionMatcher, MenuAction } from '../../common/menu/menu-types';
2121

2222
export interface AcceleratorSource {
2323
getAccelerator(context: HTMLElement | undefined): string[];
@@ -70,7 +70,7 @@ export class ActionMenuNode implements CommandMenu {
7070
this.disposables.dispose();
7171
}
7272

73-
isVisible<T>(effeciveMenuPath: MenuPath, contextMatcher: ContextExpressionMatcher<T>, context: T | undefined, ...args: unknown[]): boolean {
73+
isVisible<T>(parentChain: CompoundMenuNode[], contextMatcher: ContextExpressionMatcher<T>, context: T | undefined, ...args: unknown[]): boolean {
7474
if (!this.commands.isVisible(this.action.commandId, ...args)) {
7575
return false;
7676
}
@@ -92,13 +92,13 @@ export class ActionMenuNode implements CommandMenu {
9292
return [];
9393
}
9494

95-
isEnabled(effeciveMenuPath: MenuPath, ...args: unknown[]): boolean {
95+
isEnabled(parentChain: CompoundMenuNode[], ...args: unknown[]): boolean {
9696
return this.commands.isEnabled(this.action.commandId, ...args);
9797
}
98-
isToggled(effeciveMenuPath: MenuPath, ...args: unknown[]): boolean {
98+
isToggled(parentChain: CompoundMenuNode[], ...args: unknown[]): boolean {
9999
return this.commands.isToggled(this.action.commandId, ...args);
100100
}
101-
async run(effeciveMenuPath: MenuPath, ...args: unknown[]): Promise<void> {
101+
async run(parentChain: CompoundMenuNode[], ...args: unknown[]): Promise<void> {
102102
return this.commands.executeCommand(this.action.commandId, ...args);
103103
}
104104

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

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -105,20 +105,20 @@ export class BrowserMainMenuFactory implements MenuWidgetFactory {
105105
const menuCommandRegistry = new LuminoCommandRegistry();
106106
for (const menu of menuModel.children) {
107107
if (CompoundMenuNode.is(menu) && RenderedMenuNode.is(menu)) {
108-
const menuWidget = this.createMenuWidget(MAIN_MENU_BAR, menu, this.contextKeyService, { commands: menuCommandRegistry });
108+
const menuWidget = this.createMenuWidget([], menu, this.contextKeyService, { commands: menuCommandRegistry });
109109
menuBar.addMenu(menuWidget);
110110
}
111111
}
112112
}
113113

114114
createContextMenu(effectiveMenuPath: MenuPath, menuModel: CompoundMenuNode, contextMatcher: ContextMatcher, args?: unknown[], context?: HTMLElement): MenuWidget {
115115
const menuCommandRegistry = new LuminoCommandRegistry();
116-
const contextMenu = this.createMenuWidget(effectiveMenuPath, menuModel, contextMatcher, { commands: menuCommandRegistry, context }, args);
116+
const contextMenu = this.createMenuWidget([], menuModel, contextMatcher, { commands: menuCommandRegistry, context }, args);
117117
return contextMenu;
118118
}
119119

120-
createMenuWidget(parentPath: MenuPath, menu: CompoundMenuNode, contextMatcher: ContextMatcher, options: BrowserMenuOptions, args?: unknown[]): DynamicMenuWidget {
121-
return new DynamicMenuWidget(parentPath, menu, options, contextMatcher, this.services, args);
120+
createMenuWidget(parentChain: CompoundMenuNode[], menu: CompoundMenuNode, contextMatcher: ContextMatcher, options: BrowserMenuOptions, args?: unknown[]): DynamicMenuWidget {
121+
return new DynamicMenuWidget(parentChain, menu, options, contextMatcher, this.services, args);
122122
}
123123

124124
protected get services(): MenuServices {
@@ -212,7 +212,7 @@ export class MenuServices {
212212
}
213213

214214
export interface MenuWidgetFactory {
215-
createMenuWidget(effectiveMenuPath: MenuPath, menu: Submenu, contextMatcher: ContextMatcher, options: BrowserMenuOptions): MenuWidget;
215+
createMenuWidget(parentChain: CompoundMenuNode[], menu: Submenu, contextMatcher: ContextMatcher, options: BrowserMenuOptions, args?: unknown[]): MenuWidget;
216216
}
217217

218218
/**
@@ -226,7 +226,7 @@ export class DynamicMenuWidget extends MenuWidget {
226226
protected previousFocusedElement: HTMLElement | undefined;
227227

228228
constructor(
229-
protected readonly effectiveMenuPath: MenuPath,
229+
protected readonly parentChain: CompoundMenuNode[],
230230
protected menu: CompoundMenuNode,
231231
protected options: BrowserMenuOptions,
232232
protected contextMatcher: ContextMatcher,
@@ -242,7 +242,7 @@ export class DynamicMenuWidget extends MenuWidget {
242242
this.title.iconClass = this.menu.icon;
243243
}
244244
}
245-
this.updateSubMenus(this.effectiveMenuPath, this, this.menu, this.options.commands, this.contextMatcher, this.options.context);
245+
this.updateSubMenus([...this.parentChain, this.menu], this, this.menu, this.options.commands, this.contextMatcher, this.options.context);
246246
}
247247

248248
protected override onAfterAttach(msg: Message): void {
@@ -291,7 +291,7 @@ export class DynamicMenuWidget extends MenuWidget {
291291
this.preserveFocusedElement(previousFocusedElement);
292292
this.clearItems();
293293
this.runWithPreservedFocusContext(() => {
294-
this.updateSubMenus(this.effectiveMenuPath, this, this.menu, this.options.commands, this.contextMatcher, this.options.context);
294+
this.updateSubMenus([this.menu], this, this.menu, this.options.commands, this.contextMatcher, this.options.context);
295295
});
296296
}
297297

@@ -305,9 +305,9 @@ export class DynamicMenuWidget extends MenuWidget {
305305
super.open(x, y, options);
306306
}
307307

308-
protected updateSubMenus(parentPath: MenuPath, parent: MenuWidget, menu: CompoundMenuNode, commands: LuminoCommandRegistry,
308+
protected updateSubMenus(parentChain: CompoundMenuNode[], parent: MenuWidget, menu: CompoundMenuNode, commands: LuminoCommandRegistry,
309309
contextMatcher: ContextMatcher, context?: HTMLElement | undefined): void {
310-
const items = this.createItems(parentPath, menu.children, commands, contextMatcher, context);
310+
const items = this.createItems(parentChain, menu.children, commands, contextMatcher, context);
311311
while (items[items.length - 1]?.type === 'separator') {
312312
items.pop();
313313
}
@@ -316,21 +316,20 @@ export class DynamicMenuWidget extends MenuWidget {
316316
}
317317
}
318318

319-
protected createItems(parentPath: MenuPath, nodes: MenuNode[], phCommandRegistry: LuminoCommandRegistry,
319+
protected createItems(parentChain: CompoundMenuNode[], nodes: MenuNode[], phCommandRegistry: LuminoCommandRegistry,
320320
contextMatcher: ContextMatcher, context?: HTMLElement): MenuWidget.IItemOptions[] {
321321
const result: MenuWidget.IItemOptions[] = [];
322322

323323
for (const node of nodes) {
324-
const nodePath = [...parentPath, node.id];
325-
if (node.isVisible(nodePath, contextMatcher, context, ...(this.args || []))) {
324+
if (node.isVisible(parentChain, contextMatcher, context, ...(this.args || []))) {
326325
if (CompoundMenuNode.is(node)) {
327326
if (RenderedMenuNode.is(node)) {
328-
const submenu = this.services.menuWidgetFactory.createMenuWidget(nodePath, node, this.contextMatcher, this.options);
327+
const submenu = this.services.menuWidgetFactory.createMenuWidget([...parentChain, node], node, this.contextMatcher, this.options, this.args);
329328
if (submenu.items.length > 0) {
330329
result.push({ type: 'submenu', submenu });
331330
}
332331
} else if (node.id !== 'inline') {
333-
const items = this.createItems(nodePath, node.children, phCommandRegistry, contextMatcher, context);
332+
const items = this.createItems([...parentChain, node], node.children, phCommandRegistry, contextMatcher, context);
334333
if (items.length > 0) {
335334
if (result[result.length - 1]?.type !== 'separator') {
336335
result.push({ type: 'separator' });
@@ -343,9 +342,9 @@ export class DynamicMenuWidget extends MenuWidget {
343342
} else if (CommandMenu.is(node)) {
344343
const id = !phCommandRegistry.hasCommand(node.id) ? node.id : `${node.id}:${DynamicMenuWidget.nextCommmandId++}`;
345344
phCommandRegistry.addCommand(id, {
346-
execute: () => { node.run(nodePath, ...(this.args || [])); },
347-
isEnabled: () => node.isEnabled(nodePath, ...(this.args || [])),
348-
isToggled: () => node.isToggled ? !!node.isToggled(nodePath, ...(this.args || [])) : false,
345+
execute: () => { node.run(parentChain, ...(this.args || [])); },
346+
isEnabled: () => node.isEnabled(parentChain, ...(this.args || [])),
347+
isToggled: () => node.isToggled ? !!node.isToggled(parentChain, ...(this.args || [])) : false,
349348
isVisible: () => true,
350349
label: node.label,
351350
iconClass: node.icon,

packages/core/src/browser/menu/composite-menu-node.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,12 @@ export class SubMenuLink implements CompoundMenuNode {
2828
get icon(): string | undefined { return this.delegate.icon; };
2929

3030
get sortString(): string { return this._sortString || this.delegate.sortString; };
31-
isVisible<T>(effectiveMenuPath: MenuPath, contextMatcher: ContextExpressionMatcher<T>, context: T | undefined, ...args: unknown[]): boolean {
32-
return this.delegate.isVisible(effectiveMenuPath, contextMatcher, context) && this._when ? contextMatcher.match(this._when, context) : true;
31+
isVisible<T>(parentChain: CompoundMenuNode[], contextMatcher: ContextExpressionMatcher<T>, context: T | undefined, ...args: unknown[]): boolean {
32+
return this.delegate.isVisible(parentChain, contextMatcher, context) && this._when ? contextMatcher.match(this._when, context) : true;
3333
}
3434

35-
isEmpty<T>(effectiveMenuPath: MenuPath, contextMatcher: ContextExpressionMatcher<T>, context: T | undefined, ...args: unknown[]): boolean {
36-
return this.delegate.isEmpty(effectiveMenuPath, contextMatcher, context, args);
35+
isEmpty<T>(parentChain: CompoundMenuNode[], contextMatcher: ContextExpressionMatcher<T>, context: T | undefined, ...args: unknown[]): boolean {
36+
return this.delegate.isEmpty(parentChain, contextMatcher, context, args);
3737
}
3838
}
3939

@@ -70,14 +70,14 @@ export abstract class AbstractCompoundMenuImpl implements MenuNode {
7070
/**
7171
* Menu nodes are sorted in ascending order based on their `sortString`.
7272
*/
73-
isVisible<T>(effectiveMenuPath: MenuPath, contextMatcher: ContextExpressionMatcher<T>, context: T | undefined, ...args: unknown[]): boolean {
73+
isVisible<T>(parentChain: CompoundMenuNode[], contextMatcher: ContextExpressionMatcher<T>, context: T | undefined, ...args: unknown[]): boolean {
7474
return (!this.when || contextMatcher.match(this.when, context));
7575
}
7676

77-
isEmpty<T>(effectiveMenuPath: MenuPath, contextMatcher: ContextExpressionMatcher<T>, context: T | undefined, ...args: unknown[]): boolean {
77+
isEmpty<T>(parentChain: CompoundMenuNode[], contextMatcher: ContextExpressionMatcher<T>, context: T | undefined, ...args: unknown[]): boolean {
7878
for (const child of this.children) {
79-
if (child.isVisible(effectiveMenuPath, contextMatcher, context, args)) {
80-
if (!CompoundMenuNode.is(child) || !child.isEmpty(effectiveMenuPath, contextMatcher, context, args)) {
79+
if (child.isVisible(parentChain, contextMatcher, context, args)) {
80+
if (!CompoundMenuNode.is(child) || !child.isEmpty(parentChain, contextMatcher, context, args)) {
8181
return false;
8282
}
8383
}

0 commit comments

Comments
 (0)