Uh oh!
There was an error while loading. Please reload this page.
fix(components): announce element:text_input's description with the field - #5770
Merged
Merged
Conversation
…he field (#5735) The resolved `description` rendered as a bare sibling paragraph: no `id` on the paragraph, no `aria-describedby` on the `Input`, so the two had no programmatic relationship and a screen reader moving to the field announced the label and the value and never the helper text. The `label` half of the same block was already wired, and the identical key authored inside `renderers/form/form.tsx` has been announced all along — one authoring key, two behaviours, decided by container. The paragraph id is minted per instance with `React.useId()` (the source `FormItem` already mints the form renderer's description id from), not derived from `schema.id`: `htmlFor` needs an id on the INPUT, which only the author can supply, while `aria-describedby` needs one on the PARAGRAPH, which the renderer owns. So the description association holds whether or not the node carries an `id`, and cannot collide when two nodes share one. The attribute is emitted only when a paragraph is actually rendered, never as a dangling reference. The key's published `ComponentInput` description documented the gap; it is rewritten here. Its "prefer `label` for instructions a user must not miss" advice is kept on a new, cited basis — announcement order and AT verbosity — rather than on the text being unreachable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EuPCi56cnGyykygi3z9w4m
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-zhuang
marked this pull request as ready for review
August 23, 2026 06:29
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#5735
element:text_inputrendered its resolveddescriptionas a bare sibling paragraph: noidon the paragraph, noaria-describedbyon theInput. The two elements had no programmatic relationship, so a screen reader moving to the field announced the label and the value and never the helper text.The three facts the card rests on — verified on this worktree, not taken from the card
text-input.tsxrenders the description with noid, and theInputgets noaria-describedby{description && ...}rendered atext-sm text-muted-foregroundparagraph; theInputprop list carriedid/type/placeholder/value/defaultValue/required/disabled/onChangeand no aria attribute.labelhalf is wired:Label htmlFor={schema?.id}againstInput id={schema?.id}packages/components/src/ui/input.tsxadds no aria attributes of its ownforwardRefover a nativeinputthat spreads...props, so anaria-describedbyhanded down by a caller reaches the element unaltered. The primitive is named by triage as the thing that does not supply the attribute, and that stays true: it is not the thing that should. No edit to that file, as instructed.The mechanism, and why it is the platform's rather than a local invention
renderers/form/form.tsxhas wired this correctly all along, viaui/form.tsx:FormItemmints an id withReact.useId(),useFormFieldderivesformDescriptionIdfrom it,FormDescriptionpublishes that id, andFormControl(a RadixSlot) injectsaria-describedbyinto the control. So one authoring key behaved two ways depending on which container the author reached for.This change converges on the id source and the suffix derivation, not on the injection plumbing — there is no
Slothere and no context to hang one on, and adding either to a nine-line render body would be the locally clever fix, not the converged one. The paragraph id is minted withReact.useId()and the attribute is set directly.It deliberately does not converge on one property of the form path:
FormControlemitsaria-describedbyunconditionally, so a form field with no description publishes a reference to an element that was never rendered. This renderer emits the attribute only when a paragraph is actually rendered. (That divergence is stated here rather than filed — the form path's version is a Slot-level behaviour shared withFormMessage, and untangling it is a different card.)The absent-
schema.idpath — the decision the card asked forThe card framed this as a choice between "generate a fallback id so the association always holds" and "stay consistent with
labeland wire only when the author gave the node an id", warning that the block should not end up with two conventions.The two are not the same situation, and that is what decides it: the two associations need ids on opposite ends.
htmlFormust name the input, whose id is the author'sschema.id— the same keyusePageVariableBindingbinds on. Only the author can supply it, so that wiring can only hold when they did. Unchanged.aria-describedbymust name the paragraph, an element this renderer wholly owns and that no author ever addresses. Nothing about it depends on the node carrying anid.So there is one convention — associate where we can — applied to a case where we always can. Deriving the paragraph id from
schema.idwould have imported the label's dependency for no gain and added a failure the label wiring cannot have: two nodes sharing an authored id would publish two paragraphs sharing an id, and both fields'aria-describedbywould resolve to whichever came first in the document. That is the wrong helper text announced, which is worse than none. A per-instanceuseIdcannot collide, and there is a test for exactly that case.Measured id shape under React 19.2.8:
_r_1_-description.The published prose, and the trailing clause
#5717 rewrote this key's
ComponentInput.descriptionto document the gap. That sentence is now false, so it is rewritten in the same change. A repo-wide search found exactly one copy of it (controlled against a neighbouring "Presentational only" occurrence inAiUsageIndicator.tsx, which the same search did return, so the single hit is a reading and not a broken instrument). The published skills corpus does not document this key at all —grepfortext_input,record_pickerandaria-describedbyunderskills/returns zero, controlled against "ObjectUI", which returns three files. No diff underskills/, so the published-skills line-count ruling does not apply here.The trailing clause — "instructions a user must not miss belong in
label" — is kept, not deleted, and its basis is restated.It was originally true because the text was not exposed at all. That reason is now gone. The reason it is still true is different and weaker: a description is announced after the accessible name (it is the last element of the name/role/state/value/description order), and screen readers expose description text through verbosity settings the user can turn down — NVDA's Report object descriptions under Object Presentation, VoiceOver's hint verbosity, JAWS' verbosity levels. So it remains the half of the announcement most likely to go unheard.
This half is cited, not measured, and the code says so. The tests here run
dom-accessibility-apiover happy-dom: that can prove the description is computed and that the reference resolves, and it cannot prove what any screen reader speaks in any given verbosity mode. Deleting the clause would have told authors that a critical instruction placed indescriptionis now guaranteed to be heard, which nothing in this repo can support.Tests — the relationship, not its halves
packages/components/src/__tests__/text-input-description-association.test.tsx, 9 cases. Every one resolves the link the way assistive tech does (readaria-describedby, look each id up in the document, compare the resolved element's text) through a helper that throws on a dangling id rather than skipping it. Asserting the two attributes separately would pass on a build where they point at different things.Alongside the primary case: the two absence controls (no description, and a description resolving to an empty string, both emitting no attribute), the label association it shares an id with, the absent-
schema.idpath, the duplicate-authored-id case, description-beats-placeholder, the resolved-locale-value case, and one run throughSchemaRendererso the real render path is covered too.Ablation — both directions, committed first, restored under a trap
Tests resolve source, not
dist:vitest.config.mts:264aliases@object-ui/componentstopackages/components/src, and nodist/exists anywhere in this worktree (test -d packages/components/distexits 1; so doespackages/core/dist) while all 9 cases pass. No rebuild leg is possible or needed — there is no build artifact a stale copy could hide in.Each mutation was confirmed on disk by anchored counts in both directions before any run, and restored by
trap '...' EXIT INT TERM.A — remove
aria-describedby={descriptionId}from theInput. Injected-text count 0, paragraph-idcount still 1,git diff --stat1 deletion.The 2 survivors are the two absence controls, and they survive correctly: they assert the attribute is absent, which removing the attribute cannot break. That is the point of having them — they are the half that mutation A structurally cannot reach.
B — drop the conditional, so the id (and therefore the attribute) is always emitted. Conditional form gone (0), unconditional form landed (1).
Exactly the complement: the two absence controls, and only those, go red. Between them the two ablations account for all 9 cases, and neither leaves a case that no mutation can kill. Both restore legs confirmed on disk (anchored count back to 1;
git statusclean, i.e. byte-identical to the commit).Verification — all at
f448261f7, on a clean treepnpm exec vitest run packages/components --maxWorkers=4pnpm --filter @object-ui/components type-checktsc --noEmit && tsc -p tsconfig.test.json, both echoed) afterpnpm --workspace-concurrency=2 --filter '@object-ui/components^...' buildpnpm lint(turbo run lint, the whole repo)no-explicit-anywarnings, 0 errors repo-wide.check:control-bytescheck-control-bytes: OK (scanned 4810 tracked text file(s); skipped 85 binary)check:doc-typesEvery documented component type is registered.check:self-importNo package names itself inside its own src/.check:phantom-depsEvery in-scope import is declared by the package that publishes it.check:spec-symbolsspec alignment claims: 2 declared deliberate copies, 18 unbacked claims in 5 packages.(exit 0)check:skills-pathsOK (93/94 stated path(s) resolve across 18 guide file(s); 1 baselined)check:i18n-keys/check:i18n-driftcheck:i18n-dead-keysEvery exit code above was captured before any pipe (
cmd > file 2>&1; EXIT=$?), and each row quotes the gate's own verdict line rather than a bare$?.Changeset:
.changeset/text-input-description-aria-describedby-5735.md(@object-ui/components: patch).Out-of-scope finding — reported, not widened
Swept per the card's ask. Within this renderer nothing else is in the same position:
requiredreaches the nativerequiredattribute (announced), the asterisk is CSS generated content on theLabel(invisible to happy-dom, part of the accessible name in a real browser, and not depended on either way by any test here),placeholderreaches the native attribute, and this renderer emits no error message at all, so there is no error relationship to wire.One sibling block is, and it is filed rather than fixed:
element:record_pickerrenders alabelelement with nohtmlForwhile itsSelectTriggercarries noid— so its label is unassociated in both directions, which is a step worse than the gap this card closes. Its own registration prose advertises the caption as rendered "in alabelelement". This is the same defect class #3341 ruled on and fixed for the ActionParamDialog select branch, one surface over.Generated by Claude Code