diff --git a/.changeset/11318-page-width-padding-refusal.md b/.changeset/11318-page-width-padding-refusal.md new file mode 100644 index 0000000000..d19d32eb1a --- /dev/null +++ b/.changeset/11318-page-width-padding-refusal.md @@ -0,0 +1,28 @@ +--- +'@object-ui/types': minor +--- + +A `page` node refuses `maxWidth` and `padding` by name, and the layout guide teaches the +controls that work: `pageType` for the page's width, a `container` for a narrower column or +custom padding (objectui#11318). + +**Breaking for a `page` node that carries `maxWidth` or `padding`.** Both keys are `container` +members. The page node never declared them, and `PageRenderer` never read them: it takes its +max-width class from `pageType`, and its wrapper padding is fixed. `BaseSchema`'s +`.passthrough()` kept the keys anyway, so a page carrying `maxWidth: 'lg'` or `padding: false` +parsed clean and rendered exactly as it would without them. The layout guide taught both, in +four snippets. + +- `@object-ui/types`: `PageNodeSchema.maxWidth` and `PageNodeSchema.padding` are now `never` on + the TypeScript face, and the zod mirror refuses each key by name, at its own path + (`invalid_type`), with a message that names the door that works. For `maxWidth` that is + `pageType`, or a `container` in `children` with its own `maxWidth`. For `padding` it is a + `container` in `children` with its own `padding`. `safeValidateSchema` (what + `objectui validate` runs) gives the same refusal. So does the strict authoring face, which + refused both keys before only as an unnamed `unrecognized_keys`. +- Unchanged: `ContainerSchema.maxWidth` and `ContainerSchema.padding`, and every other key of + the page node, which stays open to unknown renderer props. + +Migration: delete the key from the page node. To narrow the page, set `pageType` (`utility` is +the narrowest). To get a narrower column or your own padding, wrap the content in +`{ "type": "container", "maxWidth": "2xl", "padding": 8, "children": [ … ] }`. diff --git a/content/docs/guide/layout.md b/content/docs/guide/layout.md index 924b6d0eed..ab1d81f43b 100644 --- a/content/docs/guide/layout.md +++ b/content/docs/guide/layout.md @@ -220,27 +220,29 @@ and `button.tsx`, which reads `schema.label`, renders a button with no text. // Content children: SchemaNode, // Main page content - - // Layout options - maxWidth?: 'sm' | 'md' | 'lg' | 'xl' | '2xl' | 'full', - padding?: boolean, // Add padding (default: true) - + + // Layout + pageType?: 'record' | 'home' | 'app' | 'utility' | 'list', // default 'record'; sets the content's max width + // NO `maxWidth` — refused by name (objectui#11318); use `pageType`, or a `container` in `children` + // NO `padding` — refused by name (objectui#11318); put a `container` with `padding` in `children` + // Styling className?: string, - headerClassName?: string, - bodyClassName?: string } ``` -### Max Width Options +### Content Width -Control page content width: +A `page` node has no `maxWidth` key. **`pageType`** sets how wide the page's content may +grow: `utility` is the narrowest and suits settings and forms, `record` (the default), +`list` and `app` share a wider cap, and `home` is the widest. `getPageMaxWidth` in +`packages/components/src/renderers/layout/page.tsx` holds the mapping. ```json { "type": "page", + "pageType": "utility", "title": "Settings", - "maxWidth": "lg", "children": { "type": "form", "fields": [] @@ -248,13 +250,35 @@ Control page content width: } ``` -Available values: -- `sm` - 640px -- `md` - 768px -- `lg` - 1024px -- `xl` - 1280px -- `2xl` - 1536px -- `full` - No maximum width (default) +`pageType` is more than a width. A `record` page leaves its heading to a `page:header` +block, so a `title` on a page with no `pageType` draws no heading. That is one more reason +the settings page above says `utility`. + +For a column narrower than the page's cap, wrap the content in a `container` and set +**its** `maxWidth`. That key is a `container` member, and the +[Container reference](/docs/components/layout/container) lists its values: + +```json +{ + "type": "page", + "pageType": "utility", + "title": "Profile", + "children": { + "type": "container", + "maxWidth": "2xl", + "children": { + "type": "form", + "fields": [] + } + } +} +``` + +> **⛔ `maxWidth` on a `page` node is refused by name** (objectui#11318). This page used to +> teach `"maxWidth": "lg"` beside `title`, with a list of sizes, and it changed **nothing**: +> `PageRenderer` has no read for the key and takes its width from `pageType`, while +> `BaseSchema`'s `.passthrough()` kept the value rather than refusing it. `PageNodeSchema` +> now declares the key as a refusal whose message names `pageType` and the `container`. ## Page Header (`page:header`) @@ -495,11 +519,14 @@ Omit `sidebar` and the content fills the width under the top bar. ### Settings Page with Tabs +A settings page is a `utility` page, which gives it the narrowest content width (see +[Content Width](#content-width)): + ```json { "type": "page", + "pageType": "utility", "title": "Settings", - "maxWidth": "2xl", "children": { "type": "tabs", "items": [ @@ -664,20 +691,31 @@ the sidebar are nodes you build, so style them where you build them, as above. ### Page Padding -Control page content padding: +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: ```json { "type": "page", - "padding": false, "children": { "type": "container", - "className": "p-8", + "maxWidth": false, + "padding": 8, "children": [] } } ``` +`"maxWidth": false` keeps the container as wide as the page. Leave it out and the container +also caps its own width at its default, `xl`. The +[Container reference](/docs/components/layout/container) lists both scales. + +> **⛔ `padding` on a `page` node is refused by name** (objectui#11318). This page used to +> teach `"padding": false` here, and it removed **nothing**: the page wrapper kept its inset, +> because `PageRenderer` has no read for the key and `BaseSchema`'s `.passthrough()` kept the +> value rather than refusing it. The refusal's message names the `container` and its `padding`. + ## Best Practices ### 1. Consistent Structure @@ -747,17 +785,23 @@ Place primary actions in the first `children` node, so they sit above the conten are **action ids** resolved from the object's own actions metadata, not nodes (see the [PageHeader reference](/docs/layout/page-header)). -### 4. Max Width for Forms +### 4. Constrained Width for Forms -Use constrained width for forms and reading content: +Give forms and reading content a narrow column: a `container` with its own `maxWidth`, +inside the page. ⛔ Not `maxWidth` on the `page` node, which is refused by name +(objectui#11318); the page's own cap comes from `pageType` (see [Content Width](#content-width)). ```json { "type": "page", - "maxWidth": "lg", + "pageType": "utility", "children": { - "type": "form", - "fields": [] + "type": "container", + "maxWidth": "2xl", + "children": { + "type": "form", + "fields": [] + } } } ``` diff --git a/packages/layout/src/__tests__/guide-layout-app-shell-doc.test.ts b/packages/layout/src/__tests__/guide-layout-app-shell-doc.test.ts index ed37e41d93..6484e7476c 100644 --- a/packages/layout/src/__tests__/guide-layout-app-shell-doc.test.ts +++ b/packages/layout/src/__tests__/guide-layout-app-shell-doc.test.ts @@ -90,9 +90,11 @@ * `### Props` table, the `` opening tags in its fences, and whether * any fence authors an `app-shell` JSON node. The same page's `page`, * `page-header` and `sidebar-nav` sections are NOT audited here and must not be - * swept in — `page` in particular declares `headerClassName` / `bodyClassName` - * for a different component (`PageRenderer` in `@object-ui/components`), so a - * file-wide ban on those spellings would be wrong. `packages/layout/README.md` + * swept in — they describe different components with pins of their own (the + * `page` node's are the `page-*-refusal-*` files in `@object-ui/types`), so a + * file-wide ban on this file's spellings would judge material it does not own. + * (The `page` section's Schema API block used to declare `headerClassName` / + * `bodyClassName`; objectui#11318 removed them as unread.) `packages/layout/README.md` * belongs to `readme-app-shell-example.test.ts` (objectui#4817) and * `content/docs/layout/app-shell.mdx` to `app-shell-docs-nav-example.test.ts` * (objectui#4793 / #4808); pinning the whole docs tree to source is diff --git a/packages/types/src/__tests__/page-app-dashboard-spec-parity.test.ts b/packages/types/src/__tests__/page-app-dashboard-spec-parity.test.ts index affbdcfd80..f2c10756ce 100644 --- a/packages/types/src/__tests__/page-app-dashboard-spec-parity.test.ts +++ b/packages/types/src/__tests__/page-app-dashboard-spec-parity.test.ts @@ -90,11 +90,19 @@ const CASES: Record = { // exempted: a local REFUSAL, not a local capability. // `../__tests__/page-breadcrumbs-refusal-8871.test.ts` owns the behaviour. // - // ⚠️ These two are the ONLY members of this row that are refusals. Adding a - // third means a third undeclared key was found surviving `.passthrough()` on + // `maxWidth` and `padding` are the THIRD and FOURTH (objectui#11318, same + // gate, same helper). They are `container` members the layout guide taught + // on this node, and that census is the one this note asked for: no other + // `type: 'page'` object in the tree carried either key, and the node's open + // envelope stays pinned below, so the answer was two more named refusals and + // not a strict node. `../__tests__/page-width-padding-refusal-11318.test.ts` + // owns the behaviour. + // + // ⚠️ These four are the ONLY members of this row that are refusals. Adding a + // fifth means another undeclared key was found surviving `.passthrough()` on // this node — which is the census that decides between one more named refusal // and finally making the node strict. ⛔ Do not grow this list reflexively. - local: ['title', 'pageType', 'body', 'children', 'actions', 'breadcrumbs'], + local: ['title', 'pageType', 'body', 'children', 'actions', 'breadcrumbs', 'maxWidth', 'padding'], }, App: { spec: SpecAppSchema, diff --git a/packages/types/src/__tests__/page-width-padding-refusal-11318.test.ts b/packages/types/src/__tests__/page-width-padding-refusal-11318.test.ts new file mode 100644 index 0000000000..f17baccdb7 --- /dev/null +++ b/packages/types/src/__tests__/page-width-padding-refusal-11318.test.ts @@ -0,0 +1,286 @@ +/** + * 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. + */ + +/** + * `maxWidth` and `padding` on a `page` NODE are refused at parse, by name and + * with the door that works, and `content/docs/guide/layout.md` teaches those + * doors instead (objectui#11318, ADR-0049 enforce-or-remove). + * + * ## The defect + * + * Both keys are `container` members. The guide taught them on a `page` node, in + * four fences: `maxWidth` three times and `padding: false` once. `PageNodeSchema` + * declared neither, so on the tolerant face (`safeValidateSchema`) the fences + * parsed green with the key kept by `.passthrough()`, and the strict authoring + * face refused them only as a bare `unrecognized_keys`. `PageRenderer` has no + * read for either key: its max-width class comes from `pageType` through + * `getPageMaxWidth`, and its wrapper inset is fixed. An author following the + * guide got a green validation and an unchanged page. + * + * ## The shape + * + * The one objectui#7926 (`actions`) and objectui#8871 (`breadcrumbs`) set on + * this same node: a `retirementTombstone` arm in `zod/layout.zod.ts` and a + * `?: never` twin in `layout.ts`. No new accepted key — the triage direction + * on objectui#11318 rules that out — and no `.strict()` node, because + * `page-app-dashboard-spec-parity.test.ts` pins the envelope open. + * + * ## The two halves below + * + * The contract half asserts each refusal on BOTH faces, because they are two + * different doors: `objectui validate` runs the tolerant one today, and the + * strict one is what the doc gates are ruled to run. The guide half runs every + * `type: "page"` JSON fence in the guide through both faces, so a corrected + * snippet that only one face accepts goes red here. + */ + +import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import { PageNodeSchema } from '../zod/layout.zod.js'; +import { safeValidateSchema, StrictAnyComponentSchema } from '../zod/index.zod.js'; +import type { PageNodeSchema as TsPageNodeSchema } from '../layout.js'; + +const REPO_ROOT = resolve(__dirname, '../../../..'); +const GUIDE_PATH = resolve(REPO_ROOT, 'content/docs/guide/layout.md'); + +interface Issue { + code: string; + path: PropertyKey[]; + message: string; + keys?: string[]; +} + +/** Both faces, side by side — the pair the triage direction pins. */ +const FACES = { + 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[]); + +/** One case per refused key: the document the guide used to teach, and the remedy it now teaches. */ +const CASES = [ + { + key: 'maxWidth', + retired: { + type: 'page', + title: 'Settings', + maxWidth: 'lg', + children: { type: 'form', fields: [] }, + }, + remedy: { + type: 'page', + pageType: 'utility', + title: 'Settings', + children: { type: 'container', maxWidth: '2xl', children: { type: 'form', fields: [] } }, + }, + // What the message must name: the key, the page's own door, and the container door. + names: ['maxWidth', 'pageType', 'container', 'children'], + }, + { + key: 'padding', + retired: { + type: 'page', + padding: false, + children: { type: 'container', className: 'p-8', children: [] }, + }, + remedy: { + type: 'page', + children: { type: 'container', maxWidth: false, padding: 8, children: [] }, + }, + names: ['padding', 'container', 'children'], + }, +] as const; + +describe.each(CASES)('objectui#11318 — the `page` node refuses `$key` (contract half)', (c) => { + it('the key is DECLARED, which is what makes the refusal loud rather than a strip', () => { + // An undeclared key on a `.passthrough()` object is kept in silence; a + // refusal has to be declared. There was nothing to delete. + expect(Object.keys(PageNodeSchema.shape)).toContain(c.key); + }); + + it.each(Object.keys(FACES) as (keyof typeof FACES)[])( + 'the %s face refuses the retired document AT the key, with the tombstone code', + (face) => { + const issue = issuesOf(FACES[face](c.retired)).find((i) => i.path.join('.') === c.key); + expect(issue).toBeDefined(); + // `retirementTombstone` is a `z.never` arm: the code and path are a bare + // `z.never()`'s; only the message is customised. + expect(issue!.code).toBe('invalid_type'); + }, + ); + + it.each(Object.keys(FACES) as (keyof typeof FACES)[])( + 'the %s face names the remedy, not just the key', + (face) => { + const issues = issuesOf(FACES[face](c.retired)); + const issue = issues.find((i) => i.path.join('.') === c.key); + expect(issue).toBeDefined(); + // Named subjects, not the sentence (assert the subject, not the copy). + for (const word of c.names) expect(issue!.message).toContain(word); + expect(issue!.message).toContain('objectui#11318'); + // Zod's own default for a `never` arm says none of this. + expect(issue!.message).not.toMatch(/^Invalid input: expected never/); + // ⭐ The strict face used to answer with a bare `unrecognized_keys` + // naming the key and no door. That reading must be gone on both faces. + const bare = issues.filter((i) => i.code === 'unrecognized_keys' && i.keys?.includes(c.key)); + expect(bare).toEqual([]); + }, + ); + + it.each(Object.keys(FACES) as (keyof typeof FACES)[])( + 'POSITIVE CONTROL — the %s face accepts the same document without the key', + (face) => { + // Without this leg, a schema that refused EVERY page document would pass + // the assertions above. + const { [c.key]: dropped, ...rest } = c.retired as Record; + expect(dropped).toBeDefined(); + expect(FACES[face](rest).success).toBe(true); + }, + ); + + it.each(Object.keys(FACES) as (keyof typeof FACES)[])( + 'the remedy the message names parses on the %s face', + (face) => { + expect(issuesOf(FACES[face](c.remedy))).toEqual([]); + }, + ); +}); + +describe('objectui#11318 — the refusals are targeted, and the TypeScript twin agrees', () => { + it('the node is NOT strict — an unknown renderer prop still passes the tolerant face', () => { + // The cheap way to refuse two more keys is `.strict()`; the parity ledger + // pins the envelope open, which is why these are named refusals. + expect(PageNodeSchema.safeParse({ type: 'page', someRendererProp: 42 }).success).toBe(true); + }); + + it('the TypeScript twin refuses both keys', () => { + // `?: never` — the pair `zod-mirror-parity.test.ts` compares. Each + // `@ts-expect-error` IS an assertion: `type-check` fails if the key ever + // becomes assignable again. + const page: TsPageNodeSchema = { + type: 'page', + title: 'Settings', + // @ts-expect-error `maxWidth` is refused by name on the page node (objectui#11318) + maxWidth: 'lg', + // @ts-expect-error `padding` is refused by name on the page node (objectui#11318) + padding: false, + }; + expect(page.type).toBe('page'); + }); +}); + +/** One `json` fence of the guide that carries a `type: "page"` document, with the `###` section it sits in. */ +interface PageFence { + heading: string; + doc: Record; +} + +function pageFences(src: string): PageFence[] { + const lines = src.split('\n'); + const out: PageFence[] = []; + let heading = ''; + for (let i = 0; i < lines.length; i++) { + const h = /^#{2,3} (.+)$/.exec(lines[i]); + if (h) { + heading = h[1]; + continue; + } + if (lines[i] !== '```json') continue; + const end = lines.indexOf('```', i + 1); + if (end === -1) break; + let doc: unknown; + try { + doc = JSON.parse(lines.slice(i + 1, end).join('\n')); + } catch { + doc = undefined; + } + if (doc && typeof doc === 'object' && (doc as Record).type === 'page') { + out.push({ heading, doc: doc as Record }); + } + i = end; + } + return out; +} + +describe('objectui#11318 — the guide teaches the doors that work', () => { + const guide = readFileSync(GUIDE_PATH, 'utf8'); + const fences = pageFences(guide); + + it('LIT CONTROL — the guide is readable and its page fences are found', () => { + // A broken reader would hand every assertion below an empty list. + expect(guide.length).toBeGreaterThan(1000); + expect(fences.length).toBeGreaterThan(4); + }); + + it('the four corrected sections each still carry a page fence', () => { + // The sections that taught the keys, found by heading text rather than by + // line. A section that loses its fence reddens here instead of shrinking + // the population below to something vacuous. + const headings = new Set(fences.map((f) => f.heading)); + for (const heading of [ + 'Content Width', + 'Settings Page with Tabs', + 'Page Padding', + '4. Constrained Width for Forms', + ]) { + expect({ heading, present: headings.has(heading) }).toEqual({ heading, present: true }); + } + }); + + it.each(Object.keys(FACES) as (keyof typeof FACES)[])( + 'every page fence in the guide passes the %s face', + (face) => { + const failing = fences + .map((f) => ({ heading: f.heading, issues: issuesOf(FACES[face](f.doc)) })) + .filter((f) => f.issues.length > 0) + .map((f) => `${f.heading}: ${f.issues.map((i) => `${i.code} at ${i.path.join('.')}`).join('; ')}`); + expect(failing).toEqual([]); + }, + ); + + it('no page fence writes `maxWidth` or `padding` on the page node itself', () => { + // Redundant with the faces while the tombstones stand, and kept for the day + // one of them is removed: then both faces would accept the key again, and + // only this read would still say the guide teaches it. + const offenders = fences + .filter((f) => 'maxWidth' in f.doc || 'padding' in f.doc) + .map((f) => f.heading); + expect(offenders).toEqual([]); + }); + + it('LIT CONTROL — the container door is what the guide teaches instead', () => { + // The scan above cannot see a fence that stopped teaching width at all; this + // reads the remedy itself, one level down, where it is a declared member. + const containerKeys = fences.flatMap((f) => { + const child = f.doc.children as Record | undefined; + return child && child.type === 'container' ? Object.keys(child) : []; + }); + expect(containerKeys).toContain('maxWidth'); + expect(containerKeys).toContain('padding'); + expect(fences.some((f) => f.doc.pageType === 'utility')).toBe(true); + }); + + it('the Schema API block no longer declares either key on the page shape', () => { + // That block is a `typescript` fence, a fence language the JSON reader above + // never visits — the blind spot objectui#8871 recorded for `breadcrumbs`. + const start = guide.indexOf(" type: 'page',"); + expect(start).toBeGreaterThan(-1); + const block = guide.slice(start, guide.indexOf('```', start)); + expect(block).toContain('pageType?:'); + expect(block).not.toMatch(/^\s*(maxWidth|padding)\?\s*:/m); + }); + + it('the retirement is EXPLAINED where it was taught, not silently dropped', () => { + expect(guide).toContain('`maxWidth` on a `page` node is refused by name'); + expect(guide).toContain('`padding` on a `page` node is refused by name'); + expect(guide).toContain('objectui#11318'); + }); +}); diff --git a/packages/types/src/layout.ts b/packages/types/src/layout.ts index 89c0e35230..69e6640134 100644 --- a/packages/types/src/layout.ts +++ b/packages/types/src/layout.ts @@ -1486,6 +1486,42 @@ export interface PageNodeSchema extends BaseSchema, Omit