Skip to content

Commit bb3f5d0

Browse files
committed
fix: address issues
Signed-off-by: Marek Dano <mk.dano@gmail.com>
1 parent b9612ec commit bb3f5d0

11 files changed

Lines changed: 145 additions & 16 deletions

src/components/gateways/VirtualServerDetailsPanel.tsx

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -587,7 +587,10 @@ export function VirtualServerDetailsPanel({
587587
<span className="truncate">{identifier}</span>
588588
<CopyButton
589589
value={identifier}
590-
label={`Copy ${title}`}
590+
label={intl.formatMessage(
591+
{ id: "common.copyValue" },
592+
{ label: title },
593+
)}
591594
className="size-5 text-muted-foreground"
592595
/>
593596
</span>
@@ -598,7 +601,10 @@ export function VirtualServerDetailsPanel({
598601
<span className="truncate">{identifier}</span>
599602
<CopyButton
600603
value={identifier}
601-
label={`Copy ${identifier}`}
604+
label={intl.formatMessage(
605+
{ id: "common.copyValue" },
606+
{ label: identifier },
607+
)}
602608
className="size-5 text-muted-foreground"
603609
/>
604610
</span>

src/components/prompts/PromptPreviewResult.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ export function PromptPreviewResult({ preview }: PromptPreviewResultProps) {
6060
<CodeBlock
6161
code={JSON.stringify({ messages: result.rendered.messages ?? [] }, null, 2)}
6262
language="json"
63-
copyLabel="JSON"
63+
copyLabel={intl.formatMessage({ id: "common.copyValue" }, { label: "JSON" })}
6464
/>
6565
)}
6666

src/components/servers/MCPServerDetailsPanel.tsx

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -490,7 +490,10 @@ export function MCPServerDetailsPanel({
490490
<span className="truncate">{identifier}</span>
491491
<CopyButton
492492
value={identifier}
493-
label={`Copy ${title}`}
493+
label={intl.formatMessage(
494+
{ id: "common.copyValue" },
495+
{ label: title },
496+
)}
494497
className="size-5 text-muted-foreground"
495498
/>
496499
</span>
@@ -501,7 +504,10 @@ export function MCPServerDetailsPanel({
501504
<span className="truncate">{identifier}</span>
502505
<CopyButton
503506
value={identifier}
504-
label={`Copy ${identifier}`}
507+
label={intl.formatMessage(
508+
{ id: "common.copyValue" },
509+
{ label: identifier },
510+
)}
505511
className="size-5 text-muted-foreground"
506512
/>
507513
</span>

src/components/servers/TestConnectionPanel.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { useCallback, useEffect, useMemo, useRef, useState } from "react";
22
import { z } from "zod";
33
import * as RadioGroupPrimitive from "@radix-ui/react-radio-group";
44
import { CircleCheck, CircleAlert, Info, Loader2 } from "lucide-react";
5+
import { useIntl } from "react-intl";
56
import { Button } from "../ui/button";
67
import { CopyButton } from "../ui/copy-button";
78
import { Input } from "../ui/input";
@@ -123,6 +124,7 @@ function FieldLabel({
123124
}
124125

125126
export function TestConnectionPanel({ serverUrl }: TestConnectionPanelProps) {
127+
const intl = useIntl();
126128
const [status, setStatus] = useState<TestStatus>("idle");
127129
const [method, setMethod] = useState<string>("Get");
128130
const [url, setUrl] = useState<string>(serverUrl);
@@ -441,7 +443,7 @@ export function TestConnectionPanel({ serverUrl }: TestConnectionPanelProps) {
441443
{responseBodyText && (
442444
<CopyButton
443445
value={responseBodyText}
444-
label="Copy response body"
446+
label={intl.formatMessage({ id: "mcpServer.testConnection.copyResponseBody" })}
445447
className="absolute right-2 top-2 size-6 bg-background/80 text-muted-foreground backdrop-blur-sm hover:bg-muted hover:text-foreground"
446448
/>
447449
)}

src/components/tools/ToolSchemaDialog.test.tsx

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, it, expect, vi } from "vitest";
2-
import { screen } from "@testing-library/react";
2+
import { screen, waitFor } from "@testing-library/react";
33
import userEvent from "@testing-library/user-event";
44
import { renderWithProviders as render } from "@/test/test-utils";
55
import { ToolSchemaDialog } from "./ToolSchemaDialog";
@@ -261,6 +261,37 @@ describe("ToolSchemaDialog", () => {
261261
expect(screen.getByText(/array/)).toBeInTheDocument();
262262
});
263263

264+
it("moves initial focus to the footer Close button, not a schema copy button", async () => {
265+
const tool = createMockTool();
266+
render(<ToolSchemaDialog tool={tool} open={true} onOpenChange={mockOnOpenChange} />);
267+
268+
await waitFor(() => {
269+
const allClose = screen.getAllByRole("button", { name: /close/i });
270+
const footerClose = allClose.find((btn) => !btn.querySelector("svg"))!;
271+
expect(footerClose).toHaveFocus();
272+
});
273+
});
274+
275+
it("closes on a single Escape press instead of dismissing a copy button tooltip first", async () => {
276+
const user = userEvent.setup();
277+
const tool = createMockTool();
278+
render(<ToolSchemaDialog tool={tool} open={true} onOpenChange={mockOnOpenChange} />);
279+
280+
// Wait for the auto-focus redirect to land on Close before pressing Escape,
281+
// otherwise focus may still be mid-transition from the default (a copy
282+
// button), which is exactly the regression this guards against.
283+
await waitFor(() => {
284+
const allClose = screen.getAllByRole("button", { name: /close/i });
285+
const footerClose = allClose.find((btn) => !btn.querySelector("svg"))!;
286+
expect(footerClose).toHaveFocus();
287+
});
288+
289+
await user.keyboard("{Escape}");
290+
291+
expect(mockOnOpenChange).toHaveBeenCalledTimes(1);
292+
expect(mockOnOpenChange).toHaveBeenCalledWith(false);
293+
});
294+
264295
it("handles schemas with long text values", () => {
265296
const tool = createMockTool({
266297
inputSchema: {

src/components/tools/ToolSchemaDialog.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { useRef } from "react";
22
import { Code } from "lucide-react";
3+
import { useIntl } from "react-intl";
34
import {
45
Dialog,
56
DialogContent,
@@ -26,6 +27,7 @@ function SchemaSection({
2627
title: string;
2728
schema: Record<string, unknown> | null | undefined;
2829
}) {
30+
const intl = useIntl();
2931
const schemaText = schema ? JSON.stringify(schema, null, 2) : "{}";
3032

3133
return (
@@ -39,7 +41,7 @@ function SchemaSection({
3941
</pre>
4042
<CopyButton
4143
value={schemaText}
42-
label={`Copy ${title.toLowerCase()}`}
44+
label={intl.formatMessage({ id: "common.copyValue" }, { label: title.toLowerCase() })}
4345
className="absolute right-2 top-2 size-6 bg-neutral-800/80 text-neutral-400 hover:bg-neutral-700 hover:text-neutral-100"
4446
/>
4547
</div>

src/hooks/useCopyToClipboard.test.ts

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,17 @@ function setClipboard(writeText: (value: string) => Promise<void>) {
1111
});
1212
}
1313

14+
/** A promise plus its resolver/rejecter, so a test can control exactly when it settles. */
15+
function deferred<T>() {
16+
let resolve!: (value: T) => void;
17+
let reject!: (reason: unknown) => void;
18+
const promise = new Promise<T>((res, rej) => {
19+
resolve = res;
20+
reject = rej;
21+
});
22+
return { promise, resolve, reject };
23+
}
24+
1425
describe("useCopyToClipboard", () => {
1526
beforeEach(() => {
1627
vi.useFakeTimers();
@@ -90,4 +101,61 @@ describe("useCopyToClipboard", () => {
90101
unmount();
91102
expect(clearTimeoutSpy).toHaveBeenCalled();
92103
});
104+
105+
it("does not let a stale request clobber a newer one that already settled", async () => {
106+
const first = deferred<void>();
107+
const second = deferred<void>();
108+
let call = 0;
109+
setClipboard(() => {
110+
call += 1;
111+
return call === 1 ? first.promise : second.promise;
112+
});
113+
const { result } = renderHook(() => useCopyToClipboard());
114+
115+
let firstCopy!: Promise<boolean>;
116+
let secondCopy!: Promise<boolean>;
117+
act(() => {
118+
firstCopy = result.current.copy("first");
119+
});
120+
act(() => {
121+
secondCopy = result.current.copy("second");
122+
});
123+
124+
// The newer (second) request resolves first...
125+
second.resolve();
126+
await act(async () => {
127+
await secondCopy;
128+
});
129+
expect(result.current.status).toBe("copied");
130+
131+
// ...and the older (first) request rejects after it. Since it's stale it
132+
// must be ignored rather than flipping status to "error".
133+
first.reject(new Error("denied"));
134+
await act(async () => {
135+
await firstCopy;
136+
});
137+
expect(result.current.status).toBe("copied");
138+
});
139+
140+
it("ignores a completion that arrives after unmount", async () => {
141+
const { promise, resolve } = deferred<void>();
142+
setClipboard(() => promise);
143+
const setTimeoutSpy = vi.spyOn(window, "setTimeout");
144+
const { result, unmount } = renderHook(() => useCopyToClipboard());
145+
146+
let copyPromise!: Promise<boolean>;
147+
act(() => {
148+
copyPromise = result.current.copy("value");
149+
});
150+
151+
unmount();
152+
resolve();
153+
await act(async () => {
154+
await copyPromise;
155+
});
156+
157+
// The write finished after teardown, so no state update or reset timer
158+
// should have been scheduled.
159+
expect(setTimeoutSpy).not.toHaveBeenCalled();
160+
});
93161
});

src/hooks/useCopyToClipboard.ts

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,28 @@ export type CopyStatus = "idle" | "copied" | "error";
1212
export function useCopyToClipboard(resetDelayMs = 1500) {
1313
const [status, setStatus] = useState<CopyStatus>("idle");
1414
const timeoutRef = useRef<number | null>(null);
15+
const mountedRef = useRef(true);
16+
const requestIdRef = useRef(0);
1517

16-
useEffect(
17-
() => () => {
18+
useEffect(() => {
19+
mountedRef.current = true;
20+
return () => {
21+
mountedRef.current = false;
1822
if (timeoutRef.current) window.clearTimeout(timeoutRef.current);
19-
},
20-
[],
21-
);
23+
};
24+
}, []);
2225

2326
const copy = useCallback(
2427
async (value: string) => {
28+
const requestId = ++requestIdRef.current;
2529
const ok = await copyToClipboard(value);
30+
31+
// Ignore this result if a later copy() has since been fired (so an
32+
// older request resolving out of order can't clobber newer feedback)
33+
// or the component has unmounted (so we don't set state or schedule
34+
// an uncleared timer after teardown).
35+
if (!mountedRef.current || requestId !== requestIdRef.current) return ok;
36+
2637
setStatus(ok ? "copied" : "error");
2738
if (timeoutRef.current) window.clearTimeout(timeoutRef.current);
2839
timeoutRef.current = window.setTimeout(() => {

src/i18n/locales/en-US/mcpServer.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,5 +175,6 @@
175175
"mcpServer.caCert.hint": "Public certificate files only (.pem, .crt, .cer, .cert)",
176176
"mcpServer.caCert.invalidFiles": "{count, plural, one {Invalid file type: {files}.} other {Invalid file types: {files}.}} Only .pem, .crt, .cer, .cert files are allowed.",
177177
"mcpServer.caCert.filesSelected": "{count, plural, one {# file selected successfully.} other {# files selected successfully.}}",
178-
"mcpServer.caCert.selected": "Selected: {files}"
178+
"mcpServer.caCert.selected": "Selected: {files}",
179+
"mcpServer.testConnection.copyResponseBody": "Copy response body"
179180
}

src/i18n/locales/es-ES/mcpServer.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,5 +175,6 @@
175175
"mcpServer.caCert.hint": "Solo archivos de certificado público (.pem, .crt, .cer, .cert)",
176176
"mcpServer.caCert.invalidFiles": "{count, plural, one {Tipo de archivo no válido: {files}.} other {Tipos de archivo no válidos: {files}.}} Solo se permiten archivos .pem, .crt, .cer y .cert.",
177177
"mcpServer.caCert.filesSelected": "{count, plural, one {# archivo seleccionado correctamente.} other {# archivos seleccionados correctamente.}}",
178-
"mcpServer.caCert.selected": "Seleccionados: {files}"
178+
"mcpServer.caCert.selected": "Seleccionados: {files}",
179+
"mcpServer.testConnection.copyResponseBody": "Copiar cuerpo de la respuesta"
179180
}

0 commit comments

Comments
 (0)