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

Detect menu items invoked via keybinding and dispatch keybinding on renderer #64228

Merged
merged 6 commits into from
Dec 19, 2018
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
12 changes: 11 additions & 1 deletion src/vs/platform/keybinding/common/abstractKeybindingService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,20 @@ export abstract class AbstractKeybindingService extends Disposable implements IK
this._currentChord = null;
}

public dispatchByUserSettingsLabel(userSettingsLabel: string, target: IContextKeyServiceTarget): void {
const keybindings = this.resolveUserBinding(userSettingsLabel);
if (keybindings.length >= 1) {
this._doDispatch(keybindings[0], target);
}
}

protected _dispatch(e: IKeyboardEvent, target: IContextKeyServiceTarget): boolean {
return this._doDispatch(this.resolveKeyboardEvent(e), target);
}

private _doDispatch(keybinding: ResolvedKeybinding, target: IContextKeyServiceTarget): boolean {
let shouldPreventDefault = false;

const keybinding = this.resolveKeyboardEvent(e);
if (keybinding.isChord()) {
console.warn('Unexpected keyboard event mapped to a chord');
return false;
Expand Down
2 changes: 2 additions & 0 deletions src/vs/platform/keybinding/common/keybinding.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ export interface IKeybindingService {
*/
softDispatch(keyboardEvent: IKeyboardEvent, target: IContextKeyServiceTarget): IResolveResult | null;

dispatchByUserSettingsLabel(userSettingsLabel: string, target: IContextKeyServiceTarget): void;

/**
* Look up keybindings for a command.
* Use `lookupKeybinding` if you are interested in the preferred keybinding.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,10 @@ export class MockKeybindingService implements IKeybindingService {
return null;
}

public dispatchByUserSettingsLabel(userSettingsLabel: string, target: IContextKeyServiceTarget): void {

}

public dispatchEvent(e: IKeyboardEvent, target: IContextKeyServiceTarget): boolean {
return false;
}
Expand Down
1 change: 1 addition & 0 deletions src/vs/platform/menubar/common/menubar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export interface IMenubarMenu {

export interface IMenubarKeybinding {
label: string;
userSettingsLabel: string;
isNative?: boolean; // Assumed true if missing
}

Expand Down
69 changes: 57 additions & 12 deletions src/vs/platform/menubar/electron-main/menubar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import * as nls from 'vs/nls';
import { isMacintosh, language } from 'vs/base/common/platform';
import { IEnvironmentService } from 'vs/platform/environment/common/environment';
import { app, shell, Menu, MenuItem, BrowserWindow } from 'electron';
import { OpenContext, IRunActionInWindowRequest, getTitleBarStyle } from 'vs/platform/windows/common/windows';
import { OpenContext, IRunActionInWindowRequest, getTitleBarStyle, IRunKeybindingInWindowRequest } from 'vs/platform/windows/common/windows';
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
import { IUpdateService, StateType } from 'vs/platform/update/common/update';
Expand All @@ -29,6 +29,15 @@ interface IMenuItemClickHandler {
inNoWindow: () => void;
}

type IMenuItemInvocation = (
{ type: 'commandId'; commandId: string; }
| { type: 'keybinding'; userSettingsLabel: string; }
);

interface IMenuItemWithKeybinding {
userSettingsLabel?: string;
}

export class Menubar {

private static readonly lastKnownMenubarStorageKey = 'lastKnownMenubarData';
Expand Down Expand Up @@ -573,17 +582,49 @@ export class Menubar {
}
}

private static _menuItemIsTriggeredViaKeybinding(event: Electron.Event, userSettingsLabel: string): boolean {
// The event coming in from Electron will inform us only about the modifier keys pressed.
// The strategy here is to check if the modifier keys match those of the keybinding,
// since it is highly unlikely to use modifier keys when clicking with the mouse
if (!userSettingsLabel) {
// There is no keybinding
return false;
}

let ctrlRequired = /ctrl/.test(userSettingsLabel);
let shiftRequired = /shift/.test(userSettingsLabel);
let altRequired = /alt/.test(userSettingsLabel);
let metaRequired = /cmd/.test(userSettingsLabel) || /super/.test(userSettingsLabel);

if (!ctrlRequired && !shiftRequired && !altRequired && !metaRequired) {
// This keybinding does not use any modifier keys, so we cannot use this heuristic
return false;
}

return (
ctrlRequired === event.ctrlKey
&& shiftRequired === event.shiftKey
&& altRequired === event.altKey
&& metaRequired === event.metaKey
);
}

private createMenuItem(label: string, commandId: string | string[], enabled?: boolean, checked?: boolean): Electron.MenuItem;
private createMenuItem(label: string, click: () => void, enabled?: boolean, checked?: boolean): Electron.MenuItem;
private createMenuItem(arg1: string, arg2: any, arg3?: boolean, arg4?: boolean): Electron.MenuItem {
const label = this.mnemonicLabel(arg1);
const click: () => void = (typeof arg2 === 'function') ? arg2 : (menuItem: Electron.MenuItem, win: Electron.BrowserWindow, event: Electron.Event) => {
const click: () => void = (typeof arg2 === 'function') ? arg2 : (menuItem: Electron.MenuItem & IMenuItemWithKeybinding, win: Electron.BrowserWindow, event: Electron.Event) => {
const userSettingsLabel = menuItem.userSettingsLabel;
let commandId = arg2;
if (Array.isArray(arg2)) {
commandId = this.isOptionClick(event) ? arg2[1] : arg2[0]; // support alternative action if we got multiple action Ids and the option key was pressed while invoking
}

this.runActionInRenderer(commandId);
if (userSettingsLabel && Menubar._menuItemIsTriggeredViaKeybinding(event, userSettingsLabel)) {
this.runActionInRenderer({ type: 'keybinding', userSettingsLabel });
} else {
this.runActionInRenderer({ type: 'commandId', commandId });
}
};
const enabled = typeof arg3 === 'boolean' ? arg3 : this.windowsMainService.getWindowCount() > 0;
const checked = typeof arg4 === 'boolean' ? arg4 : false;
Expand Down Expand Up @@ -656,7 +697,7 @@ export class Menubar {
};
}

private runActionInRenderer(id: string): void {
private runActionInRenderer(invocation: IMenuItemInvocation): void {
// We make sure to not run actions when the window has no focus, this helps
// for https://github.com/Microsoft/vscode/issues/25907 and specifically for
// https://github.com/Microsoft/vscode/issues/11928
Expand All @@ -671,18 +712,21 @@ export class Menubar {
}

if (activeWindow) {
if (!activeWindow.isReady && isMacintosh && id === 'workbench.action.toggleDevTools' && !this.environmentService.isBuilt) {
// prevent this action from running twice on macOS (https://github.com/Microsoft/vscode/issues/62719)
// we already register a keybinding in bootstrap-window.js for opening developer tools in case something
// goes wrong and that keybinding is only removed when the application has loaded (= window ready).
return;
if (invocation.type === 'commandId') {
if (!activeWindow.isReady && isMacintosh && invocation.commandId === 'workbench.action.toggleDevTools' && !this.environmentService.isBuilt) {
// prevent this action from running twice on macOS (https://github.com/Microsoft/vscode/issues/62719)
// we already register a keybinding in bootstrap-window.js for opening developer tools in case something
// goes wrong and that keybinding is only removed when the application has loaded (= window ready).
return;
}
this.windowsMainService.sendToFocused('vscode:runAction', { id: invocation.commandId, from: 'menu' } as IRunActionInWindowRequest);
} else {
this.windowsMainService.sendToFocused('vscode:runKeybinding', { userSettingsLabel: invocation.userSettingsLabel } as IRunKeybindingInWindowRequest);
}

this.windowsMainService.sendToFocused('vscode:runAction', { id, from: 'menu' } as IRunActionInWindowRequest);
}
}

private withKeybinding(commandId: string | undefined, options: Electron.MenuItemConstructorOptions): Electron.MenuItemConstructorOptions {
private withKeybinding(commandId: string | undefined, options: Electron.MenuItemConstructorOptions & IMenuItemWithKeybinding): Electron.MenuItemConstructorOptions {
const binding = typeof commandId === 'string' ? this.keybindings[commandId] : undefined;

// Apply binding if there is one
Expand All @@ -691,6 +735,7 @@ export class Menubar {
// if the binding is native, we can just apply it
if (binding.isNative !== false) {
options.accelerator = binding.label;
options.userSettingsLabel = binding.userSettingsLabel;
}

// the keybinding is not native so we cannot show it as part of the accelerator of
Expand Down
4 changes: 4 additions & 0 deletions src/vs/platform/windows/common/windows.ts
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,10 @@ export interface IRunActionInWindowRequest {
args?: any[];
}

export interface IRunKeybindingInWindowRequest {
userSettingsLabel: string;
}

export class ActiveWindowManager implements IDisposable {

private disposables: IDisposable[] = [];
Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/browser/parts/titlebar/menubarControl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -516,13 +516,13 @@ export class MenubarControl extends Disposable {
// first try to resolve a native accelerator
const electronAccelerator = binding.getElectronAccelerator();
if (electronAccelerator) {
return { label: electronAccelerator };
return { label: electronAccelerator, userSettingsLabel: binding.getUserSettingsLabel() };
}

// we need this fallback to support keybindings that cannot show in electron menus (e.g. chords)
const acceleratorLabel = binding.getLabel();
if (acceleratorLabel) {
return { label: acceleratorLabel, isNative: false };
return { label: acceleratorLabel, isNative: false, userSettingsLabel: binding.getUserSettingsLabel() };
}

return null;
Expand Down
9 changes: 8 additions & 1 deletion src/vs/workbench/electron-browser/window.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { toResource, IUntitledResourceInput } from 'vs/workbench/common/editor';
import { IEditorService, IResourceEditor } from 'vs/workbench/services/editor/common/editorService';
import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
import { IWorkspaceConfigurationService } from 'vs/workbench/services/configuration/common/configuration';
import { IWindowsService, IWindowService, IWindowSettings, IOpenFileRequest, IWindowsConfiguration, IAddFoldersRequest, IRunActionInWindowRequest, IPathData } from 'vs/platform/windows/common/windows';
import { IWindowsService, IWindowService, IWindowSettings, IOpenFileRequest, IWindowsConfiguration, IAddFoldersRequest, IRunActionInWindowRequest, IPathData, IRunKeybindingInWindowRequest } from 'vs/platform/windows/common/windows';
import { IContextMenuService } from 'vs/platform/contextview/browser/contextView';
import { ITitleService } from 'vs/workbench/services/title/common/titleService';
import { IWorkbenchThemeService, VS_HC_THEME } from 'vs/workbench/services/themes/common/workbenchThemeService';
Expand All @@ -38,6 +38,7 @@ import { AccessibilitySupport, isRootUser, isWindows, isMacintosh } from 'vs/bas
import product from 'vs/platform/node/product';
import { INotificationService } from 'vs/platform/notification/common/notification';
import { EditorServiceImpl } from 'vs/workbench/browser/parts/editor/editor';
import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding';

const TextInputActions: IAction[] = [
new Action('undo', nls.localize('undo', "Undo"), null, true, () => document.execCommand('undo') && Promise.resolve(true)),
Expand Down Expand Up @@ -71,6 +72,7 @@ export class ElectronWindow extends Themable {
@IWorkbenchThemeService protected themeService: IWorkbenchThemeService,
@INotificationService private notificationService: INotificationService,
@ICommandService private commandService: ICommandService,
@IKeybindingService private keybindingService: IKeybindingService,
@IContextMenuService private contextMenuService: IContextMenuService,
@ITelemetryService private telemetryService: ITelemetryService,
@IWorkspaceEditingService private workspaceEditingService: IWorkspaceEditingService,
Expand Down Expand Up @@ -133,6 +135,11 @@ export class ElectronWindow extends Themable {
});
});

// Support runKeybinding event
ipc.on('vscode:runKeybinding', (event: any, request: IRunKeybindingInWindowRequest) => {
this.keybindingService.dispatchByUserSettingsLabel(request.userSettingsLabel, document.activeElement);
});

// Error reporting from main
ipc.on('vscode:reportError', (event: any, error: string) => {
if (error) {
Expand Down