fix: Form primitives spacing

[gcgoncalves]

Relates to IBM/mcp-context-forge#6508

Three root causes of inconsistent label-to-control gaps across forms:

• Label: leading-none lived in the CVA base string, so a call-site text-sm silently stripped it via tailwind-merge (line-height utilities are treated as conflicting with font-size ones). Moved to a [data-slot="label"] CSS rule that no className can strip.
• SelectTrigger: height was set via data-[size=*], an attribute selector that always beats a plain utility class in specificity, so call-site h-10 overrides were silently ignored. Replaced with a CVA size variant so heights merge normally.
• SelectTrigger defaulted to w-fit while Input defaults to w-full, forcing every call site to re-add w-full. Default is now w-full.

Also introduces Field, a label/control/hint/error stack primitive, so future spacing changes live in one place instead of every form. Form migration to Field is PR2.

[marekdano]

The core fixes are solid.

The new Field primitive has a11y gaps:

1. [High] field.tsx:~116 — aria-invalid/aria-describedby silently no-op on composite children (e.g. Select) Wrapping <Select><SelectTrigger id="type">...</SelectTrigger></Select> in <Field id="type" error="Required"> clones the aria props onto <Select>, but Select (select.tsx:29-31) just spreads ...props onto SelectPrimitive.Root, a Radix context provider that renders no DOM element — the actual SelectTrigger never receives them. Verified empirically: trigger.getAttribute('aria-invalid') and aria-describedby both came back null. Since select.tsx is touched in this same PR and select-based fields are the next thing scheduled to migrate onto Field ("PR2"), this would silently drop error-state a11y wiring and styling (aria-invalid:border-destructive) on every select field.

2. [High] field.tsx:~116 — aria-invalid/aria-describedby get cloned onto every child, not just the form control <Field id="name" error="Required"><Input id="name"/><Button>Clear</Button></Field> (a field with a trailing action) marks the Clear button as aria-invalid="true" and aria-describedby="name-error" too. Verified empirically: both Input and the sibling Button received the same attributes. Screen readers will announce the Clear button as invalid and "described by: Required," which is wrong.

3. [Medium] field.tsx:~124 — hint text has no id and is never wired into aria-describedby <Field id="name" label="Name" hint="Must be unique"><Input id="name"/></Field> renders the hint paragraph with no id, and the aria-describedby injected onto the Input only ever comes from errorId (undefined here) or the child's pre-existing aria-describedby — never the hint. A screen reader user tabbing into the input gets no indication the hint exists, defeating the component's stated purpose.

4. [Low] field.tsx:~112 — labelProps spreads after the auto-derived htmlFor, silently overriding it <Field id="email" label="Email" labelProps={{ htmlFor: "wrong-id" }}><Input id="email"/></Field> produces <label for="wrong-id"> that no longer targets the actual input, breaking click-to-focus and the screen-reader label association, with no warning.

[gcgoncalves]

@marekdano Addressed.

[marekdano]

🔴 src/index.css:256 — line-height fix likely doesn't apply in production

The new line-height: 1 rule for [data-slot="label"] is wrapped in @layer components, but Tailwind's .text-sm utility lives in @layer utilities, declared later. Under CSS Cascade Layers, a rule in a later layer beats a rule in an earlier layer for the same property on the same element, regardless of selector specificity — so .text-sm's line-height (≈1.4286) still wins over this new rule. Since Label's CVA base class always includes text-sm, every rendered <Label> keeps the old line-height — this is the exact bug the PR claims to fix, and it still exists.

Verified against an actual vite build of this branch — the compiled CSS shows:

@layer components{[data-slot=label]{line-height:1}}
@layer utilities{...}.text-sm{font-size:var(--text-sm);line-height:var(--tw-leading,var(--text-sm--line-height))}

Vitest/jsdom tests pass because jsdom doesn't apply real cascade-layer semantics, so this isn't caught by CI.

The fix: declare the rule unlayered instead of inside @layer components, matching the pattern already documented a few lines above it in src/index.css ("Unlayered on purpose: Tailwind emits .overflow-y-auto into @layer utilities, which these rules have to outrank at equal specificity").

---

🟡 src/components/ui/field.tsx:28 — dual child API adds unnecessary surface (minor)

