From 564834224862d8d8391ba76b7bc6330272cf1152 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 3 Aug 2026 10:58:20 +0000 Subject: [PATCH] fix(fields,components): deliver the validation slot to field widgets as `error` (#3222) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `@objectstack/spec/ui`'s `FieldWidgetPropsSchema` declares `error?: string`; `@object-ui/fields` declared `errorMessage?: string`. That looked like a naming split, but the slot was dead under BOTH spellings: nothing in packages/ or apps/ ever produced it, so the seven widgets computing `aria-invalid={!!errorMessage}` computed it from a permanent `undefined`. `aria-invalid` had never once been set on a failing field — and because a widget writes its own attribute AFTER the props spread, those seven actively OVERWROTE the correct `aria-invalid` that ``'s Radix Slot hands down. Two halves, both required: - the form renderer now passes `fieldState.error?.message` down as `error` when it renders a registered widget (the producer that never existed); - the slot adopts the spec's name across `FieldWidgetComponentProps` and the 7 widgets, with no alias kept. #3221 had already closed the type, so the compiler — not grep — validated the rename. Responsibilities stay split: the widget consumes `error` only to drive `aria-invalid` on the control it renders; the message text remains with ``. `required` is deliberately NOT lowered into widget props — the required marker has exactly one author (``), and the a11y state a widget could carry is `aria-required`, which needs no contract change. Builtin field types strip `error` instead: they render inside ``, whose Slot already supplies `aria-invalid`, so the prop would only reach the DOM as a stray attribute. Docs synced (plugin-development guide + skill, component.prompt.md — which also used the spec's non-generic alias as a generic and destructured a `mode` prop that exists on neither type). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NVPjPzmmAJ2Ngtvgg5MSRa --- .../field-widget-error-slot-follows-spec.md | 86 ++++++++ .github/prompts/component.prompt.md | 52 +++-- content/docs/guide/plugin-development.md | 62 ++++-- .../__tests__/form-error-delivery.test.tsx | 203 ++++++++++++++++++ .../components/src/renderers/form/form.tsx | 32 ++- .../src/__tests__/spec-symbol-batch7.test.ts | 44 ++-- .../__tests__/validation-feedback.test.tsx | 35 +-- .../widget-aria-invalid-e2e.test.tsx | 188 ++++++++++++++++ .../__tests__/widget-props-contract.test.tsx | 17 +- packages/fields/src/widgets/CurrencyField.tsx | 4 +- packages/fields/src/widgets/EmailField.tsx | 4 +- packages/fields/src/widgets/PercentField.tsx | 4 +- packages/fields/src/widgets/PhoneField.tsx | 4 +- packages/fields/src/widgets/RichTextField.tsx | 4 +- packages/fields/src/widgets/TextAreaField.tsx | 4 +- packages/fields/src/widgets/UrlField.tsx | 4 +- packages/fields/src/widgets/types.ts | 45 +++- skills/objectui/guides/plugin-development.md | 42 ++-- 18 files changed, 723 insertions(+), 111 deletions(-) create mode 100644 .changeset/field-widget-error-slot-follows-spec.md create mode 100644 packages/components/src/renderers/form/__tests__/form-error-delivery.test.tsx create mode 100644 packages/fields/src/__tests__/widget-aria-invalid-e2e.test.tsx diff --git a/.changeset/field-widget-error-slot-follows-spec.md b/.changeset/field-widget-error-slot-follows-spec.md new file mode 100644 index 0000000000..0dc5c00553 --- /dev/null +++ b/.changeset/field-widget-error-slot-follows-spec.md @@ -0,0 +1,86 @@ +--- +"@object-ui/fields": minor +"@object-ui/components": minor +--- + +Field widgets are finally told when their field fails validation, and the props +slot that carries it takes the name the published contract gives it +(objectui#3222). + +**Breaking** for anyone implementing a field widget (see migration below). The +repo version policy keeps this a `minor` — objectui's major tracks +`@objectstack`'s — so read the bump as "breaking within objectui". + +## The a11y defect this fixes + +`@objectstack/spec/ui`'s `FieldWidgetPropsSchema` — the published contract that +third-party and AI-authored field widgets are written against — has always +declared `error?: string`. `@object-ui/fields` declared its own slot as +`errorMessage`. That looked like a naming split; it was worse: + +``` +producers of `errorMessage` anywhere in packages/ + apps/ : 0 +reads of `errorMessage` in packages/fields/src : 15 (7 widgets) +reads of `props.error` : 0 +``` + +The slot was dead under BOTH spellings. No host ever passed it: the form +renderer showed validation text through its own `` and never +forwarded the prop. So `EmailField`, `CurrencyField`, `UrlField`, +`RichTextField`, `PercentField`, `TextAreaField` and `PhoneField` each computed +`aria-invalid={!!errorMessage}` from a value that was `undefined` forever — +**`aria-invalid` had never once been set, and a screen reader was never told +the field had failed validation.** + +Worse than "never set": `` is a Radix `Slot` that hands its child a +CORRECT `aria-invalid`, but a widget's own attribute is written after the props +spread, so it wins. Those seven widgets were actively overwriting the right +answer with `false`. + +FROM: `renderFieldComponent` received no validation state, and the widget props +type declared `errorMessage?: string`, which nothing produced. +TO: the form renderer passes react-hook-form's `fieldState.error?.message` down +as `error` when it renders a registered widget, and the props type declares +`error?: string`. Both ends of the contract are live for the first time; a +rename alone would only have swapped one dead key for another. + +## Migration for widget authors + +```diff +-export function MyField({ value, onChange, field, readonly, errorMessage }: FieldWidgetComponentProps< string >) { +- return ; ++export function MyField({ value, onChange, field, readonly, error }: FieldWidgetComponentProps< string >) { ++ return ; +``` + +No alias is kept. `errorMessage` was retained nowhere on purpose — a tolerant +second spelling is exactly the de-facto second contract AGENTS.md #0.1 forbids, +and it is what would let a missed call site go quiet again. Because +objectui#3221 had already removed the type's `[key: string]: any`, every missed +site is a compile error rather than a silent `any`, so the compiler — not grep +— validated this rename. + +## Responsibilities are split, not duplicated + +The widget consumes `error` **only** to drive `aria-invalid` on the control it +renders (which only it can do — `aria-invalid` has to sit on the input element). +The message TEXT stays with `` in the form renderer. A widget that +also renders the text double-displays it, and the docs, the agent prompt and the +tests all now say so. + +For the same reason `required` — also declared by the spec, also never delivered +— is deliberately NOT lowered into widget props: the required marker has exactly +one author, the renderer's ``, and giving widgets the flag invites a +second asterisk. The a11y state a widget could legitimately carry is +`aria-required`, which needs no contract change at all (`AriaAttributes` is +already part of the type and widgets already forward it). + +Builtin field types are unaffected: they render inside ``, whose +Slot already supplies `aria-invalid`, so `error` is stripped there rather than +leaking into the DOM as a stray attribute. + +Docs updated to match: `content/docs/guide/plugin-development.md`, +`skills/objectui/guides/plugin-development.md` and +`.github/prompts/component.prompt.md` — the last of which additionally used the +spec's non-generic type alias as a generic (`FieldWidgetProps< number >`) and +destructured a `mode` prop that exists on neither type. diff --git a/.github/prompts/component.prompt.md b/.github/prompts/component.prompt.md index b7307fb965..f4de037391 100644 --- a/.github/prompts/component.prompt.md +++ b/.github/prompts/component.prompt.md @@ -16,7 +16,7 @@ 2. **Fields (@object-ui/fields):** * Standard Input/Display widgets (Text, Number, Date, Select). - * Must implement `FieldWidgetProps`. + * Must implement `FieldWidgetComponentProps` (the package's own generic React interface — not the spec's non-generic `FieldWidgetProps` alias; see §2.A). 3. **Layouts & Patterns (@object-ui/layout):** * Page structures (Sidebar, Header, AppLauncher). @@ -35,15 +35,22 @@ You will be asked to build components in these 3 standard slots. Refer to `packa ### A. Field Widgets (`field:*`) Responsible for **Input** (Edit Mode) and **Display** (Read Mode) of a specific data type. -* **Contract:** Must implement `FieldWidgetProps` (Ref: `src/ui/widget.zod.ts`). +* **Contract:** Two layers with the same shape and DIFFERENT names — do not mix them up. + * `FieldWidgetProps` (`@objectstack/spec/ui`, Ref: `src/ui/widget.zod.ts`) is the **declared** contract: a `z.infer` of `FieldWidgetPropsSchema`, so it is a plain **non-generic** type alias. Read it to learn what a widget receives. + * `FieldWidgetComponentProps` (`@object-ui/fields`) is the **implemented** React interface, and the one you actually import and parameterize when writing a widget in this repo. ```typescript - type FieldWidgetProps = { + import type { FieldWidgetComponentProps } from '@object-ui/fields'; + + type FieldWidgetComponentProps = { value: T; onChange: (val: T) => void; - field: FieldSchema; // Config + field: FieldMetadata; // Config readonly?: boolean; + disabled?: boolean; + error?: string; // active validation message — see below } ``` + The type is **closed**: a key it does not declare is a compile error, not a silent `any`. * **Required Types (Ref: `src/data/field.zod.ts`):** * **Textual:** `text` (Input), `textarea` (Multi-line), `password`, `email`, `url`, `phone`. * **Rich Content:** `markdown` (Editor), `html` (WYSIWYG), `code` (Monaco/Ace). @@ -168,24 +175,35 @@ Conversational and Generative UI components. ## 2. API Reference & Contracts ### A. Field Widget Implementation -**Reference:** `@objectstack/spec` -> `dist/ui/widget.zod.d.ts` +**Reference:** `packages/fields/src/widgets/types.ts` (implemented props), `@objectstack/spec` -> `dist/ui/widget.zod.d.ts` (declared contract) + +Import the **generic** `FieldWidgetComponentProps` from `@object-ui/fields`. +The spec's `FieldWidgetProps` is a non-generic alias — writing +`FieldWidgetProps< number >` does not compile. There is no `mode` prop on +either type; read-mode is `readonly`. ```typescript -import { FieldWidgetProps } from '@objectstack/spec/ui'; - -export function RatingField({ - value, - onChange, - field, - mode -}: FieldWidgetProps) { - - if (mode === 'read') { +import type { FieldWidgetComponentProps } from '@object-ui/fields'; + +export function RatingField({ + value, + onChange, + field, + readonly, + error, +}: FieldWidgetComponentProps) { + + if (readonly) { return {'★'.repeat(value || 0)}; } return ( -
+ // `error` is the ACTIVE VALIDATION MESSAGE, supplied by the form renderer. + // Consume it as a boolean signal for a11y and nothing more: the message + // text is rendered by `` and the required marker by + // ``, both in the form renderer. A widget that also prints the + // text double-displays it. +
{[1, 2, 3, 4, 5].map((star) => (
); diff --git a/packages/fields/src/widgets/TextAreaField.tsx b/packages/fields/src/widgets/TextAreaField.tsx index 18d2a90f33..ec82ff60cd 100644 --- a/packages/fields/src/widgets/TextAreaField.tsx +++ b/packages/fields/src/widgets/TextAreaField.tsx @@ -37,7 +37,7 @@ import { FieldWidgetComponentProps } from './types'; * override is ever genuinely needed, declare ONE key on * `FieldWidgetComponentProps`, stop stripping it, and have a host pass it. */ -export function TextAreaField({ value, onChange, field, readonly, errorMessage, ...props }: FieldWidgetComponentProps) { +export function TextAreaField({ value, onChange, field, readonly, error, ...props }: FieldWidgetComponentProps) { // Hooks must run before any early return (readonly) to keep hook order stable. const [fullscreenOpen, setFullscreenOpen] = useState(false); const [draft, setDraft] = useState(value ?? ''); @@ -78,7 +78,7 @@ export function TextAreaField({ value, onChange, field, readonly, errorMessage, disabled={readonly || domProps.disabled} rows={rows} maxLength={maxLength} - aria-invalid={!!errorMessage} + aria-invalid={!!error} className={domProps.className} /> {showFullscreenButton && ( diff --git a/packages/fields/src/widgets/UrlField.tsx b/packages/fields/src/widgets/UrlField.tsx index 9ed89d2436..85f2e9cebf 100644 --- a/packages/fields/src/widgets/UrlField.tsx +++ b/packages/fields/src/widgets/UrlField.tsx @@ -6,7 +6,7 @@ import { FieldWidgetComponentProps } from './types'; * UrlField - URL input with clickable link in readonly mode * Validates URLs to only render http/https links for security */ -export function UrlField({ value, onChange, field, readonly, errorMessage, ...props }: FieldWidgetComponentProps) { +export function UrlField({ value, onChange, field, readonly, error, ...props }: FieldWidgetComponentProps) { const config = field || (props as any).schema; if (readonly) { if (!value) return ; @@ -40,7 +40,7 @@ export function UrlField({ value, onChange, field, readonly, errorMessage, ...pr onChange={(e) => onChange(e.target.value)} placeholder={config?.placeholder || 'https://example.com'} disabled={readonly || domProps.disabled} - aria-invalid={!!errorMessage} + aria-invalid={!!error} /> ); } diff --git a/packages/fields/src/widgets/types.ts b/packages/fields/src/widgets/types.ts index f427e84528..233debb84c 100644 --- a/packages/fields/src/widgets/types.ts +++ b/packages/fields/src/widgets/types.ts @@ -14,13 +14,27 @@ import type { DependsOnInput, FieldMetadata } from '@object-ui/types'; * the widgets in this directory actually implement, against * `@object-ui/types`'s much richer `FieldMetadata`. * - * The two are NOT interchangeable, and the divergence is not cosmetic: the - * spec's contract names the error slot `error`, this one names it - * `errorMessage`, so a widget written to the spec's declared props renders no - * validation message here. That divergence is tracked in objectui#3222 and is - * deliberately NOT resolved here — but it is now *visible*: reading - * `props.error` or `props.required` off this type is a compile error rather - * than a silent `any`. + * The two are NOT interchangeable, but they no longer disagree on the + * validation slot: this type used to call it `errorMessage` while the spec + * calls it `error`, so a widget written to the published contract read + * `undefined` forever. objectui#3222 resolved that in the direction the + * contract points — the slot below IS the spec's `error`, and the form + * renderer now produces it (see `error`'s own doc comment). Reading + * `props.required` off this type is still a compile error rather than a + * silent `any`; that key is deliberately NOT lowered here (see below). + * + * ## Why `required` is still absent, though the spec declares it + * + * The required MARKER is drawn exactly once, by the form renderer's + * `` (the `*` with `aria-label="required"`, which the label + * association folds into the control's accessible name). Handing `required` + * to widgets would give that marker a second possible author, and the very + * next AI-written widget draws its own asterisk — the same double-display + * failure that keeps the validation TEXT out of the widget below. The a11y + * state a widget genuinely could carry is `aria-required` on the input, and + * that needs no new key at all: `AriaAttributes` is already part of this type + * and every widget already forwards it to its control. Tracked separately; + * do not add `required` here to "align" without that decision. * * ## Why there is no `[key: string]: any` (objectui#3221) * @@ -55,7 +69,22 @@ export type FieldWidgetComponentProps = { readonly?: boolean; disabled?: boolean; className?: string; - errorMessage?: string; + /** + * The active validation message for this field, named as + * `@objectstack/spec/ui`'s `FieldWidgetPropsSchema` names it (objectui#3222). + * + * **Producer**: the form renderer, from react-hook-form's + * `fieldState.error?.message` (`packages/components/src/renderers/form/ + * form.tsx`). Before #3222 nothing in the repo produced it under EITHER + * spelling, so the seven widgets computing `aria-invalid={!!errorMessage}` + * were computing it from a permanent `undefined` — `aria-invalid` was never + * once set and a screen reader was never told the field had failed. + * + * **Consumer**: a widget reads this ONLY to drive `aria-invalid` on the + * control it renders. The message TEXT stays with the form renderer's + * ``; a widget that also renders it double-displays it. + */ + error?: string; /** * Upload widgets (`file`/`image`) fire this when their in-progress state * flips, so a host can block submit until a presigned upload settles. Other diff --git a/skills/objectui/guides/plugin-development.md b/skills/objectui/guides/plugin-development.md index e7136ebb44..d2857854b1 100644 --- a/skills/objectui/guides/plugin-development.md +++ b/skills/objectui/guides/plugin-development.md @@ -230,20 +230,20 @@ export function ColorField({ readonly, disabled, className, - errorMessage, + error, }: FieldWidgetComponentProps) { return ( -
- onChange(e.target.value)} - disabled={readonly || disabled} - /> - {errorMessage && ( -

{errorMessage}

- )} -
+ onChange(e.target.value)} + disabled={readonly || disabled} + // `error` drives the a11y state ONLY. The form renderer already prints + // the message below the control via ``; printing it here + // too shows the user the same sentence twice. + aria-invalid={!!error} + /> ); } ``` @@ -258,11 +258,25 @@ type FieldWidgetComponentProps = { readonly?: boolean; // Read-only mode disabled?: boolean; // HTML disabled state className?: string; // Tailwind CSS classes - errorMessage?: string; // Validation error - [key: string]: any; // Additional forwarded props + error?: string; // Active validation message — drive `aria-invalid` with it }; ``` +The slot is named `error` because that is what `FieldWidgetPropsSchema` in +`@objectstack/spec/ui` — the published widget contract — calls it. + +The type is **closed**: it also declares the host plumbing and DOM/ARIA +pass-through keys the renderer forwards (`schema`, `dataSource`, +`dependentValues`, `dependsOn`, `emptyHint`, `compact`, `id`, `name`, +`aria-*`, `data-*`), and nothing else. A key it does not declare is a compile +error rather than a silent `any`, so a typo like `readOnly` for `readonly` is +caught at build time instead of being quietly `undefined` at runtime. + +Two things are deliberately **not** props, because each has exactly one author +in the form renderer: the validation message TEXT (``) and the +required marker (``). Rendering either inside a widget +double-displays it. + ## Package configuration ### package.json essentials