diff --git a/.changeset/11424-container-padding-set.md b/.changeset/11424-container-padding-set.md new file mode 100644 index 0000000000..22bd08b16c --- /dev/null +++ b/.changeset/11424-container-padding-set.md @@ -0,0 +1,27 @@ +--- +'@object-ui/types': minor +'@object-ui/components': minor +--- + +A `container` node's `padding` is one of the twelve steps its renderer maps: 0 to 8, 10, 12 +and 16. Any other number is refused at validation, with the set named (objectui#11424). + +**Breaking for a `container` that carries any other `padding` number.** The key was declared +as any number, but the `container` renderer has one padding class per step and nothing for +the rest. So `padding: 9` or `padding: 20` parsed clean and the container rendered with no +padding class at all, not even the default `4`, because the default applies only when the key +is absent. + +- `@object-ui/types`: `ContainerSchema.padding` is the literal union + `0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 10 | 12 | 16` on the TypeScript face, so `tsc` refuses + any other number. The zod mirror refuses one at `padding` (`invalid_value`, with the twelve + values in the issue), with a message that lists the set. `safeValidateSchema` (what + `objectui validate` runs) and the strict authoring face both give that refusal. +- `@object-ui/components`: the `container` registration's `padding` input changes from + `type: 'number'` to a closed `enum` of the same twelve numbers, in the object form + `maxWidth` already uses. In the SDUI manifest, `validateTree` now answers an unlisted number + with `invalid-enum`, and the generated intrinsics type the prop as the twelve literals. The + renderer is unchanged: it does not round or clamp, and an absent key still renders the + default step `4`. + +Migration: replace the number with the step you meant from the set. `0` means no padding. diff --git a/content/docs/components/layout/container.mdx b/content/docs/components/layout/container.mdx index fc2f6c9f5d..d274127242 100644 --- a/content/docs/components/layout/container.mdx +++ b/content/docs/components/layout/container.mdx @@ -22,7 +22,13 @@ interface ContainerSchema { maxWidth?: 'sm' | 'md' | 'lg' | 'xl' | '2xl' | '3xl' | '4xl' | '5xl' | '6xl' | '7xl' | 'full' | 'screen' | false; // default: 'xl' centered?: boolean; // default: true - padding?: number; // 0, 1-8, 10, 12, 16 — default: 4 + padding?: 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 10 | 12 | 16; // default: 4 className?: string; } ``` + +`padding` is a step on the container's spacing scale, and `0` means none. The twelve +steps are the ones the renderer maps to a padding class, so they are the only values +validation accepts: `"padding": 9` or `"padding": 20` is refused with the set named +(objectui#11424). Such a number used to pass validation and then render with no padding +at all, not even the default. diff --git a/content/docs/guide/layout.md b/content/docs/guide/layout.md index bdb99cfee8..52d796e83c 100644 --- a/content/docs/guide/layout.md +++ b/content/docs/guide/layout.md @@ -712,7 +712,8 @@ the sidebar are nodes you build, so style them where you build them, as above. A `page` node has no padding switch. Its wrapper always insets the content: `p-3`, then `md:p-4`, then `lg:p-6`. The padding you control is a `container`'s. Its `padding` is a -number on the container's spacing scale, and `0` means none: +step on the container's spacing scale: one of 0 to 8, 10, 12 or 16, and `0` means none. +Any other number is refused: ```json { diff --git a/packages/components/src/__tests__/container-padding-set-11424.test.tsx b/packages/components/src/__tests__/container-padding-set-11424.test.tsx new file mode 100644 index 0000000000..d57eca7bb4 --- /dev/null +++ b/packages/components/src/__tests__/container-padding-set-11424.test.tsx @@ -0,0 +1,88 @@ +/** + * 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 `container` padding set is ONE set in three places (objectui#11424): + * the steps the renderer's branches map to a padding class, the literal set + * `ContainerSchema.padding` declares, and the closed `enum` the registration's + * `padding` input publishes into the manifest. + * + * The renderer is the truth (the objectui#7759 ruling: `padding` is not a spec + * key, so the read site decides). This file therefore DERIVES the mapped set by + * rendering the real `container` through `SchemaRenderer` for every candidate + * number and reading which ones put a padding utility on the element, then + * compares the other two lists with it. A branch added or removed in + * `container.tsx` without the declaration and the registration following — + * or the reverse — reddens here. + * + * The derivation also reads the defect itself: an unmapped number (9, 20) + * draws no padding class at all. That is the renderer's behaviour on purpose — + * ⛔ it does not round or clamp; the declaration refuses those numbers instead. + * + * Module-scope import of the renderers, not `beforeAll` (AGENTS.md §测试纪律). + */ +import { describe, it, expect } from 'vitest'; +import { render } from '@testing-library/react'; +import '../renderers'; +import { SchemaRenderer } from '@object-ui/react'; +import { ComponentRegistry } from '@object-ui/core'; +import { ContainerSchema as ContainerMirror } from '@object-ui/types/zod'; + +/** Every candidate the derivation renders: a range past the widest step, plus fractions and a negative. */ +const CANDIDATES = [...Array.from({ length: 33 }, (_, i) => i), -1, 0.5, 1.5, 9.5]; + +/** `maxWidth: false` and `centered: false` keep the base class list free of anything but the padding ladder. */ +function classOf(schema: Record): string[] { + const { container } = render( + , + ); + return (container.firstElementChild as HTMLElement).className.split(/\s+/).filter(Boolean); +} + +/** A padding utility at any breakpoint: `p-2`, `sm:p-3`, `md:p-0.5`. */ +const isPaddingClass = (token: string) => /^(?:[a-z0-9]+:)?p-/.test(token); + +const sortNumbers = (values: Iterable) => [...values].map(Number).sort((a, b) => a - b); + +/** + * The rendered set, derived once on first use — inside a test, so RTL's + * per-test cleanup unmounts what it rendered (the class lists are read first). + */ +let renderedCache: number[] | undefined; +const renderedSet = (): number[] => + (renderedCache ??= CANDIDATES.filter((n) => classOf({ padding: n }).some(isPaddingClass))); + +describe('container padding: the rendered set, the declared set and the registered set are one (objectui#11424)', () => { + it('the derivation is not vacuous: it finds mapped steps AND unmapped numbers', () => { + const rendered = renderedSet(); + expect(rendered.length).toBeGreaterThan(0); + expect(rendered.length).toBeLessThan(CANDIDATES.length); + }); + + it('an unmapped number draws no padding class — 9 and 20 are neither rounded nor clamped', () => { + expect(classOf({ padding: 9 }).filter(isPaddingClass)).toEqual([]); + expect(classOf({ padding: 20 }).filter(isPaddingClass)).toEqual([]); + }); + + it('`ContainerSchema.padding` declares exactly the rendered set', () => { + const declared = ContainerMirror.shape.padding.unwrap().values; + expect(sortNumbers(declared)).toEqual(sortNumbers(renderedSet())); + }); + + it('the registration publishes exactly the rendered set, as a closed enum', () => { + const input = ComponentRegistry.getConfig('container')?.inputs?.find((i) => i.name === 'padding'); + expect(input?.type).toBe('enum'); + const published = (input?.enum ?? []).map((e) => (typeof e === 'object' ? e.value : e)); + expect(sortNumbers(published)).toEqual(sortNumbers(renderedSet())); + }); + + it('control: an absent key still draws the default ladder (padding 4)', () => { + expect(classOf({}).filter(isPaddingClass)).toEqual(['p-2', 'sm:p-3', 'md:p-4']); + expect(classOf({}).filter(isPaddingClass)).toEqual(classOf({ padding: 4 }).filter(isPaddingClass)); + }); +}); diff --git a/packages/components/src/renderers/layout/container.tsx b/packages/components/src/renderers/layout/container.tsx index 319226c75a..01c7bf5371 100644 --- a/packages/components/src/renderers/layout/container.tsx +++ b/packages/components/src/renderers/layout/container.tsx @@ -24,10 +24,12 @@ const ContainerRenderer = forwardRef ContainerSchema.safeParse(doc), + tolerant: (doc: unknown) => safeValidateSchema(doc), + strict: (doc: unknown) => StrictAnyComponentSchema.safeParse(doc), +} as const; + +const issuesOf = (r: { success: boolean; error?: { issues: unknown[] } }): Issue[] => + r.success ? [] : (r.error!.issues as Issue[]); + +describe('ContainerSchema.padding is closed to the renderer\'s steps (objectui#11424)', () => { + for (const [face, parse] of Object.entries(FACES)) { + describe(`${face} face`, () => { + for (const value of UNMAPPED) { + it(`refuses padding ${value} at the key, naming the set`, () => { + const issues = issuesOf(parse({ type: 'container', padding: value })); + expect(issues).toHaveLength(1); + const [issue] = issues; + expect(issue.code).toBe('invalid_value'); + expect(issue.path).toEqual(['padding']); + // The set, structurally: the issue carries the accept list itself … + expect(issue.values).toEqual([...MAPPED]); + // … and the author-facing text spells that same list out. + expect(issue.message).toContain(MAPPED.join(', ')); + }); + } + + it('accepts each mapped step', () => { + for (const value of MAPPED) { + expect(parse({ type: 'container', padding: value }).success, `padding ${value} refused`).toBe(true); + } + }); + + it('control: a container without padding still parses', () => { + expect(parse({ type: 'container', children: [] }).success).toBe(true); + }); + }); + } + + it('lit control: an undeclared key beside a mapped step stays green on the node', () => { + expect(ContainerSchema.safeParse({ type: 'container', padding: 4, [UNKNOWN_KEY]: true }).success).toBe(true); + }); + + it('the declaration takes the same set (compile-time: refused by `tsc` when it does not)', () => { + // @ts-expect-error — 9 is not one of the renderer's steps. + const nine: ContainerSchemaType = { type: 'container', padding: 9 }; + // @ts-expect-error — nor is 20. + const twenty: ContainerSchemaType = { type: 'container', padding: 20 }; + const none: ContainerSchemaType = { type: 'container', padding: 0 }; + const widest: ContainerSchemaType = { type: 'container', padding: 16 }; + expect([nine, twenty, none, widest].length).toBe(4); + }); + + it('the two faces state one set (compile-time)', () => { + type Mirror = NonNullable['padding']>; + type Declared = NonNullable; + type Same = [A] extends [B] ? ([B] extends [A] ? true : false) : false; + const same: Same = true; + expect(same).toBe(true); + }); +}); diff --git a/packages/types/src/layout.ts b/packages/types/src/layout.ts index 69e6640134..2d7a387c0c 100644 --- a/packages/types/src/layout.ts +++ b/packages/types/src/layout.ts @@ -566,9 +566,17 @@ export interface ContainerSchema extends BaseSchema { */ centered?: boolean; /** - * Padding + * Padding step on the container's spacing scale; `0` means none. + * + * The steps are the ones `container.tsx` maps to a padding class, and only + * those (objectui#11424): it reads `schema.padding ?? 4` and tests it against + * one branch per step, so any other number matched no branch and drew no + * padding class at all — not even the default. This was `number`, which let + * `9` and `20` through; the zod mirror (`zod/layout.zod.ts`) refuses them + * with the set named. + * @default 4 */ - padding?: number; + padding?: 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 10 | 12 | 16; /** * Child components */ diff --git a/packages/types/src/zod/layout.zod.ts b/packages/types/src/zod/layout.zod.ts index 9bfb0977c5..62e8a89685 100644 --- a/packages/types/src/zod/layout.zod.ts +++ b/packages/types/src/zod/layout.zod.ts @@ -298,6 +298,32 @@ export const SeparatorSchema = BaseSchema.extend({ ), }); +/** + * The `padding` steps the `container` renderer maps to a padding class + * (objectui#11424) — the read site's set, not a design choice made here. + * + * `ContainerSchema.padding` is not a key `@objectstack/spec` declares, so the + * read site is the truth (the objectui#7759 ruling, the one objectui#10286 + * applied to `maxWidth` on this same node). `container.tsx` reads + * `schema.padding ?? 4` and then tests it against one `padding === N` branch + * per step; a number that equals none of them matches no branch and draws NO + * padding class at all — not even the default, which `??` supplies only for an + * absent key. So `z.number()` accepted `9` and `20` and the container rendered + * flush. The renderer neither rounds nor clamps an unmapped number, and must + * not start: the declaration closes to the set instead. + * + * `components/src/__tests__/container-padding-set-11424.test.tsx` re-derives + * the set by rendering the real `container` and compares it with this list and + * with the registration's `padding` enum, so the three cannot part silently. + */ +const CONTAINER_PADDING_STEPS = [0, 1, 2, 3, 4, 5, 6, 7, 8, 10, 12, 16] as const; + +const CONTAINER_PADDING_REFUSAL = + `\`padding\` on a \`container\` is one of ${CONTAINER_PADDING_STEPS.join(', ')} ` + + '(objectui#11424): those are the steps the renderer maps to a padding class, and `0` means ' + + 'none. Any other number drew NO padding class at all, not even the default `4`, so it is ' + + 'refused here rather than rendered flush. Pick the step you meant from that set.'; + /** * Container Schema - Generic container component */ @@ -314,7 +340,12 @@ export const ContainerSchema = BaseSchema.extend({ z.literal(false), ]).optional().describe('Max width constraint'), centered: z.boolean().optional().describe('Center the container'), - padding: z.number().optional().describe('Padding value'), + // A literal union of the renderer's mapped steps, not `z.number()` + // (objectui#11424) — see {@link CONTAINER_PADDING_STEPS}. + padding: z + .literal(CONTAINER_PADDING_STEPS, { error: CONTAINER_PADDING_REFUSAL }) + .optional() + .describe(`Padding step, one of ${CONTAINER_PADDING_STEPS.join(', ')}; 0 is none (default 4)`), children: z.union([SchemaNodeSchema, z.array(SchemaNodeSchema)]).optional(), body: aliasKeyRefusal( 'body',