fix(ui): let users copy text from the download-log terminal
Two separate bugs had to be fixed together before copy worked: 1. `terminal-dialog.tsx` used to swallow every contextmenu event inside the dialog (`e.preventDefault(); e.stopPropagation()`) — killing Electron's built-in "Copy" menu for selected text in the xterm log. Drop the preventDefault so the native menu fires, but keep stopPropagation (now scoped to the contextmenu handler alone) so the event doesn't bubble up through the React tree to `DownloadTaskItem`'s `onContextMenu` and pop its "select / download / refresh / delete" menu instead. Radix Dialog portals the DOM to document.body, but React synthetic events still travel the virtual tree — this is the classic portal-bubbling gotcha. 2. xterm.js does not bind Ctrl+C / Cmd+C to "copy selection" by default; it forwards them as control chars. Since the log view uses `disableStdin: true`, hijacking those keys is safe. `attachCustomKeyEventHandler` now intercepts copy shortcuts, writes the selection to the clipboard via `navigator.clipboard`, and returns false so xterm stops handling the event. No selection → passthrough. Result: both right-click → Copy and Ctrl/Cmd+C paths work. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -35,6 +35,28 @@ const Terminal: FC<TerminalProps> = ({ className, id, header }) => {
|
||||
terminal.open(terminalRef.current);
|
||||
fitAddon.fit();
|
||||
|
||||
// Copy-on-shortcut. xterm doesn't bind Ctrl+C / Cmd+C to "copy
|
||||
// selection" by default (it passes them through as control chars).
|
||||
// Since stdin is disabled for this read-only log view, hijacking
|
||||
// those keys is safe — when there's a selection we copy it and tell
|
||||
// xterm to stop handling the event; otherwise we let it through.
|
||||
terminal.attachCustomKeyEventHandler((ev) => {
|
||||
if (ev.type !== "keydown") return true;
|
||||
const isCopy =
|
||||
(ev.ctrlKey || ev.metaKey) &&
|
||||
!ev.altKey &&
|
||||
!ev.shiftKey &&
|
||||
ev.key.toLowerCase() === "c";
|
||||
if (!isCopy) return true;
|
||||
const sel = terminal.getSelection();
|
||||
if (!sel) return true;
|
||||
void navigator.clipboard.writeText(sel).catch(() => {
|
||||
/* clipboard API may be unavailable in non-secure contexts; ignore */
|
||||
});
|
||||
ev.preventDefault();
|
||||
return false;
|
||||
});
|
||||
|
||||
if (data?.log) {
|
||||
terminal.write(data.log);
|
||||
}
|
||||
|
||||
@@ -20,9 +20,17 @@ interface Props {
|
||||
|
||||
export function TerminalDialog({ trigger, title, id, asChild }: Props) {
|
||||
const { t } = useTranslation();
|
||||
const handleContextMenu = useMemoizedFn(
|
||||
|
||||
// Radix Dialog portals the DOM to document.body, but React synthetic
|
||||
// events still bubble up through the React tree — that means a
|
||||
// right-click inside this dialog would otherwise propagate to the
|
||||
// underlying DownloadTaskItem's `onContextMenu` and pop its "select /
|
||||
// download / refresh / delete" menu instead of the native "Copy"
|
||||
// menu. Stop React-level bubbling so the item handler is isolated;
|
||||
// don't preventDefault, so Chromium's built-in context menu still
|
||||
// fires for the xterm selection.
|
||||
const stopContextPropagation = useMemoizedFn(
|
||||
(e: React.MouseEvent<HTMLDivElement>) => {
|
||||
e.preventDefault();
|
||||
e.stopPropagation();
|
||||
},
|
||||
);
|
||||
@@ -30,7 +38,10 @@ export function TerminalDialog({ trigger, title, id, asChild }: Props) {
|
||||
return (
|
||||
<Dialog>
|
||||
<DialogTrigger asChild={asChild}>{trigger}</DialogTrigger>
|
||||
<DialogContent onContextMenu={handleContextMenu} className="min-w-fit">
|
||||
<DialogContent
|
||||
className="min-w-fit"
|
||||
onContextMenu={stopContextPropagation}
|
||||
>
|
||||
<DialogHeader>
|
||||
<DialogTitle>{t("downloadLog")}</DialogTitle>
|
||||
<DialogDescription>{title}</DialogDescription>
|
||||
|
||||
Reference in New Issue
Block a user