From 431f63159c6aa093a7c23dd2396b2415368770fc Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 06:23:35 +0000 Subject: [PATCH 1/3] fix(components): every wide centred dialog reads one shared viewport gutter Promote ModalForm's objectui#12593 gutter to one export, DIALOG_VIEWPORT_GUTTER in @object-ui/components, and have every wide centred DialogContent read it beside its sm cap: the action modal's lg/xl, ActionParamDialog, FlowRunner, CelTestRunDialog, the fullscreen editor, ImageLightbox, RecordPickerDialog and ImportWizard. A census in @object-ui/components enumerates every wide centred dialog and fails one that does not read the gutter. Claude-Session: https://claude.ai/code/session_01XVp19E1MYUTFM3dnkwz8gB Co-authored-by: Claude --- .changeset/12603-dialog-gutter.md | 21 + ...eActionModal.viewportGutter-12603.test.tsx | 80 ++++ .../app-shell/src/hooks/useActionModal.tsx | 11 +- .../app-shell/src/views/ActionParamDialog.tsx | 3 +- packages/app-shell/src/views/FlowRunner.tsx | 3 +- .../views/metadata-admin/CelTestRunDialog.tsx | 3 +- .../dialogViewportGutterCensus-12603.test.ts | 453 ++++++++++++++++++ .../src/custom/dialog-viewport-gutter.ts | 38 ++ .../src/custom/fullscreen-editor.tsx | 3 +- packages/components/src/custom/index.ts | 1 + packages/fields/src/widgets/ImageLightbox.tsx | 4 +- .../fields/src/widgets/RecordPickerDialog.tsx | 7 +- ...PickerDialog.viewportGutter-12603.test.tsx | 85 ++++ packages/plugin-form/src/ModalForm.tsx | 36 +- packages/plugin-grid/src/ImportWizard.tsx | 4 +- 15 files changed, 713 insertions(+), 39 deletions(-) create mode 100644 .changeset/12603-dialog-gutter.md create mode 100644 packages/app-shell/src/hooks/__tests__/useActionModal.viewportGutter-12603.test.tsx create mode 100644 packages/components/src/__tests__/dialogViewportGutterCensus-12603.test.ts create mode 100644 packages/components/src/custom/dialog-viewport-gutter.ts create mode 100644 packages/fields/src/widgets/RecordPickerDialog.viewportGutter-12603.test.tsx diff --git a/.changeset/12603-dialog-gutter.md b/.changeset/12603-dialog-gutter.md new file mode 100644 index 0000000000..14df2a95c2 --- /dev/null +++ b/.changeset/12603-dialog-gutter.md @@ -0,0 +1,21 @@ +--- +'@object-ui/components': minor +'@object-ui/app-shell': patch +'@object-ui/fields': patch +'@object-ui/plugin-grid': patch +'@object-ui/plugin-form': patch +--- + +Every wide centred dialog keeps a 1rem gutter on each side of the window from `sm` (640px) up, by one shared class (objectui#12603). + +`@object-ui/components` adds one export, `DIALOG_VIEWPORT_GUTTER` (`sm:w-[calc(100%-2rem)]`): the gutter `ModalForm` introduced for its `lg` and `xl` sizes (objectui#12593), promoted so every wide dialog reads the same value. A dialog that raises its cap with `sm:max-w-2xl` or wider writes this constant beside the cap. + +Dialogs that ran edge to edge between `sm` and their max width and now keep the gutter: + +- the action modal's `lg` and `xl` sizes (`xl` is what a `type: 'modal'` action with a page target and no size opens at); +- the action parameter dialog with a long parameter list, the flow runner's object-form or long screen step, and the CEL test-run dialog; +- the full-screen text editor dialog, the image lightbox and the import wizard. + +The record picker reads the same class under every width rung; it kept a `95vw` margin of its own before, so between `sm` and its cap it is now the window less 2rem instead. `ModalForm` reads the shared export instead of its private copy, with no change in output. + +Unchanged: sheets, the phone sheet below `sm`, the `full` size (`95vw`), and every dialog's width on a window wider than its max width plus 2rem. diff --git a/packages/app-shell/src/hooks/__tests__/useActionModal.viewportGutter-12603.test.tsx b/packages/app-shell/src/hooks/__tests__/useActionModal.viewportGutter-12603.test.tsx new file mode 100644 index 0000000000..3cf6f548c0 --- /dev/null +++ b/packages/app-shell/src/hooks/__tests__/useActionModal.viewportGutter-12603.test.tsx @@ -0,0 +1,80 @@ +/** + * 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 action modal's bounded wide sizes keep the shared viewport gutter + * (objectui#12603). + * + * A `type: 'modal'` action with a page target and no size opens at `xl` + * (`resolveModalTarget`), the centred dialog's `sm:max-w-5xl`. Before + * objectui#12603 that dialog ran edge to edge in every window between `sm` + * and 1024px. `lg` and `xl` now also read `DIALOG_VIEWPORT_GUTTER`, so the + * dialog keeps 1rem either side from `sm` up and is as wide as before above + * its max width plus 2rem. + * + * CONTROLs: `full` keeps its own `95vw` and reads no gutter; `default` never + * reaches the edge at or above `sm` and reads none either; a `side` placement + * is a sheet, edge-anchored by design, and is unchanged. + */ + +import * as React from 'react'; +import { describe, it, expect, afterEach } from 'vitest'; +import { render, screen, cleanup, act } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { DIALOG_VIEWPORT_GUTTER } from '@object-ui/components'; +import { useActionModal } from '../useActionModal'; + +afterEach(cleanup); + +type Handler = (schema: unknown) => Promise; + +function Harness({ onReady }: { onReady: (h: Handler) => void }) { + const { modalHandler, modalElement } = useActionModal(); + React.useEffect(() => onReady(modalHandler), [onReady, modalHandler]); + return <>{modalElement}; +} + +async function open(descriptor: Record): Promise { + let handler: Handler | null = null; + render( { handler = h; }} />); + await act(async () => { + void handler!({ title: 'Quick view', content: { type: 'div' }, ...descriptor }); + }); + const surface = screen.getByRole('dialog'); + return (surface.getAttribute('class') ?? '').split(/\s+/).filter(Boolean); +} + +describe('useActionModal: the wide sizes keep the shared viewport gutter (objectui#12603)', () => { + it('xl, the size a page target with no size opens at, reads the gutter beside its cap', async () => { + const classes = await open({ size: 'xl' }); + expect(classes).toContain('sm:max-w-5xl'); + expect(classes).toContain(DIALOG_VIEWPORT_GUTTER); + }); + + it('lg reads the same gutter', async () => { + const classes = await open({ size: 'lg' }); + expect(classes).toContain('sm:max-w-2xl'); + expect(classes).toContain(DIALOG_VIEWPORT_GUTTER); + }); + + it('CONTROL: full keeps its own 95vw and default reads no gutter', async () => { + const full = await open({ size: 'full' }); + expect(full).toContain('sm:max-w-[95vw]'); + expect(full).toContain('sm:w-full'); + expect(full).not.toContain(DIALOG_VIEWPORT_GUTTER); + cleanup(); + const plain = await open({}); + expect(plain).not.toContain(DIALOG_VIEWPORT_GUTTER); + }); + + it('CONTROL: a side placement is a sheet and reads no gutter', async () => { + const classes = await open({ size: 'xl', placement: 'side' }); + expect(classes).toContain('sm:max-w-3xl'); + expect(classes).not.toContain(DIALOG_VIEWPORT_GUTTER); + }); +}); diff --git a/packages/app-shell/src/hooks/useActionModal.tsx b/packages/app-shell/src/hooks/useActionModal.tsx index aa5ed0c38d..c023895448 100644 --- a/packages/app-shell/src/hooks/useActionModal.tsx +++ b/packages/app-shell/src/hooks/useActionModal.tsx @@ -36,6 +36,7 @@ import { DialogDescription, DialogHeader, DialogTitle, + DIALOG_VIEWPORT_GUTTER, Drawer, DrawerContent, DrawerDescription, @@ -97,11 +98,17 @@ export interface ModalDescriptor { type ActionResult = { success: boolean; reload?: boolean; data?: any; [k: string]: any }; +/** + * The centred dialog's width per size. The bounded wide sizes, `lg` and `xl`, + * read the shared {@link DIALOG_VIEWPORT_GUTTER} so they keep 1rem from the + * window edge at every width from `sm` up (objectui#12603); `full` keeps its + * own `95vw`, and `sm` / `default` never reach the edge at or above `sm`. + */ const SIZE_CLASS: Record = { sm: 'sm:max-w-sm', default: 'sm:max-w-lg', - lg: 'sm:max-w-2xl', - xl: 'sm:max-w-5xl', + lg: `sm:max-w-2xl ${DIALOG_VIEWPORT_GUTTER}`, + xl: `sm:max-w-5xl ${DIALOG_VIEWPORT_GUTTER}`, full: 'sm:max-w-[95vw] sm:w-full', }; const SIDE_SIZE_CLASS: Partial> = { diff --git a/packages/app-shell/src/views/ActionParamDialog.tsx b/packages/app-shell/src/views/ActionParamDialog.tsx index f409340cfe..80ce5490cd 100644 --- a/packages/app-shell/src/views/ActionParamDialog.tsx +++ b/packages/app-shell/src/views/ActionParamDialog.tsx @@ -43,6 +43,7 @@ import { DialogFooter, DialogHeader, DialogTitle, + DIALOG_VIEWPORT_GUTTER, Button, Label, Collapsible, @@ -508,7 +509,7 @@ export function ActionParamDialog({ state, onOpenChange }: ActionParamDialogProp {/* Bounded (objectui#12080): a flex column at most 90vh tall, in which only the body below can give up height (`min-h-0`) and scroll, so the header and the footer always stay on screen. */} - + {state.title || t('actionDialog.title')} {/* `whitespace-pre-line` so a description composed of more than one diff --git a/packages/app-shell/src/views/FlowRunner.tsx b/packages/app-shell/src/views/FlowRunner.tsx index 9bc5ddaac1..849fe015a5 100644 --- a/packages/app-shell/src/views/FlowRunner.tsx +++ b/packages/app-shell/src/views/FlowRunner.tsx @@ -199,6 +199,7 @@ import { DialogFooter, DialogTitle, DialogDescription, + DIALOG_VIEWPORT_GUTTER, Button, cn, } from '@object-ui/components'; @@ -600,7 +601,7 @@ export function FlowRunner({ state, authFetch, baseUrl, onClose, onComplete, dat tall, in which only the body below can give up height (`min-h-0`) and scroll, so the header and the footer always stay on screen. */} {/* The flow being run (objectui#11092), above the step's own heading, diff --git a/packages/app-shell/src/views/metadata-admin/CelTestRunDialog.tsx b/packages/app-shell/src/views/metadata-admin/CelTestRunDialog.tsx index ea530a7049..42af5092b8 100644 --- a/packages/app-shell/src/views/metadata-admin/CelTestRunDialog.tsx +++ b/packages/app-shell/src/views/metadata-admin/CelTestRunDialog.tsx @@ -34,6 +34,7 @@ import { DialogFooter, DialogHeader, DialogTitle, + DIALOG_VIEWPORT_GUTTER, Label, Textarea, cn, @@ -126,7 +127,7 @@ export function CelTestRunDialog({ return ( - + diff --git a/packages/components/src/__tests__/dialogViewportGutterCensus-12603.test.ts b/packages/components/src/__tests__/dialogViewportGutterCensus-12603.test.ts new file mode 100644 index 0000000000..b5399b1c6a --- /dev/null +++ b/packages/components/src/__tests__/dialogViewportGutterCensus-12603.test.ts @@ -0,0 +1,453 @@ +/** + * 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. + */ + +/** + * Every wide centred dialog reads the ONE shared viewport gutter + * (objectui#12603). + * + * ## The defect, and why it is a census + * + * The `DialogContent` primitive draws `w-full max-w-lg`, `fixed`, centred by + * `left-[50%]` and `translate-x-[-50%]`. A consumer that raises the cap from + * `sm` up (`sm:max-w-5xl`) leaves nothing to keep a margin below that cap, so + * the dialog is as wide as the window, at x = 0, edge to edge. objectui#12593 + * repaired `ModalForm` with a gutter of its own; objectui#12603 found the same + * edge in seven more dialogs and promoted that gutter to one export, + * `DIALOG_VIEWPORT_GUTTER` (`packages/components/src/custom/dialog-viewport-gutter.ts`). + * This census is the enumeration pin that card asked for: a new wide dialog + * that forgets the gutter fails here instead of reaching a window. + * + * It lives in `@object-ui/components` because that package owns both the + * `DialogContent` primitive and the gutter it judges. It is a test in the + * ordinary suite, ⛔ not a CI gate. + * + * ## The population, stated as a rule + * + * Every source file under the `src/` tree of a `packages/*` or `apps/*` + * workspace, minus test and measurement tooling (the `TOOLING` shape the + * objectui#12143 / #12145 censuses read), whose comment-masked text + * (`scripts/js-comment-mask.mjs`) names `DialogContent`. + * + * ## What it judges + * + * A JSX element whose tag ends in `DialogContent` (`DialogContent`, + * `MobileDialogContent`, `AlertDialogContent`) is a centred dialog. Sheets + * (`SheetContent`) are edge-anchored by design and never judged, and neither + * is a `Drawer`. The census reads the element's `className` through the + * TypeScript AST, following what the same file declares: `cn()` / `clsx()` + * arguments, conditionals, `&&` / `??`, a same-file `const` (a size map, a + * ceiling), an indexed map, and a same-file function's `return`s (the record + * picker's column-count rung). Each string or template literal reached is a + * UNIT. + * + * A unit is WIDE when it carries a breakpoint max width at or above `sm` that + * a window at that breakpoint cannot hold with 1rem either side: the cap is + * larger than the breakpoint less 2rem (`sm:` above 608px, `md:` above 736px, + * `lg:` above 992px, and so on). `sm:max-w-2xl` (672px) is wide; + * `sm:max-w-xl` (576px) is not, since the narrowest `sm` window (640px) still + * leaves it 32px a side. A viewport-relative cap below 100 (`sm:max-w-[95vw]`, + * the `full` size) keeps its own margin and is not wide. A cap the census + * cannot size (`sm:max-w-[var(--w)]`, `sm:max-w-none`) is wide: it fails + * closed. + * + * A wide unit READS THE GUTTER when it is a template literal that interpolates + * `DIALOG_VIEWPORT_GUTTER` beside the cap, or when the element's `className` + * passes `DIALOG_VIEWPORT_GUTTER` unconditionally (a direct `cn()` argument). + * A gutter literal of a site's own does not count: the rule is one export, + * not a literal per site. + * + * ## ⛔ What this instrument CANNOT see, stated rather than assumed + * + * - A class that arrives from outside the file (a prop, `schema.className`, a + * value imported from another module): the census sizes only what the file + * itself declares. An author-supplied `className` is the author's. + * - An inline `style` width, or a wrapper component that renders a + * `DialogContent` under another tag name. + * - Whether Tailwind emits the class: that is the console's own CSS build. + * + * ## The census, declared + * + * `WIDE_SITES` lists every wide centred dialog by file and tag, once per + * element. Declared and found must agree exactly, so a new wide dialog fails + * until it reads the gutter and is listed. `OWN_GUTTER` names the sites that + * keep a viewport gutter of their own and are not yet converged; it takes + * deletions only. + */ + +import { describe, it, expect } from 'vitest'; +import { existsSync, readdirSync, readFileSync } from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import ts from 'typescript'; +import { maskComments } from '../../../../scripts/js-comment-mask.mjs'; +import { DIALOG_VIEWPORT_GUTTER } from '../custom/dialog-viewport-gutter'; + +const here = path.dirname(fileURLToPath(import.meta.url)); +// packages/components/src/__tests__ -> repository root +const repoRoot = path.resolve(here, '../../../..'); + +const GUTTER_NAME = 'DIALOG_VIEWPORT_GUTTER'; + +/* -------------------------------------------------------------------------- */ +/* The population */ +/* -------------------------------------------------------------------------- */ + +function workspaceSrcDirs(group: 'packages' | 'apps'): string[] { + const base = path.join(repoRoot, group); + if (!existsSync(base)) return []; + return readdirSync(base, { withFileTypes: true }) + .filter((e) => e.isDirectory() && existsSync(path.join(base, e.name, 'package.json'))) + .map((e) => `${group}/${e.name}/src`) + .filter((dir) => existsSync(path.join(repoRoot, dir))); +} + +const SKIP_DIRS = new Set(['node_modules', 'dist', 'build', 'coverage', 'out', '.turbo', '.next']); +/** Test and measurement tooling never ships; the shape the objectui#12143 census reads. */ +const TOOLING = /(^|\/)(__tests__|__mocks__|__benchmarks__|e2e)\/|\.(test|spec|bench|stories)\.[cm]?[jt]sx?$/; + +function walk(dir: string, out: string[] = []): string[] { + for (const entry of readdirSync(dir, { withFileTypes: true })) { + if (entry.name.startsWith('.')) continue; + const full = path.join(dir, entry.name); + if (entry.isDirectory()) { + if (!SKIP_DIRS.has(entry.name)) walk(full, out); + } else if (/\.[cm]?[jt]sx?$/.test(entry.name) && !entry.name.endsWith('.d.ts')) { + out.push(full); + } + } + return out; +} + +const sources: string[] = [...workspaceSrcDirs('packages'), ...workspaceSrcDirs('apps')].flatMap((dir) => + walk(path.join(repoRoot, dir)) + .map((f) => path.relative(repoRoot, f).split(path.sep).join('/')) + .filter((rel) => !TOOLING.test(rel)), +); + +/* -------------------------------------------------------------------------- */ +/* Sizing a class token */ +/* -------------------------------------------------------------------------- */ + +/** Tailwind's default breakpoints, in px. */ +const BREAKPOINT_PX: Record = { sm: 640, md: 768, lg: 1024, xl: 1280, '2xl': 1536 }; +/** Tailwind's default `max-w-*` container scale, in px. */ +const MAX_W_SCALE_PX: Record = { + '3xs': 256, '2xs': 288, xs: 320, sm: 384, md: 448, lg: 512, xl: 576, + '2xl': 672, '3xl': 768, '4xl': 896, '5xl': 1024, '6xl': 1152, '7xl': 1280, +}; +/** 1rem either side. */ +const GUTTER_PX = 32; + +/** + * The cap a `max-w-*` value sets, in px; `'viewport'` for a viewport-relative + * cap below 100 that keeps its own margin; `null` when the census cannot size + * it (judged wide: fail closed). + */ +export function capPx(value: string): number | 'viewport' | null { + if (value in MAX_W_SCALE_PX) return MAX_W_SCALE_PX[value]; + const arb = /^\[(\d+(?:\.\d+)?)(px|rem|vw|%)\]$/.exec(value); + if (!arb) return null; + const n = Number(arb[1]); + if (arb[2] === 'px') return n; + if (arb[2] === 'rem') return n * 16; + return n < 100 ? 'viewport' : null; +} + +/** The wide breakpoint caps a unit's text carries, as the tokens themselves. */ +export function wideTokens(text: string): string[] { + const out: string[] = []; + for (const token of text.split(/\s+/)) { + const m = /^(sm|md|lg|xl|2xl):max-w-(.+)$/.exec(token); + if (!m) continue; + const cap = capPx(m[2]); + if (cap === 'viewport') continue; + if (cap === null || cap > BREAKPOINT_PX[m[1]] - GUTTER_PX) out.push(token); + } + return out; +} + +/* -------------------------------------------------------------------------- */ +/* The matcher */ +/* -------------------------------------------------------------------------- */ + +interface Unit { + text: string; + /** A template literal that interpolates the gutter beside its text. */ + carriesGutter: boolean; +} + +interface Site { + file: string; + tag: string; + line: number; + wide: string[]; + readsGutter: boolean; +} + +function unwrap(e: ts.Expression): ts.Expression { + let cur = e; + while ( + ts.isParenthesizedExpression(cur) || + ts.isAsExpression(cur) || + ts.isSatisfiesExpression(cur) || + ts.isNonNullExpression(cur) + ) { + cur = cur.expression; + } + return cur; +} + +const isGutterRef = (e: ts.Expression): boolean => { + const u = unwrap(e); + return ts.isIdentifier(u) && u.text === GUTTER_NAME; +}; + +export function scanSource(file: string, text: string): { dialogs: number; sites: Site[] } { + const out = { dialogs: 0, sites: [] as Site[] }; + if (!maskComments(text).includes('DialogContent')) return out; + const kind = /\.[cm]?[jt]sx$/.test(file) ? ts.ScriptKind.TSX : ts.ScriptKind.TS; + const sf = ts.createSourceFile(file, text, ts.ScriptTarget.Latest, true, kind); + + // Same-file declarations a className can reach by name. + const values = new Map(); + const functions = new Map(); + const collect = (node: ts.Node): void => { + if (ts.isVariableDeclaration(node) && ts.isIdentifier(node.name) && node.initializer) { + const init = unwrap(node.initializer); + if (ts.isArrowFunction(init) || ts.isFunctionExpression(init)) { + if (!functions.has(node.name.text)) functions.set(node.name.text, init.body); + } else if (!values.has(node.name.text)) { + values.set(node.name.text, node.initializer); + } + } + if (ts.isFunctionDeclaration(node) && node.name && node.body && !functions.has(node.name.text)) { + functions.set(node.name.text, node.body); + } + ts.forEachChild(node, collect); + }; + collect(sf); + + const returnsOf = (body: ts.Node): ts.Expression[] => { + if (!ts.isBlock(body)) return [body as ts.Expression]; + const found: ts.Expression[] = []; + const visit = (n: ts.Node): void => { + if (ts.isFunctionLike(n) && n !== body.parent) return; + if (ts.isReturnStatement(n) && n.expression) found.push(n.expression); + ts.forEachChild(n, visit); + }; + ts.forEachChild(body, visit); + return found; + }; + + const unitsOf = (expr: ts.Expression, seen: Set, acc: Unit[]): void => { + const e = unwrap(expr); + if (ts.isStringLiteral(e) || ts.isNoSubstitutionTemplateLiteral(e)) { + acc.push({ text: e.text, carriesGutter: false }); + } else if (ts.isTemplateExpression(e)) { + const text = [e.head.text, ...e.templateSpans.map((s) => s.literal.text)].join(' '); + acc.push({ text, carriesGutter: e.templateSpans.some((s) => isGutterRef(s.expression)) }); + for (const s of e.templateSpans) if (!isGutterRef(s.expression)) unitsOf(s.expression, seen, acc); + } else if (ts.isIdentifier(e)) { + if (e.text === GUTTER_NAME || seen.has(e.text)) return; + const next = new Set(seen).add(e.text); + const v = values.get(e.text); + if (v) unitsOf(v, next, acc); + } else if (ts.isCallExpression(e)) { + const callee = unwrap(e.expression); + if (ts.isIdentifier(callee) && functions.has(callee.text) && !seen.has(callee.text)) { + const next = new Set(seen).add(callee.text); + for (const r of returnsOf(functions.get(callee.text)!)) unitsOf(r, next, acc); + } + for (const a of e.arguments) unitsOf(a, seen, acc); + } else if (ts.isConditionalExpression(e)) { + unitsOf(e.whenTrue, seen, acc); + unitsOf(e.whenFalse, seen, acc); + } else if (ts.isBinaryExpression(e)) { + unitsOf(e.left, seen, acc); + unitsOf(e.right, seen, acc); + } else if (ts.isElementAccessExpression(e)) { + unitsOf(e.expression, seen, acc); + } else if (ts.isPropertyAccessExpression(e)) { + unitsOf(e.expression, seen, acc); + } else if (ts.isArrayLiteralExpression(e)) { + for (const el of e.elements) if (!ts.isSpreadElement(el)) unitsOf(el, seen, acc); + } else if (ts.isObjectLiteralExpression(e)) { + for (const p of e.properties) if (ts.isPropertyAssignment(p)) unitsOf(p.initializer, seen, acc); + } + }; + + /** The gutter passed so that it applies whatever else the className picks. */ + const unconditionalGutter = (expr: ts.Expression): boolean => { + const e = unwrap(expr); + if (isGutterRef(e)) return true; + if (ts.isCallExpression(e)) return e.arguments.some(isGutterRef); + if (ts.isTemplateExpression(e)) return e.templateSpans.some((s) => isGutterRef(s.expression)); + return false; + }; + + const judge = (tagName: ts.JsxTagNameExpression, attrs: ts.JsxAttributes, at: ts.Node): void => { + const tag = tagName.getText(sf); + if (!/DialogContent$/.test(tag.split('.').pop() ?? '')) return; + out.dialogs += 1; + const cls = attrs.properties.find( + (a): a is ts.JsxAttribute => ts.isJsxAttribute(a) && a.name.getText(sf) === 'className', + ); + if (!cls?.initializer) return; + const init = cls.initializer; + const expr = ts.isJsxExpression(init) ? init.expression : (init as ts.Expression); + if (!expr) return; + const units: Unit[] = []; + unitsOf(expr, new Set(), units); + const wideUnits = units.filter((u) => wideTokens(u.text).length > 0); + if (wideUnits.length === 0) return; + const blanket = unconditionalGutter(expr); + out.sites.push({ + file, + tag, + line: sf.getLineAndCharacterOfPosition(at.getStart(sf)).line + 1, + wide: [...new Set(wideUnits.flatMap((u) => wideTokens(u.text)))], + readsGutter: blanket || wideUnits.every((u) => u.carriesGutter), + }); + }; + + const visit = (node: ts.Node): void => { + if (ts.isJsxElement(node)) judge(node.openingElement.tagName, node.openingElement.attributes, node); + else if (ts.isJsxSelfClosingElement(node)) judge(node.tagName, node.attributes, node); + ts.forEachChild(node, visit); + }; + visit(sf); + return out; +} + +const scans = sources.map((rel) => scanSource(rel, readFileSync(path.join(repoRoot, rel), 'utf8'))); +const dialogCount = scans.reduce((n, s) => n + s.dialogs, 0); +const sites: Site[] = scans.flatMap((s) => s.sites); + +/* -------------------------------------------------------------------------- */ +/* The declared census — file · tag, once per element */ +/* -------------------------------------------------------------------------- */ + +/** + * Every wide centred dialog, by file and tag, once per element. Cited by + * content, never by line. A new wide dialog fails until it reads the gutter + * and is added here; a dialog that narrows leaves in the same commit. + */ +const WIDE_SITES: Array<[file: string, tags: string[]]> = [ + ['packages/app-shell/src/hooks/useActionModal.tsx', ['DialogContent']], + ['packages/app-shell/src/views/ActionParamDialog.tsx', ['DialogContent']], + ['packages/app-shell/src/views/FlowRunner.tsx', ['DialogContent']], + ['packages/app-shell/src/views/metadata-admin/CelTestRunDialog.tsx', ['DialogContent']], + ['packages/components/src/custom/fullscreen-editor.tsx', ['DialogContent']], + ['packages/components/src/custom/navigation-overlay.tsx', ['DialogContent']], + ['packages/fields/src/widgets/ImageLightbox.tsx', ['DialogContent']], + ['packages/fields/src/widgets/RecordPickerDialog.tsx', ['DialogContent']], + ['packages/plugin-form/src/ModalForm.tsx', ['MobileDialogContent', 'MobileDialogContent']], + ['packages/plugin-grid/src/ImportWizard.tsx', ['DialogContent']], +]; + +/** + * Wide dialogs that keep a viewport gutter of their own instead of the shared + * export. Deletions only: converging one removes it from here. + * + * - `navigation-overlay.tsx`'s modal mode draws `w-[calc(100vw-1rem)]` under + * a host-set `--ov-w` cap (default 42rem): never edge to edge, but a 0.5rem + * literal of its own. It was outside objectui#12603's enumeration and file + * surface. + */ +const OWN_GUTTER: ReadonlySet = new Set(['packages/components/src/custom/navigation-overlay.tsx · DialogContent']); + +function tally(entries: Array<{ file: string; tag: string }>): Map { + const out = new Map(); + for (const { file, tag } of entries) { + const k = `${file} · ${tag}`; + out.set(k, (out.get(k) ?? 0) + 1); + } + return out; +} + +const declared = tally(WIDE_SITES.flatMap(([file, tags]) => tags.map((tag) => ({ file, tag })))); +const found = tally(sites); + +describe('every wide centred dialog reads the shared viewport gutter (objectui#12603)', () => { + it('the gutter is the one class objectui#12593 measured', () => { + expect(DIALOG_VIEWPORT_GUTTER).toBe('sm:w-[calc(100%-2rem)]'); + }); + + it('the census reads a real population', () => { + // Floors, not counts: a matcher that stops seeing dialogs reads as a pass + // over nothing, so the population has to be visibly there. + expect(sources.length).toBeGreaterThan(500); + expect(dialogCount).toBeGreaterThan(WIDE_SITES.length); + }); + + it('sizes caps the way a window at that breakpoint sees them', () => { + expect(wideTokens('sm:max-w-2xl')).toEqual(['sm:max-w-2xl']); + expect(wideTokens('sm:max-w-xl')).toEqual([]); + expect(wideTokens('sm:max-w-[560px]')).toEqual([]); + expect(wideTokens('sm:max-w-[1100px]')).toEqual(['sm:max-w-[1100px]']); + expect(wideTokens('lg:max-w-4xl')).toEqual([]); + expect(wideTokens('lg:max-w-5xl')).toEqual(['lg:max-w-5xl']); + // CONTROL: the `full` size keeps its own viewport margin. + expect(wideTokens('sm:max-w-[95vw] sm:w-full')).toEqual([]); + // Unsizable caps fail closed. + expect(wideTokens('sm:max-w-[var(--ov-w,42rem)]')).toEqual(['sm:max-w-[var(--ov-w,42rem)]']); + // Below `sm` is the phone sheet: an unprefixed cap is not judged. + expect(wideTokens('max-w-[98vw] max-w-5xl')).toEqual([]); + }); + + it('a wide cap without the shared gutter is a finding, with it is not', () => { + const bare = scanSource('probe.tsx', `const A = () => ;`); + expect(bare.sites.map((s) => s.readsGutter)).toEqual([false]); + const own = scanSource( + 'probe.tsx', + `const A = () => ;`, + ); + expect(own.sites.map((s) => s.readsGutter)).toEqual([false]); + const paired = scanSource( + 'probe.tsx', + `const A = () => ;`, + ); + expect(paired.sites.map((s) => s.readsGutter)).toEqual([true]); + // A map entry: the wide entry must carry it itself. + const mapped = scanSource( + 'probe.tsx', + `const M = { a: 'sm:max-w-lg', b: \`sm:max-w-5xl \${DIALOG_VIEWPORT_GUTTER}\`, c: 'sm:max-w-4xl' }; + const A = ({ k }) => ;`, + ); + expect(mapped.sites.map((s) => s.readsGutter)).toEqual([false]); + // Unconditional: one direct `cn()` argument covers every rung. + const blanket = scanSource( + 'probe.tsx', + `function w(n) { return n > 3 ? 'sm:max-w-3xl lg:max-w-5xl' : 'sm:max-w-xl'; } + const A = ({ n }) => ;`, + ); + expect(blanket.sites.map((s) => s.readsGutter)).toEqual([true]); + // CONTROL: sheets are edge-anchored and never judged. + const sheet = scanSource('probe.tsx', `const A = () => ;`); + expect(sheet.sites).toEqual([]); + }); + + it('declared and found agree: every wide dialog is listed', () => { + expect([...found.entries()].sort()).toEqual([...declared.entries()].sort()); + }); + + it('every listed wide dialog reads the shared gutter', () => { + const missing = sites + .filter((s) => !s.readsGutter && !OWN_GUTTER.has(`${s.file} · ${s.tag}`)) + .map((s) => `${s.file} · ${s.tag} · ${s.wide.join(' ')}`); + expect(missing).toEqual([]); + }); + + it('OWN_GUTTER takes deletions only: each entry is still a wide site without the shared gutter', () => { + for (const key of OWN_GUTTER) { + const hit = sites.find((s) => `${s.file} · ${s.tag}` === key); + expect(hit, key).toBeDefined(); + expect(hit!.readsGutter, key).toBe(false); + } + }); +}); diff --git a/packages/components/src/custom/dialog-viewport-gutter.ts b/packages/components/src/custom/dialog-viewport-gutter.ts new file mode 100644 index 0000000000..20f3504cfd --- /dev/null +++ b/packages/components/src/custom/dialog-viewport-gutter.ts @@ -0,0 +1,38 @@ +/** + * 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 ONE viewport gutter every wide centred dialog reads from `sm` up + * (objectui#12593, promoted to a shared export by objectui#12603). + * + * The `DialogContent` primitive draws `w-full max-w-lg`. A consumer that raises + * the cap with `sm:max-w-2xl` or wider makes the dialog exactly as wide as any + * window narrower than that cap, at x = 0, edge to edge. With this class the + * dialog is the smaller of its max width and the window less 2rem, and the + * primitive's `left-[50%]` plus `translate-x-[-50%]` centres it with an equal + * 1rem either side. Above the max width plus 2rem the dialog is exactly as wide + * as before. + * + * `100%`, not `100vw`: the dialog is `fixed`, so `100%` is its containing + * block, the viewport less any classic scrollbar, the same box `left-[50%]` + * centres in. `100vw` counts the scrollbar, so each gutter would lose half of + * the scrollbar's width. + * + * `sm:` only: below `sm` a dialog is the phone sheet, which this leaves alone. + * A dialog that keeps its own viewport-relative cap (`sm:max-w-[95vw] + * sm:w-full`, the `full` size) does not read it. + * + * The class string is literal here so Tailwind's source scan emits it; an app + * that `@source`-scans this package gets the rule. A site writes its wide cap + * and this constant together (`` `sm:max-w-5xl ${DIALOG_VIEWPORT_GUTTER}` ``, + * or the constant as an unconditional `cn()` argument), never a gutter literal + * of its own. The census + * `packages/components/src/__tests__/dialogViewportGutterCensus-12603.test.ts` + * enumerates every wide `DialogContent` and fails one that does not. + */ +export const DIALOG_VIEWPORT_GUTTER = 'sm:w-[calc(100%-2rem)]'; diff --git a/packages/components/src/custom/fullscreen-editor.tsx b/packages/components/src/custom/fullscreen-editor.tsx index ed505bbae5..da8984ba32 100644 --- a/packages/components/src/custom/fullscreen-editor.tsx +++ b/packages/components/src/custom/fullscreen-editor.tsx @@ -8,6 +8,7 @@ import React, { useId, useState } from 'react'; import { cn } from '../lib/utils'; +import { DIALOG_VIEWPORT_GUTTER } from './dialog-viewport-gutter'; import { Button } from '../ui/button'; import { Dialog, @@ -367,7 +368,7 @@ export function FullscreenEditor({ diff --git a/packages/components/src/custom/index.ts b/packages/components/src/custom/index.ts index 6f02572f0c..33caeec42c 100644 --- a/packages/components/src/custom/index.ts +++ b/packages/components/src/custom/index.ts @@ -26,3 +26,4 @@ export * from './refresh-indicator'; export * from './view-states'; export * from './empty-state-link'; export * from './mobile-dialog-content'; +export * from './dialog-viewport-gutter'; diff --git a/packages/fields/src/widgets/ImageLightbox.tsx b/packages/fields/src/widgets/ImageLightbox.tsx index eabd91b95e..cebda481b3 100644 --- a/packages/fields/src/widgets/ImageLightbox.tsx +++ b/packages/fields/src/widgets/ImageLightbox.tsx @@ -7,7 +7,7 @@ */ import * as React from 'react'; -import { Dialog, DialogContent, DialogTitle } from '@object-ui/components'; +import { Dialog, DialogContent, DialogTitle, DIALOG_VIEWPORT_GUTTER } from '@object-ui/components'; import { useObjectTranslation } from '@object-ui/i18n'; import { ChevronLeft, ChevronRight } from 'lucide-react'; @@ -66,7 +66,7 @@ export function ImageLightbox({ images, index, open, onOpenChange, onIndexChange return ( {/* Title is required for a labelled dialog but visually redundant here. */} diff --git a/packages/fields/src/widgets/RecordPickerDialog.tsx b/packages/fields/src/widgets/RecordPickerDialog.tsx index a927c20f80..86a887b247 100644 --- a/packages/fields/src/widgets/RecordPickerDialog.tsx +++ b/packages/fields/src/widgets/RecordPickerDialog.tsx @@ -7,6 +7,7 @@ import { DialogHeader, DialogTitle, DialogFooter, + DIALOG_VIEWPORT_GUTTER, Input, Label, Select, @@ -1193,10 +1194,14 @@ export function RecordPickerDialog({ title-to-search distance is the one `DialogHeader` gets in every other dialog. `flex flex-col` is what lets the table region below be the one bounded scroll area (`flex-1 min-h-0`); the width is - `dialogWidthClass`'s rung for the drawn column count. */} + `dialogWidthClass`'s rung for the drawn column count. From `sm` up + the shared `DIALOG_VIEWPORT_GUTTER` (objectui#12603) keeps 1rem from + the window edge under every rung, the `lg:` ceiling included; below + `sm` the dialog stays `95vw`. */} ; + +const ROWS = [{ id: 'u01', name: 'User 01', email: 'user01@example.com' }]; +const FIELDS: Record = { + name: { type: 'text', label: 'Name' }, + email: { type: 'text', label: 'Email' }, + id: { type: 'text', label: 'Id' }, + department: { type: 'text', label: 'Department' }, + title: { type: 'text', label: 'Title' }, + phone: { type: 'text', label: 'Phone' }, +}; + +function renderPicker(props: Partial) { + const dataSource = { + find: vi.fn(async () => ({ data: ROWS, total: ROWS.length })), + findOne: vi.fn(async () => ROWS[0]), + getObjectSchema: vi.fn(async () => ({ name: 'sys_user', fields: FIELDS })), + } as any; + return render( + {}} + dataSource={dataSource} + objectName="sys_user" + fieldsMeta={FIELDS} + cellRenderer={getCellRenderer} + onSelect={() => {}} + {...props} + />, + ); +} + +const tokens = () => + (screen.getByTestId('record-picker-dialog').getAttribute('class') ?? '').split(/\s+/).filter(Boolean); + +describe('RecordPickerDialog keeps the shared viewport gutter (objectui#12603)', () => { + it('the widest rung keeps 1rem from the window at both of its breakpoints', async () => { + renderPicker({ columns: ['name', 'email', 'id', 'department', 'title', 'phone'], multiple: false }); + await waitFor(() => expect(screen.getByTestId('record-row-u01')).toBeInTheDocument()); + const classes = tokens(); + expect(classes).toContain('sm:max-w-3xl'); + expect(classes).toContain('lg:max-w-5xl'); + expect(classes).toContain(DIALOG_VIEWPORT_GUTTER); + // Below `sm` the dialog is still `95vw`, the phone width it always had. + expect(classes).toContain('w-[95vw]'); + }); + + it('the narrowest rung reads the same gutter (a no-op under a cap the window holds)', async () => { + renderPicker({ columns: ['name'], multiple: false }); + await waitFor(() => expect(screen.getByTestId('record-row-u01')).toBeInTheDocument()); + const classes = tokens(); + expect(classes).toContain('sm:max-w-xl'); + expect(classes).toContain(DIALOG_VIEWPORT_GUTTER); + }); +}); diff --git a/packages/plugin-form/src/ModalForm.tsx b/packages/plugin-form/src/ModalForm.tsx index 86e75a7758..39a080aac4 100644 --- a/packages/plugin-form/src/ModalForm.tsx +++ b/packages/plugin-form/src/ModalForm.tsx @@ -18,6 +18,7 @@ import type { FormField, FormSchema, DataSource, ObjectFormSchema, ObjectFormSec import { Dialog, MobileDialogContent, + DIALOG_VIEWPORT_GUTTER, DialogHeader, DialogTitle, DialogDescription, @@ -252,30 +253,6 @@ export interface ModalFormProps { className?: string; } -/** - * The viewport gutter the bounded wide sizes keep from `sm` up - * (objectui#12593): a width of the window less 1rem on each side, under the - * size's own `max-w`. ONE value, read by both `lg` and `xl`. - * - * Without it, the dialog primitive's `w-full` makes an `lg` (672px) or `xl` - * (1024px) dialog exactly as wide as any window narrower than its max width, - * at x = 0, edge to edge. With it, the dialog is the smaller of its max width - * and the window less 2rem, and the primitive's `left-[50%]` plus - * `translate-x-[-50%]` centres it with an equal 1rem either side. Above the - * max width plus 2rem the size is exactly as wide as before. - * - * `100%`, not `100vw`: the dialog is `fixed`, so `100%` is its containing - * block, the viewport less any classic scrollbar, the same box `left-[50%]` - * centres in. `100vw` counts the scrollbar, so each gutter would lose half - * the scrollbar's width. - * - * `sm:` only: below `sm` the dialog is the full-screen phone sheet, unchanged. - * `sm` (384px) and `default` (512px) never reach the window edge at or above - * 640px, and `full` keeps its own `95vw`, so none of the three reads it. The - * class string is literal so Tailwind's source scan emits it. - */ -const BOUNDED_SIZE_VIEWPORT_GUTTER = 'sm:w-[calc(100%-2rem)]'; - /** * Size class map for the dialog content. * @@ -283,14 +260,17 @@ const BOUNDED_SIZE_VIEWPORT_GUTTER = 'sm:w-[calc(100%-2rem)]'; * with MobileDialogContent's base `sm:max-w-lg` class. On mobile (< sm) the * dialog is already full-screen, so max-width only matters at sm+ breakpoints. * - * The bounded wide sizes, `lg` and `xl`, also read - * {@link BOUNDED_SIZE_VIEWPORT_GUTTER} (objectui#12593). + * The bounded wide sizes, `lg` and `xl`, also read the shared + * {@link DIALOG_VIEWPORT_GUTTER} (objectui#12593, promoted to one export by + * objectui#12603): 1rem from the window edge at every width from `sm` up. + * `sm` (384px) and `default` (512px) never reach the edge at or above 640px, + * and `full` keeps its own `95vw`, so none of the three reads it. */ const modalSizeClasses: Record = { sm: 'sm:max-w-sm', default: 'sm:max-w-lg', - lg: `sm:max-w-2xl ${BOUNDED_SIZE_VIEWPORT_GUTTER}`, - xl: `sm:max-w-5xl ${BOUNDED_SIZE_VIEWPORT_GUTTER}`, + lg: `sm:max-w-2xl ${DIALOG_VIEWPORT_GUTTER}`, + xl: `sm:max-w-5xl ${DIALOG_VIEWPORT_GUTTER}`, full: 'sm:max-w-[95vw] sm:w-full', }; diff --git a/packages/plugin-grid/src/ImportWizard.tsx b/packages/plugin-grid/src/ImportWizard.tsx index 88e448b9c2..9ad0474600 100644 --- a/packages/plugin-grid/src/ImportWizard.tsx +++ b/packages/plugin-grid/src/ImportWizard.tsx @@ -6,7 +6,7 @@ import React, { useState, useCallback, useMemo, useEffect } from 'react'; import { cn, Button, Badge, Progress, Input, Checkbox, Label, EmptyValue, - Dialog, DialogContent, DialogHeader, DialogFooter, DialogTitle, DialogDescription, + Dialog, DialogContent, DialogHeader, DialogFooter, DialogTitle, DialogDescription, DIALOG_VIEWPORT_GUTTER, Select, SelectContent, SelectItem, SelectTrigger, SelectValue, Table, TableBody, TableCell, TableHead, TableHeader, TableRow, } from '@object-ui/components'; @@ -2777,7 +2777,7 @@ export const ImportWizard: React.FC = ({ return ( { if (!v) handleClose(); else onOpenChange?.(v); }}> Date: Fri, 16 Oct 2026 06:43:09 +0000 Subject: [PATCH 2/3] test(components): type the comment-mask import in the dialog gutter census Claude-Session: https://claude.ai/code/session_01XVp19E1MYUTFM3dnkwz8gB Co-authored-by: Claude --- .../__tests__/useActionModal.viewportGutter-12603.test.tsx | 2 +- .../src/__tests__/dialogViewportGutterCensus-12603.test.ts | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/app-shell/src/hooks/__tests__/useActionModal.viewportGutter-12603.test.tsx b/packages/app-shell/src/hooks/__tests__/useActionModal.viewportGutter-12603.test.tsx index 3cf6f548c0..41999fa153 100644 --- a/packages/app-shell/src/hooks/__tests__/useActionModal.viewportGutter-12603.test.tsx +++ b/packages/app-shell/src/hooks/__tests__/useActionModal.viewportGutter-12603.test.tsx @@ -43,7 +43,7 @@ async function open(descriptor: Record): Promise { let handler: Handler | null = null; render( { handler = h; }} />); await act(async () => { - void handler!({ title: 'Quick view', content: { type: 'div' }, ...descriptor }); + void handler!({ title: 'Quick view', content: { type: 'box' }, ...descriptor }); }); const surface = screen.getByRole('dialog'); return (surface.getAttribute('class') ?? '').split(/\s+/).filter(Boolean); diff --git a/packages/components/src/__tests__/dialogViewportGutterCensus-12603.test.ts b/packages/components/src/__tests__/dialogViewportGutterCensus-12603.test.ts index b5399b1c6a..8d8bb5e646 100644 --- a/packages/components/src/__tests__/dialogViewportGutterCensus-12603.test.ts +++ b/packages/components/src/__tests__/dialogViewportGutterCensus-12603.test.ts @@ -84,6 +84,7 @@ import { existsSync, readdirSync, readFileSync } from 'node:fs'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; import ts from 'typescript'; +// @ts-expect-error — plain-JS shared helper, intentionally untyped (`allowJs: false`) import { maskComments } from '../../../../scripts/js-comment-mask.mjs'; import { DIALOG_VIEWPORT_GUTTER } from '../custom/dialog-viewport-gutter'; @@ -91,6 +92,9 @@ const here = path.dirname(fileURLToPath(import.meta.url)); // packages/components/src/__tests__ -> repository root const repoRoot = path.resolve(here, '../../../..'); +/** Local annotation, since the import above is untyped — the call site stays checked. */ +const mask: (source: string) => string = maskComments; + const GUTTER_NAME = 'DIALOG_VIEWPORT_GUTTER'; /* -------------------------------------------------------------------------- */ @@ -209,7 +213,7 @@ const isGutterRef = (e: ts.Expression): boolean => { export function scanSource(file: string, text: string): { dialogs: number; sites: Site[] } { const out = { dialogs: 0, sites: [] as Site[] }; - if (!maskComments(text).includes('DialogContent')) return out; + if (!mask(text).includes('DialogContent')) return out; const kind = /\.[cm]?[jt]sx$/.test(file) ? ts.ScriptKind.TSX : ts.ScriptKind.TS; const sf = ts.createSourceFile(file, text, ts.ScriptTarget.Latest, true, kind); From 5bfde250b208529a3fc7347b030ae39f77d3a1e3 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 06:44:19 +0000 Subject: [PATCH 3/3] test(fields): type the picker gutter pin's data source without a cast to any Claude-Session: https://claude.ai/code/session_01XVp19E1MYUTFM3dnkwz8gB Co-authored-by: Claude --- .../widgets/RecordPickerDialog.viewportGutter-12603.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/fields/src/widgets/RecordPickerDialog.viewportGutter-12603.test.tsx b/packages/fields/src/widgets/RecordPickerDialog.viewportGutter-12603.test.tsx index a927c79ebd..d90660b59f 100644 --- a/packages/fields/src/widgets/RecordPickerDialog.viewportGutter-12603.test.tsx +++ b/packages/fields/src/widgets/RecordPickerDialog.viewportGutter-12603.test.tsx @@ -45,7 +45,7 @@ function renderPicker(props: Partial) { find: vi.fn(async () => ({ data: ROWS, total: ROWS.length })), findOne: vi.fn(async () => ROWS[0]), getObjectSchema: vi.fn(async () => ({ name: 'sys_user', fields: FIELDS })), - } as any; + } as unknown as PickerProps['dataSource']; return render(