Skip to content

Commit 3902794

Browse files
committed
fix(webapp): stop Cmd-J opening Chrome's downloads
Shortcuts can now ask for the browser default to be prevented, and the agent's keystroke does.
1 parent 8d233c2 commit 3902794

3 files changed

Lines changed: 96 additions & 7 deletions

File tree

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import { describe, expect, it } from "vitest";
2+
import { hotkeyOptions } from "~/hooks/useShortcutKeys";
3+
import { LEGACY_ASK_AI_SHORTCUT, TOGGLE_PANEL_SHORTCUT } from "./dashboardAgentLauncher";
4+
5+
const enabled = { isEnabled: true };
6+
7+
describe("the agent's shortcuts", () => {
8+
it("asks for Cmd-J's browser default to be prevented", () => {
9+
expect(TOGGLE_PANEL_SHORTCUT.key).toBe("j");
10+
expect(TOGGLE_PANEL_SHORTCUT.modifiers).toEqual(["mod"]);
11+
expect(hotkeyOptions({ shortcut: TOGGLE_PANEL_SHORTCUT, ...enabled }).preventDefault).toBe(
12+
true
13+
);
14+
});
15+
16+
it("fires from inside the composer", () => {
17+
const options = hotkeyOptions({ shortcut: TOGGLE_PANEL_SHORTCUT, ...enabled });
18+
expect(options.enableOnFormTags).toBe(true);
19+
expect(options.enableOnContentEditable).toBe(true);
20+
});
21+
22+
it("leaves Cmd-I's default alone", () => {
23+
expect(hotkeyOptions({ shortcut: LEGACY_ASK_AI_SHORTCUT, ...enabled }).preventDefault).toBe(
24+
false
25+
);
26+
});
27+
});
28+
29+
describe("hotkeyOptions", () => {
30+
it("defaults to leaving the browser default alone", () => {
31+
expect(hotkeyOptions({ shortcut: { key: "k" }, ...enabled })).toEqual({
32+
enabled: true,
33+
enableOnFormTags: false,
34+
enableOnContentEditable: false,
35+
preventDefault: false,
36+
});
37+
});
38+
39+
// The library calls preventDefault before it checks `enabled`.
40+
it("does not prevent the default while the shortcut is disabled", () => {
41+
expect(
42+
hotkeyOptions({ shortcut: TOGGLE_PANEL_SHORTCUT, isEnabled: false }).preventDefault
43+
).toBe(false);
44+
});
45+
46+
it("lets the call site turn on input elements for a shortcut that did not", () => {
47+
const options = hotkeyOptions({
48+
shortcut: { key: "k" },
49+
isEnabled: true,
50+
enabledOnInputElements: true,
51+
});
52+
expect(options.enableOnFormTags).toBe(true);
53+
});
54+
55+
it("survives an undefined shortcut", () => {
56+
expect(hotkeyOptions({ shortcut: undefined, isEnabled: false })).toEqual({
57+
enabled: false,
58+
enableOnFormTags: false,
59+
enableOnContentEditable: false,
60+
preventDefault: false,
61+
});
62+
});
63+
});

apps/webapp/app/components/dashboard-agent/dashboardAgentLauncher.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ export const TOGGLE_PANEL_SHORTCUT: Shortcut = {
1212
// The composer holds focus while the panel is open, so the key must fire from
1313
// inside a text field.
1414
enabledOnInputElements: true,
15+
// Chrome binds Cmd/Ctrl-J to Show Downloads.
16+
preventDefault: true,
1517
};
1618

1719
type DashboardAgentContextValue = {

apps/webapp/app/hooks/useShortcutKeys.tsx

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ export type Shortcut = {
99
modifiers?: Modifier[];
1010
enabledOnInputElements?: boolean;
1111
enabled?: boolean;
12+
/** Set when the browser binds the same keystroke to something of its own. */
13+
preventDefault?: boolean;
1214
};
1315

1416
export type ShortcutDefinition =
@@ -48,16 +50,38 @@ export function useShortcutKeys({
4850
action(event);
4951
}
5052
},
51-
{
52-
enabled: isEnabled,
53-
enableOnFormTags:
54-
isEnabled && (enabledOnInputElements ?? relevantShortcut?.enabledOnInputElements),
55-
enableOnContentEditable:
56-
isEnabled && (enabledOnInputElements ?? relevantShortcut?.enabledOnInputElements),
57-
}
53+
hotkeyOptions({ shortcut: relevantShortcut, isEnabled, enabledOnInputElements })
5854
);
5955
}
6056

57+
export type HotkeyOptions = {
58+
enabled: boolean;
59+
enableOnFormTags: boolean;
60+
enableOnContentEditable: boolean;
61+
preventDefault: boolean;
62+
};
63+
64+
// react-hotkeys-hook runs `preventDefault` before it checks `enabled`, so a
65+
// disabled shortcut must not ask for it.
66+
export function hotkeyOptions({
67+
shortcut,
68+
isEnabled,
69+
enabledOnInputElements,
70+
}: {
71+
shortcut: Shortcut | undefined;
72+
isEnabled: boolean;
73+
enabledOnInputElements?: boolean;
74+
}): HotkeyOptions {
75+
const onInputElements = enabledOnInputElements ?? shortcut?.enabledOnInputElements ?? false;
76+
77+
return {
78+
enabled: isEnabled,
79+
enableOnFormTags: isEnabled && onInputElements,
80+
enableOnContentEditable: isEnabled && onInputElements,
81+
preventDefault: isEnabled && (shortcut?.preventDefault ?? false),
82+
};
83+
}
84+
6185
function createKeysFromShortcut(shortcut: Shortcut | undefined) {
6286
if (!shortcut) {
6387
return [];

0 commit comments

Comments
 (0)