From e57b1cdfd4149747bdedbc285705835eb2026305 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 05:18:56 +0000 Subject: [PATCH] fix(app-shell,components,plugin-detail): deliver aria-required at the five renderer sites outside the object form (#3299) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five renderers computed `required`, painted a red asterisk, and put NO required state on the control — the same defect #3290/#3298 fixed in form.tsx, at the five sites its scope fence excluded. Four of them were worse than pre-fix form.tsx: the bare `*` had no `aria-hidden`, so the only "signal" was a literal asterisk folded into the accessible name ("Title asterisk"). Every site converges on the reference shape (EmbeddableForm.tsx:481,489): - control: `aria-required={required || undefined}` — absence, not "false", for optional fields; - asterisk: `aria-hidden="true"` — announced once, as a state, never as part of the name. Sites: - app-shell/ActionParamDialog: both branches (boolean row + default); the state rides the widget props and reaches the DOM through the fields' `toDomProps` whitelist, which forwards `aria-*` by prefix. - app-shell/CreateViewDialog: display label, machine name, and each type-specific required-field selector (statically required, so a literal `aria-required="true"`). - components/custom/action-param-dialog: all five typed branches; for select the state lands on the Radix trigger (the focusable combobox — the root renders no element). - components/custom/field: FieldContainer injects `aria-required` via the same Slot that already injects id/aria-describedby/aria-invalid — one line covers every consumer. Its asterisk is a CSS pseudo-element, which never enters the a11y tree, so no marker change is needed there. - plugin-detail/InlineCreateRelated: create-tab inputs. Deliberately NOT native `required` (#3290 ruling): each host runs its own validation; native required would arm the browser's constraint- validation bubble beside it. The SDUI controls that already set native `required` (renderers/form/{input,textarea,select,checkbox}.tsx, basic/text-input.tsx) are excluded per the issue — no second validator there, so their channel is already correct. Tests per site, with mutation verification on the two load-bearing deliveries (FieldContainer's Slot injection; the app-shell dialog's widget-chain delivery): attribute removed -> red, restored -> green. Fixes #3299 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01NVPjPzmmAJ2Ngtvgg5MSRa --- .changeset/aria-required-five-more-sites.md | 15 ++ .../ActionParamDialog.ariaRequired.test.tsx | 101 +++++++++++++ .../app-shell/src/views/ActionParamDialog.tsx | 23 ++- .../CreateViewDialog.ariaRequired.test.tsx | 96 +++++++++++++ .../app-shell/src/views/CreateViewDialog.tsx | 12 +- ...action-param-dialog-aria-required.test.tsx | 136 ++++++++++++++++++ .../field-container-aria-required.test.tsx | 106 ++++++++++++++ .../src/custom/action-param-dialog.tsx | 26 +++- packages/components/src/custom/field.tsx | 11 +- .../plugin-detail/src/InlineCreateRelated.tsx | 9 +- .../InlineCreateRelated.ariaRequired.test.tsx | 81 +++++++++++ 11 files changed, 603 insertions(+), 13 deletions(-) create mode 100644 .changeset/aria-required-five-more-sites.md create mode 100644 packages/app-shell/src/views/ActionParamDialog.ariaRequired.test.tsx create mode 100644 packages/app-shell/src/views/CreateViewDialog.ariaRequired.test.tsx create mode 100644 packages/components/src/__tests__/action-param-dialog-aria-required.test.tsx create mode 100644 packages/components/src/__tests__/field-container-aria-required.test.tsx create mode 100644 packages/plugin-detail/src/__tests__/InlineCreateRelated.ariaRequired.test.tsx diff --git a/.changeset/aria-required-five-more-sites.md b/.changeset/aria-required-five-more-sites.md new file mode 100644 index 0000000000..b3a3f72be7 --- /dev/null +++ b/.changeset/aria-required-five-more-sites.md @@ -0,0 +1,15 @@ +--- +'@object-ui/components': patch +'@object-ui/app-shell': patch +'@object-ui/plugin-detail': patch +--- + +Deliver the required state to the control in the five renderers outside the object form that still painted it as an asterisk only (objectui#3299 — the same defect #3290/#3298 fixed in `form.tsx`). + +Each site converges on the reference shape (`EmbeddableForm.tsx`): the control carries `aria-required={required || undefined}` and the asterisk is `aria-hidden="true"`, so assistive tech announces required once, as a state — instead of hearing a bare "asterisk" folded into the accessible name, or nothing at all. + +- `@object-ui/app-shell` — `ActionParamDialog` (both the boolean row and the default branch, delivered through the real field widgets' `toDomProps` whitelist) and `CreateViewDialog` (display label, machine name, and every type-specific required-field selector). +- `@object-ui/components` — the custom `ActionParamDialog` (all five typed branches, including the Radix select trigger) and `FieldContainer`, whose existing Slot injection (`id` / `aria-describedby` / `aria-invalid`) now also injects `aria-required`, covering every consumer in one place. +- `@object-ui/plugin-detail` — `InlineCreateRelated`'s create-tab inputs. + +Deliberately NOT the native `required` attribute (#3290 ruling): each of these hosts runs its own validation, and native `required` would arm the browser's constraint-validation bubble beside it. The SDUI controls that already use native `required` (`renderers/form/{input,textarea,select,checkbox}.tsx`, `basic/text-input.tsx`) are unchanged — they don't have a second validator, so their channel is already correct. diff --git a/packages/app-shell/src/views/ActionParamDialog.ariaRequired.test.tsx b/packages/app-shell/src/views/ActionParamDialog.ariaRequired.test.tsx new file mode 100644 index 0000000000..8d5144ce49 --- /dev/null +++ b/packages/app-shell/src/views/ActionParamDialog.ariaRequired.test.tsx @@ -0,0 +1,101 @@ +/** + * 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. + */ + +/** + * ActionParamDialog (app-shell) — required must reach the field WIDGET's + * control as a state, not sit in the label as a bare `*` (objectui#3299; same + * shape as #3290/#3298). + * + * This dialog routes params through the real `@object-ui/fields` widgets + * (ADR-0059), so the state travels host → widget props → `toDomProps` (whose + * whitelist forwards `aria-*` by prefix, objectui#3291) → the rendered + * control. These tests drive that FULL delivery chain with real widgets — a + * widget-side strip of `aria-*` would fail here, not just a host-side + * omission. + * + * Deliberately NOT native `required` (#3290 ruling): the dialog runs its own + * required validation (`requiredError` messages); native required would arm + * the browser's constraint-validation bubble beside it. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { render, screen, waitFor } from '@testing-library/react'; +import type { ActionParamDef } from '@object-ui/core'; +import { ActionParamDialog } from './ActionParamDialog'; + +/** Mount the dialog open with the given params (mirrors ActionParamDialog.test.tsx). */ +function openDialog(params: ActionParamDef[]) { + const resolve = vi.fn(); + render( + {}} + />, + ); + return resolve; +} + +const def = (over: Partial): ActionParamDef => ({ + name: 'p1', + label: 'Param One', + type: 'text', + ...over, +}); + +describe('app-shell ActionParamDialog — `aria-required` reaches the widget control (objectui#3299)', () => { + it('sets aria-required="true" on a required text param, delivered through the real widget', async () => { + openDialog([def({ name: 'note', type: 'text', required: true })]); + + const input = await screen.findByLabelText(/Param One/); + expect(input).toHaveAttribute('aria-required', 'true'); + }); + + it('omits the attribute entirely on an optional param, rather than writing "false"', async () => { + openDialog([def({ name: 'note', type: 'text' })]); + + const input = await screen.findByLabelText(/Param One/); + expect(input).not.toHaveAttribute('aria-required'); + }); + + it('sets aria-required="true" on the boolean branch too (checkbox row)', async () => { + // The boolean branch is a SEPARATE render path (inline checkbox row) — + // fixing only the default branch would leave this one silent. + openDialog([def({ name: 'force', type: 'boolean', required: true })]); + + const checkbox = await screen.findByRole('checkbox'); + expect(checkbox).toHaveAttribute('aria-required', 'true'); + }); + + it('keeps the asterisk OUT of the accessible name — state announced once, not "asterisk"', async () => { + // Pre-fix the bare `*` inside `