Uh oh!
There was an error while loading. Please reload this page.
fix(components): forward RadioGroupSchema.orientation to the rendered radio group - #6244
Merged
Merged
Conversation
Red on the current renderer, for the reason the card measured: the two rendered radiogroup roots come back character-for-character equal, because `RadioGroupSchema.orientation` is declared by the TS type and the zod mirror and read by nothing. The pin asserts the INEQUALITY rather than "horizontal renders something" — the latter is true before and after any fix and would pin nothing. Fixtures are authored here rather than reused from the schema catalog, whose radio-group fixtures still spell the key `direction` (objectui#6157). Part of #6158
… group `orientation` was declared in two layers and read by none: the shipped TS type (`@default 'vertical'`, enum) and the zod mirror both carry it, while the renderer contained neither the string `orientation` nor `direction`. Every radiogroup root the library rendered was therefore byte-identical on that axis — no `data-orientation`, no `aria-orientation` — so the docs page's `## Layout Options` section demonstrated a distinction the product could not make, and the horizontal demo rendered vertically. Radix's `RadioGroup` accepts `orientation` natively with the same two-value vocabulary (verified against the installed 1.4.7: `RovingFocusGroupProps ['orientation']` = `AriaAttributes['aria-orientation']`), and puts it on the root as `aria-orientation` plus, via RovingFocusGroup, `data-orientation`. It is forwarded BY NAME rather than by reopening the DOM spread — `toFormControlDomProps` is a closed whitelist and this is the objectui#4435 route that file documents. The declared `@default 'vertical'` is applied here rather than left to Radix's own `undefined`: a default the type documents and nothing applies is the same declared-but-unenforced defect as the key itself. Vertical is also what the group has always looked like, so this makes the announced orientation agree with the rendered one instead of being absent. The layout utilities follow the key, since `## Layout Options` promises a visible difference; author `className` composes last so tailwind-merge resolves conflicts in the author's favour. Registry meta `inputs` gains `orientation` in the same change — the third surface that omitted it. Part of #6158
… changeset `as const` made `options` a readonly tuple, which `RadioGroupSchema.options: RadioOption[]` rejects, and a `satisfies Omit<…>` widened `type` back to `string`. A factory with a return-type annotation contextually types both, and carries the "differ in EXACTLY one key" property in the code rather than in a comment beside it. Part of #6158
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
|
yinlianghui
marked this pull request as ready for review
August 25, 2026 04:11
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#6158
RadioGroupSchema.orientationwas declared in two layers and read by none. This forwards it to the underlying RadixRadioGroup, restoring declared = enforced without widening the acceptance set — no new key becomes legal, and no spelling outside the already-declared enum is accepted.What was re-derived, not assumed
All four of the dispatch's mechanism assumptions were checked against this branch's tree before any code was written. All four held, but two of them moved:
packages/components/src/renderers/form/radio-group.tsxorientationnordirection.orientationdeclared on the TS type and the zod mirror, with enum + defaultdist/. The source sites arepackages/types/src/form.ts:383(orientation?: 'horizontal' | 'vertical',@default 'vertical') andpackages/types/src/zod/form.zod.ts:282(z.enum(['horizontal', 'vertical']).optional()).orientationnatively with the same vocabularynode_modules, not memory: installed@radix-ui/react-radio-group@1.4.7, whoseRadioGroupProps.orientationisRovingFocusGroupProps['orientation']=AriaAttributes['aria-orientation']='horizontal' | 'vertical'. Exact match — so this is a real forward, not a silent no-op.inputsomitsorientationdefaultValue,id,options,className.On the default. The declared default is
'vertical', and it matters: the unfixed renderer'sgrid gap-2stack already looked vertical, so it accidentally matched the vertical state visually — but it emitted nodata-orientationand noaria-orientation, so in the DOM it matched neither state. That is why the pin asserts attributes and not just appearance.The change
schema.orientationis forwarded by name, not by reopening the DOM spread.toFormControlDomPropsis a closed whitelist that does not carryorientation, and forwarding-by-name is exactly the objectui#4435 route that file documents for a key like this.@default 'vertical'is applied rather than left to Radix's ownundefined. A default the type documents and nothing applies is the same declared-but-unenforced defect as the key itself.## Layout Optionspromises a visible difference. AuthorclassNamecomposes last, so tailwind-merge resolves every conflict in the author's favour.inputsgainsorientation, in the housetype: 'enum'shape (matchingoverlay/sheet.tsx:48, which likewise carries adefaultValue). This is the third surface the card measured as omitting the key.Behaviour note for existing radio groups
A group that never authored the key keeps the vertical stack it already rendered, and now additionally announces
aria-orientation="vertical"— the announced orientation agrees with the rendered one instead of being absent. Arrow-key roving focus narrows from all-arrows to Up/Down for those groups, which is the correct pairing for a vertical stack. Called out here because it is the one user-visible change that reaches groups which did not opt in.This branch deliberately does not touch
examples/schema-catalog/src/schemas/components-form-radio-group/*. Those fixtures — includinghorizontal-layout.jsonandvertical-layout.json, the two demos behind the docs page's## Layout Optionsheading — currently spell the keydirection, which nothing declares. That divergence is #6157's scope.Consequence, stated plainly: with this PR alone, the two shipped docs demos still render identically to each other. The renderer can now tell the orientations apart; those two fixtures still do not ask it to. Triage asked that "the horizontal demo's markup actually diverges once both land" be asserted — that assertion is not possible from this branch and is not attempted here. The pin asserts divergence against its own fixtures instead. This is the serial boundary, not an incomplete card; the docs page becomes correct when #6157 lands.
Verification
Pin written first and proven red on the unfixed renderer before the fix existed, then re-run as an ablation against the final tree so the red measurement matches what is shipping.
Ablation restored the renderer to the branch point with
git checkout f66072d1b -- packages/components/src/renderers/form/radio-group.tsx, undertrap … EXIT INT TERM. Mutation proven on disk in both directions —orientationoccurrences10 → 0,ORIENTATION_CLASS→ 0,1 file changed, 1 insertion(+), 34 deletions(-);git diff HEAD --statempty after restore:The core failure is the defect itself: an
AssertionErrorreporting that the horizontal root's markupnot to bethe vertical root's markup, with the two truncated strings printed identically —Object.isequality on two roots that are character-for-character the same.With the fix:
Test Files 1 passed (1)/Tests 5 passed (5).Which assertions would still pass on a revert
Stated because a pin that is green either way pins nothing:
lets an author className still win over the orientation layout classes— passes on a revert. It assertsgap-8present andgap-4absent; on the unfixed renderer nogap-4is ever emitted, so it is vacuously true. Kept because it guards the composition order going forward, but it is not evidence for this card.enforces the declared @default 'vertical'— passes on a revert. The byte-identity of the omitted case against explicitverticalheld before the fix too, because both were orientation-less. Thedata-orientation/aria-orientationhalf of that same test is red on a revert, and that is the half that does the work.Gates run (all on
fbc603df7, the final commit; tree clean)Derived by enumerating each CI job's own step list, then re-run as a union after the last commit.
pnpm test(targeted):packages/components/— 186 files / 1706 tests passed; plus the three suites that referenceradio-group(widget-dom-leak-sweep,form-control-dom-leak-5632,form-renderers) — 239 passed. The DOM-leak sweep passing matters: the two new attributes are not read as leaks.type-check:@object-ui/componentsgreen, plus the dependent direction — every one of the 30 dependent packages that has atype-checkscript ran a realtscand passed. (@object-ui/example-hello-worldhas no such script and says so loudly —ERR_PNPM_RECURSIVE_RUN_NO_SCRIPT, exit 1 — rather than exiting 0 on a zero-script match.)lint:@object-ui/componentswith--force(Cached: 0 cached) — 0 errors, 907 pre-existing warnings.check:designer-field-key-parity,check:doc-types,check:doc-snippets,check:control-bytes,check:phantom-deps,check:self-import,check:action-forward-parity,check:icon-record-names,check:i18n-keys,check:doc-fences— all green.check:eager-closure— first run exited 2 ("broken gauge, not a passing budget") because it needs a console build; built and re-run rather than counted as a pass:Console eager closure 3222.6 KB / 3990.2 KB budget, andui-components 382.0 KB / 389.6 KB ceiling.check-changeset-presence/check-changeset-no-major— green. Changeset isminor(nevermajor, per the fixed-group rule).Two local red readings were chased down and are not regressions — both were unbuilt local dependencies, proven by building them and re-running to green:
@object-ui/console(missingplugin-gantt/plugin-mapbuilds) and@object-ui/site(missingexample-schema-catalogbuild).Scope
Bounded to the radio-group renderer, its registry meta entry, its changeset, and one new test file.
packages/components/src/ui/radio-group.tsxis untouched — the shadcn wrapper already spreads its remaining props afterclassName, so the prop reaches Radix without editing a no-touch file. Nothing needed to reach wider.⛔ Left draft deliberately — the PM lands it.
Generated by Claude Code