Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions apps/mapgen-studio/src/features/configOverrides/rjsfTemplates.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ const FORM = {
card: "bg-card border-border",
nested: "bg-muted/40 border-border-subtle",
divider: "border-border",
// Field labels sit a full tier above prose (Pass-2 form hierarchy): labels are
// foreground anchors the eye scans; descriptions/help/gs-comments recede on the
// muted tier. Same 11px size — the split is color/weight, not scale.
fieldLabel: "text-foreground",
label: "text-muted-foreground",
muted: "text-muted-foreground/70",
text: "text-foreground",
Expand Down Expand Up @@ -59,7 +63,7 @@ function renderGsComments(args: { schema: unknown; className: string }): ReactNo
export function BrowserConfigFieldTemplate(
props: FieldTemplateProps<unknown, RJSFSchema, BrowserConfigFormContext>
){
const { id, label, required, description, errors, help, children, hidden, classNames, displayLabel } = props;
const { id, label, required, description, errors, help, children, hidden, classNames, displayLabel, rawErrors } = props;
if (hidden) return <div style={{ display: "none" }} />;
const prettyLabel = label ? humanizeSchemaLabel(label) : "";
const schemaType = props.schema?.type;
Expand All @@ -74,15 +78,18 @@ export function BrowserConfigFieldTemplate(
// `role="alert"` live region, and the widget mirrors that id through
// `aria-describedby` + `aria-invalid` (see `rjsfWidgets.tsx`), so assistive tech
// announces validation against the control rather than as orphaned text.
// Gated on `rawErrors`: rjsf's `errors` prop is an always-truthy element, so
// rendering on it mounts an empty live region per field (~40 phantom alerts).
const errorId = `${id}__error`;
const hasErrors = (rawErrors?.length ?? 0) > 0;

if (!showLabel) {
return (
<div className={["flex flex-col gap-1", classNames].filter(Boolean).join(" ")}>
<div className={textClass}>{children}</div>
{description && !suppressDescription ? <div className={`text-data ${labelClass}`}>{description}</div> : null}
{renderGsComments({ schema: props.schema, className: labelClass })}
{errors ? <div id={errorId} role="alert" className="text-data text-destructive">{errors}</div> : null}
{hasErrors ? <div id={errorId} role="alert" className="text-data text-destructive">{errors}</div> : null}
{help ? <div className={`text-data ${mutedClass}`}>{help}</div> : null}
</div>
);
Expand All @@ -91,15 +98,15 @@ export function BrowserConfigFieldTemplate(
return (
<div className={["flex flex-col gap-1", classNames].filter(Boolean).join(" ")}>
<FieldRow>
<label className={`text-data min-w-[96px] ${labelClass}`} htmlFor={id}>
<label className={`text-data min-w-[96px] ${FORM.fieldLabel}`} htmlFor={id}>
<span className="font-medium">{prettyLabel}</span>
{required ? <span className="text-data text-destructive">*</span> : null}
</label>
<div className={`flex-1 min-w-[120px] ${textClass}`}>{children}</div>
</FieldRow>
{description && !suppressDescription ? <div className={`text-data ${labelClass}`}>{description}</div> : null}
{renderGsComments({ schema: props.schema, className: labelClass })}
{errors ? <div id={errorId} role="alert" className="text-data text-destructive">{errors}</div> : null}
{hasErrors ? <div id={errorId} role="alert" className="text-data text-destructive">{errors}</div> : null}
{help ? <div className={`text-data ${mutedClass}`}>{help}</div> : null}
</div>
);
Expand Down
58 changes: 58 additions & 0 deletions apps/mapgen-studio/test/config/rjsfFieldTemplateErrors.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import { renderToStaticMarkup } from "react-dom/server";
import { describe, expect, it } from "vitest";
import type { FieldTemplateProps, RJSFSchema } from "@rjsf/utils";

import {
BrowserConfigFieldTemplate,
type BrowserConfigFormContext,
} from "../../src/features/configOverrides/rjsfTemplates";

type TemplateProps = FieldTemplateProps<unknown, RJSFSchema, BrowserConfigFormContext>;

// rjsf's `errors` prop is a ReactElement (always truthy, even with no errors);
// the template must gate the `role="alert"` live region on `rawErrors` so a
// pristine form mounts zero phantom alert regions (Pass-2 form-hierarchy spec).
function renderField(overrides: Partial<TemplateProps>): string {
const base = {
id: "root_test_field",
label: "plateActivity",
required: false,
hidden: false,
displayLabel: true,
classNames: "",
description: <span>Plate activity scalar.</span>,
errors: <span />, // always-truthy element, like rjsf passes
help: null,
rawErrors: [] as string[],
schema: { type: "number" } as RJSFSchema,
children: <input id="root_test_field" />,
};
return renderToStaticMarkup(
<BrowserConfigFieldTemplate {...({ ...base, ...overrides } as unknown as TemplateProps)} />
);
}

describe("BrowserConfigFieldTemplate error live regions", () => {
it("mounts no alert region when the field has no raw errors", () => {
const html = renderField({ rawErrors: [] });
expect(html).not.toContain('role="alert"');
});

it("renders the associated alert region when raw errors exist", () => {
const html = renderField({
rawErrors: ["must be <= 1"],
errors: <span>must be &lt;= 1</span>,
});
expect(html).toContain('role="alert"');
expect(html).toContain('id="root_test_field__error"');
expect(html).toContain("must be &lt;= 1");
});

it("keeps the label on the foreground tier and the description muted", () => {
const html = renderField({ rawErrors: [] });
// Label anchor: foreground tier; description prose: muted tier.
expect(html).toMatch(/<label[^>]*class="[^"]*text-foreground/);
expect(html).toMatch(/Plate activity scalar/);
expect(html).toMatch(/text-muted-foreground/);
});
});
39 changes: 39 additions & 0 deletions openspec/changes/mapgen-studio-form-hierarchy/design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
## Context

Pass-2 form hierarchy: measured in the live app, rjsf labels and descriptions
were both 11px `text-muted-foreground` (identical color), and 40 empty
`role="alert"` regions were mounted on a pristine form.

## Decisions

### 1. Tier split is color/weight only

`FORM.fieldLabel = text-foreground` is a new entry beside the muted prose tier;
labels keep `text-data font-medium` (no size change). Density is untouched — the
fix is contrast hierarchy, per the Pass-2 amendment in
`.interface-design/system.md`.

### 2. Gate alerts on `rawErrors`, render `errors`

rjsf's `errors` prop is an always-truthy ReactElement; `rawErrors` is the actual
string list. The template now mounts the live region only when
`rawErrors.length > 0` and still renders the richer `errors` element inside it.
The `id="${id}__error"` association contract from `mapgen-studio-a11y-fixes` is
unchanged — widgets already gate `aria-describedby`/`aria-invalid` on
`rawErrors`, so the id appears exactly when the region exists.

### 3. "Hand-rolled field set" resolved as FieldRow-only (verified)

The proposal scoped a matching split for `src/ui/components/fields/styles.ts` —
that file no longer exists: the legacy field set was deleted in the theming
slices and only the layout-only `FieldRow` wrapper remains, which carries no
label styling. No second edit site exists; the spec's "hand-rolled fields match"
scenario is satisfied by the absence of any other label styler (eyebrow labels in
the chrome are a deliberate different pattern, not field labels).

### 4. Proof shape

The form has no live validation (no `liveValidate`, no submit button), so the
error path cannot be forced from the running UI. Proof is a unit test
(`test/config/rjsfFieldTemplateErrors.test.tsx`) rendering the template both
ways, plus a live DOM count of `[role="alert"]` (0 on pristine form, was 40).
12 changes: 6 additions & 6 deletions openspec/changes/mapgen-studio-form-hierarchy/tasks.md
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
## 1. rjsf templates

- [ ] 1.1 `rjsfTemplates.tsx`: split `FORM.label` usage — labels take a new
- [x] 1.1 `rjsfTemplates.tsx`: split `FORM.label` usage — labels take a new
foreground-tier class; descriptions/help/gs-comments keep the muted tier.
- [ ] 1.2 `rjsfTemplates.tsx`: gate the error region on `props.rawErrors?.length`
- [x] 1.2 `rjsfTemplates.tsx`: gate the error region on `props.rawErrors?.length`
(both the labeled and unlabeled template branches), preserving
`id="${id}__error"` + `role="alert"` when errors exist.

## 2. Hand-rolled fields

- [ ] 2.1 `src/ui/components/fields/styles.ts` (and any field component that styles
- [x] 2.1 `src/ui/components/fields/styles.ts` (and any field component that styles
labels directly): apply the same label/description tier split.

## 3. Verification

- [ ] 3.1 `bun run openspec -- validate mapgen-studio-form-hierarchy --strict`
- [ ] 3.2 tsc + mapgen-studio vitest project green
- [ ] 3.3 Visual on :5173 (dark + light): labels read as anchors; DOM query counts
- [x] 3.1 `bun run openspec -- validate mapgen-studio-form-hierarchy --strict`
- [x] 3.2 tsc + mapgen-studio vitest project green
- [x] 3.3 Visual on :5173 (dark + light): labels read as anchors; DOM query counts
zero empty `[role="alert"]`; force an invalid value and confirm the alert
region renders with the association contract intact.