diff --git a/.changeset/12521-console-record-dialog-interim.md b/.changeset/12521-console-record-dialog-interim.md new file mode 100644 index 0000000000..1d7dfae569 --- /dev/null +++ b/.changeset/12521-console-record-dialog-interim.md @@ -0,0 +1,7 @@ +--- +'@object-ui/app-shell': patch +--- + +**The console's New and Edit dialog on a field-heavy object opens bounded on desktop** (objectui#12521). For an object with twelve or more authorable fields, `@objectstack/spec`'s `deriveRecordFlowSurface` answers a full-screen modal for a create, edit or child task, and the console opened that dialog at 95% of the viewport. The console now gives that answer the bounded shape the spec is moving to (objectstack-ai/objectstack#23734), so `ModalForm`'s own default sizes the dialog: `xl` or `lg`, never `full`. + +This is an interim with a pinned exit: the bound is one function in the console's record-dialog host, and a test fails, naming its deletion, once objectui installs a spec that derives the bound itself. On a phone the dialog is the full-screen sheet it was. No type, prop or export changes. diff --git a/.changeset/12521-dialog-default-layout.md b/.changeset/12521-dialog-default-layout.md new file mode 100644 index 0000000000..8b592ca2f6 --- /dev/null +++ b/.changeset/12521-dialog-default-layout.md @@ -0,0 +1,10 @@ +--- +'@object-ui/plugin-form': minor +--- + +**A record dialog with no authored layout opens bounded, and draws two fields per row wherever no `columns` is declared** (objectui#12521). Three defaults of `ModalForm` change. None reaches a setting the author declared. + +- **Size.** A dialog with no `modalSize` derived its width from its inferred column count, and three or more columns derived `full` (95vw). It now derives `lg` or `xl` from the grid it renders and its field count, and never `full`. `inferModalSize` returns `'lg'` for one column and `'xl'` for two or more. An authored `modalSize`, `'full'` included, is used as written. +- **Flat grid.** A dialog whose form declares no `columns` and draws no sections inferred one to four columns from its field count. It now lays out at two columns from the container's `@md` tier up and one below it, so the phone sheet is unchanged. Every field takes one cell, so fields of one kind share one width. A wide kind (textarea, rich text, markdown, html, an inline grid) and a `span: 'full'` field take the whole row. The object's `fieldGroups` fallback lays out the same way. +- **Section density.** In a form that declares no `columns`, a section that declares none draws two fields per row from `@md`, whatever the other sections declare. Beside a `columns: 3` section it draws two per row on the 12-track grid of objectui#12548, where before it filled three; with no section declaring any, the grid is two wide. A declared count, `1` included, is drawn as written, and a form-level `columns` still sets an undeclared section's count. Until `@objectstack/spec` drops `FormSectionSchema.columns`' `default(1)` (objectstack-ai/objectstack#23740), the spec's parse writes `columns: 1` into every section that omits it, so a section from an authored stack arrives declared and draws one column, as before. +- **What does not move.** A declared form `columns`, a declared section `columns` and a field's `colSpan` / `span` render the same grid classes as before. `inferColumns`, and the page form, drawer and detail view that read it, are unchanged. A host that passes a `modalSize` of its own keeps it. diff --git a/content/docs/plugins/plugin-form.mdx b/content/docs/plugins/plugin-form.mdx index ec1e0b98cf..7c296ea46a 100644 --- a/content/docs/plugins/plugin-form.mdx +++ b/content/docs/plugins/plugin-form.mdx @@ -178,6 +178,30 @@ resolve it that way, so the same metadata lays out identically in every host so each keeps its own authored width). The grid is applied to the field container inside the form, never wrapped around the `
`. +One default differs: a dialog. `ModalForm`, whether `ObjectForm` opens it with +`formType: 'modal'` or a host mounts it directly, draws two fields per row from +the container's `@md` tier up wherever nothing declares `columns`: the flat +field list, the object's `fieldGroups`, and every section that declares no +`columns` in a form that declares none. Such a section draws two per row beside +any other section's count; when the counts do not all fit one grid, the form +switches to a twelve-track grid so each section keeps its own count. A declared +count, `1` included, is drawn as written. Every field takes one cell; a wide +kind (textarea, rich text, markdown, html) and a `span: 'full'` field take the +row. Below `@md`, as on a phone, it is one column. + +Until `@objectstack/spec` drops `FormSectionSchema.columns`' `default(1)`, the +spec's parse writes `columns: 1` into a section that omits it, so a section from +a `defineView` or `defineStack` stack reaches the dialog declared and draws one +column. + +A dialog with no `modalSize` is bounded: `lg` for a short form or a single +column, `xl` otherwise, and never `full`. Declare `modalSize: 'full'` for the +95vw dialog. The console's New and Edit dialog on a field-heavy object (twelve +or more authorable fields) stays within that bound on desktop too: the +record-surface derivation still answers a full-screen modal for it, and the +console bounds that answer until the spec derives the bound itself. On a phone +the dialog is a full-screen sheet whatever its size. + ### Tabbed field layout (`fieldTabs`) A sectioned form stays **one** form: declare the tabs on it and the renderer diff --git a/packages/app-shell/src/console/AppContent.tsx b/packages/app-shell/src/console/AppContent.tsx index 884ef09e06..91e724d1d8 100644 --- a/packages/app-shell/src/console/AppContent.tsx +++ b/packages/app-shell/src/console/AppContent.tsx @@ -33,7 +33,7 @@ import { useTrackRouteAsRecent } from '../hooks/useTrackRouteAsRecent.js'; import { useHomePath } from '../hooks/useHomePath.js'; import { useSignedInUserLocale } from '../hooks/useUserLocale.js'; import { resolveRecordFormTarget, resolveFormViewLayout, resolveNavigateCreateUrl, resolveNavigateEditUrl, resolvePostCreateTarget, recordFormCreateValues, readRecordFormCreateValues, RECORD_FORM_CREATE_VALUES_STATE } from '../utils/recordFormNavigation.js'; -import { deriveRecordSurface, deriveRecordFlowSurface } from '@object-ui/plugin-view'; +import { deriveRecordSurface, deriveRecordFlowSurface, type RecordFlowSurface } from '@object-ui/plugin-view'; import { RECORD_FORM_PARAM, RECORD_FORM_OBJECT_PARAM, RECORD_FORM_LINK_PARAM } from '../urlParams.js'; import { appRouteSegment, matchAppBySegment } from '../utils/appRoute.js'; import { resolveHref, type NavTemplateContext } from '@object-ui/layout'; @@ -102,6 +102,36 @@ interface AppContentProps { extraRoutesNoApp?: ReactNode; } +/** + * INTERIM — objectui#12521, ruled in comment 6205068311 (the maintainer's + * 「可以，12521 走快速方案」), the early outcome of letter B (6203721639). + * + * `deriveRecordFlowSurface` answers `size: 'full'` for a field-heavy create / + * edit / child-create / child-edit flow (objectstack-ai/objectstack#2604 D1), + * and the record dialog below spreads that into `modalSize: 'full'`: a 95vw + * task canvas on desktop. Letter B moves the bound into the protocol — on + * desktop a field-heavy task flow derives a bounded modal, `size: 'auto'` — + * through objectstack-ai/objectstack#23734. This gives the installed + * derivation's answer that shape early, so `ModalForm`'s own bounded default + * (`lg` / `xl`, never `full`) sizes the dialog until that spec is installed. + * + * Desktop only, with no posture check of its own: this host calls the + * derivation with no `viewport`, so the answer it receives is the desktop + * one, and the phone presentation is `ModalForm`'s full-screen sheet below + * `sm`, which no `modalSize` reaches (every size class is `sm:`-prefixed). + * Only the derived default changes: a `modalSize` handed to `ModalForm` is + * used as written. + * + * EXIT: once objectui installs an `@objectstack/spec` carrying + * objectstack-ai/objectstack#23734, delete this function and call + * `deriveRecordFlowSurface` directly at its one call site. The pin + * `AppContent.recordDialogInterimBound-12521.test.tsx` fails on that bump and + * names this deletion. + */ +function interimBoundedTaskFlowSurface(surface: RecordFlowSurface): RecordFlowSurface { + return surface.size === 'full' ? { ...surface, size: 'auto' } : surface; +} + /** * Bridges the global chat's "Review N change(s)" affordance (ADR-0033 Phase B) * to the metadata designer. The chat publishes a review target on `assistantBus`; @@ -1191,18 +1221,19 @@ export function AppContent({ extraRoutes, extraRoutesNoApp }: AppContentProps = ? { initialValues: formLinkValues ?? overlayCreateValues } : {}), // objectstack-ai/objectstack#2604 D1: create/edit follow the flow-surface derivation — - // field-heavy → full-screen modal (the same big canvas the - // detail page gets, with overlay return semantics); light - // objects keep the existing auto-sized modal (ModalForm - // infers from columns when modalSize is unset). Derived - // default only — anything spread later (form view layout) - // would win. Child tasks size to the CHILD object's def. - ...(deriveRecordFlowSurface( + // field-heavy → full-screen modal, which the objectui#12521 + // interim bounds on desktop until the spec derives it + // (`interimBoundedTaskFlowSurface`); light objects keep the + // auto-sized modal (ModalForm's bounded default when + // modalSize is unset). Derived default only — anything spread + // later (form view layout) would win. Child tasks size to the + // CHILD object's def. + ...(interimBoundedTaskFlowSurface(deriveRecordFlowSurface( formObjectDef, isChildFormTask ? (editingRecord ? 'child-edit' : 'child-create') : (editingRecord ? 'edit' : 'create'), - ).size === 'full' + )).size === 'full' ? { modalSize: 'full' as const } : {}), // Honor the object's DEFAULT FORM VIEW: curated sections (field diff --git a/packages/app-shell/src/console/__tests__/AppContent.recordDialogInterimBound-12521.test.tsx b/packages/app-shell/src/console/__tests__/AppContent.recordDialogInterimBound-12521.test.tsx new file mode 100644 index 0000000000..7d151eb1f5 --- /dev/null +++ b/packages/app-shell/src/console/__tests__/AppContent.recordDialogInterimBound-12521.test.tsx @@ -0,0 +1,266 @@ +/** + * 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. + */ + +/** + * objectui#12521 — the console's record dialog on a field-heavy object opens + * bounded on desktop, as an INTERIM with a pinned exit (ruled in comment + * 6205068311, the early outcome of letter B, 6203721639). + * + * `deriveRecordFlowSurface` (installed `@objectstack/spec`, re-exported by + * `@object-ui/plugin-view`, the import `AppContent` uses) answers + * `size: 'full'` for a create / edit / child task flow on an object with + * `RECORD_SURFACE_PAGE_THRESHOLD` or more authorable fields, and `AppContent` + * spread that into `modalSize: 'full'`: a 95vw dialog. Letter B makes the spec + * derive a bounded modal on desktop instead (objectstack-ai/objectstack#23734); + * until objectui installs that spec, `AppContent`'s module-private + * `interimBoundedTaskFlowSurface` gives the derivation's answer that shape, so + * `ModalForm`'s own bounded default sizes the dialog. + * + * Measured from a real `AppContent` mount (the `AppContent.createSeed-12291` + * harness): the global record-form modal records the schema it is handed and, + * where a cell reads the dialog itself, renders the real `ModalForm`. A + * dialog's width is its `sm:max-w-*` class; jsdom evaluates no media query, so + * "below `sm`" is read as the classes that carry no responsive prefix. + * + * Direction, written before the run: with the interim removed (the call site + * back on the bare derivation), the desktop cells and the EXIT cell RED (the + * schema carries `modalSize: 'full'`, the dialog `sm:max-w-[95vw]`); the + * authored-size CONTROL and the phone cell GREEN on both sides. + */ + +import '@testing-library/jest-dom/vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import React from 'react'; +import { render, cleanup, waitFor, screen } from '@testing-library/react'; +import { MemoryRouter, Routes, Route } from 'react-router-dom'; +import { registerAllFields } from '@object-ui/fields'; +import { + deriveRecordFlowSurface, + deriveRecordSurface, + type RecordFlow, +} from '@object-ui/plugin-view'; + +registerAllFields(); + +const probe = vi.hoisted(() => ({ + /** Every schema the global record-form modal was rendered with. */ + modalSchemas: [] as any[], + /** Render the real `ModalForm` under the probe. */ + realModal: false, + /** The real `ModalForm`, for cells that render a schema of their own. */ + RealModalForm: null as null | React.ComponentType, +})); + +vi.mock('@object-ui/plugin-form', async (importOriginal) => { + const actual = await importOriginal>(); + const RealModalForm = actual.ModalForm; + probe.RealModalForm = RealModalForm; + return { + ...actual, + ModalForm: (props: any) => { + probe.modalSchemas.push(props.schema); + return probe.realModal ? :
; + }, + }; +}); + +vi.mock('@object-ui/plugin-designer', async (importOriginal) => ({ + ...(await importOriginal()), + CreateAppPage: () =>
, + EditAppPage: () =>
, + DashboardDesignPage: () =>
, +})); + +vi.mock('../../layout/ConsoleLayout', () => ({ + ConsoleLayout: ({ children }: { children?: React.ReactNode }) => ( +
{children}
+ ), +})); +vi.mock('../../chrome/CommandPalette', () => ({ CommandPalette: () => null })); +vi.mock('../../chrome/KeyboardShortcutsDialog', () => ({ KeyboardShortcutsDialog: () => null })); +vi.mock('../../chrome/OnboardingWalkthrough', () => ({ OnboardingWalkthrough: () => null })); +vi.mock('../../views/ObjectView', () => ({ ObjectView: () =>
})); +vi.mock('../../views/RecordFormPage', () => ({ + RecordFormPage: () =>
, +})); + +vi.mock('@object-ui/auth', async (importOriginal) => ({ + ...(await importOriginal>()), + useAuth: () => ({ + user: { id: 'u1', name: 'Ada', email: 'ada@example.com', role: 'user' }, + getAuthConfig: async () => ({ features: {} }), + activeOrganization: null, + }), + useWorkspaceAdminStatus: () => ({ isAdmin: true, isResolved: true }), +})); + +/** Fourteen plain fields, the size of HotCRM's lead: a field-heavy object. */ +const HEAVY_FIELDS = Object.fromEntries( + Array.from({ length: 14 }, (_, i) => [`f${i}`, { type: 'text', label: `F${i}` }]), +); + +const HEAVY = { name: 'crm_lead', label: 'Lead', fields: HEAVY_FIELDS }; +const PARENT = { name: 'crm_account', label: 'Account', fields: { name: { type: 'text', label: 'Name' } } }; +const OBJECTS = [HEAVY, PARENT]; + +const dataSourceStub = { + onConnectionStateChange: () => () => {}, + getConnectionState: () => 'connected', + getObjectSchema: vi.fn(async (name: string) => OBJECTS.find((o) => o.name === name)), + findOne: vi.fn(async () => ({ id: 'r1' })), + find: vi.fn(async () => ({ data: [] })), + create: vi.fn(), + update: vi.fn(), +}; +vi.mock('../../providers/AdapterProvider', async (importOriginal) => ({ + ...(await importOriginal>()), + useAdapter: () => dataSourceStub, +})); + +const APPS = [{ name: 'crm', label: 'CRM', isDefault: true, navigation: [] }]; + +// One value for the whole run: effects key on these members. +const METADATA = { + apps: APPS, + objects: OBJECTS, + loading: false, + ensureType: undefined, + error: null, + refresh: async () => {}, +}; +vi.mock('../../providers/MetadataProvider', async (importOriginal) => ({ + ...(await importOriginal>()), + useMetadata: () => METADATA, +})); + +const ACTION_RUNNER = { execute: () => {}, runner: { registerHandler: () => {}, getContext: () => ({}) } }; +vi.mock('@object-ui/react', async (importOriginal) => ({ + ...(await importOriginal>()), + useActionRunner: () => ACTION_RUNNER, + useGlobalUndo: () => {}, + useMutationInvalidationBridge: () => {}, +})); + +import { AppContent } from '../AppContent'; + +/** The URL that opens each task flow on the heavy object. */ +const FLOW_URL: Record, string> = { + create: '/apps/crm/crm_lead?form=new', + edit: '/apps/crm/crm_lead?form=r1', + 'child-create': '/apps/crm/crm_account?form=new&formObject=crm_lead', + 'child-edit': '/apps/crm/crm_account?form=r1&formObject=crm_lead', +}; + +/** Open a task flow on the console and return the schema the record dialog was handed. */ +async function openFlow(flow: Exclude) { + render( + + + } /> + + , + ); + await waitFor(() => expect(lastModal()?.open).toBe(true)); + expect(lastModal().objectName).toBe('crm_lead'); + return lastModal(); +} + +const lastModal = () => probe.modalSchemas[probe.modalSchemas.length - 1]; + +/** The rendered dialog, once its fields are drawn. */ +async function dialogOnScreen(): Promise { + await waitFor(() => { + if (!document.body.querySelector('[role="dialog"] [name="f0"]')) throw new Error('form not drawn yet'); + }, { timeout: 5000 }); + return screen.getByRole('dialog'); +} + +/** Render the real `ModalForm` with a schema of the cell's own, and return its dialog. */ +async function renderDialog(schema: Record): Promise { + const RealModalForm = probe.RealModalForm!; + render(); + return dialogOnScreen(); +} + +const tokens = (el: HTMLElement) => el.className.split(/\s+/).filter(Boolean); +/** The classes that apply below `sm`: those with no responsive or state prefix. */ +const belowSm = (el: HTMLElement) => tokens(el).filter((t) => !t.includes(':')).sort(); + +beforeEach(() => { + probe.modalSchemas.length = 0; + probe.realModal = false; + vi.clearAllMocks(); +}); +afterEach(cleanup); + +describe('objectui#12521 interim — a desktop task flow on a field-heavy object opens bounded', () => { + it('the object is field-heavy: the installed derivation pages it', () => { + expect(deriveRecordSurface(HEAVY)).toBe('page'); + }); + + it('create: the real dialog is within the xl bound, not 95vw', async () => { + probe.realModal = true; + const schema = await openFlow('create'); + expect(schema.modalSize).toBeUndefined(); + + const dialog = await dialogOnScreen(); + expect(dialog.className).toContain('sm:max-w-5xl'); + expect(dialog.className).not.toContain('sm:max-w-[95vw]'); + }); + + it.each(['edit', 'child-create', 'child-edit'] as const)( + '%s: the dialog is handed no derived full size', + async (flow) => { + const schema = await openFlow(flow); + expect(schema.mode).toBe(flow.endsWith('edit') ? 'edit' : 'create'); + expect(schema.modalSize).toBeUndefined(); + }, + ); + + it('the phone sheet is unchanged: below sm the dialog carries the same classes as the 95vw dialog did', async () => { + probe.realModal = true; + const schema = await openFlow('create'); + const bounded = belowSm(await dialogOnScreen()); + cleanup(); + + // What the console handed before the interim: the derivation's `full`. + const full = belowSm(await renderDialog({ ...schema, modalSize: 'full' })); + + expect(bounded).toEqual(full); + // Both are the full-screen sheet. + expect(bounded).toEqual(expect.arrayContaining(['fixed', 'inset-0', 'w-full', 'h-[100dvh]'])); + // No width cap reaches a phone. + expect(bounded.some((t) => t.startsWith('max-w-'))).toBe(false); + }); + + it('CONTROL: a modalSize handed to the dialog is used as written — full still opens at 95vw', async () => { + const schema = await openFlow('create'); + cleanup(); + const dialog = await renderDialog({ ...schema, modalSize: 'full' }); + expect(dialog.className).toContain('sm:max-w-[95vw]'); + }); +}); + +describe('objectui#12521 interim — EXIT', () => { + it('retires when the installed spec bounds the field-heavy desktop task flow itself', async () => { + // The import `AppContent` uses, called the way it calls it: no viewport. + const derived = deriveRecordFlowSurface(HEAVY, 'create'); + expect( + derived.size, + 'The installed @objectstack/spec no longer derives `size: \'full\'` for a field-heavy desktop ' + + 'task flow: objectstack-ai/objectstack#23734 is installed. Delete the interim ' + + '`interimBoundedTaskFlowSurface` in packages/app-shell/src/console/AppContent.tsx, call ' + + '`deriveRecordFlowSurface` directly at its call site, and delete this file ' + + '(objectui#12521, interim ruled in comment 6205068311).', + ).toBe('full'); + + // While the spec still answers `full`, the interim is what bounds the dialog. + const schema = await openFlow('create'); + expect(schema.modalSize).toBeUndefined(); + }); +}); diff --git a/packages/plugin-form/src/ModalForm.tsx b/packages/plugin-form/src/ModalForm.tsx index 0a97d769a6..05d4eec38f 100644 --- a/packages/plugin-form/src/ModalForm.tsx +++ b/packages/plugin-form/src/ModalForm.tsx @@ -267,6 +267,66 @@ const modalSizeClasses: Record = { full: 'sm:max-w-[95vw] sm:w-full', }; +/** + * How many fields per row the dialog draws where the author declared no + * `columns` (objectui#12521): two from the container's `@md` tier up, one + * below it, so the phone sheet keeps its single column. It rides the spec's + * own `columns` / `colSpan` path, so every field takes one cell — fields of + * one kind share one width — while a wide kind (`isWideFieldType`: textarea, + * rich text, markdown, html, an inline `grid`) and a `span: 'full'` field take + * the whole row. Line items never reach this grid: they arrive as `subforms`, + * which the master-detail arm below lays out across the dialog. + * + * It applies to a flat form with no `columns`, to the object's `fieldGroups`, + * and to every SECTION that declares no `columns` while the form declares + * none either: an undeclared section density is the renderer's choice, and a + * declared one — `1` included — is drawn as written (the section-`columns` + * fork on objectui#12521, ruled 6204264665). + * + * ⚠️ Until `@objectstack/spec`'s `FormSectionSchema.columns` drops its + * `default(1)` (objectstack-ai/objectstack#23740), the spec's parse writes + * `columns: 1` into every section that omitted it — `defineView` and + * `defineStack` both return the parsed form — so a section from an authored + * stack reaches this dialog DECLARED and draws one column. The undeclared + * rule then covers only sections that arrive unparsed. ⛔ Never read a + * declared `1` as undeclared to get ahead of that release. + * + * Dialog-only on purpose: a page form, a drawer and the detail view keep + * `inferColumns`, which they mirror between them. + */ +const DIALOG_DEFAULT_COLUMNS = 2; + +/** Clamp an authored column count into the 1–4 grid keys; anything else reads as not authored. */ +const clampColumns = (n: unknown): number | undefined => + typeof n === 'number' && n > 0 ? Math.min(Math.floor(n), 4) : undefined; + +/** + * One section's fields per row in the dialog: its declared `columns` as + * written, else the form's declared `columns` (what an undeclared section + * always filled), else {@link DIALOG_DEFAULT_COLUMNS}. + */ +const dialogSectionColumns = (sectionColumns: unknown, formColumns: unknown): number => + clampColumns(sectionColumns) ?? clampColumns(formColumns) ?? DIALOG_DEFAULT_COLUMNS; + +/** + * The ONE grid width of a sectioned dialog: the form's own `columns`, else the + * widest section's {@link dialogSectionColumns}. Read by the stacked and + * tabbed layouts and by the dialog's derived size, so the size is chosen for + * the grid that renders. Each section then draws its own count on it through + * `sectionedGridLayout`, which switches to the 12-track grid when the counts + * do not all fit (objectui#12548) — so an undeclared section beside a + * `columns: 3` one draws two per row, not three. + */ +function sectionedDialogColumns( + sections: ReadonlyArray<{ columns?: unknown }>, + formColumns: unknown, +): 1 | 2 | 3 | 4 { + const widest = sections.length + ? Math.max(...sections.map((s) => dialogSectionColumns(s.columns, formColumns))) + : DIALOG_DEFAULT_COLUMNS; + return (clampColumns(formColumns) ?? widest) as 1 | 2 | 3 | 4; +} + export const ModalForm: React.FC = ({ schema, dataSource, @@ -359,9 +419,9 @@ export const ModalForm: React.FC = ({ // Field-group fallback (object-designer metadata): when the caller passes no // explicit sections, honor the object's declared `fieldGroups` the same way // ObjectForm's simple path does — one section per group, with flat-path - // auto-layout parity (system/auto-generated fields filtered, columns - // inferred from field count, wide fields spanning the row). An explicit - // curated `sections` list from a form view always wins over this fallback. + // auto-layout parity (system/auto-generated fields filtered, the dialog's + // default columns, wide fields spanning the row). An explicit curated + // `sections` list from a form view always wins over this fallback. const derivedSections = useMemo(() => { // `customFields` does NOT switch the fallback off (objectui#10073): the // members are merged into `formFields` above, and ObjectForm derives its @@ -371,9 +431,7 @@ export const ModalForm: React.FC = ({ if (schema.mode === 'create') fs = filterAutoGeneratedFields(fs, objectSchema); const sections = deriveFieldGroupSections(fs, (objectSchema as any)?.fieldGroups); if (!sections) return null; - const columns = (schema.columns && schema.columns > 0 - ? Math.min(Math.floor(schema.columns), 4) - : inferColumns(fs.length)) as 1 | 2 | 3 | 4; + const columns = (clampColumns(schema.columns) ?? DIALOG_DEFAULT_COLUMNS) as 1 | 2 | 3 | 4; // A member's definition reaches a group's body through `buildSectionFields`, // the route an explicit section's takes (objectui#10254). return sections.map((s) => ({ ...s, columns })) as ModalFormSectionConfig[]; @@ -384,24 +442,29 @@ export const ModalForm: React.FC = ({ // Compute auto-layout for flat fields (no sections) to determine inferred columns // (`customFields` does not switch it off: the members are merged into // `formFields`, and ObjectForm auto-lays-out that merged list — objectui#10073.) + // A form that declares no `columns` lays out at the dialog's default + // (objectui#12521) through the same explicit-columns route an authored + // `columns` takes, so `applyAutoLayout` never infers 3–4 columns here. + const flatColumns = schema.columns ?? DIALOG_DEFAULT_COLUMNS; const autoLayoutResult = useMemo(() => { if (effectiveSections?.length) return null; - return applyAutoLayout(formFields, objectSchema, schema.columns, schema.mode); - }, [formFields, objectSchema, schema.columns, schema.mode, effectiveSections]); + return applyAutoLayout(formFields, objectSchema, flatColumns, schema.mode); + }, [formFields, objectSchema, flatColumns, schema.mode, effectiveSections]); - // Auto-upgrade modal size when auto-layout infers multi-column and user hasn't set modalSize + // The dialog's size when the author declared none (objectui#12521): bounded + // — `lg` or `xl`, never `full` — and chosen from the grid that renders and + // the field count, so a short form is not stretched across a wide dialog. + // An authored `modalSize` (including `'full'`) is used as written. const effectiveModalSize = useMemo(() => { if (schema.modalSize) return schema.modalSize; - if (autoLayoutResult?.columns && autoLayoutResult.columns > 1) { - return inferModalSize(autoLayoutResult.columns); - } - // Auto-upgrade for sections: use the max columns across all sections - if (effectiveSections?.length) { - const maxCols = Math.max(...effectiveSections.map(s => Number(s.columns) || 1)); - if (maxCols > 1) return inferModalSize(maxCols); - } - return 'default'; - }, [schema.modalSize, autoLayoutResult, effectiveSections]); + const columns = effectiveSections?.length + ? sectionedDialogColumns(effectiveSections, schema.columns) + : (autoLayoutResult?.columns ?? DIALOG_DEFAULT_COLUMNS); + const fieldCount = effectiveSections?.length + ? effectiveSections.reduce((n, s) => n + (Array.isArray(s.fields) ? s.fields.length : 0), 0) + : (autoLayoutResult?.fields.length ?? 0); + return inferModalSize(Math.min(columns, inferColumns(fieldCount))); + }, [schema.modalSize, schema.columns, autoLayoutResult, effectiveSections]); const sizeClass = modalSizeClasses[effectiveModalSize] || modalSizeClasses.default; @@ -833,19 +896,15 @@ export const ModalForm: React.FC = ({ sec.name ? sectionLabel(schema.objectName, sec.name, sec.label || sec.name) : sec.label; // The form is ONE grid. Its width is the explicit form `columns`, else the - // widest section; each section then lays ITS fields out at its own - // declared density within that grid via colSpan (objectstack-ai/objectstack#2578), exactly like the + // widest section's count, an undeclared section counting the dialog's + // default (objectui#12521); each section then lays ITS fields out at its + // own count within that grid via colSpan (objectstack-ai/objectstack#2578), exactly like the // full-page sectioned form — by the one sectioned-form rule, which // switches to a 12-track grid when the sections' counts do not all fit // the widest one (objectui#12548). - const clampCol = (n: unknown): number | undefined => - typeof n === 'number' && n > 0 ? Math.min(Math.floor(n), 4) : undefined; - const declaredCols = sections - .map((s) => clampCol(s.columns)) - .filter((c): c is number => c != null); - const formColumns = (clampCol(schema.columns) - ?? (declaredCols.length ? Math.max(...declaredCols) : 1)) as 1 | 2 | 3 | 4; - const sectionGrid = sectionedGridLayout(formColumns, sections.map((s) => clampCol(s.columns))); + const sectionColumnsOf = (sec: ModalFormSectionConfig) => dialogSectionColumns(sec.columns, schema.columns); + const formColumns = sectionedDialogColumns(sections, schema.columns); + const sectionGrid = sectionedGridLayout(formColumns, sections.map(sectionColumnsOf)); const containerFieldClass = sectionGrid.fieldContainerClass; // FLS first, so a section whose every field is non-readable drops out @@ -876,7 +935,7 @@ export const ModalForm: React.FC = ({ visibleWhen: section.visibleWhen, className: section.className, gridClassName: section.gridClassName, - fields: sectionGrid.layOut(body, clampCol(section.columns)), + fields: sectionGrid.layOut(body, sectionColumnsOf(section)), }; }) .filter((g) => g.fields.length > 0); @@ -998,7 +1057,7 @@ export const ModalForm: React.FC = ({ } // Reuse pre-computed auto-layout result for flat fields - const layoutResult = autoLayoutResult ?? applyAutoLayout(formFields, objectSchema, schema.columns, schema.mode); + const layoutResult = autoLayoutResult ?? applyAutoLayout(formFields, objectSchema, flatColumns, schema.mode); // Flat fields layout — use container-query grid classes so the form // responds to the modal width, not the viewport width. diff --git a/packages/plugin-form/src/__tests__/autoLayout.test.ts b/packages/plugin-form/src/__tests__/autoLayout.test.ts index aafecd13c8..aff7cac1bf 100644 --- a/packages/plugin-form/src/__tests__/autoLayout.test.ts +++ b/packages/plugin-form/src/__tests__/autoLayout.test.ts @@ -83,19 +83,19 @@ describe('autoLayout', () => { }); describe('inferModalSize', () => { - it('returns default for 0 or 1 column', () => { - expect(inferModalSize(0)).toBe('default'); - expect(inferModalSize(1)).toBe('default'); + it('returns lg for 0 or 1 column', () => { + expect(inferModalSize(0)).toBe('lg'); + expect(inferModalSize(1)).toBe('lg'); }); it('returns xl for 2 columns', () => { expect(inferModalSize(2)).toBe('xl'); }); - it('returns full for 3+ columns', () => { - expect(inferModalSize(3)).toBe('full'); - expect(inferModalSize(4)).toBe('full'); - expect(inferModalSize(5)).toBe('full'); + it('stays bounded at xl for 3+ columns, never full (objectui#12521)', () => { + expect(inferModalSize(3)).toBe('xl'); + expect(inferModalSize(4)).toBe('xl'); + expect(inferModalSize(5)).toBe('xl'); }); }); diff --git a/packages/plugin-form/src/__tests__/dialogDefaultLayout-12521.test.tsx b/packages/plugin-form/src/__tests__/dialogDefaultLayout-12521.test.tsx new file mode 100644 index 0000000000..5b0f556ec0 --- /dev/null +++ b/packages/plugin-form/src/__tests__/dialogDefaultLayout-12521.test.tsx @@ -0,0 +1,292 @@ +/** + * 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 record dialog's defaults when the author declared no layout + * (objectui#12521). + * + * ## What changed + * + * - SIZE: a dialog with no `modalSize` derived its width from the inferred + * column count, and three or more columns mapped to `full` (95vw). It is now + * bounded: `lg` or `xl`, chosen from the grid that renders and the field + * count, ⛔ never `full`. An authored `modalSize: 'full'` is used as written. + * - GRID: a form that declares no `columns` (on the form, and on no section) + * inferred one to four columns from its field count. It now lays out at two + * columns from the container's `@md` tier up, through the explicit-columns + * route, so every field takes one cell and a wide kind or a `span: 'full'` + * field takes the row. An authored `columns` wins. + * - SECTION DENSITY (the section-`columns` fork, ruled 6204264665): a section + * that declares no `columns`, in a form that declares none, draws two per + * row from `@md`, beside any other section's count, on objectui#12548's + * per-section grid. A declared count, `1` included, is drawn as written. + * + * ## What it reads + * + * The real `ModalForm` with the real form renderer and field registry, as + * `AppContent` mounts it (no `ObjectForm` above it). The dialog's size is its + * `sm:max-w-*` class; a field's width is the col-span classes on its grid + * cell. jsdom evaluates no container query, so "two columns from `@md`" is + * read as the grid's class list, which the form renderer emits verbatim. + */ + +import React from 'react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, waitFor, cleanup } from '@testing-library/react'; +import { registerAllFields } from '@object-ui/fields'; +import { ModalForm } from '../ModalForm'; + +registerAllFields(); + +afterEach(() => cleanup()); + +const opt = (v: string) => [{ label: v, value: v }]; + +/** Twelve plain fields plus one textarea — "an object form with twelve fields". */ +const TWELVE = { + name: 'thing', + fields: { + name: { type: 'text', label: 'Name' }, + code: { type: 'text', label: 'Code' }, + email: { type: 'email', label: 'Email' }, + phone: { type: 'text', label: 'Phone' }, + website: { type: 'url', label: 'Website' }, + status: { type: 'select', label: 'Status', options: opt('open') }, + stage: { type: 'select', label: 'Stage', options: opt('new') }, + source: { type: 'select', label: 'Source', options: opt('web') }, + amount: { type: 'currency', label: 'Amount' }, + quantity: { type: 'number', label: 'Quantity' }, + due: { type: 'date', label: 'Due' }, + notes: { type: 'textarea', label: 'Notes' }, + }, +}; + +/** A two-field form: short enough that a wide dialog would only stretch it. */ +const TWO = { + name: 'thing', + fields: { + name: { type: 'text', label: 'Name' }, + code: { type: 'text', label: 'Code' }, + }, +}; + +const ds = (def: unknown) => ({ + getObjectSchema: vi.fn().mockResolvedValue(def), + findOne: vi.fn().mockResolvedValue({}), + find: vi.fn().mockResolvedValue({ data: [] }), + create: vi.fn(), + update: vi.fn(), +}); + +/** Mount the dialog, wait for its fields, and read its size and grid. */ +async function openDialog(def: { fields: Record }, extra: Record = {}) { + const first = Object.keys(def.fields)[0]; + render( + , + ); + await waitFor(() => { + if (!document.body.querySelector(`[name="${first}"]`)) throw new Error('form not drawn yet'); + }); + const dialog = document.body.querySelector('[role="dialog"]') as HTMLElement; + const form = document.body.querySelector('form') as HTMLElement; + const grid = form.querySelector('.grid') as HTMLElement | null; + /** The col-span classes on the grid cell that holds `name`'s control. */ + const spanOf = (name: string): string[] => { + const control = form.querySelector(`[name="${name}"]`); + let cell = control as HTMLElement | null; + while (cell && cell.parentElement !== grid) cell = cell.parentElement; + if (!cell) throw new Error(`no grid cell holds ${name}`); + return cell.className.split(/\s+/).filter((c) => c.includes('col-span')); + }; + return { dialog, grid, spanOf }; +} + +const DEFAULT_GRID = 'grid gap-4 grid-cols-1 @md:grid-cols-2'; +/** objectui#12548's per-section grid, drawn when the sections' counts do not all fit one grid. */ +const TWELVE_TRACK_GRID = 'grid gap-4 grid-cols-1 @md:grid-cols-12'; + +describe('a dialog with no authored layout (objectui#12521)', () => { + it('twelve fields: a bounded xl dialog, a two-column grid from @md, one cell per field, the textarea across the row', async () => { + const { dialog, grid, spanOf } = await openDialog(TWELVE); + + expect(dialog.className).toContain('sm:max-w-5xl'); + expect(dialog.className).not.toContain('sm:max-w-[95vw]'); + + expect(grid?.className).toBe(DEFAULT_GRID); + + // Fields of one kind share one width: every non-wide field is one cell. + for (const name of Object.keys(TWELVE.fields).filter((n) => n !== 'notes')) { + expect(spanOf(name), name).toEqual([]); + } + // The wide kind spans both columns. + expect(spanOf('notes')).toEqual(['@md:col-span-2']); + }); + + it('a short form gets the lg bound, not a dialog wider than its fields', async () => { + const { dialog, grid } = await openDialog(TWO); + expect(dialog.className).toContain('sm:max-w-2xl'); + expect(grid?.className).toBe(DEFAULT_GRID); + }); + + it('below sm the grid is one column, and the dialog is still the phone sheet', async () => { + const { dialog, grid } = await openDialog(TWELVE); + // Every multi-column tier is a container query from @md up; the base is one column. + const tokens = (grid?.className ?? '').split(/\s+/); + expect(tokens).toContain('grid-cols-1'); + expect(tokens.filter((t) => /grid-cols-(?!1$)/.test(t)).every((t) => t.startsWith('@md:'))).toBe(true); + // The phone sheet's own classes are untouched. + expect(dialog.className).toContain('h-[100dvh]'); + expect(dialog.className).toContain('inset-0'); + }); + + it('object fieldGroups with no layout get the same two-column default', async () => { + const grouped = { + ...TWELVE, + fieldGroups: [ + { key: 'main', label: 'Main' }, + { key: 'more', label: 'More' }, + ], + fields: Object.fromEntries( + Object.entries(TWELVE.fields).map(([k, f], i) => [k, { ...f, group: i < 6 ? 'main' : 'more' }]), + ), + }; + const { dialog, grid, spanOf } = await openDialog(grouped); + expect(dialog.className).toContain('sm:max-w-5xl'); + expect(grid?.className).toBe(DEFAULT_GRID); + expect(spanOf('name')).toEqual([]); + expect(spanOf('notes')).toEqual(['@md:col-span-2']); + }); + + it('sections that declare no columns anywhere get the two-column default too', async () => { + const { grid, spanOf } = await openDialog(TWELVE, { + sections: [ + { name: 'a', label: 'A', fields: ['name', 'code', 'email', 'phone', 'website', 'status'] }, + { name: 'b', label: 'B', fields: ['stage', 'source', 'amount', 'quantity', 'due', 'notes'] }, + ], + }); + expect(grid?.className).toBe(DEFAULT_GRID); + expect(spanOf('name')).toEqual([]); + expect(spanOf('notes')).toEqual(['@md:col-span-2']); + }); +}); + +describe('an authored layout is used as written (objectui#12521)', () => { + it("an authored modalSize: 'full' is honoured", async () => { + const { dialog } = await openDialog(TWELVE, { modalSize: 'full' }); + expect(dialog.className).toContain('sm:max-w-[95vw]'); + }); + + it('an authored columns: 1 is honoured: one stacked column, no grid tiers', async () => { + const { grid } = await openDialog(TWELVE, { columns: 1 }); + expect(grid).toBeNull(); + expect(document.body.querySelector('[class*="@md:grid-cols-2"]')).toBeNull(); + }); + + it("a field's authored span: 'full' takes the row in the default grid", async () => { + const { spanOf } = await openDialog(TWELVE, { + sections: [{ name: 'a', label: 'A', fields: [{ field: 'name', span: 'full' }, 'code', 'email'] }], + }); + expect(spanOf('name')).toEqual(['@md:col-span-2']); + expect(spanOf('code')).toEqual([]); + }); + + it('CONTROL: an authored multi-section layout keeps every grid class and col-span main draws', async () => { + // The hotcrm lead form's shape: a two-column section and a three-column + // one. These are the classes objectui#12548's per-section grid draws on + // `main` — the defaults of objectui#12521 reach none of them. Only the + // size moves: no `modalSize` is authored, so the derived bound applies, + // and twelve fields on a three-wide grid are what used to derive `full`. + const { dialog, grid, spanOf } = await openDialog(TWELVE, { + sections: [ + { name: 'a', label: 'A', columns: 2, fields: ['name', { field: 'code', span: 'full' }, 'email', 'phone', 'website'] }, + { name: 'b', label: 'B', columns: 3, fields: ['status', 'stage', 'source', 'amount', 'quantity', 'due', 'notes'] }, + ], + }); + expect(grid?.className).toBe(TWELVE_TRACK_GRID); + expect(spanOf('name')).toEqual(['@md:col-span-6']); + expect(spanOf('code')).toEqual(['@md:col-span-12']); + expect(spanOf('status')).toEqual(['@md:col-span-6', '@2xl:col-span-4']); + expect(dialog.className).toContain('sm:max-w-5xl'); + expect(dialog.className).not.toContain('sm:max-w-[95vw]'); + }); +}); + +describe('section density: undeclared is the dialog\'s choice, declared is drawn as written (objectui#12521)', () => { + it('an undeclared section draws two per row from @md, and a declared columns: 1 section draws one', async () => { + const { grid, spanOf } = await openDialog(TWELVE, { + sections: [ + { name: 'a', label: 'A', fields: ['name', 'code', 'email'] }, + { name: 'b', label: 'B', columns: 1, fields: ['phone', 'website'] }, + ], + }); + // Both counts fit a two-wide grid, so objectui#12548's rule keeps it. + expect(grid?.className).toBe(DEFAULT_GRID); + expect(spanOf('name')).toEqual([]); + expect(spanOf('code')).toEqual([]); + expect(spanOf('phone')).toEqual(['@md:col-span-2']); + expect(spanOf('website')).toEqual(['@md:col-span-2']); + }); + + it('beside a declared columns: 3 section, an undeclared section draws two per row on the 12-track grid', async () => { + const { grid, spanOf } = await openDialog(TWELVE, { + sections: [ + { name: 'a', label: 'A', fields: ['name', 'code', 'email', 'notes'] }, + { name: 'b', label: 'B', columns: 3, fields: ['status', 'stage', 'source'] }, + ], + }); + expect(grid?.className).toBe(TWELVE_TRACK_GRID); + // Two per row at every tier from @md: six twelfths each. + expect(spanOf('name')).toEqual(['@md:col-span-6']); + expect(spanOf('email')).toEqual(['@md:col-span-6']); + // A wide kind still takes the row. + expect(spanOf('notes')).toEqual(['@md:col-span-12']); + // The declared three: two per row at @md, three from @2xl. + expect(spanOf('status')).toEqual(['@md:col-span-6', '@2xl:col-span-4']); + }); + + it('a declared columns: 3 section draws three per row', async () => { + const { grid, spanOf } = await openDialog(TWELVE, { + sections: [{ name: 'b', label: 'B', columns: 3, fields: ['name', 'code', 'email', 'phone', 'website', 'status'] }], + }); + expect(grid?.className).toBe('grid gap-4 grid-cols-1 @md:grid-cols-2 @2xl:grid-cols-3'); + for (const name of ['name', 'code', 'email', 'phone', 'website', 'status']) { + expect(spanOf(name), name).toEqual([]); + } + }); + + it('a declared columns: 1 is never read as undeclared: every section at 1 — what the spec\'s parse writes today — draws one column', async () => { + const { grid } = await openDialog(TWELVE, { + sections: [ + { name: 'a', label: 'A', columns: 1, fields: ['name', 'code', 'email'] }, + { name: 'b', label: 'B', columns: 1, fields: ['phone', 'website'] }, + ], + }); + expect(grid).toBeNull(); + expect(document.body.querySelector('[class*="@md:grid-cols-"]')).toBeNull(); + }); + + it('CONTROL: under a declared form columns, an undeclared section fills the form\'s count as before', async () => { + const { grid, spanOf } = await openDialog(TWELVE, { + columns: 3, + sections: [{ name: 'a', label: 'A', fields: ['name', 'code', 'email', 'phone', 'website', 'status'] }], + }); + expect(grid?.className).toBe('grid gap-4 grid-cols-1 @md:grid-cols-2 @2xl:grid-cols-3'); + expect(spanOf('name')).toEqual([]); + }); +}); diff --git a/packages/plugin-form/src/autoLayout.ts b/packages/plugin-form/src/autoLayout.ts index 2635fef127..afd645b53a 100644 --- a/packages/plugin-form/src/autoLayout.ts +++ b/packages/plugin-form/src/autoLayout.ts @@ -460,18 +460,23 @@ export function sectionFormLayout( } /** - * Infer an appropriate modal size based on the number of layout columns. - * Used to auto-upgrade the modal width when auto-layout detects multi-column forms. + * Infer the modal size for a dialog whose author declared none, from the + * number of layout columns it will show. * * Mapping: - * - 1 column → 'default' (max-w-lg) - * - 2 columns → 'xl' (max-w-5xl) - * - 3+ columns → 'full' (max-w-[95vw]) + * - 0–1 column → 'lg' (max-w-2xl) + * - 2+ columns → 'xl' (max-w-5xl) + * + * ⛔ Never 'full' (objectui#12521): a derived dialog is bounded, and its width + * follows its content (the objectui#12080 ruling). 'full' (95vw) stays + * available to an author who declares `modalSize: 'full'`. The bound changes + * no grid class an authored layout carries: the field grid's tiers are + * container queries, so an authored section draws as many of its columns as + * the bounded dialog's width admits. */ export function inferModalSize(columns: number): 'sm' | 'default' | 'lg' | 'xl' | 'full' { - if (columns <= 1) return 'default'; - if (columns === 2) return 'xl'; - return 'full'; + if (columns <= 1) return 'lg'; + return 'xl'; } /**