Field supports both a cloned single ReactElement child and a render-prop function. The render-prop form alone already covers every call site (including plain Input/Textarea), so the cloneElement branch is extra surface to maintain — it has subtly different prop-merging behavior (id always overrides, but aria-invalid/aria-describedby only fall back to the child's own value) and requires two Record<string, unknown> casts.

Suggest dropping the cloneElement branch and always using the render-prop form ({(p) => <Input {...p} />}) — removes ~10 lines and one path callers could pick incorrectly.

[marekdano]

The a11y and CSS-layering issues from the last round are properly fixed — Field now uses the render-prop API exclusively (no more cloneElement cross-contamination), labelProps spreads before htmlFor so it can't hijack the label association, hint text gets an id wired into aria-describedby, and the [data-slot="label"] rule is unlayered so it actually beats .text-sm in production. Thanks for addressing those.

One regression turned up in the fix for the w-fit → w-full default, plus a few smaller items.

🔴 src/components/layout/HeaderProfileMenu.tsx:87 — language picker breaks under the new w-full default

SelectTrigger's base class now starts with w-full (was w-fit). Every other call site in the repo already passes an explicit width class, but this one doesn't:

<SelectTrigger
  size="sm"
  aria-label={intl.formatMessage({ id: "common.language" })}
  className="h-auto gap-1.5 border-0 bg-transparent px-2 py-1 text-xs font-medium text-secondary-foreground shadow-none"
>

It sits in a flex items-center justify-between row next to the "Language" label and relied on shrinking to its content. With the new default it now stretches to fill the row — the compact pill styling breaks. Needs w-fit (or similar) added explicitly at this call site.

🟡 src/components/server-catalog/CatalogApiKeyDialog.tsx:183 — stale comment, now false

{/* SelectTrigger is w-fit by default; full width lines it up with the inputs above. */}
<SelectTrigger id="catalog-server-visibility" className="w-full">

The default is w-full now, so this comment is backwards. Either delete it or update it - as written it'll mislead the next person into thinking w-full is load-bearing here when it's redundant.

[marekdano]

Code quality / correctness

🔴 Confirmed regression: global leading-none CSS rule clobbers an existing label's intentional leading-relaxed

src/index.css:270

[data-slot="label"] { line-height: 1; }

This rule is deliberately placed unlayered (outside any @layer) so it can outrank .text-sm's line-height, which lives in Tailwind's @layer utilities. But per the CSS cascade-layers spec, an unlayered rule beats every layered rule regardless of specificity — not just the one collision this was meant to fix.

src/components/gateways/ExposeComponentsForm.tsx:618 (untouched by this PR, already on main) passes className="text-sm font-normal leading-relaxed ..." to Label for a long, wrapping OAuth checkbox description. That leading-relaxed is now silently forced to line-height: 1, cramping a multi-line label that used to breathe. Worse: no className can ever override this again — the fix closed off line-height customization for every Label in the app, not just the one bug it targeted.

Suggestion: a narrower selector, or scoping the override to where the original bug actually occurred, rather than a blanket unlayered rule.

🟡 Latent, not yet triggered: CardTag has the identical bug this PR just fixed in Label

src/components/ui/card-tag.tsx:8 — leading-none sits in the same CVA base string alongside implicit text-sizing, with an open className prop. No current call site passes a text-size override to CardTag, so it's not an active bug, but it's the exact root cause this PR diagnosed and fixed elsewhere. Worth a follow-up if CardTag ever grows a caller that needs a different text size.

🟡 Field treats error="" the same as "no error"

src/components/ui/field.tsx:32 uses bare truthiness (error ? ... : undefined, {error && (...)}). If a caller ever passes an empty-string error (e.g. a validation message that hasn't resolved yet, or a schema check with no message), Field silently drops aria-invalid, aria-describedby, and the visible error — the control renders as valid when the caller intended otherwise. Given this component exists specifically to make invalid-state wiring foolproof, an explicit error !== undefined check (or typing error as non-empty) would close this gap the same way the rest of the PR closes others.

Test coverage

• field.test.tsx, label.test.tsx, select.test.tsx are thorough for what they cover: htmlFor linkage, aria-invalid/describedby wiring, error-vs-hint precedence, the render-prop composite-control case, the CVA height-variant override, and the w-full default.
• Gap: nothing catches the CSS cascade-layer regression above — that's a cross-component interaction (index.css change vs. an unrelated component's className) that unit tests targeting Label in isolation can't see. A visual regression test, or at minimum a repo-wide check for leading-* passed to Label, would have caught it.
• Gap: no test for the error="" edge case on Field.
• Independently verified every SelectTrigger call site in the repo for width regressions from the new w-full default — all other call sites already set w-full explicitly, so no additional breakage there. Good catch by the PR author on HeaderProfileMenu.

Accessibility

• Field's aria wiring (htmlFor, aria-invalid, aria-describedby chaining to error-over-hint, role="alert" on errors, labelProps spread order preventing htmlFor hijacking) is well-designed and well-tested.
• The leading-none regression above is itself an a11y regression, not just visual: compressing a wrapped, multi-line label to line-height: 1 hurts readability (WCAG 1.4.8 recommends ≥1.5 for body-like text), and it currently affects a real OAuth consent description in ExposeComponentsForm.
• The error="" gap above is also an a11y gap: a screen reader user would get no indication the field is invalid if a caller passes an empty-string error.

[gcgoncalves]

🟡 Latent, not yet triggered: CardTag has the identical bug this PR just fixed in Label

src/components/ui/card-tag.tsx:8 — leading-none sits in the same CVA base string alongside implicit text-sizing, with an open className prop. No current call site passes a text-size override to CardTag, so it's not an active bug, but it's the exact root cause this PR diagnosed and fixed elsewhere. Worth a follow-up if CardTag ever grows a caller that needs a different text size.

Separate issue, unrelated to the scope of this task.

🟡 Field treats error="" the same as "no error"

src/components/ui/field.tsx:32 uses bare truthiness (error ? ... : undefined, {error && (...)}). If a caller ever passes an empty-string error (e.g. a validation message that hasn't resolved yet, or a schema check with no message), Field silently drops aria-invalid, aria-describedby, and the visible error — the control renders as valid when the caller intended otherwise. Given this component exists specifically to make invalid-state wiring foolproof, an explicit error !== undefined check (or typing error as non-empty) would close this gap the same way the rest of the PR closes others.

NIT/Preference. IMHO, An empty error message is not helpful and the aria attrubutes should indeed be hidden. If the message gets loaded, then we display it.

I'm addressing the remaining items (🔴 and test/a11y gaps).

[marekdano]

The PR looks good now. Thanks for addressing all issue!

LGTM 🚀