From e204f630e814ede0c6906ed9f83df88874cf5fe1 Mon Sep 17 00:00:00 2001 From: Rene Turcios <95189389+CloudCorpRecords@users.noreply.github.com> Date: Thu, 8 Oct 2026 13:55:12 -0700 Subject: [PATCH 1/2] refactor(servers): use shared Field in TestConnectionPanel Replace the local FieldLabel and hand-rolled error markup with the shared Field component, so required/aria-invalid/aria-describedby wiring and the info popover come from one place. The Method radiogroup keeps a plain Label (out of scope per #7007). The handshake credentials hint still describes the Headers field alongside its error. Refs IBM/mcp-context-forge#7007 Co-Authored-By: Claude Sonnet 5.5 Signed-off-by: Rene Turcios <95189389+CloudCorpRecords@users.noreply.github.com> --- .../servers/TestConnectionPanel.test.tsx | 35 +- .../servers/TestConnectionPanel.tsx | 312 ++++++++---------- 2 files changed, 156 insertions(+), 191 deletions(-) diff --git a/src/components/servers/TestConnectionPanel.test.tsx b/src/components/servers/TestConnectionPanel.test.tsx index 6b1dbc43..b197de41 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 () => { @@ -636,7 +636,10 @@ describe("TestConnectionPanel", () => { 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(screen.getByLabelText(/^headers/i)).toHaveAttribute( + "aria-describedby", + "headers-hint", + ); }); it("describes the Headers field by both the error and the hint", async () => { @@ -644,11 +647,11 @@ 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", ); @@ -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..c84b940c 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 - -