refactor(servers): use shared Field in TestConnectionPanel

[CloudCorpRecords]

Summary

First checklist item of IBM/mcp-context-forge#7007: replace the local FieldLabel in TestConnectionPanel.tsx and its hand-rolled label/error markup with the shared Field component.

• Deletes FieldLabel and converts its call sites: test-mode, url, path, content-type, headers, body
• required / aria-invalid / aria-describedby / the error <p role="alert"> now come from Field instead of being re-implemented per input
• The old <title> tooltip hints become Field's info popover, which is keyboard-reachable
• The Method radiogroup keeps a plain Label (out of scope in #7007: it's a group legend, not a single control)

Behavior notes

• The handshake-mode "stored credentials" hint is still rendered inside the render prop and merged into aria-describedby, so Headers is still described by both its error and the hint (an existing test asserts this). Field's own hint prop would hide the hint while an error is showing.
• Field's info trigger has the accessible name "More info about {label}", so loose getByLabelText(/headers/i) / /body/i queries in TestConnectionPanel.test.tsx became ambiguous. I anchored them (/^headers/i, /^body/i), matching the existing /^url/i and /^path/i.

Not touched: CatalogOAuthDialog (open PR #177 rewrites it), and the other checklist files. Happy to do those in follow-ups.

Test plan

• vitest run src/components/servers/TestConnectionPanel.test.tsx (47/47)
• vitest run on servers, pages/Servers, pages/Teams, mcp-servers, server-catalog (311/311 in isolation)
• eslint and prettier --check clean; pre-commit secret scan passes
• Note: the full vitest run showed load-related timeouts in unrelated files on my machine; those files pass individually, with and without this change.

Refs IBM/mcp-context-forge#7007

🤖 Generated with Claude Code

[marekdano]

@CloudCorpRecords - thanks for your contribution!!!

Findings

Severity	Location	Problem	Fix
blocking	CI: DCO check	Commit cb53f710 has no Signed-off-by trailer.	Add it: git commit -s --amend (or a follow-up commit with -s), then force-push.
minor	TestConnectionPanel.tsx:490	Headers field's hint paragraph now renders before the error paragraph; the visible error reads second instead of first.	Optional: reorder so the error paragraph still renders first.
minor	TestConnectionPanel.tsx:494	Headers field manually splices aria-describedby because Field suppresses hint whenever error is set.	Optional: extend Field to support hint + error together natively.

Merge is blocked only on the DCO check. The two UI notes are optional polish.

[CloudCorpRecords]

Thanks for the review, @marekdano. DCO sign-off added (amended and force-pushed), so that check is green now.

On the two optional notes:

• Hint/error order (line 490): agreed it's a small visual difference. I kept the hint inside the render prop so the aria-describedby chain still includes both (an existing test asserts that). I'm happy to flip the order if you'd like.
• Extend Field for hint + error (line 494): I'd rather not change the shared component's contract inside a refactor PR. If you want it, I can open a follow-up that lets Field render hint alongside error and then drop the manual splice here.

Let me know which way you'd like to go.

[marekdano]

@CloudCorpRecords - Thanks for your fixes!

New findings

1. Hint/error DOM order (functionally-impacting)

Root cause: Field's hint prop hides whenever an error is present (it's meant as a redundant explainer, not needed once the error message appears). The Headers field needs an always-visible informational note (stored-credentials hint) that coexists with a validation error, so the component bypassed hint and hand-rolled the paragraph inside the children render-prop — which runs before Field's own error paragraph. That's the swap.

Fix — add a new prop to the shared field.tsx instead of patching around it locally:

// field.tsx
interface FieldProps {
  // ...
  /** Always-visible info text, shown even while `error` is set — unlike `hint`,
   *  which hides once an error explains the problem. */
  persistentHint?: React.ReactNode;
}

function Field({ /* ... */ persistentHint, /* ... */ }: FieldProps) {
  const persistentHintId = persistentHint ? `${id}-persistent-hint` : undefined;

  const controlProps: FieldControlProps = {
    id,
    "aria-invalid": hasError,
    "aria-describedby":
      [errorId, hintId, persistentHintId].filter(Boolean).join(" ") || undefined,
    // ...
  };

  return (
    // ...
    {children(controlProps)}
    {hintId && <p id={hintId} ...>{hint}</p>}
    {hasError && <p id={errorId} role="alert" ...>{error}</p>}
    {persistentHintId && (
      <p id={persistentHintId} className="text-[13px] text-muted-foreground">
        {persistentHint}
      </p>
    )}
    // ...
  );
}

// TestConnectionPanel.tsx — Headers field
<Field
  id="headers"
  label="Headers"
  info="Request headers as a JSON object."
  error={errors.headers}
  persistentHint={
    mode === "handshake"
      ? intl.formatMessage({ id: "mcpServer.testConnection.storedCredentialsHint" })
      : undefined
  }
>
  {(controlProps) => (
    <Textarea
      {...controlProps}
      value={headers}
      onChange={(e) => {
        setHeaders(e.target.value);
        clearError("headers");
      }}
      onBlur={() => setErrors((prev) => ({ ...prev, headers: validateHeaders(headers) }))}
      placeholder="Add request headers as JSON..."
      className="bg-transparent font-mono text-sm focus-visible:ring-1 focus-visible:ring-offset-0"
      disabled={isTesting}
    />
  )}
</Field>

Drop the manual aria-describedby merge and the hand-rolled <p id="headers-hint"> — Field now owns both.

2. Undocumented workaround (suggestion)

Solved as a side effect: persistentHint carries its own doc comment explaining why it's separate from hint, so there's no silent bypass left to misread or "simplify" away.

3. Explicit aria-invalid="false" on neutral fields (minor)

// field.tsx
const controlProps: FieldControlProps = {
  id,
  "aria-invalid": hasError || undefined,   // was: hasError
  "aria-describedby": [errorId, hintId, persistentHintId].filter(Boolean).join(" ") || undefined,
  "aria-required": required !== undefined ? required : undefined,
};

Omits the attribute entirely when there's no error (matches pre-refactor DOM), still renders aria-invalid="true" on real errors. One line, benefits every Field-based field, not just this one.

[vishu-bh]

Keep the details drawer open when dismissing field help. These new info props open Radix popovers, but MCPServerDetailsPanel closes on every Escape, including events the popover already consumed. Repro: open a server's Try it panel, open URL help, and press Escape; the drawer closes too. Please make the drawer respect consumed Escape events and add a container-level regression test so dismissing help preserves the connection form.

[CloudCorpRecords]

Thanks @marekdano and @vishu-bh. Pushed 6482cdc (rebased on current main, DCO sign-off included) addressing all of it:

@marekdano

1. Hint/error order: added persistentHint to Field as you sketched. It renders after the error, stays visible while error is set, and is wired into aria-describedby (headers-error headers-persistent-hint). Headers now uses it, and the hand-rolled <p> and manual aria-describedby merge are gone.
2. Undocumented workaround: covered by the doc comment on persistentHint.
3. aria-invalid: now hasError || undefined, so it is omitted on neutral controls. One existing assertion in PasswordInput.test.tsx expected "false" under a title saying it should be absent; I updated it to match the title. Nothing else in the suite depended on it.

@vishu-bh

• MCPServerDetailsPanel now ignores an Escape that was already consumed (defaultPrevented, which Radix sets when it dismisses a popover), so dismissing field help keeps the drawer and the form. The next Escape, with no popover open, closes the drawer as before.
• Added a container-level test: open the Try it panel, type a URL, open URL help, press Escape. It asserts the help closes, onClose is not called and the typed URL is preserved, then a second Escape closes the drawer. I confirmed it fails without the fix (onClose called once) and passes with it.
• Heads-up, not changed here: ToolDetailsPanel, ResourceDetailsPanel, PromptDetailsPanel and VirtualServerDetailsPanel use the same unconditional Escape → onClose pattern. They don't contain field-help popovers today, so I left them alone, but a shared hook might be worth a follow-up.

Lint is clean and the component and page suites pass locally (2,749 tests); full CI should confirm.