| author | |
| committer | |
| log | ce8ca5bb3d005d8653d1b09c1e7c9e02d3ef8ae4 |
| tree | b955938c804bceecaee3d1d6c0ebea7406f3fe55 |
| parent | f3fcfe1f63ac58cc7bdad2b459931a52ae5f6fa2 |
| signature | Signed by PGP key B5690EEEBB952194 |
6 files changed, 96 insertions(+), 2 deletions(-)
apps/web/src/components/LegacySidebar.tsx+3-1| ... | @@ -73,6 +73,7 @@ import { | ... | @@ -73,6 +73,7 @@ import { |
| 73 | import { isDesktopLocalConnectionTarget } from "../connection/desktopLocal"; | 73 | import { isDesktopLocalConnectionTarget } from "../connection/desktopLocal"; |
| 74 | import { useDesktopLocalBootstraps } from "../connection/useDesktopLocalBootstraps"; | 74 | import { useDesktopLocalBootstraps } from "../connection/useDesktopLocalBootstraps"; |
| 75 | import { isElectron } from "../env"; | 75 | import { isElectron } from "../env"; |
| 76 | import { useTerminalFocus } from "../hooks/useTerminalFocus"; | ||
| 76 | import { useOpenPrLink } from "../lib/openPullRequestLink"; | 77 | import { useOpenPrLink } from "../lib/openPullRequestLink"; |
| 77 | import { isTerminalFocused } from "../lib/terminalFocus"; | 78 | import { isTerminalFocused } from "../lib/terminalFocus"; |
| 78 | import { isMacPlatform } from "../lib/utils"; | 79 | import { isMacPlatform } from "../lib/utils"; |
| ... | @@ -3075,6 +3076,7 @@ export default function LegacySidebar() { | ... | @@ -3075,6 +3076,7 @@ export default function LegacySidebar() { |
| 3075 | const setSelectionAnchor = useThreadSelectionStore((s) => s.setAnchor); | 3076 | const setSelectionAnchor = useThreadSelectionStore((s) => s.setAnchor); |
| 3076 | const platform = navigator.platform; | 3077 | const platform = navigator.platform; |
| 3077 | const shortcutModifiers = useShortcutModifierState(); | 3078 | const shortcutModifiers = useShortcutModifierState(); |
| 3079 | const terminalFocused = useTerminalFocus(); | ||
| 3078 | const { environments } = useEnvironments(); | 3080 | const { environments } = useEnvironments(); |
| 3079 | const primaryEnvironmentId = usePrimaryEnvironmentId(); | 3081 | const primaryEnvironmentId = usePrimaryEnvironmentId(); |
| 3080 | const environmentLabelById = useMemo( | 3082 | const environmentLabelById = useMemo( |
| ... | @@ -3397,7 +3399,7 @@ export default function LegacySidebar() { | ... | @@ -3397,7 +3399,7 @@ export default function LegacySidebar() { |
| 3397 | [threadJumpCommandByKey], | 3399 | [threadJumpCommandByKey], |
| 3398 | ); | 3400 | ); |
| 3399 | const sidebarShortcutContext = { | 3401 | const sidebarShortcutContext = { |
| 3400 | terminalFocus: false, | 3402 | terminalFocus: terminalFocused, |
| 3401 | terminalOpen: routeTerminalOpen, | 3403 | terminalOpen: routeTerminalOpen, |
| 3402 | modelPickerOpen: isModelPickerOpen(), | 3404 | modelPickerOpen: isModelPickerOpen(), |
| 3403 | }; | 3405 | }; |
apps/web/src/components/Sidebar.tsx+10-1| ... | @@ -83,6 +83,7 @@ import { | ... | @@ -83,6 +83,7 @@ import { |
| 83 | threadTraversalDirectionFromCommand, | 83 | threadTraversalDirectionFromCommand, |
| 84 | } from "../keybindings"; | 84 | } from "../keybindings"; |
| 85 | import { useShortcutModifierState } from "../shortcutModifierState"; | 85 | import { useShortcutModifierState } from "../shortcutModifierState"; |
| 86 | import { useTerminalFocus } from "../hooks/useTerminalFocus"; | ||
| 86 | import { isTerminalFocused } from "../lib/terminalFocus"; | 87 | import { isTerminalFocused } from "../lib/terminalFocus"; |
| 87 | import { isModelPickerOpen } from "../modelPickerVisibility"; | 88 | import { isModelPickerOpen } from "../modelPickerVisibility"; |
| 88 | import { selectThreadTerminalUiState, useTerminalUiStateStore } from "../terminalUiStateStore"; | 89 | import { selectThreadTerminalUiState, useTerminalUiStateStore } from "../terminalUiStateStore"; |
| ... | @@ -3312,10 +3313,18 @@ export default function Sidebar() { | ... | @@ -3312,10 +3313,18 @@ export default function Sidebar() { |
| 3312 | // match a thread-jump binding. Adding Shift (screenshots) or Alt no | 3313 | // match a thread-jump binding. Adding Shift (screenshots) or Alt no |
| 3313 | // longer matches ⌘1..9, so the overlay hides for chords like ⌘⇧4. | 3314 | // longer matches ⌘1..9, so the overlay hides for chords like ⌘⇧4. |
| 3314 | const shortcutModifiers = useShortcutModifierState(); | 3315 | const shortcutModifiers = useShortcutModifierState(); |
| 3316 | const terminalFocused = useTerminalFocus(); | ||
| 3315 | const shouldShowJumpHintsNow = shouldShowThreadJumpHintsForModifiers( | 3317 | const shouldShowJumpHintsNow = shouldShowThreadJumpHintsForModifiers( |
| 3316 | shortcutModifiers, | 3318 | shortcutModifiers, |
| 3317 | keybindings, | 3319 | keybindings, |
| 3318 | { platform: navigator.platform }, | 3320 | { |
| 3321 | platform: navigator.platform, | ||
| 3322 | context: { | ||
| 3323 | terminalFocus: terminalFocused, | ||
| 3324 | terminalOpen: routeTerminalOpen, | ||
| 3325 | modelPickerOpen: isModelPickerOpen(), | ||
| 3326 | }, | ||
| 3327 | }, | ||
| 3319 | ); | 3328 | ); |
| 3320 | useEffect(() => { | 3329 | useEffect(() => { |
| 3321 | setShowJumpHints(shouldShowJumpHintsNow); | 3330 | setShowJumpHints(shouldShowJumpHintsNow); |
apps/web/src/hooks/useTerminalFocus.test.ts created+42| ... | @@ -0,0 +1,42 @@ | ||
| 1 | import { afterEach, describe, expect, it } from "vite-plus/test"; | ||
| 2 | |||
| 3 | import { subscribeToTerminalFocusChanges } from "./useTerminalFocus"; | ||
| 4 | |||
| 5 | const originalWindow = globalThis.window; | ||
| 6 | |||
| 7 | afterEach(() => { | ||
| 8 | if (originalWindow === undefined) { | ||
| 9 | delete (globalThis as { window?: Window }).window; | ||
| 10 | } else { | ||
| 11 | globalThis.window = originalWindow; | ||
| 12 | } | ||
| 13 | }); | ||
| 14 | |||
| 15 | describe("subscribeToTerminalFocusChanges", () => { | ||
| 16 | it("notifies on focus transitions until unsubscribed", () => { | ||
| 17 | const listeners = new Map<string, Set<() => void>>(); | ||
| 18 | globalThis.window = { | ||
| 19 | addEventListener: (type: string, listener: EventListenerOrEventListenerObject) => { | ||
| 20 | const callbacks = listeners.get(type) ?? new Set<() => void>(); | ||
| 21 | callbacks.add(listener as () => void); | ||
| 22 | listeners.set(type, callbacks); | ||
| 23 | }, | ||
| 24 | removeEventListener: (type: string, listener: EventListenerOrEventListenerObject) => { | ||
| 25 | listeners.get(type)?.delete(listener as () => void); | ||
| 26 | }, | ||
| 27 | } as unknown as Window & typeof globalThis; | ||
| 28 | let notifications = 0; | ||
| 29 | |||
| 30 | const unsubscribe = subscribeToTerminalFocusChanges(() => { | ||
| 31 | notifications += 1; | ||
| 32 | }); | ||
| 33 | |||
| 34 | for (const listener of listeners.get("focusin") ?? []) listener(); | ||
| 35 | for (const listener of listeners.get("focusout") ?? []) listener(); | ||
| 36 | expect(notifications).toBe(2); | ||
| 37 | |||
| 38 | unsubscribe(); | ||
| 39 | for (const listener of listeners.get("focusin") ?? []) listener(); | ||
| 40 | expect(notifications).toBe(2); | ||
| 41 | }); | ||
| 42 | }); | ||
apps/web/src/hooks/useTerminalFocus.ts created+16| ... | @@ -0,0 +1,16 @@ | ||
| 1 | import { useSyncExternalStore } from "react"; | ||
| 2 | |||
| 3 | import { isTerminalFocused } from "../lib/terminalFocus"; | ||
| 4 | |||
| 5 | export function subscribeToTerminalFocusChanges(listener: () => void): () => void { | ||
| 6 | window.addEventListener("focusin", listener, true); | ||
| 7 | window.addEventListener("focusout", listener, true); | ||
| 8 | return () => { | ||
| 9 | window.removeEventListener("focusin", listener, true); | ||
| 10 | window.removeEventListener("focusout", listener, true); | ||
| 11 | }; | ||
| 12 | } | ||
| 13 | |||
| 14 | export function useTerminalFocus(): boolean { | ||
| 15 | return useSyncExternalStore(subscribeToTerminalFocusChanges, isTerminalFocused, () => false); | ||
| 16 | } | ||
apps/web/src/keybindings.test.ts+15| ... | @@ -451,6 +451,21 @@ describe("thread navigation helpers", () => { | ... | @@ -451,6 +451,21 @@ describe("thread navigation helpers", () => { |
| 451 | }), | 451 | }), |
| 452 | ); | 452 | ); |
| 453 | }); | 453 | }); |
| 454 | |||
| 455 | it("never shows jump hints while the terminal is focused, even with an unrestricted binding", () => { | ||
| 456 | assert.isFalse( | ||
| 457 | shouldShowThreadJumpHints(event({ metaKey: true }), DEFAULT_BINDINGS, { | ||
| 458 | platform: "MacIntel", | ||
| 459 | context: { terminalFocus: true }, | ||
| 460 | }), | ||
| 461 | ); | ||
| 462 | assert.isTrue( | ||
| 463 | shouldShowThreadJumpHints(event({ metaKey: true }), DEFAULT_BINDINGS, { | ||
| 464 | platform: "MacIntel", | ||
| 465 | context: { terminalFocus: false }, | ||
| 466 | }), | ||
| 467 | ); | ||
| 468 | }); | ||
| 454 | }); | 469 | }); |
| 455 | 470 | ||
| 456 | describe("model picker navigation helpers", () => { | 471 | describe("model picker navigation helpers", () => { |
apps/web/src/keybindings.ts+10| ... | @@ -301,6 +301,16 @@ export function shouldShowThreadJumpHintsForModifiers( | ... | @@ -301,6 +301,16 @@ export function shouldShowThreadJumpHintsForModifiers( |
| 301 | keybindings: ResolvedKeybindingsConfig, | 301 | keybindings: ResolvedKeybindingsConfig, |
| 302 | options?: ShortcutMatchOptions, | 302 | options?: ShortcutMatchOptions, |
| 303 | ): boolean { | 303 | ): boolean { |
| 304 | // The embedded terminal owns keystrokes while it has focus: the Ghostty | ||
| 305 | // surface encodes the keydown and can write the pressed key into the shell | ||
| 306 | // before our window-level shortcut handling ever runs, regardless of any | ||
| 307 | // configured `when` clause on the jump command. Advertising jump hints | ||
| 308 | // here would promise a shortcut that instead types into the terminal, so | ||
| 309 | // never show them while the terminal is focused. | ||
| 310 | if (resolveContext(options).terminalFocus) { | ||
| 311 | return false; | ||
| 312 | } | ||
| 313 | |||
| 304 | const platform = resolvePlatform(options); | 314 | const platform = resolvePlatform(options); |
| 305 | 315 | ||
| 306 | for (const command of THREAD_JUMP_KEYBINDING_COMMANDS) { | 316 | for (const command of THREAD_JUMP_KEYBINDING_COMMANDS) { |