Skip to content

Commit c92e1a4

Browse files
authored
Allow to open the contextmenu through keyboard shortcuts when in widgets (#2226)
## Context In browsers, I can usually press the Menu key, Shift+F10, or, more specifically on macOS, Ctrl+Enter, to open a context menu, which is the menu that appears when right-clicking something with my mouse. But for now in Grist, when we are focused on a widget, we can't open the active cell's context menu through the keyboard. This is because when focused on a widget, the actual keyboard focus is on the hidden clipboard element. So, pressing the key triggers a browser's native contextmenu on the hidden element, appearing on the top left corner of the screen. We'd rather make it so that pressing the key correctly opens the custom context menu of the active cell, just like right-clicking. ## Proposed solution - make it so that the clipboard's hidden element just passes through the `contextmenu` events it receives, to the active cursor, if any. This makes the keyboard handling rather straightforward in the code: we don't have to code anything more than what is already there to handle the `contextmenu` event of cells, - add a way to describe commands with different shortcuts for macOS to allow us to better match each OS habits. This enables us to show to the user, in the keyboard shorcuts list, either `Menu` or `^+Enter` depending on wether we are on macOS. ## Has this been tested? <!-- Put an `x` in the box that applies: --> - [x] 👍 yes, I added tests to the test suite - [ ] 💭 no, because this PR is a draft and still needs work - [ ] 🙅 no, because this is not relevant here - [ ] 🙋 no, because I need help <!-- Detail how we can help you --> ## Screenshots / Screencasts Before: https://github.com/user-attachments/assets/6508215e-1acf-4f39-8a0b-c22a2f701957 After: https://github.com/user-attachments/assets/2d9ae04b-1e7f-499a-97fb-d785fdb4ffa3
1 parent 904d5da commit c92e1a4

8 files changed

Lines changed: 141 additions & 11 deletions

File tree

app/client/components/Clipboard.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,26 @@ export class Clipboard extends Disposable {
112112
dom.on("copy", this._onCopy.bind(this)),
113113
dom.on("cut", this._onCut.bind(this)),
114114
dom.on("paste", this._onPaste.bind(this)),
115+
// The contextmenu event can be triggered with keyboard shortcuts. In that case, we redirect the event to the
116+
// active cursor element, if any.
117+
dom.on("contextmenu", (event) => {
118+
redirectContextmenuEvent(event);
119+
}),
120+
// On Mac, no contextmenu event is triggered when pressing usual shortcuts, so we
121+
// manually check for Shift+F10 or Ctrl+Enter (the default ContextMenu shortcut on macOS)
122+
dom.on("keydown", (event) => {
123+
if (!isMac) {
124+
return;
125+
}
126+
if (event.key === "F10" && event.shiftKey && !event.metaKey && !event.ctrlKey && !event.altKey) {
127+
redirectContextmenuEvent(event);
128+
return;
129+
}
130+
if (event.key === "Enter" && event.ctrlKey && !event.metaKey && !event.altKey && !event.shiftKey) {
131+
redirectContextmenuEvent(event);
132+
return;
133+
}
134+
}),
115135
);
116136
document.body.appendChild(this.copypasteField);
117137
this.onDispose(() => { dom.domDispose(this.copypasteField); this.copypasteField.remove(); });
@@ -285,6 +305,21 @@ const FOCUS_TARGET_TAGS = new Set([
285305
"IFRAME",
286306
]);
287307

308+
function redirectContextmenuEvent(event: MouseEvent | KeyboardEvent) {
309+
event.preventDefault();
310+
event.stopPropagation();
311+
const activeCursor = document.querySelector(".active_cursor");
312+
if (activeCursor) {
313+
const rect = activeCursor.getBoundingClientRect();
314+
activeCursor.dispatchEvent(new PointerEvent("contextmenu", {
315+
bubbles: true,
316+
cancelable: true,
317+
clientX: rect.left,
318+
clientY: rect.bottom,
319+
}));
320+
}
321+
}
322+
288323
/**
289324
* Returns data formatted as a 2D array of strings, suitable for pasting within Grist.
290325
*

app/client/components/commandList.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ export type CommandName =
125125
"detachEditor" |
126126
"activateAssistant" |
127127
"viewAsCard" |
128+
"openContextMenu" |
128129
"showColumns" |
129130
"createForm" |
130131
"insertField" |
@@ -133,7 +134,7 @@ export type CommandName =
133134

134135
export interface CommandDef {
135136
name: CommandName;
136-
keys: string[];
137+
keys: CommandKey[];
137138
desc: (() => string) | null;
138139
bindKeys?: boolean;
139140
/**
@@ -143,6 +144,13 @@ export interface CommandDef {
143144
deprecated?: boolean;
144145
}
145146

147+
export interface PlatformSpecificCommandKey {
148+
default: string;
149+
mac?: string;
150+
}
151+
152+
export type CommandKey = string | PlatformSpecificCommandKey;
153+
146154
export interface MenuCommand {
147155
humanKeys: string[];
148156
run: (...args: any[]) => any;
@@ -410,6 +418,12 @@ export const groups: CommendGroupDef[] = [{
410418
name: "viewAsCard",
411419
keys: ["Space"],
412420
desc: () => t("Show the record card widget of the selected record"),
421+
}, {
422+
// This matches browser's default `contextmenu` event keybindings. This is here to show the keyboard shortcut
423+
// to the user, but we rely on the `contextmenu` event rather than the `openContextMenu` command in the code.
424+
name: "openContextMenu",
425+
keys: [{ default: "Menu", mac: "Ctrl+Enter" }, "Shift+F10"],
426+
desc: () => t("Open the context menu"),
413427
},
414428
],
415429
}, {

app/client/components/commands.ts

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,13 @@
77
* command is active at any time.
88
*/
99

10-
import { CommandDef, CommandName, CommendGroupDef, groups } from "app/client/components/commandList";
10+
import {
11+
CommandDef,
12+
CommandKey,
13+
CommandName,
14+
CommendGroupDef,
15+
groups,
16+
} from "app/client/components/commandList";
1117
import { get as getBrowserGlobals } from "app/client/lib/browserGlobals";
1218
import dom from "app/client/lib/dom";
1319
import * as Mousetrap from "app/client/lib/Mousetrap";
@@ -122,6 +128,13 @@ export function getHumanKey(key: string, mac: boolean): string {
122128
return keys.join(mac ? "" : " + ");
123129
}
124130

131+
function resolveKeyForPlatform(key: CommandKey) {
132+
if (typeof key === "string") {
133+
return key;
134+
}
135+
return (isMac && key.mac) ? key.mac : key.default;
136+
}
137+
125138
export interface CommandOptions {
126139
bindKeys?: boolean;
127140
deprecated?: boolean;
@@ -152,11 +165,12 @@ export class Command implements CommandDef {
152165
private _implGroupStack: CommandGroup[] = [];
153166
private _activeFunc: (...args: any[]) => any = _.noop;
154167

155-
constructor(name: CommandName, desc: (() => string) | null, keys: string[], options: CommandOptions = {}) {
168+
constructor(name: CommandName, desc: (() => string) | null, keys: CommandKey[], options: CommandOptions = {}) {
169+
const platformKeys = keys.map(resolveKeyForPlatform);
156170
this.name = name;
157171
this.desc = desc;
158-
this.humanKeys = keys.map(key => getHumanKey(key, isMac));
159-
this.keys = keys.map(function(k) { return k.trim().toLowerCase().replace(/ *\+ */g, "+"); });
172+
this.humanKeys = platformKeys.map(key => getHumanKey(key, isMac));
173+
this.keys = platformKeys.map(function(k) { return k.trim().toLowerCase().replace(/ *\+ */g, "+"); });
160174
this.bindKeys = options.bindKeys ?? true;
161175
this.alwaysOn = options.alwaysOn ?? false;
162176
this.isActive = ko.observable(false);

app/client/lib/Mousetrap.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ if (typeof window === "undefined") {
2121
// Minus is different on Gecko:
2222
// see https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/keyCode
2323
// and https://github.com/ccampbell/mousetrap/pull/215
24-
Mousetrap.addKeycodes({173: "-"});
24+
Mousetrap.addKeycodes({173: "-", 93: "menu"});
2525

2626
var customStopCallbacks = new WeakMap();
2727

app/client/ui/contextMenu.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,21 @@ class ContextMenuController extends Disposable implements IOpenController {
5050

5151
// On click anywhere on the page (outside popup content), close it.
5252
const onClick = (evt: MouseEvent) => {
53-
const target: Node | null = evt.target as Node;
54-
if (target && !content.contains(target)) {
53+
if (evt.target && !content.contains(evt.target as Node)) {
5554
this.close();
5655
}
5756
};
58-
this.autoDispose(dom.onElem(document, "contextmenu", onClick, { useCapture: true }));
57+
// Handle the specific case if right-clicking/pressing the Menu key inside the menu itself (we ignore it)
58+
const onContextMenu = (evt: MouseEvent) => {
59+
if (evt.target && content.contains(evt.target as Node)) {
60+
evt.preventDefault();
61+
evt.stopPropagation();
62+
return;
63+
}
64+
onClick(evt);
65+
};
5966
this.autoDispose(dom.onElem(document, "click", onClick, { useCapture: true }));
67+
this.autoDispose(dom.onElem(document, "contextmenu", onContextMenu, { useCapture: true }));
6068

6169
// Cleanup involves removing the element.
6270
this.onDispose(() => {

static/locales/en.client.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2589,7 +2589,8 @@
25892589
"When in the search bar, close it and focus the current match": "When in the search bar, close it and focus the current match",
25902590
"When typed at the start of a cell, make this a formula column": "When typed at the start of a cell, make this a formula column",
25912591
"showing a behavioral popup": "showing a behavioral popup",
2592-
"Filter this column by just this cell's value": "Filter this column by just this cell's value"
2592+
"Filter this column by just this cell's value": "Filter this column by just this cell's value",
2593+
"Open the context menu": "Open the context menu"
25932594
},
25942595
"GridViewMenusDateHelpers": {
25952596
"12-hour format": "12-hour format",

static/locales/fr.client.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2549,7 +2549,8 @@
25492549
"When in the search bar, close it and focus the current match": "Quand on est dans la barre de recherche, la fermer et mettre le focus sur la correspondance actuelle",
25502550
"When typed at the start of a cell, make this a formula column": "Quand tapé au début d'une cellule, transformer celle-ci en colonne de formule",
25512551
"showing a behavioral popup": "affichage d'une fenêtre contextuelle comportementale",
2552-
"Filter this column by just this cell's value": "Filtrer cette colonne par la valeur de cette cellule uniquement"
2552+
"Filter this column by just this cell's value": "Filtrer cette colonne par la valeur de cette cellule uniquement",
2553+
"Open the context menu": "Ouvrir le menu contextuel"
25532554
},
25542555
"GridViewMenusDateHelpers": {
25552556
"12-hour format": "format 12-heures",

test/nbrowser/ViewContextMenu.ts

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
import * as gu from "test/nbrowser/gristUtils";
2+
import { setupTestSuite } from "test/nbrowser/testUtils";
3+
4+
import { assert, driver, Key } from "mocha-webdriver";
5+
6+
describe("ViewContextMenu", function() {
7+
this.timeout(20000);
8+
const cleanup = setupTestSuite();
9+
10+
before(async function() {
11+
const session = await gu.session().login();
12+
await session.tempNewDoc(cleanup);
13+
});
14+
15+
afterEach(() => gu.checkForErrors());
16+
17+
it("should support opening a GridView cell context menu with keyboard", async function() {
18+
await assertShiftF10Works();
19+
});
20+
21+
it("should support opening a RecordView context menu with keyboard", async function() {
22+
await gu.addNewSection("Card", "Table1");
23+
await assertShiftF10Works();
24+
});
25+
26+
it("should not open a context menu twice when opening it with keyboard", async function() {
27+
await pressShiftF10();
28+
assert.isTrue(await gu.findOpenMenu().isDisplayed());
29+
await pressShiftF10(true);
30+
await driver.sleep(100);
31+
const openedMenus = await driver.findAll(".grist-floating-menu");
32+
assert.equal(openedMenus.length, 1);
33+
34+
await gu.sendKeys(Key.ESCAPE);
35+
await gu.waitForMenuToClose();
36+
});
37+
38+
it("should hide the context menu when clicking outside of it after opening it with keyboard", async function() {
39+
await pressShiftF10();
40+
assert.isTrue(await gu.findOpenMenu().isDisplayed());
41+
await gu.selectSectionByTitle("Table1");
42+
await gu.waitForMenuToClose();
43+
});
44+
});
45+
46+
async function assertShiftF10Works() {
47+
await gu.waitAppFocus();
48+
await pressShiftF10();
49+
assert.isTrue(await gu.findOpenMenu().isDisplayed());
50+
await gu.sendKeys(Key.ESCAPE);
51+
await gu.waitForMenuToClose();
52+
}
53+
54+
// Using gu.sendKeys doesn't work with Shift+F10 so we have to deal with the testing environment limitations.
55+
async function pressShiftF10(inMenu: boolean = false) {
56+
return driver.find(inMenu ? ".grist-floating-menu" : ".copypaste").sendKeys(Key.chord(Key.SHIFT, Key.F10));
57+
}

0 commit comments

Comments
 (0)