diff --git a/src/components/servers/MCPServerDetailsPanel.test.tsx b/src/components/servers/MCPServerDetailsPanel.test.tsx index 1bc83440..63a863e3 100644 --- a/src/components/servers/MCPServerDetailsPanel.test.tsx +++ b/src/components/servers/MCPServerDetailsPanel.test.tsx @@ -510,6 +510,36 @@ describe("MCPServerDetailsPanel", () => { expect(onClose).toHaveBeenCalledTimes(1); }); + it("keeps the drawer open when Escape dismisses a field-help popover, then closes on the next Escape", async () => { + const user = userEvent.setup(); + const onClose = vi.fn(); + + renderWithProviders( + , + ); + + // "Try it" is the default tab; type into the connection form first so we + // can prove dismissing help doesn't discard it. + const urlField = await screen.findByLabelText(/^url/i); + await user.clear(urlField); + await user.type(urlField, "https://mcp.example.com/mcp"); + + await user.click(screen.getByRole("button", { name: /more information about url/i })); + expect(await screen.findByText(/full url of the mcp server to test/i)).toBeInTheDocument(); + + // Escape consumed by the popover: help closes, drawer and form stay. + await user.keyboard("{Escape}"); + await waitFor(() => + expect(screen.queryByText(/full url of the mcp server to test/i)).not.toBeInTheDocument(), + ); + expect(onClose).not.toHaveBeenCalled(); + expect(screen.getByLabelText(/^url/i)).toHaveValue("https://mcp.example.com/mcp"); + + // With no popover open, Escape closes the drawer as before. + await user.keyboard("{Escape}"); + expect(onClose).toHaveBeenCalledTimes(1); + }); + it("does not close on Escape when already closed", async () => { const onClose = vi.fn(); diff --git a/src/components/servers/MCPServerDetailsPanel.tsx b/src/components/servers/MCPServerDetailsPanel.tsx index f2232674..7ac4ed51 100644 --- a/src/components/servers/MCPServerDetailsPanel.tsx +++ b/src/components/servers/MCPServerDetailsPanel.tsx @@ -191,11 +191,13 @@ export function MCPServerDetailsPanel({ }; }, [open]); - // ESC closes while open. + // ESC closes while open. An Escape a nested Radix layer (e.g. a field-help + // popover) already consumed arrives with defaultPrevented set; leave the + // drawer open for it so dismissing help doesn't discard the form behind it. useEffect(() => { if (!open) return; const onKey = (e: globalThis.KeyboardEvent) => { - if (e.key === "Escape") onClose(); + if (e.key === "Escape" && !e.defaultPrevented) onClose(); }; window.addEventListener("keydown", onKey); return () => window.removeEventListener("keydown", onKey); diff --git a/src/components/servers/TestConnectionPanel.test.tsx b/src/components/servers/TestConnectionPanel.test.tsx index 6b1dbc43..427bce1b 100644 --- a/src/components/servers/TestConnectionPanel.test.tsx +++ b/src/components/servers/TestConnectionPanel.test.tsx @@ -39,7 +39,7 @@ describe("TestConnectionPanel", () => { expect(screen.getByRole("radiogroup", { name: /method/i })).toBeInTheDocument(); expect(screen.getByLabelText(/^path/i)).toBeInTheDocument(); expect(screen.getByLabelText(/content type/i)).toBeInTheDocument(); - expect(screen.getByLabelText(/headers/i)).toBeInTheDocument(); + expect(screen.getByLabelText(/^headers/i)).toBeInTheDocument(); }); it("exposes HTTP methods as radio options", () => { @@ -55,11 +55,11 @@ describe("TestConnectionPanel", () => { render(); // GET is selected by default — no body field. - expect(screen.queryByLabelText(/body/i)).not.toBeInTheDocument(); + expect(screen.queryByLabelText(/^body/i)).not.toBeInTheDocument(); await user.click(screen.getByRole("radio", { name: "Post" })); - expect(screen.getByLabelText(/body/i)).toBeInTheDocument(); + expect(screen.getByLabelText(/^body/i)).toBeInTheDocument(); }); it("calls the connectivity endpoint and shows a successful response", async () => { @@ -136,7 +136,7 @@ describe("TestConnectionPanel", () => { render(); await user.click(screen.getByRole("radio", { name: "Post" })); - await user.type(screen.getByLabelText(/body/i), '{{"hello":"world"}'); + await user.type(screen.getByLabelText(/^body/i), '{{"hello":"world"}'); await user.click(screen.getByRole("button", { name: /^test connection$/i })); await waitFor(() => { @@ -159,7 +159,7 @@ describe("TestConnectionPanel", () => { render(); await user.type( - screen.getByLabelText(/headers/i), + screen.getByLabelText(/^headers/i), '{{"Authorization":"Bearer tok","X-Trace":"1"}', ); await user.click(screen.getByRole("button", { name: /^test connection$/i })); @@ -208,7 +208,7 @@ describe("TestConnectionPanel", () => { await user.click(screen.getByRole("combobox", { name: /content type/i })); await user.click(screen.getByRole("option", { name: /x-www-form-urlencoded/i })); // A non-JSON body must NOT trigger the "Invalid body JSON" validation. - await user.type(screen.getByLabelText(/body/i), "name=subway&line=A"); + await user.type(screen.getByLabelText(/^body/i), "name=subway&line=A"); await user.click(screen.getByRole("button", { name: /^test connection$/i })); await waitFor(() => { @@ -308,7 +308,7 @@ describe("TestConnectionPanel", () => { const user = userEvent.setup(); render(); - const headersField = screen.getByLabelText(/headers/i); + const headersField = screen.getByLabelText(/^headers/i); await user.clear(headersField); await user.type(headersField, "invalid json"); @@ -326,7 +326,7 @@ describe("TestConnectionPanel", () => { // Body is only available for non-GET methods. await user.click(screen.getByRole("radio", { name: "Post" })); - const bodyField = screen.getByLabelText(/body/i); + const bodyField = screen.getByLabelText(/^body/i); await user.type(bodyField, "not json"); await user.click(screen.getByRole("button", { name: /^test connection$/i })); @@ -410,10 +410,10 @@ describe("TestConnectionPanel", () => { expect(screen.getByLabelText(/^url/i)).toBeInTheDocument(); expect(screen.getByLabelText(/^path/i)).toBeInTheDocument(); - expect(screen.getByLabelText(/headers/i)).toBeInTheDocument(); + expect(screen.getByLabelText(/^headers/i)).toBeInTheDocument(); expect(screen.queryByRole("radiogroup", { name: /method/i })).not.toBeInTheDocument(); expect(screen.queryByLabelText(/content type/i)).not.toBeInTheDocument(); - expect(screen.queryByLabelText(/body/i)).not.toBeInTheDocument(); + expect(screen.queryByLabelText(/^body/i)).not.toBeInTheDocument(); }); it("shows the stored-credentials hint under Headers", async () => { @@ -635,8 +635,11 @@ describe("TestConnectionPanel", () => { await selectHandshakeMode(user); const hint = screen.getByText(/stored credentials for registered servers/i); - expect(hint.id).toBe("headers-hint"); - expect(screen.getByLabelText(/headers/i)).toHaveAttribute("aria-describedby", "headers-hint"); + expect(hint.id).toBe("headers-persistent-hint"); + expect(screen.getByLabelText(/^headers/i)).toHaveAttribute( + "aria-describedby", + "headers-persistent-hint", + ); }); it("describes the Headers field by both the error and the hint", async () => { @@ -644,13 +647,13 @@ describe("TestConnectionPanel", () => { render(); await selectHandshakeMode(user); - await user.type(screen.getByLabelText(/headers/i), "not json"); + await user.type(screen.getByLabelText(/^headers/i), "not json"); await user.click(screen.getByRole("button", { name: /^test connection$/i })); await waitFor(() => expect(screen.getByText(/invalid headers json/i)).toBeInTheDocument()); - expect(screen.getByLabelText(/headers/i)).toHaveAttribute( + expect(screen.getByLabelText(/^headers/i)).toHaveAttribute( "aria-describedby", - "headers-error headers-hint", + "headers-error headers-persistent-hint", ); }); @@ -666,7 +669,7 @@ describe("TestConnectionPanel", () => { render(); await selectHandshakeMode(user); - await user.type(screen.getByLabelText(/headers/i), '{{"X-Retry": 3}'); + await user.type(screen.getByLabelText(/^headers/i), '{{"X-Retry": 3}'); await user.click(screen.getByRole("button", { name: /^test connection$/i })); await waitFor(() => @@ -723,7 +726,7 @@ describe("TestConnectionPanel", () => { // Only HTTP mode validates the body, and the handshake payload never // carries one — leftover invalid JSON must not be parsed on this path. await user.click(screen.getByRole("radio", { name: "Post" })); - await user.type(screen.getByLabelText(/body/i), "not json"); + await user.type(screen.getByLabelText(/^body/i), "not json"); await selectHandshakeMode(user); await user.click(screen.getByRole("button", { name: /^test connection$/i })); @@ -874,7 +877,7 @@ describe("TestConnectionPanel", () => { render(); await selectHandshakeMode(user); - await user.click(screen.getByLabelText(/headers/i)); + await user.click(screen.getByLabelText(/^headers/i)); await user.paste('{"X-Api-Key": "k"}'); await user.click(screen.getByRole("button", { name: /^test connection$/i })); diff --git a/src/components/servers/TestConnectionPanel.tsx b/src/components/servers/TestConnectionPanel.tsx index 76a58fee..64a92e64 100644 --- a/src/components/servers/TestConnectionPanel.tsx +++ b/src/components/servers/TestConnectionPanel.tsx @@ -1,11 +1,12 @@ import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { z } from "zod"; import * as RadioGroupPrimitive from "@radix-ui/react-radio-group"; -import { Info, Loader2 } from "lucide-react"; +import { Loader2 } from "lucide-react"; import { STATUS_ICON } from "@/lib/status"; import { useIntl, type IntlShape } from "react-intl"; import { Button } from "../ui/button"; import { CopyButton } from "../ui/copy-button"; +import { Field } from "../ui/field"; import { Input } from "../ui/input"; import { Label } from "../ui/label"; import { RadioGroup } from "../ui/radio-group"; @@ -161,32 +162,6 @@ function validateBody(value: string, method: string, contentType: string): strin return undefined; } -function FieldLabel({ - htmlFor, - children, - required, - hint, -}: { - htmlFor?: string; - children: React.ReactNode; - required?: boolean; - hint?: string; -}) { - return ( - - ); -} - export function TestConnectionPanel({ serverUrl }: TestConnectionPanelProps) { const intl = useIntl(); const [status, setStatus] = useState("idle"); @@ -364,58 +339,58 @@ export function TestConnectionPanel({ serverUrl }: TestConnectionPanelProps) { {/* Left column — request form */}
{/* Test type */} -
- - {intl.formatMessage({ id: "mcpServer.testConnection.mode.label" })} - - -
+ + {(controlProps) => ( + + )} + {/* URL */} -
- - URL - - { - setUrl(e.target.value); - clearError("url"); - }} - onBlur={() => setErrors((prev) => ({ ...prev, url: validateUrl(url) }))} - placeholder="https://mcp.github.com/mcp" - disabled={isTesting} - aria-invalid={!!errors.url} - aria-describedby={errors.url ? "url-error" : undefined} - className="bg-transparent dark:bg-transparent" - /> - {errors.url && ( -

- {errors.url} -

+ + {(controlProps) => ( + { + setUrl(e.target.value); + clearError("url"); + }} + onBlur={() => setErrors((prev) => ({ ...prev, url: validateUrl(url) }))} + placeholder="https://mcp.github.com/mcp" + disabled={isTesting} + className="bg-transparent dark:bg-transparent" + /> )} -
+ {/* Method */} {mode === "http" && (
- Method + - - Path - - { - setPath(e.target.value); - clearError("path"); - }} - onBlur={() => setErrors((prev) => ({ ...prev, path: validatePath(path) }))} - placeholder="/health" - disabled={isTesting} - aria-invalid={!!errors.path} - aria-describedby={errors.path ? "path-error" : undefined} - className="bg-transparent dark:bg-transparent" - /> - {errors.path && ( -

- {errors.path} -

+ + {(controlProps) => ( + { + setPath(e.target.value); + clearError("path"); + }} + onBlur={() => setErrors((prev) => ({ ...prev, path: validatePath(path) }))} + placeholder="/health" + disabled={isTesting} + className="bg-transparent dark:bg-transparent" + /> )} -
+ {/* Content type */} {mode === "http" && ( -
- Content type - -
+ + {(controlProps) => ( + + )} + )} {/* Headers */} -
- - Headers - -