| author | |
| committer | |
| log | 553867262cf22f982622df3eb7f3174f62b4ba2b |
| tree | 1156016af1ad40e379e2e31ea9bea40463dbb5db |
| parent | 11639bf4391d9d49e48052bae755f748cbd22ff2 |
| signature | Signed by PGP key B5690EEEBB952194 |
6 files changed, 195 insertions(+), 2 deletions(-)
apps/desktop/src/window/DesktopWindow.test.ts+45| ... | @@ -438,6 +438,51 @@ describe("DesktopWindow", () => { | ... | @@ -438,6 +438,51 @@ describe("DesktopWindow", () => { |
| 438 | }), | 438 | }), |
| 439 | ); | 439 | ); |
| 440 | 440 | ||
| 441 | it.effect("blocks only repeated Cmd+W input before it reaches the native window menu", () => | ||
| 442 | Effect.gen(function* () { | ||
| 443 | const fakeWindow = makeFakeBrowserWindow(); | ||
| 444 | const createCount = yield* Ref.make(0); | ||
| 445 | const mainWindow = yield* Ref.make<Option.Option<Electron.BrowserWindow>>(Option.none()); | ||
| 446 | const layer = makeTestLayer({ | ||
| 447 | window: fakeWindow.window, | ||
| 448 | createCount, | ||
| 449 | mainWindow, | ||
| 450 | }); | ||
| 451 | |||
| 452 | yield* Effect.gen(function* () { | ||
| 453 | const desktopWindow = yield* DesktopWindow.DesktopWindow; | ||
| 454 | yield* desktopWindow.handleBackendReady(new URL("http://127.0.0.1:3773")); | ||
| 455 | |||
| 456 | const beforeInput = fakeWindow.webContentsListeners.get("before-input-event"); | ||
| 457 | if (!beforeInput) { | ||
| 458 | return yield* Effect.die("before-input-event listener was not registered"); | ||
| 459 | } | ||
| 460 | |||
| 461 | let prevented = false; | ||
| 462 | const event = { preventDefault: () => (prevented = true) }; | ||
| 463 | const input = { | ||
| 464 | type: "keyDown", | ||
| 465 | isAutoRepeat: true, | ||
| 466 | key: "W", | ||
| 467 | meta: true, | ||
| 468 | control: false, | ||
| 469 | alt: false, | ||
| 470 | shift: false, | ||
| 471 | }; | ||
| 472 | beforeInput(event, input); | ||
| 473 | assert.isTrue(prevented); | ||
| 474 | |||
| 475 | prevented = false; | ||
| 476 | beforeInput(event, { ...input, isAutoRepeat: false }); | ||
| 477 | assert.isFalse(prevented); | ||
| 478 | |||
| 479 | prevented = false; | ||
| 480 | beforeInput(event, { ...input, meta: false }); | ||
| 481 | assert.isFalse(prevented); | ||
| 482 | }).pipe(Effect.provide(layer)); | ||
| 483 | }), | ||
| 484 | ); | ||
| 485 | |||
| 441 | it.effect("uses the persisted main window bounds when opening the window", () => | 486 | it.effect("uses the persisted main window bounds when opening the window", () => |
| 442 | Effect.gen(function* () { | 487 | Effect.gen(function* () { |
| 443 | const fakeWindow = makeFakeBrowserWindow(); | 488 | const fakeWindow = makeFakeBrowserWindow(); |
apps/desktop/src/window/DesktopWindow.ts+12| ... | @@ -514,6 +514,18 @@ export const make = Effect.gen(function* () { | ... | @@ -514,6 +514,18 @@ export const make = Effect.gen(function* () { |
| 514 | } | 514 | } |
| 515 | }); | 515 | }); |
| 516 | 516 | ||
| 517 | // Electron's windowMenu close role owns CmdOrCtrl+W. Holding the | ||
| 518 | // close-terminal shortcut can outlive the terminal that handled its first | ||
| 519 | // press, so reject repeats before they reach the native window accelerator. | ||
| 520 | // Deliberate presses still flow through the renderer or native menu. | ||
| 521 | window.webContents.on("before-input-event", (event, input) => { | ||
| 522 | if (input.type !== "keyDown" || !input.isAutoRepeat) return; | ||
| 523 | const modifier = environment.platform === "darwin" ? input.meta : input.control; | ||
| 524 | if (modifier && !input.alt && !input.shift && input.key.toLowerCase() === "w") { | ||
| 525 | event.preventDefault(); | ||
| 526 | } | ||
| 527 | }); | ||
| 528 | |||
| 517 | window.on("page-title-updated", (event) => { | 529 | window.on("page-title-updated", (event) => { |
| 518 | event.preventDefault(); | 530 | event.preventDefault(); |
| 519 | window.setTitle(environment.displayName); | 531 | window.setTitle(environment.displayName); |
apps/web/src/components/ChatView.tsx+5| ... | @@ -171,6 +171,7 @@ import { useNowMinute } from "../hooks/useNowMinute"; | ... | @@ -171,6 +171,7 @@ import { useNowMinute } from "../hooks/useNowMinute"; |
| 171 | import { useNewThreadHandler } from "../hooks/useHandleNewThread"; | 171 | import { useNewThreadHandler } from "../hooks/useHandleNewThread"; |
| 172 | import { resolveAppModelSelectionForInstance } from "../modelSelection"; | 172 | import { resolveAppModelSelectionForInstance } from "../modelSelection"; |
| 173 | import { getTerminalFocusOwner } from "../lib/terminalFocus"; | 173 | import { getTerminalFocusOwner } from "../lib/terminalFocus"; |
| 174 | import { preventRepeatedTerminalCloseShortcut } from "../lib/terminalCloseShortcut"; | ||
| 174 | import { resolveNewDraftStartFromOrigin } from "../lib/chatThreadActions"; | 175 | import { resolveNewDraftStartFromOrigin } from "../lib/chatThreadActions"; |
| 175 | import { | 176 | import { |
| 176 | deriveLogicalProjectKeyFromSettings, | 177 | deriveLogicalProjectKeyFromSettings, |
| ... | @@ -4354,6 +4355,10 @@ function ChatViewContent(props: ChatViewProps) { | ... | @@ -4354,6 +4355,10 @@ function ChatViewContent(props: ChatViewProps) { |
| 4354 | 4355 | ||
| 4355 | useEffect(() => { | 4356 | useEffect(() => { |
| 4356 | const handler = (event: globalThis.KeyboardEvent) => { | 4357 | const handler = (event: globalThis.KeyboardEvent) => { |
| 4358 | if (preventRepeatedTerminalCloseShortcut(event, keybindings)) { | ||
| 4359 | event.stopPropagation(); | ||
| 4360 | return; | ||
| 4361 | } | ||
| 4357 | if (!activeThreadId || isCommandPaletteOpen()) { | 4362 | if (!activeThreadId || isCommandPaletteOpen()) { |
| 4358 | return; | 4363 | return; |
| 4359 | } | 4364 | } |
apps/web/src/components/ThreadTerminalDrawer.tsx+4-2| ... | @@ -43,7 +43,6 @@ import { isTerminalLinkActivation, resolvePathLinkTarget } from "../terminal-lin | ... | @@ -43,7 +43,6 @@ import { isTerminalLinkActivation, resolvePathLinkTarget } from "../terminal-lin |
| 43 | import { | 43 | import { |
| 44 | isDiffToggleShortcut, | 44 | isDiffToggleShortcut, |
| 45 | isTerminalClearShortcut, | 45 | isTerminalClearShortcut, |
| 46 | isTerminalCloseShortcut, | ||
| 47 | isTerminalNewShortcut, | 46 | isTerminalNewShortcut, |
| 48 | isTerminalSplitShortcut, | 47 | isTerminalSplitShortcut, |
| 49 | isTerminalSplitVerticalShortcut, | 48 | isTerminalSplitVerticalShortcut, |
| ... | @@ -63,6 +62,7 @@ import { previewEnvironment } from "../state/preview"; | ... | @@ -63,6 +62,7 @@ import { previewEnvironment } from "../state/preview"; |
| 63 | import { terminalEnvironment } from "../state/terminal"; | 62 | import { terminalEnvironment } from "../state/terminal"; |
| 64 | import { openTerminalLinkInPreview } from "./preview/openTerminalLinkInPreview"; | 63 | import { openTerminalLinkInPreview } from "./preview/openTerminalLinkInPreview"; |
| 65 | import { useAtomCommand } from "../state/use-atom-command"; | 64 | import { useAtomCommand } from "../state/use-atom-command"; |
| 65 | import { preventTerminalCloseShortcut } from "../lib/terminalCloseShortcut"; | ||
| 66 | 66 | ||
| 67 | const MIN_DRAWER_HEIGHT = 180; | 67 | const MIN_DRAWER_HEIGHT = 180; |
| 68 | const MAX_DRAWER_HEIGHT_RATIO = 0.75; | 68 | const MAX_DRAWER_HEIGHT_RATIO = 0.75; |
| ... | @@ -527,12 +527,14 @@ export function TerminalViewport({ | ... | @@ -527,12 +527,14 @@ export function TerminalViewport({ |
| 527 | function handleBeforeKey(event: KeyboardEvent): boolean { | 527 | function handleBeforeKey(event: KeyboardEvent): boolean { |
| 528 | const currentKeybindings = keybindingsRef.current; | 528 | const currentKeybindings = keybindingsRef.current; |
| 529 | const options = { context: { terminalFocus: true, terminalOpen: true } }; | 529 | const options = { context: { terminalFocus: true, terminalOpen: true } }; |
| 530 | if (preventTerminalCloseShortcut(event, currentKeybindings)) { | ||
| 531 | return false; | ||
| 532 | } | ||
| 530 | if ( | 533 | if ( |
| 531 | isTerminalToggleShortcut(event, currentKeybindings, options) || | 534 | isTerminalToggleShortcut(event, currentKeybindings, options) || |
| 532 | isTerminalSplitShortcut(event, currentKeybindings, options) || | 535 | isTerminalSplitShortcut(event, currentKeybindings, options) || |
| 533 | isTerminalSplitVerticalShortcut(event, currentKeybindings, options) || | 536 | isTerminalSplitVerticalShortcut(event, currentKeybindings, options) || |
| 534 | isTerminalNewShortcut(event, currentKeybindings, options) || | 537 | isTerminalNewShortcut(event, currentKeybindings, options) || |
| 535 | isTerminalCloseShortcut(event, currentKeybindings, options) || | ||
| 536 | isDiffToggleShortcut(event, currentKeybindings, options) | 538 | isDiffToggleShortcut(event, currentKeybindings, options) |
| 537 | ) { | 539 | ) { |
| 538 | return false; | 540 | return false; |
apps/web/src/lib/terminalCloseShortcut.test.ts created+93| ... | @@ -0,0 +1,93 @@ | ||
| 1 | import { describe, expect, it } from "vite-plus/test"; | ||
| 2 | |||
| 3 | import type { ResolvedKeybindingsConfig } from "@t3tools/contracts"; | ||
| 4 | |||
| 5 | import { | ||
| 6 | preventRepeatedTerminalCloseShortcut, | ||
| 7 | preventTerminalCloseShortcut, | ||
| 8 | type TerminalCloseShortcutEvent, | ||
| 9 | } from "./terminalCloseShortcut"; | ||
| 10 | |||
| 11 | const keybindings = [ | ||
| 12 | { | ||
| 13 | command: "terminal.close", | ||
| 14 | shortcut: { | ||
| 15 | key: "w", | ||
| 16 | metaKey: false, | ||
| 17 | ctrlKey: false, | ||
| 18 | shiftKey: false, | ||
| 19 | altKey: false, | ||
| 20 | modKey: true, | ||
| 21 | }, | ||
| 22 | whenAst: { type: "identifier", name: "terminalFocus" }, | ||
| 23 | }, | ||
| 24 | ] satisfies ResolvedKeybindingsConfig; | ||
| 25 | |||
| 26 | type KeyboardEventOverrides = Partial< | ||
| 27 | Pick< | ||
| 28 | TerminalCloseShortcutEvent, | ||
| 29 | "key" | "code" | "metaKey" | "ctrlKey" | "shiftKey" | "altKey" | "repeat" | ||
| 30 | > | ||
| 31 | >; | ||
| 32 | |||
| 33 | function keyboardEvent( | ||
| 34 | overrides: KeyboardEventOverrides = {}, | ||
| 35 | ): TerminalCloseShortcutEvent & { readonly defaultPrevented: boolean } { | ||
| 36 | let defaultPrevented = false; | ||
| 37 | return { | ||
| 38 | key: "w", | ||
| 39 | code: "KeyW", | ||
| 40 | metaKey: false, | ||
| 41 | ctrlKey: true, | ||
| 42 | shiftKey: false, | ||
| 43 | altKey: false, | ||
| 44 | repeat: false, | ||
| 45 | ...overrides, | ||
| 46 | preventDefault: () => { | ||
| 47 | defaultPrevented = true; | ||
| 48 | }, | ||
| 49 | get defaultPrevented() { | ||
| 50 | return defaultPrevented; | ||
| 51 | }, | ||
| 52 | }; | ||
| 53 | } | ||
| 54 | |||
| 55 | describe("terminal close shortcut guards", () => { | ||
| 56 | it("prevents the browser default for a deliberate terminal close", () => { | ||
| 57 | const event = keyboardEvent(); | ||
| 58 | |||
| 59 | expect(preventTerminalCloseShortcut(event, keybindings, "Linux x86_64")).toBe(true); | ||
| 60 | expect(event.defaultPrevented).toBe(true); | ||
| 61 | }); | ||
| 62 | |||
| 63 | it("keeps held close repeats from closing the browser after the last terminal unmounts", () => { | ||
| 64 | const firstPress = keyboardEvent(); | ||
| 65 | expect(preventTerminalCloseShortcut(firstPress, keybindings, "Linux x86_64")).toBe(true); | ||
| 66 | |||
| 67 | let browserCloseCount = 0; | ||
| 68 | for (const repeat of [true, true, true]) { | ||
| 69 | const event = keyboardEvent({ repeat }); | ||
| 70 | preventRepeatedTerminalCloseShortcut(event, keybindings, "Linux x86_64"); | ||
| 71 | if (!event.defaultPrevented) browserCloseCount += 1; | ||
| 72 | } | ||
| 73 | |||
| 74 | expect(browserCloseCount).toBe(0); | ||
| 75 | expect(preventRepeatedTerminalCloseShortcut(keyboardEvent(), keybindings, "Linux x86_64")).toBe( | ||
| 76 | false, | ||
| 77 | ); | ||
| 78 | }); | ||
| 79 | |||
| 80 | it("leaves a non-repeated window close and unrelated repeats alone", () => { | ||
| 81 | const deliberateWindowClose = keyboardEvent({ repeat: false }); | ||
| 82 | const unrelatedRepeat = keyboardEvent({ key: "q", code: "KeyQ", repeat: true }); | ||
| 83 | |||
| 84 | expect( | ||
| 85 | preventRepeatedTerminalCloseShortcut(deliberateWindowClose, keybindings, "Linux x86_64"), | ||
| 86 | ).toBe(false); | ||
| 87 | expect(deliberateWindowClose.defaultPrevented).toBe(false); | ||
| 88 | expect(preventRepeatedTerminalCloseShortcut(unrelatedRepeat, keybindings, "Linux x86_64")).toBe( | ||
| 89 | false, | ||
| 90 | ); | ||
| 91 | expect(unrelatedRepeat.defaultPrevented).toBe(false); | ||
| 92 | }); | ||
| 93 | }); | ||
apps/web/src/lib/terminalCloseShortcut.ts created+36| ... | @@ -0,0 +1,36 @@ | ||
| 1 | import type { ResolvedKeybindingsConfig } from "@t3tools/contracts"; | ||
| 2 | |||
| 3 | import { isTerminalCloseShortcut, type ShortcutEventLike } from "../keybindings"; | ||
| 4 | |||
| 5 | export interface TerminalCloseShortcutEvent extends ShortcutEventLike { | ||
| 6 | readonly repeat?: boolean; | ||
| 7 | readonly preventDefault: () => void; | ||
| 8 | } | ||
| 9 | |||
| 10 | function terminalCloseShortcutOptions(platform?: string) { | ||
| 11 | return { | ||
| 12 | ...(platform === undefined ? {} : { platform }), | ||
| 13 | context: { terminalFocus: true, terminalOpen: true }, | ||
| 14 | }; | ||
| 15 | } | ||
| 16 | |||
| 17 | export function preventTerminalCloseShortcut( | ||
| 18 | event: TerminalCloseShortcutEvent, | ||
| 19 | keybindings: ResolvedKeybindingsConfig, | ||
| 20 | platform?: string, | ||
| 21 | ): boolean { | ||
| 22 | if (!isTerminalCloseShortcut(event, keybindings, terminalCloseShortcutOptions(platform))) { | ||
| 23 | return false; | ||
| 24 | } | ||
| 25 | event.preventDefault(); | ||
| 26 | return true; | ||
| 27 | } | ||
| 28 | |||
| 29 | export function preventRepeatedTerminalCloseShortcut( | ||
| 30 | event: TerminalCloseShortcutEvent, | ||
| 31 | keybindings: ResolvedKeybindingsConfig, | ||
| 32 | platform?: string, | ||
| 33 | ): boolean { | ||
| 34 | if (!event.repeat) return false; | ||
| 35 | return preventTerminalCloseShortcut(event, keybindings, platform); | ||
| 36 | } | ||