From ea1cdcdac670d064652a1c7d6d4dcf78884e211e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 16:28:46 +0000 Subject: [PATCH 1/2] fix(fields): drive the grid widget's per-cell aria-invalid from form-level failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A required `grid` submitted while EMPTY failed at the form level, rendered its "is required" message, and marked nothing: every row was a ghost row, and ghost rows were skipped by the per-cell validity channel. A sighted user saw red text; a screen-reader user was told nothing at all. The container's deliberate strip of the host `aria-invalid` (objectui#4857) STANDS — a container is not a control. What was missing is the other half that strip implied: the host's failure now drives the per-cell channel the widget already owns. When the `error` slot is set on an empty grid, the ghost entry row stops being skipped and its required cells flag. The mark also moves off the `` and onto the cell's own control (Input, SelectTrigger, and LookupField via its published `error` slot). A `td` is not focusable, and assistive tech reads validity from the control, not the wrapper. Also tightens the registry sweep's DELIVERING assertion to require a FOCUSABLE carrier (objectui#5223), closing the hole where a wrapper mark satisfied a row-wide query while the control announced nothing. Measured first, per the ruling's sequencing caveat: all 37 previously-green rows already carry on a focusable element, so the tightening reds none of them. NOT_YET_DELIVERED is now empty (29 -> 20 -> 9 -> 1 -> 0) and a new case asserts it stays empty, making the header's long-standing "do not add to this list" prose mechanical. Part of #3318 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RV6yuVCxymHYE16PL9vQkE --- .../widget-aria-invalid-registry-e2e.test.tsx | 117 +++++++++++++++--- packages/fields/src/widgets/GridField.tsx | 85 +++++++++++-- 2 files changed, 178 insertions(+), 24 deletions(-) diff --git a/packages/fields/src/__tests__/widget-aria-invalid-registry-e2e.test.tsx b/packages/fields/src/__tests__/widget-aria-invalid-registry-e2e.test.tsx index 7ab9cbc69..bdd3496ad 100644 --- a/packages/fields/src/__tests__/widget-aria-invalid-registry-e2e.test.tsx +++ b/packages/fields/src/__tests__/widget-aria-invalid-registry-e2e.test.tsx @@ -25,6 +25,12 @@ * * ## The ratchet, and its two ledgers * + * The positive assertion requires the `aria-invalid="true"` carrier to be + * FOCUSABLE (objectui#5223). A mark on a wrapper satisfies a row-wide query + * while telling a keyboard or screen-reader user nothing, so without that + * requirement the cheapest way to clear a ledger row also left the defect + * alive and invisible to the next agent. + * * Widgets listed in {@link NOT_YET_DELIVERED} are known not to deliver the * attribute today. They are asserted in the OPPOSITE direction — the sweep * proves they still do NOT deliver — so fixing one turns its entry red and @@ -55,8 +61,10 @@ * 3. The two ledgers are asserted DISJOINT and to name only real types, so a * row cannot be double-booked or left pointing at a widget that is gone. * - * What survives in `NOT_YET_DELIVERED` is tracked as follow-up work; this file - * is the source of truth for what is left. + * `NOT_YET_DELIVERED` is now EMPTY (objectui#3318 closed the last row, `grid`), + * and a case below asserts it stays that way. This file remains the source of + * truth for the registry's state: 45 types, every one of them either delivering + * on a focusable control or carrying a measured NOT_APPLICABLE verdict. * * ## Discipline inherited from the #3291 sweep * @@ -186,16 +194,26 @@ const WIDGETS: Record> = { * accessibility gap tracked in objectui#3318, not an accepted state: the field * shows its red message while assistive tech is told nothing. * - * The objectui#3318 delivery batch cleared 20 of the original 29 entries with - * the objectui#3222/#3306 pattern, and the remainder pass cleared `slider` and - * reclassified seven (see {@link NOT_APPLICABLE}). One is left, and it is the - * one that needs a DESIGN rather than a spread: + * ## THE LEDGER IS EMPTY — 29 -> 20 -> 9 -> 1 -> 0 + * + * Every registered widget now either delivers `aria-invalid` on a focusable + * control or carries a measured {@link NOT_APPLICABLE} verdict. The history, + * so the zero is readable as an achievement rather than an oversight: * - * - `grid` — composite line-item editor with its own per-CELL `aria-invalid` - * for line validation; driving it from a FORM-level failure has no obvious - * target (which cell? the add-row button?). It is emphatically NOT a - * `NOT_APPLICABLE` candidate: its row DOES offer a focusable control, which - * is exactly what that ledger's guard measures. + * - objectui#3345 cleared 20 with the objectui#3222/#3306 spread pattern; + * - objectui#4933 cleared `slider` and reclassified seven (NOT_APPLICABLE); + * - objectui#3318's final pass cleared `grid`, the one that needed a DESIGN + * rather than a spread. It is a composite line-item editor that reports + * validity per CELL, so a FORM-level failure had no obvious target (which + * cell? the add-row button?). The adopted answer drives the widget's own + * per-cell channel from the host's `error` slot: a required grid that fails + * while EMPTY now flags the ghost entry row's required cells, marking each + * cell's own input. It was never a `NOT_APPLICABLE` candidate — its row DOES + * offer a focusable control, which is what that ledger's guard measures. + * + * The set stays here, empty, because it is the RATCHET and not a to-do list: + * emptiness is asserted below, so the next widget that cannot deliver has to + * face that choice in review instead of quietly acquiring a ledger row. * * Do NOT add to this list to make a new widget pass; fix the widget (the * objectui#3222/#3306 pattern: spread `toDomProps(props)` onto the real @@ -205,7 +223,7 @@ const WIDGETS: Record> = { * widget turns its own ledger row red until it is removed here. The ledger only * shrinks. */ -const NOT_YET_DELIVERED: ReadonlySet = new Set(['grid']); +const NOT_YET_DELIVERED: ReadonlySet = new Set([]); /** * THE SECOND LEDGER — widget types for which `aria-invalid` is not a delivery @@ -289,6 +307,34 @@ const OPTIONS = [ { label: 'Beta', value: 'beta' }, ]; +/** + * `grid` is the same shape of dependency as the option widgets above, and it + * was measured, not assumed (objectui#3318). A grid with no `columns` renders + * an empty table: MEASURED on `origin/main` at `bd977f86f`, its whole row after + * a real failure is + * + * ``` + * bare grid focusables=[button] inputs=0 + * grid with columns focusables=[input(Item), input(Qty), button] inputs=2 + * ``` + * + * — the lone focusable in the bare state is the auxiliary "Add line" BUTTON, + * whose action is not what is invalid and which objectui#4857 already refused + * to route field identity onto. So the bare state has no control to mark, and + * asserting delivery there would only ever be satisfiable by the one carrier + * this sweep forbids. + * + * Giving it columns renders the widget's REAL editing surface, which is what + * every realistic config is (the widget's own header: "Every realistic config + * is a composite: many cell inputs, each with its own `aria-label`"). This + * makes the assertion stricter, not looser — the sweep now has to find the + * mark on a genuine cell control. + */ +const GRID_COLUMNS = [ + { name: 'item', label: 'Item', required: true }, + { name: 'qty', label: 'Qty', type: 'number', required: true }, +]; + /** `null` is MISSING for the presence-check `required` (cloud#972). */ const MISSING_VALUE = null; @@ -299,6 +345,7 @@ function fieldConfig(type: string) { type: `field:${type}`, required: true, ...(OPTION_TYPES.has(type) ? { options: OPTIONS } : {}), + ...(type === 'grid' ? { columns: GRID_COLUMNS } : {}), }; } @@ -378,6 +425,23 @@ describe('every registered field widget announces a failed validation (objectui# expect(Object.keys(WIDGETS).sort()).toEqual([...FORM_FIELD_TYPES].sort()); }); + it('the NOT_YET_DELIVERED ledger is empty and stays empty', () => { + // The ratchet's terminal state, made mechanical (objectui#3318). The + // header has always said "do NOT add to this list to make a new widget + // pass", but that was prose: a new widget could acquire a ledger row and + // the sweep would happily assert it still fails, which reads in review as + // a considered decision rather than an unfixed gap. + // + // Reaching zero is what makes enforcing it possible, so it is enforced + // here. If you are reading this because your new widget turned it red: + // deliver `aria-invalid` on the widget's focusable control (the + // objectui#3222/#3306 pattern), or — if it genuinely renders no control — + // put it in NOT_APPLICABLE, whose guard will MEASURE that claim. Editing + // this assertion is the third road, and it is deliberately a visible, + // reviewable act rather than a quiet one. + expect([...NOT_YET_DELIVERED]).toEqual([]); + }); + it('both ledgers only name types that exist', () => { // A renamed/removed widget must not leave a stale ledger entry that would // silently assert against nothing. @@ -414,7 +478,7 @@ describe('every registered field widget announces a failed validation (objectui# }); it.each(DELIVERING)( - 'field:%s — carries aria-invalid inside its row after a real failure', + 'field:%s — carries aria-invalid on a FOCUSABLE control in its row after a real failure', async (type) => { renderForm(fieldConfig(type)); const row = await formRow(); @@ -426,10 +490,33 @@ describe('every registered field widget announces a failed validation (objectui# expect(row.querySelector('[aria-invalid="true"]')).toBeNull(); const after = await failValidation(); + const carriers = Array.from(after.querySelectorAll('[aria-invalid="true"]')); + expect( - after.querySelector('[aria-invalid="true"]'), + carriers.length, `field:${type} rendered its "is required" message but no element in its row carries aria-invalid="true" — assistive tech is never told the field failed`, - ).not.toBeNull(); + ).toBeGreaterThan(0); + + // THE WRAPPER-MARK HOLE, closed (objectui#5223). Until this half existed, + // the assertion above was a query over the WHOLE ROW, so a mark on a + // non-focusable wrapper — a `div`, a `td`, a text span — passed as + // "delivered" while the control the user actually edits still announced + // nothing. That is not a hypothetical: it is the cheapest way to turn a + // ledger row green, it reads clean in review, and nothing else in this + // file would have caught it (the wrapper check guards NOT_APPLICABLE rows + // only). `aria-invalid` is control-channel state; assistive tech reads it + // from the element a keyboard user can land on, so that is where it must + // sit. + const describeEl = (el: Element) => { + const role = el.getAttribute('role'); + return `${el.tagName.toLowerCase()}${role ? `[role=${role}]` : ''}`; + }; + expect( + carriers.filter((el) => el.matches(FOCUSABLE)).map(describeEl), + `field:${type} carries aria-invalid ONLY on non-focusable element(s) [${carriers + .map(describeEl) + .join(', ')}] — a wrapper mark (objectui#5223). It satisfies a row-wide query while the control the user edits announces nothing. Put the state on the focusable control itself.`, + ).not.toEqual([]); }, ); diff --git a/packages/fields/src/widgets/GridField.tsx b/packages/fields/src/widgets/GridField.tsx index d6ad68d6e..7f36aebe2 100644 --- a/packages/fields/src/widgets/GridField.tsx +++ b/packages/fields/src/widgets/GridField.tsx @@ -55,6 +55,12 @@ import { toHostGroupProps } from './toHostGroupProps.js'; * `aria-invalid` (control-channel state; this grid reports validity per CELL, * with its own inline marks), answering `role="group"` only when a host * actually named it — `CheckboxesField`'s split, key for key. + * + * That strip STANDS (objectui#3318 upheld it rather than overturning it): + * the container is not a control, so the host's state is not re-routed onto + * it. What #3318 added is the other half the strip implied but nobody had + * built — the host's failure now DRIVES the per-cell channel this comment + * already claimed as the reporting path. See `hostFailedEmpty`. * - readonly: the table replaces the inputs entirely, so that surface takes the * name AND the description via `toHostGroupProps` — `'instead-of-the-inputs'`. * @@ -416,6 +422,7 @@ export function GridField({ readonly, disabled, className, + error, onRowExpand, displayMode, onAdd, @@ -801,9 +808,42 @@ export function GridField({ const hasGhost = !isList && allowAdd && (maxRows == null || rows.length < maxRows); const displayRows: Row[] = hasGhost ? [...rows, blankRow()] : rows; + /** + * FORM-level failure driving the per-CELL channel (objectui#3318). + * + * This widget reports validity per cell, and the container deliberately does + * NOT take the host's `aria-invalid` (objectui#4857 — it is control-channel + * state and the container is not a control). That left one real gap: a + * REQUIRED grid submitted while still EMPTY fails at the form level, renders + * its "is required" message, and marked nothing — every row was a ghost, and + * ghosts were skipped, so assistive tech was told nothing at all. A sighted + * user saw the red message; a screen-reader user saw no field state. + * + * So when the host reports a failure on an empty grid, the ghost row — the + * entry line the user would actually type into to fix it — stops being + * skipped and its required cells flag like any other. Deliberately narrow: + * a POPULATED grid already marks its own empty required cells inline, so + * this changes nothing there, and an empty grid that has NOT failed stays + * unmarked (no premature alarm before validation runs). + */ + const hostFailedEmpty = !!error && rows.length === 0; + /** Cell content: read-only display (list mode / computed columns) or an - * editable borderless control (spreadsheet feel). */ - const renderCellInput = (c: GridColumn, colIdx: number, rowIdx: number, row: Row) => { + * editable borderless control (spreadsheet feel). + * + * `invalid` is the cell's validity, and it lands on the CONTROL this + * renders — the focusable element a keyboard user edits and the only one + * assistive tech reads a control state from (objectui#3318). The read-only + * branches below ignore it: list-mode and computed cells cannot be invalid + * (the caller's `invalid` is false for both), and a text span is exactly + * the wrapper the sweep forbids marking. */ + const renderCellInput = ( + c: GridColumn, + colIdx: number, + rowIdx: number, + row: Row, + invalid = false, + ) => { const val = row?.[c.name]; // A readonlyWhen-TRUE cell is locked: treat like the form-wide `disabled`. const locked = disabled || cellRules(c, row).readonly; @@ -842,6 +882,9 @@ export function GridField({ compact field={{ reference: c.reference, display_field: c.displayField, id_field: c.idField, multiple: c.multiple, options: c.options, placeholder: '—' } as any} disabled={locked} + // The published `error` slot, not a hand-rolled attribute: LookupField + // already puts `aria-invalid` on its own focusable trigger from it. + error={invalid ? `${c.label || c.name} is required` : undefined} /> ); } @@ -849,6 +892,10 @@ export function GridField({ // not a text input: chips for uploaded files + a compact picker button. if (c.type === 'file') { return ( + // Not marked by `invalid`: `FileCell`'s prop set is closed and its + // control is its own (objectui#3318 kept the scope to the controls this + // file renders directly). A required FILE column therefore still marks + // only the cell's visual ring — tracked, not silently accepted. setCellValue(rowIdx, c.name, v)} @@ -863,7 +910,11 @@ export function GridField({ if (c.type === 'select') { return ( onCellKeyDown(e, rowIdx, colIdx)} className={cn( 'h-8 rounded-none border-0 bg-transparent px-2 shadow-none focus-visible:ring-1 focus-visible:ring-ring/60', @@ -1023,15 +1075,30 @@ export function GridField({ )} {columns.map((c, colIdx) => { // Inline validation: a required, non-computed cell that's - // empty on a real (non-ghost) row flags red in place. The - // "required" verdict honors a column's `requiredWhen` CEL - // rule (B2), evaluated against the row + parent header. + // empty flags red in place. The "required" verdict honors + // a column's `requiredWhen` CEL rule (B2), evaluated + // against the row + parent header. + // + // The ghost row joins in only when the FORM said this + // grid failed while empty (objectui#3318) — see + // `hostFailedEmpty`. const required = cellRules(c, row).required; - const invalid = !isGhost && !isList && required && !c.computed && (row[c.name] == null || row[c.name] === ''); + const invalid = + (!isGhost || hostFailedEmpty) && + !isList && + required && + !c.computed && + (row[c.name] == null || row[c.name] === ''); return ( - {renderCellInput(c, colIdx, rowIdx, row)} + {renderCellInput(c, colIdx, rowIdx, row, invalid)} ); })} From 7023649b9c06902f348e920e4c09b870626f5ec5 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 16:54:02 +0000 Subject: [PATCH 2/2] chore(changeset): grid widget aria-invalid delivery (patch) Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RV6yuVCxymHYE16PL9vQkE --- .changeset/grid-widget-aria-invalid-3318.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 .changeset/grid-widget-aria-invalid-3318.md diff --git a/.changeset/grid-widget-aria-invalid-3318.md b/.changeset/grid-widget-aria-invalid-3318.md new file mode 100644 index 000000000..e644f0adf --- /dev/null +++ b/.changeset/grid-widget-aria-invalid-3318.md @@ -0,0 +1,17 @@ +--- +'@object-ui/fields': patch +--- + +Grid field widget: announce a form-level validation failure to assistive tech. + +A required `grid` submitted while still empty rendered its "is required" message +but marked nothing — every row was a ghost row, and ghost rows were skipped by +the widget's per-cell validity channel. A sighted user saw the red message; a +screen-reader user was told nothing at all. + +The host failure now drives the per-cell channel the widget already owns: when +the `error` slot is set on an empty grid, the ghost entry row's required cells +flag, and the mark sits on each cell's own control rather than on the `td` +wrapper (a `td` is not focusable, and assistive tech reads validity from the +control). Populated grids are unaffected — they already marked their own empty +required cells inline.