| author | |
| committer | |
| log | d47059a9db4546ce507341bbc8e7c25d0f60300a |
| tree | 53b95a3337f19ae9fb5ad9b1e172d0d5794a6aad |
| parent | 10c1cae6abb3c87887b3d4a9cc6d2d4788145bd2 |
- Bind `mod+d` to `diff.toggle` when terminal is not focused
- Wire ChatView to handle `diff.toggle` and show its shortcut in the diff button tooltip
- Limit sidebar `mod+b` listener to mobile to avoid desktop shortcut conflicts
- Extend web/contracts keybinding tests and command list for `diff.toggle`7 files changed, 68 insertions(+), 11 deletions(-)
apps/server/src/keybindings.ts+1| ... | @@ -65,6 +65,7 @@ export const DEFAULT_KEYBINDINGS: ReadonlyArray<KeybindingRule> = [ | ... | @@ -65,6 +65,7 @@ export const DEFAULT_KEYBINDINGS: ReadonlyArray<KeybindingRule> = [ |
| 65 | { key: "mod+d", command: "terminal.split", when: "terminalFocus" }, | 65 | { key: "mod+d", command: "terminal.split", when: "terminalFocus" }, |
| 66 | { key: "mod+n", command: "terminal.new", when: "terminalFocus" }, | 66 | { key: "mod+n", command: "terminal.new", when: "terminalFocus" }, |
| 67 | { key: "mod+w", command: "terminal.close", when: "terminalFocus" }, | 67 | { key: "mod+w", command: "terminal.close", when: "terminalFocus" }, |
| 68 | { key: "mod+d", command: "diff.toggle", when: "!terminalFocus" }, | ||
| 68 | { key: "mod+n", command: "chat.new", when: "!terminalFocus" }, | 69 | { key: "mod+n", command: "chat.new", when: "!terminalFocus" }, |
| 69 | { key: "mod+shift+o", command: "chat.new", when: "!terminalFocus" }, | 70 | { key: "mod+shift+o", command: "chat.new", when: "!terminalFocus" }, |
| 70 | { key: "mod+shift+n", command: "chat.newLocal", when: "!terminalFocus" }, | 71 | { key: "mod+shift+n", command: "chat.newLocal", when: "!terminalFocus" }, |
apps/web/src/components/ChatView.tsx+28-10| ... | @@ -615,6 +615,20 @@ export default function ChatView({ threadId }: ChatViewProps) { | ... | @@ -615,6 +615,20 @@ export default function ChatView({ threadId }: ChatViewProps) { |
| 615 | () => shortcutLabelForCommand(keybindings, "terminal.close"), | 615 | () => shortcutLabelForCommand(keybindings, "terminal.close"), |
| 616 | [keybindings], | 616 | [keybindings], |
| 617 | ); | 617 | ); |
| 618 | const diffPanelShortcutLabel = useMemo( | ||
| 619 | () => shortcutLabelForCommand(keybindings, "diff.toggle"), | ||
| 620 | [keybindings], | ||
| 621 | ); | ||
| 622 | const onToggleDiff = useCallback(() => { | ||
| 623 | void navigate({ | ||
| 624 | to: "/$threadId", | ||
| 625 | params: { threadId }, | ||
| 626 | search: (previous) => { | ||
| 627 | const rest = stripDiffSearchParams(previous); | ||
| 628 | return diffOpen ? rest : { ...rest, diff: "1" }; | ||
| 629 | }, | ||
| 630 | }); | ||
| 631 | }, [diffOpen, navigate, threadId]); | ||
| 618 | 632 | ||
| 619 | const envLocked = Boolean( | 633 | const envLocked = Boolean( |
| 620 | activeThread && | 634 | activeThread && |
| ... | @@ -1147,6 +1161,13 @@ export default function ChatView({ threadId }: ChatViewProps) { | ... | @@ -1147,6 +1161,13 @@ export default function ChatView({ threadId }: ChatViewProps) { |
| 1147 | return; | 1161 | return; |
| 1148 | } | 1162 | } |
| 1149 | 1163 | ||
| 1164 | if (command === "diff.toggle") { | ||
| 1165 | event.preventDefault(); | ||
| 1166 | event.stopPropagation(); | ||
| 1167 | onToggleDiff(); | ||
| 1168 | return; | ||
| 1169 | } | ||
| 1170 | |||
| 1150 | const scriptId = projectScriptIdFromCommand(command); | 1171 | const scriptId = projectScriptIdFromCommand(command); |
| 1151 | if (!scriptId || !activeProject) return; | 1172 | if (!scriptId || !activeProject) return; |
| 1152 | const script = activeProject.scripts.find((entry) => entry.id === scriptId); | 1173 | const script = activeProject.scripts.find((entry) => entry.id === scriptId); |
| ... | @@ -1168,6 +1189,7 @@ export default function ChatView({ threadId }: ChatViewProps) { | ... | @@ -1168,6 +1189,7 @@ export default function ChatView({ threadId }: ChatViewProps) { |
| 1168 | runProjectScript, | 1189 | runProjectScript, |
| 1169 | splitTerminal, | 1190 | splitTerminal, |
| 1170 | keybindings, | 1191 | keybindings, |
| 1192 | onToggleDiff, | ||
| 1171 | toggleTerminalVisibility, | 1193 | toggleTerminalVisibility, |
| 1172 | ]); | 1194 | ]); |
| 1173 | 1195 | ||
| ... | @@ -1612,16 +1634,6 @@ export default function ChatView({ threadId }: ChatViewProps) { | ... | @@ -1612,16 +1634,6 @@ export default function ChatView({ threadId }: ChatViewProps) { |
| 1612 | const onExpandTimelineImage = useCallback((image: ExpandedImagePreview) => { | 1634 | const onExpandTimelineImage = useCallback((image: ExpandedImagePreview) => { |
| 1613 | setExpandedImage(image); | 1635 | setExpandedImage(image); |
| 1614 | }, []); | 1636 | }, []); |
| 1615 | const onToggleDiff = useCallback(() => { | ||
| 1616 | void navigate({ | ||
| 1617 | to: "/$threadId", | ||
| 1618 | params: { threadId }, | ||
| 1619 | search: (previous) => { | ||
| 1620 | const rest = stripDiffSearchParams(previous); | ||
| 1621 | return diffOpen ? rest : { ...rest, diff: "1" }; | ||
| 1622 | }, | ||
| 1623 | }); | ||
| 1624 | }, [diffOpen, navigate, threadId]); | ||
| 1625 | const onOpenTurnDiff = useCallback( | 1637 | const onOpenTurnDiff = useCallback( |
| 1626 | (turnId: TurnId, filePath?: string) => { | 1638 | (turnId: TurnId, filePath?: string) => { |
| 1627 | void navigate({ | 1639 | void navigate({ |
| ... | @@ -1692,6 +1704,7 @@ export default function ChatView({ threadId }: ChatViewProps) { | ... | @@ -1692,6 +1704,7 @@ export default function ChatView({ threadId }: ChatViewProps) { |
| 1692 | activeProject ? (lastInvokedScriptByProjectId[activeProject.id] ?? null) : null | 1704 | activeProject ? (lastInvokedScriptByProjectId[activeProject.id] ?? null) : null |
| 1693 | } | 1705 | } |
| 1694 | keybindings={keybindings} | 1706 | keybindings={keybindings} |
| 1707 | diffToggleShortcutLabel={diffPanelShortcutLabel} | ||
| 1695 | gitCwd={gitCwd} | 1708 | gitCwd={gitCwd} |
| 1696 | diffOpen={diffOpen} | 1709 | diffOpen={diffOpen} |
| 1697 | onRunProjectScript={(script) => { | 1710 | onRunProjectScript={(script) => { |
| ... | @@ -2043,6 +2056,7 @@ interface ChatHeaderProps { | ... | @@ -2043,6 +2056,7 @@ interface ChatHeaderProps { |
| 2043 | activeProjectScripts: ProjectScript[] | undefined; | 2056 | activeProjectScripts: ProjectScript[] | undefined; |
| 2044 | preferredScriptId: string | null; | 2057 | preferredScriptId: string | null; |
| 2045 | keybindings: ResolvedKeybindingsConfig; | 2058 | keybindings: ResolvedKeybindingsConfig; |
| 2059 | diffToggleShortcutLabel: string | null; | ||
| 2046 | gitCwd: string | null; | 2060 | gitCwd: string | null; |
| 2047 | diffOpen: boolean; | 2061 | diffOpen: boolean; |
| 2048 | onRunProjectScript: (script: ProjectScript) => void; | 2062 | onRunProjectScript: (script: ProjectScript) => void; |
| ... | @@ -2058,6 +2072,7 @@ const ChatHeader = memo(function ChatHeader({ | ... | @@ -2058,6 +2072,7 @@ const ChatHeader = memo(function ChatHeader({ |
| 2058 | activeProjectScripts, | 2072 | activeProjectScripts, |
| 2059 | preferredScriptId, | 2073 | preferredScriptId, |
| 2060 | keybindings, | 2074 | keybindings, |
| 2075 | diffToggleShortcutLabel, | ||
| 2061 | gitCwd, | 2076 | gitCwd, |
| 2062 | diffOpen, | 2077 | diffOpen, |
| 2063 | onRunProjectScript, | 2078 | onRunProjectScript, |
| ... | @@ -2101,6 +2116,9 @@ const ChatHeader = memo(function ChatHeader({ | ... | @@ -2101,6 +2116,9 @@ const ChatHeader = memo(function ChatHeader({ |
| 2101 | pressed={diffOpen} | 2116 | pressed={diffOpen} |
| 2102 | onPressedChange={onToggleDiff} | 2117 | onPressedChange={onToggleDiff} |
| 2103 | aria-label="Toggle diff panel" | 2118 | aria-label="Toggle diff panel" |
| 2119 | title={ | ||
| 2120 | diffToggleShortcutLabel ? `Toggle diff panel (${diffToggleShortcutLabel})` : undefined | ||
| 2121 | } | ||
| 2104 | variant="outline" | 2122 | variant="outline" |
| 2105 | size="xs" | 2123 | size="xs" |
| 2106 | > | 2124 | > |
apps/web/src/components/ui/sidebar.tsx+2-1| ... | @@ -132,6 +132,7 @@ function SidebarProvider({ | ... | @@ -132,6 +132,7 @@ function SidebarProvider({ |
| 132 | // Adds a keyboard shortcut to toggle the sidebar. | 132 | // Adds a keyboard shortcut to toggle the sidebar. |
| 133 | React.useEffect(() => { | 133 | React.useEffect(() => { |
| 134 | const handleKeyDown = (event: KeyboardEvent) => { | 134 | const handleKeyDown = (event: KeyboardEvent) => { |
| 135 | if (!isMobile) return; | ||
| 135 | if (event.key === SIDEBAR_KEYBOARD_SHORTCUT && (event.metaKey || event.ctrlKey)) { | 136 | if (event.key === SIDEBAR_KEYBOARD_SHORTCUT && (event.metaKey || event.ctrlKey)) { |
| 136 | event.preventDefault(); | 137 | event.preventDefault(); |
| 137 | toggleSidebar(); | 138 | toggleSidebar(); |
| ... | @@ -140,7 +141,7 @@ function SidebarProvider({ | ... | @@ -140,7 +141,7 @@ function SidebarProvider({ |
| 140 | 141 | ||
| 141 | window.addEventListener("keydown", handleKeyDown); | 142 | window.addEventListener("keydown", handleKeyDown); |
| 142 | return () => window.removeEventListener("keydown", handleKeyDown); | 143 | return () => window.removeEventListener("keydown", handleKeyDown); |
| 143 | }, [toggleSidebar]); | 144 | }, [isMobile, toggleSidebar]); |
| 144 | 145 | ||
| 145 | // We add a state so that we can do data-state="expanded" or "collapsed". | 146 | // We add a state so that we can do data-state="expanded" or "collapsed". |
| 146 | // This makes it easier to style the sidebar with Tailwind classes. | 147 | // This makes it easier to style the sidebar with Tailwind classes. |
apps/web/src/keybindings.test.ts+22| ... | @@ -10,6 +10,7 @@ import { | ... | @@ -10,6 +10,7 @@ import { |
| 10 | formatShortcutLabel, | 10 | formatShortcutLabel, |
| 11 | isChatNewShortcut, | 11 | isChatNewShortcut, |
| 12 | isChatNewLocalShortcut, | 12 | isChatNewLocalShortcut, |
| 13 | isDiffToggleShortcut, | ||
| 13 | isOpenFavoriteEditorShortcut, | 14 | isOpenFavoriteEditorShortcut, |
| 14 | isTerminalClearShortcut, | 15 | isTerminalClearShortcut, |
| 15 | isTerminalCloseShortcut, | 16 | isTerminalCloseShortcut, |
| ... | @@ -91,6 +92,11 @@ const DEFAULT_BINDINGS = compile([ | ... | @@ -91,6 +92,11 @@ const DEFAULT_BINDINGS = compile([ |
| 91 | command: "terminal.close", | 92 | command: "terminal.close", |
| 92 | whenAst: whenIdentifier("terminalFocus"), | 93 | whenAst: whenIdentifier("terminalFocus"), |
| 93 | }, | 94 | }, |
| 95 | { | ||
| 96 | shortcut: modShortcut("d"), | ||
| 97 | command: "diff.toggle", | ||
| 98 | whenAst: whenNot(whenIdentifier("terminalFocus")), | ||
| 99 | }, | ||
| 94 | { shortcut: modShortcut("o", { shiftKey: true }), command: "chat.new" }, | 100 | { shortcut: modShortcut("o", { shiftKey: true }), command: "chat.new" }, |
| 95 | { shortcut: modShortcut("n", { shiftKey: true }), command: "chat.newLocal" }, | 101 | { shortcut: modShortcut("n", { shiftKey: true }), command: "chat.newLocal" }, |
| 96 | { shortcut: modShortcut("o"), command: "editor.openFavorite" }, | 102 | { shortcut: modShortcut("o"), command: "editor.openFavorite" }, |
| ... | @@ -230,6 +236,7 @@ describe("shortcutLabelForCommand", () => { | ... | @@ -230,6 +236,7 @@ describe("shortcutLabelForCommand", () => { |
| 230 | 236 | ||
| 231 | it("returns labels for non-terminal commands", () => { | 237 | it("returns labels for non-terminal commands", () => { |
| 232 | assert.strictEqual(shortcutLabelForCommand(DEFAULT_BINDINGS, "chat.new", "MacIntel"), "⇧⌘O"); | 238 | assert.strictEqual(shortcutLabelForCommand(DEFAULT_BINDINGS, "chat.new", "MacIntel"), "⇧⌘O"); |
| 239 | assert.strictEqual(shortcutLabelForCommand(DEFAULT_BINDINGS, "diff.toggle", "Linux"), "Ctrl+D"); | ||
| 233 | assert.strictEqual( | 240 | assert.strictEqual( |
| 234 | shortcutLabelForCommand(DEFAULT_BINDINGS, "editor.openFavorite", "Linux"), | 241 | shortcutLabelForCommand(DEFAULT_BINDINGS, "editor.openFavorite", "Linux"), |
| 235 | "Ctrl+O", | 242 | "Ctrl+O", |
| ... | @@ -276,6 +283,21 @@ describe("chat/editor shortcuts", () => { | ... | @@ -276,6 +283,21 @@ describe("chat/editor shortcuts", () => { |
| 276 | }), | 283 | }), |
| 277 | ); | 284 | ); |
| 278 | }); | 285 | }); |
| 286 | |||
| 287 | it("matches diff.toggle shortcut outside terminal focus", () => { | ||
| 288 | assert.isTrue( | ||
| 289 | isDiffToggleShortcut(event({ key: "d", metaKey: true }), DEFAULT_BINDINGS, { | ||
| 290 | platform: "MacIntel", | ||
| 291 | context: { terminalFocus: false }, | ||
| 292 | }), | ||
| 293 | ); | ||
| 294 | assert.isFalse( | ||
| 295 | isDiffToggleShortcut(event({ key: "d", metaKey: true }), DEFAULT_BINDINGS, { | ||
| 296 | platform: "MacIntel", | ||
| 297 | context: { terminalFocus: true }, | ||
| 298 | }), | ||
| 299 | ); | ||
| 300 | }); | ||
| 279 | }); | 301 | }); |
| 280 | 302 | ||
| 281 | describe("cross-command precedence", () => { | 303 | describe("cross-command precedence", () => { |
apps/web/src/keybindings.ts+8| ... | @@ -198,6 +198,14 @@ export function isTerminalCloseShortcut( | ... | @@ -198,6 +198,14 @@ export function isTerminalCloseShortcut( |
| 198 | return matchesCommandShortcut(event, keybindings, "terminal.close", options); | 198 | return matchesCommandShortcut(event, keybindings, "terminal.close", options); |
| 199 | } | 199 | } |
| 200 | 200 | ||
| 201 | export function isDiffToggleShortcut( | ||
| 202 | event: ShortcutEventLike, | ||
| 203 | keybindings: ResolvedKeybindingsConfig, | ||
| 204 | options?: ShortcutMatchOptions, | ||
| 205 | ): boolean { | ||
| 206 | return matchesCommandShortcut(event, keybindings, "diff.toggle", options); | ||
| 207 | } | ||
| 208 | |||
| 201 | export function isChatNewShortcut( | 209 | export function isChatNewShortcut( |
| 202 | event: ShortcutEventLike, | 210 | event: ShortcutEventLike, |
| 203 | keybindings: ResolvedKeybindingsConfig, | 211 | keybindings: ResolvedKeybindingsConfig, |
packages/contracts/src/keybindings.test.ts+6| ... | @@ -35,6 +35,12 @@ it.effect("parses keybinding rules", () => | ... | @@ -35,6 +35,12 @@ it.effect("parses keybinding rules", () => |
| 35 | }); | 35 | }); |
| 36 | assert.strictEqual(parsedClose.command, "terminal.close"); | 36 | assert.strictEqual(parsedClose.command, "terminal.close"); |
| 37 | 37 | ||
| 38 | const parsedDiffToggle = yield* decode(KeybindingRule, { | ||
| 39 | key: "mod+d", | ||
| 40 | command: "diff.toggle", | ||
| 41 | }); | ||
| 42 | assert.strictEqual(parsedDiffToggle.command, "diff.toggle"); | ||
| 43 | |||
| 38 | const parsedLocal = yield* decode(KeybindingRule, { | 44 | const parsedLocal = yield* decode(KeybindingRule, { |
| 39 | key: "mod+shift+n", | 45 | key: "mod+shift+n", |
| 40 | command: "chat.newLocal", | 46 | command: "chat.newLocal", |
packages/contracts/src/keybindings.ts+1| ... | @@ -14,6 +14,7 @@ export const STATIC_KEYBINDING_COMMANDS = [ | ... | @@ -14,6 +14,7 @@ export const STATIC_KEYBINDING_COMMANDS = [ |
| 14 | "terminal.split", | 14 | "terminal.split", |
| 15 | "terminal.new", | 15 | "terminal.new", |
| 16 | "terminal.close", | 16 | "terminal.close", |
| 17 | "diff.toggle", | ||
| 17 | "chat.new", | 18 | "chat.new", |
| 18 | "chat.newLocal", | 19 | "chat.newLocal", |
| 19 | "editor.openFavorite", | 20 | "editor.openFavorite", |