diff --git a/.changeset/6158-radio-group-orientation.md b/.changeset/6158-radio-group-orientation.md new file mode 100644 index 0000000000..51472aa062 --- /dev/null +++ b/.changeset/6158-radio-group-orientation.md @@ -0,0 +1,41 @@ +--- +'@object-ui/components': minor +--- + +`radio-group` now renders the `orientation` its own type has always declared (objectui#6158). + +`RadioGroupSchema.orientation` was declared in two layers and read by none. The shipped TS +type carries `orientation?: 'horizontal' | 'vertical'` with `@default 'vertical'` +(`packages/types/src/form.ts:383`) and the zod mirror carries the matching +`z.enum(['horizontal', 'vertical'])` (`packages/types/src/zod/form.zod.ts:282`), while +`packages/components/src/renderers/form/radio-group.tsx` contained neither the string +`orientation` nor `direction` and forwarded only `defaultValue`, `className`, the +form-control DOM whitelist and the designer props. + +The consequence was measurable rather than cosmetic: every radiogroup root the library +rendered came back 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 its horizontal demo rendered vertically. An +author reading the shipped type had every reason to write `orientation: 'horizontal'` and +no way to discover it was inert. + +The key is now forwarded to the underlying Radix `RadioGroup`, which accepts it natively +with the same two-value vocabulary and puts it on the root as `aria-orientation` and +`data-orientation`; the layout utilities follow it so the visible difference the docs +promise is real. This restores declared = enforced **without widening the acceptance +set** — no new key is accepted, and no spelling outside the declared enum becomes legal. + +Two behaviour notes for anyone already shipping radio groups: + +- The declared `@default 'vertical'` is now actually applied instead of being left to + Radix's own `undefined`. A group that never authored the key keeps the vertical stack it + already rendered, and additionally announces `aria-orientation="vertical"` — the + announced orientation now agrees with the rendered one rather than being absent. Arrow + key roving focus narrows to Up/Down for those groups, which is the correct pairing for a + vertical stack. +- Author `className` still wins: the orientation layout utilities compose first and the + authored class last, so tailwind-merge resolves every conflict in the author's favour. + +Registry meta `inputs` for `radio-group` gains `orientation` in the same change — it was +the third surface that omitted the key, and leaving it out would have kept the designer +palette disagreeing with the type. diff --git a/packages/components/src/renderers/form/__tests__/radio-group-orientation.test.tsx b/packages/components/src/renderers/form/__tests__/radio-group-orientation.test.tsx new file mode 100644 index 0000000000..29777b91c1 --- /dev/null +++ b/packages/components/src/renderers/form/__tests__/radio-group-orientation.test.tsx @@ -0,0 +1,153 @@ +/** + * 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. + */ + +/** + * `RadioGroupSchema.orientation` must reach the DOM (objectui#6158). + * + * The key was DECLARED in two layers and read by none: + * + * - `packages/types/src/form.ts:383` — `orientation?: 'horizontal' | 'vertical'` + * with `@default 'vertical'`; + * - `packages/types/src/zod/form.zod.ts:282` — + * `z.enum(['horizontal', 'vertical']).optional()`; + * - `packages/components/src/renderers/form/radio-group.tsx` — contained + * neither the string `orientation` nor `direction`, and forwarded only + * `defaultValue`, `className`, the form-control DOM whitelist and the + * designer props. + * + * The measurable consequence: EVERY radiogroup root the library rendered was + * 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. The horizontal demo rendered vertically. + * + * ## Why these assertions are shaped the way they are + * + * A pin that asserts `orientation: 'horizontal'` renders SOMETHING is a phantom: + * it passes on the unfixed renderer too, because the unfixed renderer renders + * something for every input. The defect is an EQUALITY — two authored values + * producing one output — so the pin has to assert the INEQUALITY. `renders + * different markup for the two orientations` below is that assertion, and it is + * red on the unfixed renderer for the right reason: the two roots come back + * character-for-character equal. + * + * Both fixtures are authored HERE rather than reusing + * `examples/schema-catalog/src/schemas/components-form-radio-group/*`. Those + * catalog fixtures currently spell the key `direction`, which nothing declares — + * that divergence is objectui#6157's scope and is deliberately not touched by + * this branch. Until it lands, the two shipped docs demos still render + * identically to each other even with this renderer fix in place. + */ + +import { describe, it, expect } from 'vitest'; +import { render } from '@testing-library/react'; +import { ComponentRegistry } from '@object-ui/core'; +import type { RadioGroupSchema } from '@object-ui/types'; +// Module scope, not `beforeAll` — the cold transform must not be billed to +// `hookTimeout`. See object-ui/no-dynamic-import-in-test-hook (objectui#3010). +import '../../../renderers'; + +/** + * One factory, one parameter — so "the fixtures differ in EXACTLY one key" is + * a property of the code rather than a claim in a comment. Same `id`, same + * options, same order; a difference in the rendered roots cannot come from + * anywhere but `orientation`. + */ +function fixture(orientation?: 'horizontal' | 'vertical'): RadioGroupSchema { + return { + type: 'radio-group', + id: 'os6158-size', + options: [ + { value: 'sm', label: 'Small' }, + { value: 'md', label: 'Medium' }, + { value: 'lg', label: 'Large' }, + ], + ...(orientation ? { orientation } : {}), + }; +} + +const HORIZONTAL = fixture('horizontal'); +const VERTICAL = fixture('vertical'); +/** `orientation` omitted entirely — the `@default 'vertical'` case. */ +const DEFAULTED = fixture(); + +function renderRoot(schema: RadioGroupSchema): HTMLElement { + const Component = ComponentRegistry.get(schema.type); + if (!Component) throw new Error('radio-group is not registered'); + const { container } = render(); + const root = container.querySelector('[role="radiogroup"]'); + if (!root) throw new Error('no [role="radiogroup"] root was rendered'); + return root; +} + +describe('radio-group renderer — orientation (objectui#6158)', () => { + it('renders DIFFERENT markup for the two orientations', () => { + const horizontal = renderRoot(HORIZONTAL).outerHTML; + const vertical = renderRoot(VERTICAL).outerHTML; + + // The whole defect in one line. On the unfixed renderer these two strings + // are equal, which is precisely the bug the card measured off the built + // site. Anything weaker than an inequality passes before AND after the fix. + expect(horizontal).not.toBe(vertical); + }); + + it('carries the authored orientation onto the radiogroup root', () => { + const horizontal = renderRoot(HORIZONTAL); + const vertical = renderRoot(VERTICAL); + + // Asserted on the DOM, not on a spy over the props reaching Radix: a Radix + // version that accepted the prop and ignored it would satisfy a spy and + // still ship the byte-identical markup this card is about. + expect(horizontal.getAttribute('data-orientation')).toBe('horizontal'); + expect(horizontal.getAttribute('aria-orientation')).toBe('horizontal'); + expect(vertical.getAttribute('data-orientation')).toBe('vertical'); + expect(vertical.getAttribute('aria-orientation')).toBe('vertical'); + }); + + it('lays the horizontal group out as a row and the vertical group as a stack', () => { + const horizontal = renderRoot(HORIZONTAL); + const vertical = renderRoot(VERTICAL); + + // The attributes above are the a11y/keyboard half. This is the half a + // reader of the docs page actually sees: `## Layout Options` promises a + // visual difference, so the layout utilities have to diverge too. + expect(horizontal.className).toContain('flex'); + expect(horizontal.className).not.toContain('grid'); + expect(vertical.className).toContain('grid'); + expect(vertical.className).not.toContain('flex'); + }); + + it('enforces the declared `@default \'vertical\'` when orientation is omitted', () => { + const defaulted = renderRoot(DEFAULTED); + + // Red before the fix: the unfixed renderer emitted no orientation + // attribute at all, so the declared default was as unenforced as the + // explicit values were. + expect(defaulted.getAttribute('data-orientation')).toBe('vertical'); + expect(defaulted.getAttribute('aria-orientation')).toBe('vertical'); + + // ⚠️ This half passes on a revert as well — before the fix the two roots + // were equal because BOTH were orientation-less. It is kept because it is + // what makes "the default is vertical" (rather than merely "some default") + // fall out of the assertion above, and it fails loudly if a later change + // gives the omitted case its own branch. + expect(defaulted.outerHTML).toBe(renderRoot(VERTICAL).outerHTML); + }); + + it('lets an author className still win over the orientation layout classes', () => { + const Component = ComponentRegistry.get('radio-group')!; + const { container } = render( + , + ); + const root = container.querySelector('[role="radiogroup"]'); + + // tailwind-merge resolves the conflict in the author's favour; the + // orientation classes are a default, not an override. + expect(root?.className).toContain('gap-8'); + expect(root?.className).not.toContain('gap-4'); + }); +}); diff --git a/packages/components/src/renderers/form/radio-group.tsx b/packages/components/src/renderers/form/radio-group.tsx index 593d666fdf..c865fac09e 100644 --- a/packages/components/src/renderers/form/radio-group.tsx +++ b/packages/components/src/renderers/form/radio-group.tsx @@ -9,9 +9,32 @@ import { ComponentRegistry } from '@object-ui/core'; import type { RadioGroupSchema } from '@object-ui/types'; import { RadioGroup, RadioGroupItem, Label } from '../../ui'; +import { cn } from '../../lib/utils'; import { toControlValue } from './option-value'; import { toFormControlDomProps } from '../../lib/form-control-dom-props'; +/** + * The declared default (`packages/types/src/form.ts` — `@default 'vertical'`). + * Applied here rather than left to Radix's own `undefined`, because a default + * the type documents and nothing applies is the same declared-but-unenforced + * defect as the key itself was (objectui#6158). Vertical is also what the + * group has always LOOKED like — the `grid gap-2` stack — so this makes the + * announced orientation agree with the rendered one instead of being absent. + */ +const DEFAULT_ORIENTATION = 'vertical' as const; + +/** + * Layout utilities per orientation. `vertical` deliberately names no class: + * the `ui/radio-group` wrapper already applies `grid gap-2`, and that stack IS + * the vertical layout. Author `className` is composed LAST so tailwind-merge + * resolves every conflict in the author's favour — these are a default, not an + * override. + */ +const ORIENTATION_CLASS: Record<'horizontal' | 'vertical', string | undefined> = { + horizontal: 'flex flex-row flex-wrap items-center gap-4', + vertical: undefined, +}; + ComponentRegistry.register('radio-group', ({ schema, className, ...props }: { schema: RadioGroupSchema; className?: string; [key: string]: any }) => { // Extract designer-related props @@ -22,12 +45,21 @@ ComponentRegistry.register('radio-group', ...radioProps } = props; + // Forwarded BY NAME, not by reopening the spread: `toFormControlDomProps` + // is a closed whitelist and `orientation` is not on it, which is exactly + // the objectui#4435 route that file documents for a key like this. Radix's + // `RadioGroup` takes `orientation` natively with the same two-value + // vocabulary, and puts it on the root as both `aria-orientation` and (via + // RovingFocusGroup) `data-orientation`. + const orientation = schema.orientation ?? DEFAULT_ORIENTATION; + return ( // Radix speaks strings — stringify authored (possibly numeric) values for // the control; ids stay stable via the same stringification (#3090).