Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix #7619: don't wrap menu action args into another array #7622

Merged
merged 2 commits into from
Apr 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions examples/api-tests/src/menus.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,15 @@
// @ts-check
describe('Menus', function () {

const { assert } = chai;

const { BrowserMenuBarContribution } = require('@theia/core/lib/browser/menu/browser-menu-plugin');
const { BrowserContextMenuAccess } = require('@theia/core/lib/browser/menu/browser-context-menu-renderer');
const { ApplicationShell } = require('@theia/core/lib/browser/shell/application-shell');
const { ViewContainer } = require('@theia/core/lib/browser/view-container');
const { waitForRevealed, waitForHidden } = require('@theia/core/lib/browser/widgets/widget');
const { CallHierarchyContribution } = require('@theia/callhierarchy/lib/browser/callhierarchy-contribution');
const { EXPLORER_VIEW_CONTAINER_ID } = require('@theia/navigator/lib/browser/navigator-widget');
const { FileNavigatorContribution } = require('@theia/navigator/lib/browser/navigator-contribution');
const { ScmContribution } = require('@theia/scm/lib/browser/scm-contribution');
const { ScmHistoryContribution } = require('@theia/scm-extra/lib/browser/history/scm-history-contribution');
Expand All @@ -28,11 +34,19 @@ describe('Menus', function () {
const { PluginFrontendViewContribution } = require('@theia/plugin-ext/lib/main/browser/plugin-frontend-view-contribution');
const { ProblemContribution } = require('@theia/markers/lib/browser/problem/problem-contribution');
const { SearchInWorkspaceFrontendContribution } = require('@theia/search-in-workspace/lib/browser/search-in-workspace-frontend-contribution');
const { HostedPluginSupport } = require('@theia/plugin-ext/lib/hosted/browser/hosted-plugin');

const container = window.theia.container;
const shell = container.get(ApplicationShell);
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);

before(async function () {
await pluginService.didStart;
// register views for the explorer view container
await pluginService.activatePlugin('vscode.npm');
});

for (const contribution of [
container.get(CallHierarchyContribution),
Expand All @@ -52,4 +66,35 @@ describe('Menus', function () {
});
}

it('reveal more context menu in the explorer view container toolbar', async function () {
const viewContainer = await shell.revealWidget(EXPLORER_VIEW_CONTAINER_ID);
if (!(viewContainer instanceof ViewContainer)) {
assert.isTrue(viewContainer instanceof ViewContainer);
return;
}

const contribution = container.get(FileNavigatorContribution);
const waitForParts = [];
for (const part of viewContainer.getParts()) {
if (part.wrapped.id !== contribution.viewId) {
part.hide();
waitForParts.push(waitForHidden(part.wrapped));
} else {
part.show();
waitForParts.push(waitForRevealed(part.wrapped));
}
}
await Promise.all(waitForParts);

const contextMenuAccess = shell.leftPanelHandler.toolBar.showMoreContextMenu({ x: 0, y: 0 });
if (!(contextMenuAccess instanceof BrowserContextMenuAccess)) {
assert.isTrue(contextMenuAccess instanceof BrowserContextMenuAccess);
return;
}
const contextMenu = contextMenuAccess.menu;

await waitForRevealed(contextMenu);
assert.notEqual(contextMenu.items.length, 0);
});

});
7 changes: 5 additions & 2 deletions packages/core/src/browser/context-menu-renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,14 @@ export function toAnchor(anchor: HTMLElement | { x: number, y: number }): Anchor
return anchor instanceof HTMLElement ? { x: anchor.offsetLeft, y: anchor.offsetTop } : anchor;
}

export interface ContextMenuAccess {
}

export const ContextMenuRenderer = Symbol('ContextMenuRenderer');
export interface ContextMenuRenderer {
render(options: RenderContextMenuOptions): void;
render(options: RenderContextMenuOptions): ContextMenuAccess;
/** @deprecated since 0.7.2 pass `RenderContextMenuOptions` instead */
render(menuPath: MenuPath, anchor: Anchor, onHide?: () => void): void;
render(menuPath: MenuPath, anchor: Anchor, onHide?: () => void): ContextMenuAccess;
}

export interface RenderContextMenuOptions {
Expand Down
12 changes: 10 additions & 2 deletions packages/core/src/browser/menu/browser-context-menu-renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,31 @@

import { inject, injectable } from 'inversify';
import { MenuPath } from '../../common/menu';
import { ContextMenuRenderer, Anchor, RenderContextMenuOptions } from '../context-menu-renderer';
import { Menu } from '../widgets';
import { ContextMenuAccess, ContextMenuRenderer, Anchor, RenderContextMenuOptions } from '../context-menu-renderer';
import { BrowserMainMenuFactory } from './browser-menu-plugin';

export class BrowserContextMenuAccess implements ContextMenuAccess {
constructor(
public readonly menu: Menu
) { }
}

@injectable()
export class BrowserContextMenuRenderer implements ContextMenuRenderer {

constructor(@inject(BrowserMainMenuFactory) private menuFactory: BrowserMainMenuFactory) {
}

render(arg: MenuPath | RenderContextMenuOptions, arg2?: Anchor, arg3?: () => void): void {
render(arg: MenuPath | RenderContextMenuOptions, arg2?: Anchor, arg3?: () => void): BrowserContextMenuAccess {
const { menuPath, anchor, args, onHide } = RenderContextMenuOptions.resolve(arg, arg2, arg3);
const contextMenu = this.menuFactory.createContextMenu(menuPath, args);
const { x, y } = anchor instanceof MouseEvent ? { x: anchor.clientX, y: anchor.clientY } : anchor!;
if (onHide) {
contextMenu.aboutToClose.connect(() => onHide!());
}
contextMenu.open(x, y);
return new BrowserContextMenuAccess(contextMenu);
}

}
2 changes: 1 addition & 1 deletion packages/core/src/browser/menu/browser-menu-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ class MenuCommandRegistry extends PhosphorCommandRegistry {
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any
registerActionMenu(menu: ActionMenuNode, ...args: any[]): void {
registerActionMenu(menu: ActionMenuNode, args: any[]): void {
const { commandId } = menu.action;
const { commandRegistry } = this.services;
const command = commandRegistry.getCommand(commandId);
Expand Down
10 changes: 10 additions & 0 deletions packages/core/src/browser/shell/side-panel-toolbar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { TabBarToolbar, TabBarToolbarRegistry, TabBarToolbarFactory } from './ta
import { Message } from '@phosphor/messaging';
import { BaseWidget } from '../widgets';
import { Emitter } from '../../common/event';
import { ContextMenuAccess, Anchor } from '../context-menu-renderer';

export class SidePanelToolbar extends BaseWidget {

Expand Down Expand Up @@ -101,4 +102,13 @@ export class SidePanelToolbar extends BaseWidget {
this.update();
}
}

/* eslint-disable-next-line @typescript-eslint/no-explicit-any */
showMoreContextMenu(anchor: Anchor): ContextMenuAccess {
akosyakov marked this conversation as resolved.
Show resolved Hide resolved
if (this.toolbar) {
return this.toolbar.renderMoreContextMenu(anchor);
}
throw new Error(this.id + ' widget is not attached');
}

}
13 changes: 9 additions & 4 deletions packages/core/src/browser/shell/tab-bar-toolbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { CommandRegistry } from '../../common/command';
import { Disposable, DisposableCollection } from '../../common/disposable';
import { ContextKeyService } from '../context-key-service';
import { Event, Emitter } from '../../common/event';
import { ContextMenuRenderer } from '../context-menu-renderer';
import { ContextMenuRenderer, Anchor } from '../context-menu-renderer';
import { MenuModelRegistry } from '../../common/menu';

/**
Expand Down Expand Up @@ -145,6 +145,11 @@ export class TabBarToolbar extends ReactWidget {
event.stopPropagation();
event.preventDefault();

this.renderMoreContextMenu(event.nativeEvent);
};

/* eslint-disable-next-line @typescript-eslint/no-explicit-any */
renderMoreContextMenu(anchor: Anchor): any {
const menuPath = ['TAB_BAR_TOOLBAR_CONTEXT_MENU'];
const toDisposeOnHide = new DisposableCollection();
for (const [, item] of this.more) {
Expand All @@ -154,13 +159,13 @@ export class TabBarToolbar extends ReactWidget {
when: item.when
}));
}
this.contextMenuRenderer.render({
return this.contextMenuRenderer.render({
menuPath,
args: [this.current],
anchor: event.nativeEvent,
anchor,
onHide: () => toDisposeOnHide.dispose()
});
};
}

shouldHandleMouseEvent(event: MouseEvent): boolean {
return event.target instanceof Element && (!!this.inline.get(event.target.id) || event.target.id === '__more__');
Expand Down
24 changes: 20 additions & 4 deletions packages/core/src/browser/widgets/widget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -261,21 +261,37 @@ export function addClipboardListener<K extends 'cut' | 'copy' | 'paste'>(element
);
}

/**
* Resolves when the given widget is detached and hidden.
*/
export function waitForClosed(widget: Widget): Promise<void> {
return waitForVisible(widget, false);
return waitForVisible(widget, false, false);
}

/**
* Resolves when the given widget is attached and visible.
*/
export function waitForRevealed(widget: Widget): Promise<void> {
return waitForVisible(widget, true, true);
}

/**
* Resolves when the given widget is hidden regardless of attachment.
*/
export function waitForHidden(widget: Widget): Promise<void> {
return waitForVisible(widget, true);
}

function waitForVisible(widget: Widget, visible: boolean): Promise<void> {
if (widget.isAttached === visible && (widget.isVisible === visible || (widget.node.style.visibility !== 'hidden') === visible)) {
function waitForVisible(widget: Widget, visible: boolean, attached?: boolean): Promise<void> {
if ((typeof attached !== 'boolean' || widget.isAttached === attached) &&
(widget.isVisible === visible || (widget.node.style.visibility !== 'hidden') === visible)
) {
return new Promise(resolve => window.requestAnimationFrame(() => resolve()));
}
return new Promise(resolve => {
const waitFor = () => window.requestAnimationFrame(() => {
if (widget.isAttached === visible && (widget.isVisible === visible || (widget.node.style.visibility !== 'hidden') === visible)) {
if ((typeof attached !== 'boolean' || widget.isAttached === attached) &&
(widget.isVisible === visible || (widget.node.style.visibility !== 'hidden') === visible)) {
window.requestAnimationFrame(() => resolve());
} else {
waitFor();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,19 @@

/* eslint-disable @typescript-eslint/no-explicit-any */

import * as electron from 'electron';
import { inject, injectable } from 'inversify';
import { MenuPath } from '../../common';
import { ContextMenuRenderer, Anchor, RenderContextMenuOptions } from '../../browser';
import { ContextMenuRenderer, Anchor, RenderContextMenuOptions, ContextMenuAccess } from '../../browser';
import { ElectronMainMenuFactory } from './electron-main-menu-factory';
import { ContextMenuContext } from '../../browser/menu/context-menu-context';

export class ElectronContextMenuAccess implements ContextMenuAccess {
constructor(
public readonly menu: electron.Menu
) { }
}

@injectable()
export class ElectronContextMenuRenderer implements ContextMenuRenderer {

Expand All @@ -31,7 +38,7 @@ export class ElectronContextMenuRenderer implements ContextMenuRenderer {
constructor(@inject(ElectronMainMenuFactory) private menuFactory: ElectronMainMenuFactory) {
}

render(arg: MenuPath | RenderContextMenuOptions, arg2?: Anchor, arg3?: () => void): void {
render(arg: MenuPath | RenderContextMenuOptions, arg2?: Anchor, arg3?: () => void): ElectronContextMenuAccess {
const { menuPath, args, onHide } = RenderContextMenuOptions.resolve(arg, arg2, arg3);
const menu = this.menuFactory.createContextMenu(menuPath, args);
menu.popup({});
Expand All @@ -40,6 +47,7 @@ export class ElectronContextMenuRenderer implements ContextMenuRenderer {
if (onHide) {
menu.once('menu-will-close', () => onHide());
}
return new ElectronContextMenuAccess(menu);
}

}