Skip to content

Commit 1add47b

Browse files
fix(web): add copying terminal selection with ctrl+c in the web app (#5638)
1 parent 8099140 commit 1add47b

9 files changed

Lines changed: 159 additions & 21 deletions

File tree

apps/web/src/components/ThreadTerminalDrawer.tsx

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -440,7 +440,6 @@ export function TerminalViewport({
440440
onData: (data) => handleData(data),
441441
onResize: (cols, rows) => void resizeTerminal(cols, rows),
442442
onSelectionChange: () => handleSelectionChange(),
443-
onCopy: (text) => handleCopy(text),
444443
beforeKey: (event) => handleBeforeKey(event),
445444
onLinkActivate: (text, event) => handleLinkActivate(text, event),
446445
};
@@ -668,17 +667,6 @@ export function TerminalViewport({
668667
})();
669668
}
670669

671-
function handleCopy(text: string): void {
672-
void writeTextToClipboard(text, "terminal selection").catch((error: unknown) => {
673-
const activeTerminal = terminalRef.current;
674-
if (!activeTerminal) return;
675-
writeSystemMessage(
676-
activeTerminal,
677-
error instanceof Error ? error.message : "Unable to copy terminal selection",
678-
);
679-
});
680-
}
681-
682670
function handleData(data: string): void {
683671
void (async () => {
684672
const result = await writeTerminal(data);
@@ -696,6 +684,12 @@ export function TerminalViewport({
696684
return;
697685
}
698686
clearSelectionAction();
687+
// A copy shortcut that clears the selection (Ctrl+C) must also close
688+
// the context menu that appears with the selection, but a clear that
689+
// never opened a menu must not dismiss an unrelated one.
690+
if (selectionActionMenuOpenRef.current) {
691+
void localApi?.contextMenu.close();
692+
}
699693
}
700694

701695
const handleMouseUp = (event: MouseEvent) => {

apps/web/src/components/settings/SettingsFontPreviews.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,6 @@ export function TerminalFontPreview({ family, size }: { family: string; size: nu
238238
onData: echo,
239239
onResize: noop,
240240
onSelectionChange: noop,
241-
onCopy: (text) => void navigator.clipboard?.writeText(text).catch(noop),
242241
// Tab keeps walking the settings page instead of feeding the echo loop.
243242
beforeKey: (event) => event.key !== "Tab",
244243
onLinkActivate: noop,

apps/web/src/contextMenuFallback.test.ts

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test";
22

3-
import { showContextMenuFallback } from "./contextMenuFallback";
3+
import { dismissContextMenu, showContextMenuFallback } from "./contextMenuFallback";
44

55
type FakeListener = (event: FakeDomEvent) => void;
66

@@ -236,3 +236,37 @@ describe("showContextMenuFallback", () => {
236236
await expect(selectionPromise).resolves.toBe("rename:project-b");
237237
});
238238
});
239+
240+
describe("dismissContextMenu", () => {
241+
it("resolves an open menu with null", async () => {
242+
const selectionPromise = showContextMenuFallback([
243+
{ id: "rename", label: "Rename" },
244+
{ id: "delete", label: "Delete" },
245+
]);
246+
expect(findButton("Rename")).toBeTruthy();
247+
248+
dismissContextMenu();
249+
250+
await expect(selectionPromise).resolves.toBeNull();
251+
expect(findButton("Rename")).toBeUndefined();
252+
});
253+
254+
it("is a no-op when no menu is open", async () => {
255+
dismissContextMenu();
256+
expect(findButton("Rename")).toBeUndefined();
257+
});
258+
259+
it("dismisses the prior menu when a new one opens", async () => {
260+
const firstPromise = showContextMenuFallback([{ id: "first", label: "First" }]);
261+
expect(findButton("First")).toBeTruthy();
262+
263+
const secondPromise = showContextMenuFallback([{ id: "second", label: "Second" }]);
264+
265+
await expect(firstPromise).resolves.toBeNull();
266+
expect(findButton("First")).toBeUndefined();
267+
expect(findButton("Second")).toBeTruthy();
268+
269+
dismissContextMenu();
270+
await expect(secondPromise).resolves.toBeNull();
271+
});
272+
});

apps/web/src/contextMenuFallback.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,21 @@ function isNodeWithinMenuStack(target: EventTarget | null, menuStack: readonly H
101101
return false;
102102
}
103103

104+
// Only one fallback menu exists at a time in the renderer; the active one is
105+
// tracked so a state change (for example a terminal selection clearing) can
106+
// dismiss it with the same result as an outside click or Escape.
107+
let activeContextMenuDismiss: (() => void) | null = null;
108+
109+
/**
110+
* Closes the currently open fallback context menu, resolving its show() with
111+
* null (the same result as dismissing by outside click or Escape). No-op when
112+
* no fallback menu is open.
113+
*/
114+
export function dismissContextMenu(): void {
115+
activeContextMenuDismiss?.();
116+
activeContextMenuDismiss = null;
117+
}
118+
104119
/**
105120
* Imperative DOM-based context menu for non-Electron environments.
106121
* Supports nested submenus and resolves with the clicked leaf item id.
@@ -114,11 +129,16 @@ export function showContextMenuFallback<T extends string>(
114129
let isDisposed = false;
115130
let canDismissFromPointer = false;
116131

132+
const dismiss = () => cleanup(null);
133+
117134
const cleanup = (result: T | null) => {
118135
if (isDisposed) {
119136
return;
120137
}
121138
isDisposed = true;
139+
if (activeContextMenuDismiss === dismiss) {
140+
activeContextMenuDismiss = null;
141+
}
122142
document.removeEventListener("keydown", onKeyDown);
123143
document.removeEventListener("pointerdown", onPointerDown, true);
124144
document.removeEventListener("contextmenu", onContextMenu, true);
@@ -299,6 +319,13 @@ export function showContextMenuFallback<T extends string>(
299319
document.addEventListener("pointerdown", onPointerDown, true);
300320
document.addEventListener("contextmenu", onContextMenu, true);
301321
openMenu(items, position?.x ?? 0, position?.y ?? 0, 0);
322+
// Only one fallback menu can be open at a time: a new show must dismiss
323+
// any prior one, or its DOM and listeners leak and close() can only ever
324+
// reach the newest menu.
325+
if (activeContextMenuDismiss) {
326+
activeContextMenuDismiss();
327+
}
328+
activeContextMenuDismiss = dismiss;
302329

303330
requestAnimationFrame(() => {
304331
canDismissFromPointer = true;

apps/web/src/localApi.test.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,14 @@ const showContextMenuFallbackMock =
1313
position?: { x: number; y: number },
1414
) => Promise<T | null>
1515
>();
16+
const dismissContextMenuMock = vi.fn<() => void>();
1617

1718
const requestConfirmDialogMock =
1819
vi.fn<(message: string, options?: ConfirmDialogOptions) => Promise<boolean> | undefined>();
1920

2021
vi.mock("./contextMenuFallback", () => ({
2122
showContextMenuFallback: showContextMenuFallbackMock,
23+
dismissContextMenu: dismissContextMenuMock,
2224
}));
2325

2426
vi.mock("./confirmDialog", () => ({
@@ -85,6 +87,14 @@ describe("LocalApi", () => {
8587
expect(showContextMenuFallbackMock).toHaveBeenCalledWith(items, { x: 4, y: 5 });
8688
});
8789

90+
it("dismisses an open browser context menu without a desktop bridge", async () => {
91+
const { createLocalApi } = await import("./localApi");
92+
93+
await createLocalApi().contextMenu.close();
94+
95+
expect(dismissContextMenuMock).toHaveBeenCalledOnce();
96+
});
97+
8898
it("uses the themed confirmation host when it is available", async () => {
8999
requestConfirmDialogMock.mockResolvedValue(true);
90100
const { createLocalApi } = await import("./localApi");

apps/web/src/localApi.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import type { ConfirmDialogOptions, ContextMenuItem, LocalApi } from "@t3tools/contracts";
22

33
import { requestConfirmDialog } from "./confirmDialog";
4-
import { showContextMenuFallback } from "./contextMenuFallback";
4+
import { dismissContextMenu, showContextMenuFallback } from "./contextMenuFallback";
55
import { readBrowserClientSettings, writeBrowserClientSettings } from "./clientPersistenceStorage";
66
import { resetRequestLatencyStateForTests } from "./rpc/requestLatencyState";
77

@@ -41,6 +41,14 @@ function createBrowserLocalApi(): LocalApi {
4141
}
4242
return showContextMenuFallback(items, position);
4343
},
44+
// A native desktop menu blocks keyboard input and closes on outside
45+
// interaction, so nothing to do there; the DOM fallback needs an explicit
46+
// dismiss when the state behind it goes away.
47+
close: async () => {
48+
if (!window.desktopBridge) {
49+
dismissContextMenu();
50+
}
51+
},
4452
},
4553
persistence: {
4654
getClientSettings: async () => {

apps/web/src/terminal/ghostty/surface.test.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -219,11 +219,12 @@ describe("isTerminalCopyShortcut", () => {
219219
expect(isTerminalCopyShortcut(event({ metaKey: true }), "MacIntel")).toBe(true);
220220
});
221221

222-
it("uses the conventional Ctrl+Shift+C shortcut elsewhere", () => {
223-
expect(isTerminalCopyShortcut(event({ ctrlKey: true }), "Linux x86_64")).toBe(false);
222+
it("copies with Ctrl+C and Ctrl+Shift+C elsewhere", () => {
223+
expect(isTerminalCopyShortcut(event({ ctrlKey: true }), "Linux x86_64")).toBe(true);
224224
expect(isTerminalCopyShortcut(event({ ctrlKey: true, shiftKey: true }), "Linux x86_64")).toBe(
225225
true,
226226
);
227+
expect(isTerminalCopyShortcut(event({}), "Linux x86_64")).toBe(false);
227228
});
228229

229230
it("uses the produced character instead of the physical key position", () => {

apps/web/src/terminal/ghostty/surface.ts

Lines changed: 68 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ export function isTerminalCopyShortcut(
333333
platform = navigator.platform,
334334
) {
335335
if (event.key.toLowerCase() !== "c") return false;
336-
return isMacPlatform(platform) ? event.metaKey : event.ctrlKey && event.shiftKey;
336+
return isMacPlatform(platform) ? event.metaKey : event.ctrlKey;
337337
}
338338

339339
export function isTerminalPasteShortcut(
@@ -463,7 +463,6 @@ export interface GhosttyTerminalSurfaceOptions {
463463
readonly onData: (data: string) => void;
464464
readonly onResize: (cols: number, rows: number) => void;
465465
readonly onSelectionChange: () => void;
466-
readonly onCopy: (text: string) => void;
467466
readonly beforeKey: (event: KeyboardEvent) => boolean;
468467
readonly onLinkActivate: (text: string, event: MouseEvent) => void;
469468
}
@@ -531,6 +530,8 @@ export class GhosttyTerminalSurface {
531530
private theme: GhosttyTheme;
532531
private readonly suppressedKeyCodes = new Set<string>();
533532
private pasteShortcutToken = 0;
533+
private copyShortcutToken = 0;
534+
private clearSelectionAfterCopy = false;
534535
private wheelRemainder = 0;
535536
private dprMedia: MediaQueryList | null = null;
536537
// Read live on every blink decision, and watched so that dropping the
@@ -901,9 +902,58 @@ export class GhosttyTerminalSurface {
901902
return;
902903
}
903904
if (isTerminalCopyShortcut(event) && this.hasSelection()) {
904-
event.preventDefault();
905+
// A plain Ctrl+C/Cmd+C fires the browser's native copy event, caught in
906+
// onCopyEvent; not preventing the default keeps that path alive. WebKit
907+
// omits the keyboard copy event without a DOM selection, so race the
908+
// clipboard write against it the same way paste races its read. The
909+
// Shift variant has no native event (Chrome binds Ctrl+Shift+C to
910+
// inspect), so synthesize one with execCommand("copy").
911+
if (event.shiftKey) {
912+
event.preventDefault();
913+
document.execCommand("copy");
914+
} else {
915+
// A plain Ctrl+C is also SIGINT on non-mac: clear the selection once
916+
// it copies so the next Ctrl+C reaches the shell. The Shift chord and
917+
// Cmd+C are copy-only, so they keep the selection; resetting the flag
918+
// up front also drops any clear owed by an earlier gesture that never
919+
// completed.
920+
this.clearSelectionAfterCopy = !event.shiftKey && !isMacPlatform(navigator.platform);
921+
const clipboard = navigator.clipboard;
922+
if (typeof clipboard?.writeText === "function") {
923+
// Defer the write past the default action: the native copy event
924+
// (dispatched synchronously with the default action) claims the
925+
// token first when it fires, and the write covers browsers whose
926+
// shortcut produces no copy event. Skipping a write the native
927+
// event already handled stops a stale resolution from clobbering a
928+
// clipboard the user filled after this copy.
929+
const token = ++this.copyShortcutToken;
930+
const selection = this.getSelection();
931+
void Promise.resolve().then(() => {
932+
if (this.disposed || this.copyShortcutToken !== token) return;
933+
void clipboard.writeText(selection).then(
934+
() => {
935+
// The write may have been superseded while in flight; only
936+
// touch the selection if this gesture still owns the token.
937+
if (this.disposed || this.copyShortcutToken !== token) return;
938+
if (this.clearSelectionAfterCopy) {
939+
this.clearSelectionAfterCopy = false;
940+
this.clearSelection();
941+
}
942+
},
943+
() => {
944+
// The write failed and the native event has already had its
945+
// chance, so nothing copied and no clear is owed by this
946+
// gesture; a newer one may have just set the flag, so only
947+
// drop it if this gesture still owns the token.
948+
if (this.copyShortcutToken === token) {
949+
this.clearSelectionAfterCopy = false;
950+
}
951+
},
952+
);
953+
});
954+
}
955+
}
905956
this.suppressedKeyCodes.add(event.code);
906-
this.options.onCopy(this.getSelection());
907957
return;
908958
}
909959
if (isTerminalPasteShortcut(event)) {
@@ -989,6 +1039,18 @@ export class GhosttyTerminalSurface {
9891039
this.dprMedia.addEventListener("change", this.onDevicePixelRatioChange);
9901040
}
9911041

1042+
private readonly onCopyEvent = (event: ClipboardEvent) => {
1043+
if (!this.hasSelection()) return;
1044+
event.preventDefault();
1045+
event.clipboardData?.setData("text/plain", this.getSelection());
1046+
// The native event beat any deferred write; drop the in-flight fallback.
1047+
this.copyShortcutToken += 1;
1048+
if (this.clearSelectionAfterCopy) {
1049+
this.clearSelectionAfterCopy = false;
1050+
this.clearSelection();
1051+
}
1052+
};
1053+
9921054
private readonly onPaste = (event: ClipboardEvent) => {
9931055
// Always suppress the browser's default insertion: content the textarea
9941056
// would receive (for example an html-only clipboard converted to text)
@@ -1384,6 +1446,7 @@ export class GhosttyTerminalSurface {
13841446
this.input.addEventListener("blur", this.onBlur);
13851447
this.input.addEventListener("input", this.onInput);
13861448
this.input.addEventListener("paste", this.onPaste);
1449+
this.input.addEventListener("copy", this.onCopyEvent);
13871450
this.input.addEventListener("compositionstart", this.onCompositionStart);
13881451
this.input.addEventListener("compositionend", this.onCompositionEnd);
13891452
this.canvas.addEventListener("pointerdown", this.onPointerDown);
@@ -1408,6 +1471,7 @@ export class GhosttyTerminalSurface {
14081471
this.input.removeEventListener("blur", this.onBlur);
14091472
this.input.removeEventListener("input", this.onInput);
14101473
this.input.removeEventListener("paste", this.onPaste);
1474+
this.input.removeEventListener("copy", this.onCopyEvent);
14111475
this.input.removeEventListener("compositionstart", this.onCompositionStart);
14121476
this.input.removeEventListener("compositionend", this.onCompositionEnd);
14131477
this.canvas.removeEventListener("pointerdown", this.onPointerDown);

packages/contracts/src/ipc.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1189,6 +1189,7 @@ export interface LocalApi {
11891189
items: readonly ContextMenuItem<T>[],
11901190
position?: { x: number; y: number },
11911191
) => Promise<T | null>;
1192+
close: () => Promise<void>;
11921193
};
11931194
persistence: {
11941195
getClientSettings: () => Promise<ClientSettings | null>;

0 commit comments

Comments
 (0)