diff --git a/.changeset/7126-inline-widgets-read-error.md b/.changeset/7126-inline-widgets-read-error.md new file mode 100644 index 0000000000..2229bcffde --- /dev/null +++ b/.changeset/7126-inline-widgets-read-error.md @@ -0,0 +1,41 @@ +--- +'@object-ui/fields': patch +--- + +The last five inline edit widgets read the delivered `error` slot, so a failed +required `text` / `boolean` / `date` / `datetime` / `time` control finally +reports `aria-invalid` (objectui#7126). + +objectui#7008 made `FieldEditWidget` DELIVER the declared `error` key to +whichever widget it resolves. Of the 27 distinct components in `EDIT_WIDGETS`, +21 read it; five did not — `TextField`, `BooleanField` (serving both `boolean` +and `toggle`), `DateField`, `DateTimeField` and `TimeField` — so for their field +types the delivery was inert and the attribute was still never set. + +`text` being in that set is what made this a live defect rather than tidiness. +It is the most common field type in any object, so it is the likeliest thing a +kanban column makes required: `RequiredFieldsDialog` computed the failure, drew +the red "Required" hint, handed the state to the control, and the control said +nothing to assistive tech. The grid's inline cell editor and the detail page's +inline edit (`InlineFieldInput`) compose the same seam. + +Each of the five now computes `aria-invalid={!!error}` **after** its DOM +pass-through spread — one existing idiom, the objectui#3222 discipline the other +21 already share, so a valid field says an explicit `"false"` rather than staying +mute. Two judgements worth stating: + +- **The FORM path was never broken and is unchanged.** `` is a + Radix `Slot` whose `aria-invalid` reached each control through the props + spread; the form also produces `error`, so the widget's own computation now + agrees with the value it replaces. The gap was every host WITHOUT that Slot. +- **`BooleanField` is the one composite here, and the mark goes on the + control.** Its Radix `Checkbox` / `Switch` renders a real + `button[role=checkbox]` / `button[role=switch]`; the wrapping flex `div` is + deliberately not the target, because a wrapper mark satisfies a subtree query + while telling a screen-reader user nothing (objectui#5223). The three + date/time widgets each render one native input, so the browser's picker raises + no second-element question. + +This buys the MARKING only. The objectui#3222 slot drives `aria-invalid` and +renders no text: the visible message stays with the host, and nothing that was +invisible becomes visible. diff --git a/packages/fields/src/__tests__/inline-widget-aria-invalid-7126.test.tsx b/packages/fields/src/__tests__/inline-widget-aria-invalid-7126.test.tsx new file mode 100644 index 0000000000..e9c214a541 --- /dev/null +++ b/packages/fields/src/__tests__/inline-widget-aria-invalid-7126.test.tsx @@ -0,0 +1,193 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * The five widgets that never read the delivered `error` now report + * `aria-invalid` on a FOCUSABLE control, through the real inline seam + * (objectui#7126). + * + * ## The defect + * + * objectui#7008 made `FieldEditWidget` DELIVER the declared `error` key + * (`toHostProps`, landed as f08bcd9af). Of the 27 distinct components in + * `EDIT_WIDGETS`, 21 read it; five did not — `TextField`, `BooleanField` + * (`boolean` + `toggle`), `DateField`, `DateTimeField`, `TimeField` — so for + * their field types the delivery was inert and `aria-invalid` was still never + * set. `text` being in that set is what made it live rather than tidy: it is + * the most common type in any object, and the kanban `RequiredFieldsDialog` + * renders whatever types the target column made required. + * + * ## Why the FACTORY and not the widgets directly + * + * In the FORM these five were already announced correctly and always had been: + * `` is a Radix `Slot`, its `aria-invalid` reached the control + * through each widget's props spread untouched, and + * `widget-aria-invalid-registry-e2e.test.tsx` sweeps exactly that path with an + * EMPTY `NOT_YET_DELIVERED` ledger. So a form-based test would have been green + * before this change and proves nothing about it. + * + * `FieldEditWidget` renders no Slot. It is the seam every NON-form host + * composes — the grid's inline cell editor, the detail page's inline edit + * (`InlineFieldInput`), the kanban required-fields dialog — and the only way + * the state reaches the control there is the declared `error` prop. That is + * the path that was broken, so that is the path measured here. + * + * ## What is NOT claimed + * + * The marking only. objectui#3222's slot drives `aria-invalid` and renders no + * text; the visible message stays with the host. Nothing here becomes visible + * that was not visible before. + */ +import { describe, it, expect, afterEach } from 'vitest'; +import { render, cleanup } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import React from 'react'; + +import { FieldEditWidget } from '../FieldEditWidget'; + +afterEach(() => cleanup()); + +/** + * HTML's own focusability rules, as a selector — copied from + * `widget-aria-invalid-registry-e2e.test.tsx` on purpose, so both sweeps judge + * "the control a keyboard user can land on" by one definition. + * + * This is the objectui#5223 line: a mark on a non-focusable wrapper satisfies a + * subtree query while telling a screen-reader user nothing, and it is the + * cheapest way to make an assertion like the ones below go green without + * helping anyone. + */ +const FOCUSABLE = [ + 'a[href]', + 'button:not([disabled])', + 'input:not([disabled])', + 'select:not([disabled])', + 'textarea:not([disabled])', + '[tabindex]:not([tabindex="-1"])', + '[contenteditable="true"]', +].join(','); + +function describeEl(el: Element): string { + const role = el.getAttribute('role'); + const type = el.getAttribute('type'); + return `${el.tagName.toLowerCase()}${type ? `[type=${type}]` : ''}${role ? `[role=${role}]` : ''}`; +} + +function renderInline(field: Record, error?: string) { + const { container } = render( + {}} + error={error} + />, + ); + return container; +} + +/** + * The population of objectui#7126, by the field TYPE each widget serves inline + * — six types, five widgets (`boolean` and `toggle` both resolve to + * `BooleanField`), plus the two branch variants that a type key alone does not + * reach: `TextField`'s textarea branch (`rows > 1`) and `BooleanField`'s + * checkbox branch (`widget: 'checkbox'`). Both are real authored configs, and + * each renders a DIFFERENT element, so a fix applied to only one branch of + * either widget still fails here. + */ +const CASES: ReadonlyArray]> = [ + ['text', { name: 'f', type: 'text', label: 'F' }], + ['text (rows > 1 -> textarea branch)', { name: 'f', type: 'text', label: 'F', rows: 4 }], + ['boolean (switch branch)', { name: 'f', type: 'boolean', label: 'F' }], + ['boolean (widget: checkbox branch)', { name: 'f', type: 'boolean', label: 'F', widget: 'checkbox' }], + ['toggle', { name: 'f', type: 'toggle', label: 'F' }], + ['date', { name: 'f', type: 'date', label: 'F' }], + ['datetime', { name: 'f', type: 'datetime', label: 'F' }], + ['time', { name: 'f', type: 'time', label: 'F' }], +]; + +describe('inline field widgets announce a delivered `error` (objectui#7126)', () => { + it.each(CASES)( + '%s — carries aria-invalid="true" on a FOCUSABLE control when the host delivers `error`', + (_label, field) => { + const container = renderInline(field, 'Required'); + + const carriers = Array.from(container.querySelectorAll('[aria-invalid="true"]')); + expect( + carriers.map(describeEl), + 'the host delivered `error` and nothing in the rendered widget says so — assistive tech is never told the field failed', + ).not.toEqual([]); + + // THE WRAPPER-MARK HOLE (objectui#5223). `BooleanField` is the case this + // exists for: it renders its control inside a flex `div`, and marking + // that `div` would satisfy the query above while the switch the user + // actually operates announces nothing. + expect( + carriers.filter((el) => el.matches(FOCUSABLE)).map(describeEl), + `aria-invalid sits ONLY on non-focusable element(s) [${carriers.map(describeEl).join(', ')}] — that is a wrapper mark, not a control mark`, + ).not.toEqual([]); + }, + ); + + it.each(CASES)( + '%s — says an explicit aria-invalid="false" when the host delivers no `error`', + (_label, field) => { + // The load-bearing half, and the reason this is a two-state reading + // rather than "the attribute exists": `!!undefined` must yield `"false"`, + // so a valid field SAYS it is valid instead of staying mute (the + // objectui#3222 discipline). Without this, an unconditional + // `aria-invalid="true"` would pass the case above. + const container = renderInline(field); + + const control = container.querySelector(FOCUSABLE); + expect(control, 'no focusable control rendered at all').not.toBeNull(); + expect(control).toHaveAttribute('aria-invalid', 'false'); + expect(container.querySelector('[aria-invalid="true"]')).toBeNull(); + }, + ); + + it('CONTROL: a widget that ALREADY read `error` reports the same way through the same harness', () => { + // Without this, a green sweep above could not be distinguished from a + // harness that marks everything it renders. `select` -> `SelectField` was + // one of the 21 readers before this change (objectui#3306 / #7008's pin), + // so it must read `true`/`false` here for exactly the reasons the five now + // do — same factory, same delivery, same assertion. + const SELECT_FIELD = { + name: 'stage', + type: 'select', + label: 'Stage', + options: [{ label: 'New', value: 'new' }], + }; + + const invalid = renderInline(SELECT_FIELD, 'Required'); + const trigger = invalid.querySelector('[role="combobox"]')!; + expect(trigger.tagName).toBe('BUTTON'); + expect(trigger).toHaveAttribute('aria-invalid', 'true'); + + cleanup(); + + const valid = renderInline(SELECT_FIELD); + expect(valid.querySelector('[role="combobox"]')).toHaveAttribute('aria-invalid', 'false'); + }); + + it('CONTROL: `user` was a FALSE zero in the census and is NOT in the population', () => { + // The one trap in the measurement that produced this card. A word-boundary + // `error` grep over the 27 `EDIT_WIDGETS` components returns SIX zeroes, + // and `UserField` is one of them — but it renders `LookupField` with a + // props spread, so it delivers `error` transitively and has always marked. + // A naive census reports six and is wrong about one; this pins the sixth so + // the next reader does not "fix" a widget that was never broken (and so a + // future refactor that flattens the delegation cannot silently drop it). + const container = renderInline( + { name: 'owner_id', type: 'user', label: 'Owner', reference_to: 'sys_user' }, + 'Required', + ); + + const carriers = Array.from(container.querySelectorAll('[aria-invalid="true"]')); + expect(carriers.filter((el) => el.matches(FOCUSABLE)).map(describeEl)).not.toEqual([]); + }); +}); diff --git a/packages/fields/src/widgets/BooleanField.tsx b/packages/fields/src/widgets/BooleanField.tsx index b99f516701..c8325fff67 100644 --- a/packages/fields/src/widgets/BooleanField.tsx +++ b/packages/fields/src/widgets/BooleanField.tsx @@ -7,7 +7,7 @@ import { toDomProps } from './toDomProps.js'; * BooleanField - Toggle input supporting switch and checkbox variants * Renders as Switch or Checkbox based on field widget configuration */ -export function BooleanField({ value, onChange, field, readonly, ...props }: FieldWidgetComponentProps) { +export function BooleanField({ value, onChange, field, readonly, error, ...props }: FieldWidgetComponentProps) { const config = field as any; // Use simple type assertion for arbitrary custom properties not in BaseFieldMetadata const widget = config?.widget; @@ -56,6 +56,26 @@ export function BooleanField({ value, onChange, field, readonly, ...props }: Fie const domProps = toDomProps(props); + /** + * WHICH ELEMENT carries `aria-invalid`, since this widget is the one of the + * five in objectui#7126 that renders a composite: a control plus its + * `sr-only` label inside a flex `div`. + * + * It goes on the Radix `Checkbox` / `Switch` -- each renders a real + * `