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
17 changes: 17 additions & 0 deletions .changeset/grid-widget-aria-invalid-3318.md
Original file line numberDiff line numberDiff line change
@@ -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.
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand DownExpand Up@@ -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
*
Expand DownExpand Up@@ -186,16 +194,26 @@ const WIDGETS: Record<string, ComponentType<any>> = {
* 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
Expand All@@ -205,7 +223,7 @@ const WIDGETS: Record<string, ComponentType<any>> = {
* widget turns its own ledger row red until it is removed here. The ledger only
* shrinks.
*/
const NOT_YET_DELIVERED: ReadonlySet<string> = new Set(['grid']);
const NOT_YET_DELIVERED: ReadonlySet<string> = new Set<string>([]);

/**
* THE SECOND LEDGER — widget types for which `aria-invalid` is not a delivery
Expand DownExpand Up@@ -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;

Expand All@@ -299,6 +345,7 @@ function fieldConfig(type: string) {
type: `field:${type}`,
required: true,
...(OPTION_TYPES.has(type) ? { options: OPTIONS } : {}),
...(type === 'grid' ? { columns: GRID_COLUMNS } : {}),
};
}

Expand DownExpand Up@@ -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.
Expand DownExpand Up@@ -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();
Expand All@@ -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([]);
},
);

Expand Down
85 changes: 76 additions & 9 deletions packages/fields/src/widgets/GridField.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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'`.
*
Expand DownExpand Up@@ -416,6 +422,7 @@ export function GridField({
readonly,
disabled,
className,
error,
onRowExpand,
displayMode,
onAdd,
Expand DownExpand Up@@ -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;
Expand DownExpand Up@@ -842,13 +882,20 @@ 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}
/>
);
}
// File / image column → a real upload control in the cell (objectui#2360),
// 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.
<FileCell
value={val}
onChange={(v: any) => setCellValue(rowIdx, c.name, v)}
Expand All@@ -863,7 +910,11 @@ export function GridField({
if (c.type === 'select') {
return (
<Select value={val != null ? String(val) : ''} onValueChange={(v) => setCell(rowIdx, c, v)} disabled={locked}>
<SelectTrigger className="h-8 rounded-none border-0 bg-transparent px-2 shadow-none focus:ring-1 focus:ring-ring/60" aria-label={c.label || c.name}>
<SelectTrigger
className="h-8 rounded-none border-0 bg-transparent px-2 shadow-none focus:ring-1 focus:ring-ring/60"
aria-label={c.label || c.name}
aria-invalid={invalid || undefined}
>
<SelectValue placeholder="—" />
</SelectTrigger>
<SelectContent>
Expand All@@ -881,6 +932,7 @@ export function GridField({
)}
<Input
data-cell={`${rowIdx}-${colIdx}`}
aria-invalid={invalid || undefined}
onKeyDown={(e) => 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',
Expand DownExpand Up@@ -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 (
<td
key={c.name}
aria-invalid={invalid || undefined}
// NOTE: `aria-invalid` belongs on the cell's CONTROL,
// not here. A `td` is not focusable, and assistive
// tech reads a control's validity from the control —
// marking the wrapper is the move the registry sweep
// exists to forbid (objectui#3318 / #5223). The td
// keeps the VISUAL ring and the test hook; the state
// travels with `invalid` into `renderCellInput`.
title={invalid ? `${c.label || c.name} is required` : undefined}
data-testid={invalid ? `line-items-invalid-${rowIdx}-${c.name}` : undefined}
className={cn(
Expand All@@ -1040,7 +1107,7 @@ export function GridField({
invalid && 'bg-destructive/5 ring-1 ring-inset ring-destructive/50',
)}
>
{renderCellInput(c, colIdx, rowIdx, row)}
{renderCellInput(c, colIdx, rowIdx, row, invalid)}
</td>
);
})}
Expand Down
Loading