Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions .changeset/11424-container-padding-set.md
Original file line number Diff line number Diff line change
@@ -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.
8 changes: 7 additions & 1 deletion content/docs/components/layout/container.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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.
3 changes: 2 additions & 1 deletion content/docs/guide/layout.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
Original file line number Diff line number Diff line change
@@ -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, unknown>): string[] {
const { container } = render(
<SchemaRenderer schema={{ type: 'container', maxWidth: false, centered: false, children: [], ...schema } as never} />,
);
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<unknown>) => [...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));
});
});
36 changes: 29 additions & 7 deletions packages/components/src/renderers/layout/container.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,12 @@ const ContainerRenderer = forwardRef<HTMLDivElement, { schema: ContainerSchema;
// path from the day it was declared (objectui#4889). Same read as `padding`
// below, and the one `stack.tsx` / `grid.tsx` have always used.
const maxWidth = schema.maxWidth ?? 'xl';
// `??`, not `||`: `padding` is a declared `number`, and `0` is a legal value
// `??`, not `||`: `padding` is a declared step, and `0` is a legal value
// that `||` folds into the default — which left the `padding === 0 && 'p-0'`
// branch below permanently unreachable, so a container asking for no padding
// silently rendered `p-2 sm:p-3 md:p-4` (objectui#4003).
// ⛔ No rounding or clamping here: the branches below ARE the accept set, and
// `ContainerSchema` refuses every other number at the door (objectui#11424).
const padding = schema.padding ?? 4;
const centered = schema.centered !== false; // Default to true

Expand Down Expand Up @@ -148,12 +150,32 @@ ComponentRegistry.register('container',
{ label: 'full', value: 'full' },
{ label: 'screen', value: 'screen' },
] },
{
name: 'padding',
type: 'number',


description: 'Padding value (0, 1-8, 10, 12, 16)'
{
name: 'padding',
// A closed list, in `maxWidth`'s object form above, not `type: 'number'`
// (objectui#11424): the branches in this file map exactly these steps,
// and any other number drew no padding class at all. `ContainerSchema`
// refuses the rest on both faces; this list carries the same set into
// the manifest, so `validateTree` answers `padding: 9` with
// `invalid-enum` and the generated intrinsics type the prop as the
// twelve literals. `container-padding-set-11424.test.tsx` holds this
// list, the declaration and the rendered branches to one set.
type: 'enum',
enum: [
{ label: '0 (none)', value: 0 },
{ label: '1', value: 1 },
{ label: '2', value: 2 },
{ label: '3', value: 3 },
{ label: '4', value: 4 },
{ label: '5', value: 5 },
{ label: '6', value: 6 },
{ label: '7', value: 7 },
{ label: '8', value: 8 },
{ label: '10', value: 10 },
{ label: '12', value: 12 },
{ label: '16', value: 16 },
],
description: 'Padding step on the container spacing scale; 0 is none. Default 4.'
},
{
name: 'centered',
Expand Down
117 changes: 117 additions & 0 deletions packages/types/src/__tests__/container-padding-set-11424.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
/**
* 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.
*/

/**
* `ContainerSchema.padding` is closed to the steps the `container` renderer
* maps, on both faces, and the refusal names the set (objectui#11424).
*
* ## The defect
*
* The key was `z.number()` on the mirror and `number` on the declaration. The
* renderer reads `schema.padding ?? 4` and tests it against one branch per step
* (0 through 8, 10, 12, 16), so `padding: 9` or `padding: 20` parsed green on
* the tolerant face (`safeValidateSchema`, what `objectui validate` runs) and
* on the strict authoring face, and the container rendered with no padding
* class at all — not the default either, because `??` supplies it only for an
* absent key.
*
* ## The pins (triage `5944196293`)
*
* 9 and 20 are refused on both faces with the set named; each mapped value
* parses; an absent key still parses (the control — the renderer half of it,
* that the absent key draws the default ladder, is in
* `components/src/__tests__/container-padding-set-11424.test.tsx`, which also
* re-derives this set from the rendered branches).
*
* `BaseSchema` is `.passthrough()`, so the refusal also gets a lit control: an
* undeclared key on the same document stays green, which shows the refusal is
* the declared key's own verdict and not a strict object refusing everything.
*/

import { describe, it, expect } from 'vitest';
import type { z } from 'zod';
import { ContainerSchema } from '../zod/layout.zod.js';
import { safeValidateSchema, StrictAnyComponentSchema } from '../zod/index.zod.js';
import type { ContainerSchema as ContainerSchemaType } from '../layout.js';

/** The twelve steps the triage direction names — the renderer's branches. */
const MAPPED = [0, 1, 2, 3, 4, 5, 6, 7, 8, 10, 12, 16] as const;

/** Off-set numbers the triage direction pins as refusals. */
const UNMAPPED = [9, 20] as const;

const UNKNOWN_KEY = 'zzzNotAKeyAnySurfaceDeclares11424';

interface Issue {
code: string;
path: PropertyKey[];
message: string;
values?: unknown[];
}

/** The three doors a `container` document passes through. */
const FACES = {
node: (doc: unknown) => 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<z.infer<typeof ContainerSchema>['padding']>;
type Declared = NonNullable<ContainerSchemaType['padding']>;
type Same<A, B> = [A] extends [B] ? ([B] extends [A] ? true : false) : false;
const same: Same<Mirror, Declared> = true;
expect(same).toBe(true);
});
});
12 changes: 10 additions & 2 deletions packages/types/src/layout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
33 changes: 32 additions & 1 deletion packages/types/src/zod/layout.zod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand All @@ -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',
Expand Down
Loading