diff --git a/.changeset/tabbed-modal-keeps-every-tab.md b/.changeset/tabbed-modal-keeps-every-tab.md new file mode 100644 index 0000000000..c67ab60fee --- /dev/null +++ b/.changeset/tabbed-modal-keeps-every-tab.md @@ -0,0 +1,35 @@ +--- +"@object-ui/types": patch +"@object-ui/components": patch +"@object-ui/plugin-form": patch +--- + +fix(form): a tabbed/sectioned create-edit form no longer loses the tabs you are not looking at (#2959, #2153) + +The explicit-`sections` path rendered one `SchemaRenderer` — one react-hook-form +instance and one `
` element — **per section**, all sharing the same +`formId`. Two failures compounded: + +1. the footer submit button (`form={formId}`) can only be associated with the + **first** of those forms, so section 2+ never reached the payload; and +2. in the `tabbed` variant Radix unmounted the inactive panel, destroying that + tab's form state outright. + +Reported flow (HotCRM, 3 tabs, required `description` on tab 3): fill tab 1 → +submit → server 400 `description is required` → switch to tab 3, fill it → +submit → the server now reports `subject; description; status; priority` **all** +missing, because the second submit's body had lost every earlier value. + +`ModalForm` (stacked and `contentLayout: 'tabbed'`) and `TabbedForm` now render +ONE form for all sections, matching `ObjectForm` / `DrawerForm`. Stacked sections +use the existing inline `section-divider` header (which now also renders the +section's `description`); tabbed sections go through a new +`FormSchema.fieldTabs` (+ `defaultFieldTab`, `fieldTabsPosition`) that the form +renderer distributes into **force-mounted** Radix panels — CSS-hidden rather +than unmounted, since react-hook-form skips validation for unmounted fields, +which is how a required field on a tab nobody opened used to sail past the +client and come back as a server 400. + +Validation feedback now points at the tab: a rejected field activates its tab and +every tab holding one is marked on its trigger, for client-side rules and server +`fields[]` rejections alike. diff --git a/content/docs/plugins/plugin-form.mdx b/content/docs/plugins/plugin-form.mdx index f8c403d8da..5af61278f7 100644 --- a/content/docs/plugins/plugin-form.mdx +++ b/content/docs/plugins/plugin-form.mdx @@ -62,6 +62,35 @@ interface FormField { } ``` +### Tabbed field layout (`fieldTabs`) + +A sectioned form stays **one** form: declare the tabs on it and the renderer +distributes the fields into panels, instead of rendering a form per section +(which strands every section but the first outside the submit, and lets an +inactive tab unmount along with its values). + +```plaintext +{ + type: 'form', + fields: [/* every tab's fields, in one flat list */], + fieldTabs: [ + { key: 'basics', label: 'Basics', fields: ['subject', 'status'] }, + { key: 'detail', label: 'Detail', description: 'Anything else', fields: ['description'] }, + ], + defaultFieldTab?: 'basics', // defaults to the first tab + fieldTabsPosition?: 'top', // 'top' | 'bottom' | 'left' | 'right' +} +``` + +- Panels are force-mounted and only CSS-hidden, so a tab the user leaves keeps + its values **and** its validation. +- A failed submit activates the tab holding the first offending field and marks + every tab with a rejected field — client rules and server `fields[]` alike. +- Fields no tab claims render above the tab strip rather than disappearing. +- Needs at least two tabs; ignored when the form uses `children`. + +`ModalForm` (`contentLayout: 'tabbed'`) and `TabbedForm` build on this. + ## Usage ### Auto-registration (Side-effect Import) diff --git a/packages/components/src/renderers/form/__tests__/form-field-tabs.test.tsx b/packages/components/src/renderers/form/__tests__/form-field-tabs.test.tsx new file mode 100644 index 0000000000..bab137222a --- /dev/null +++ b/packages/components/src/renderers/form/__tests__/form-field-tabs.test.tsx @@ -0,0 +1,181 @@ +/** + * 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. + */ + +/** + * #2959: `FormSchema.fieldTabs` — a tabbed field layout inside ONE form. + * + * The contract these pin: + * - one `` / react-hook-form instance for every tab; + * - panels are force-mounted (only CSS-hidden), so a tab the user leaves keeps + * its values AND its validation — react-hook-form skips *unmounted* fields, + * which is how a required field on an unopened tab used to reach the server; + * - a failed submit activates the tab holding the first offending field and + * marks every tab that has one; + * - a field no tab claims is still rendered (never silently dropped). + */ + +import { describe, it, expect, beforeAll, beforeEach, afterEach, vi } from 'vitest'; +import { render, screen, fireEvent, waitFor, cleanup } from '@testing-library/react'; +import { ComponentRegistry } from '@object-ui/core'; +import { toast } from '../../../ui/sonner'; + +beforeAll(async () => { + await import('../../../renderers'); +}, 30000); + +beforeEach(() => { + vi.spyOn(toast, 'error').mockImplementation(() => 'id' as any); + if (!(Element.prototype as any).scrollIntoView) { + (Element.prototype as any).scrollIntoView = () => {}; + } +}); + +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +const FIELDS = [ + { name: 'subject', label: 'Subject', type: 'input', required: true }, + { name: 'status', label: 'Status', type: 'input' }, + { name: 'description', label: 'Description', type: 'input', required: true }, +]; + +const TABS = [ + { key: 'basics', label: 'Basics', fields: ['subject', 'status'] }, + { key: 'detail', label: 'Detail', description: 'Anything else', fields: ['description'] }, +]; + +function renderTabbedForm(overrides: Record = {}) { + const onSubmit = vi.fn(); + const Form = ComponentRegistry.get('form')!; + const utils = render( + , + ); + return { ...utils, onSubmit }; +} + +const submit = () => fireEvent.click(screen.getByRole('button', { name: /create/i })); +const fill = (name: string, value: string) => { + const input = document.body.querySelector(`[data-field="${name}"] input`)!; + fireEvent.change(input, { target: { value } }); +}; +const tab = (name: RegExp) => screen.getByRole('tab', { name }); + +describe('form renderer — fieldTabs (#2959)', () => { + it('renders one form and one panel per tab, with every field mounted', () => { + const { container } = renderTabbedForm(); + + expect(container.querySelectorAll('form')).toHaveLength(1); + expect(screen.getAllByRole('tab')).toHaveLength(2); + // Both panels exist up front — the inactive one is hidden by CSS, not + // unmounted, so its fields are present and registered. + expect(screen.getByTestId('form-tab-panel:basics')).toBeTruthy(); + expect(screen.getByTestId('form-tab-panel:detail')).toBeTruthy(); + for (const name of ['subject', 'status', 'description']) { + expect(container.querySelector(`[data-field="${name}"]`)).toBeTruthy(); + } + // The tab's blurb rides along with its panel. + expect(screen.getByText('Anything else')).toBeTruthy(); + }); + + it('marks the inactive panel with data-state=inactive (CSS-hidden, still mounted)', () => { + renderTabbedForm(); + expect(screen.getByTestId('form-tab-panel:basics')).toHaveAttribute('data-state', 'active'); + expect(screen.getByTestId('form-tab-panel:detail')).toHaveAttribute('data-state', 'inactive'); + expect(screen.getByTestId('form-tab-panel:detail').className).toContain( + 'data-[state=inactive]:hidden', + ); + }); + + it('submits values entered across different tabs', async () => { + const { onSubmit } = renderTabbedForm(); + + fill('subject', 'From tab 1'); + fireEvent.click(tab(/Detail/)); + fill('description', 'From tab 2'); + submit(); + + await waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(1)); + expect(onSubmit.mock.calls[0][0]).toMatchObject({ + subject: 'From tab 1', + description: 'From tab 2', + }); + }); + + it('validates fields on tabs the user never opened, and jumps to the offender', async () => { + const { onSubmit } = renderTabbedForm(); + + fill('subject', 'Only tab 1 filled'); + submit(); + + // `description` is required and lives on a tab that was never opened. + await waitFor(() => expect(tab(/Detail/)).toHaveAttribute('data-state', 'active')); + expect(onSubmit).not.toHaveBeenCalled(); + expect(tab(/Detail/)).toHaveAttribute('data-error', 'true'); + expect(tab(/Basics/)).not.toHaveAttribute('data-error'); + }); + + it('honors defaultFieldTab', () => { + renderTabbedForm({ defaultFieldTab: 'detail' }); + expect(screen.getByTestId('form-tab-panel:detail')).toHaveAttribute('data-state', 'active'); + }); + + it('keeps the tab schema keys off the DOM element', () => { + // Callers/the SDUI dispatch spread the whole form node at the top level too, + // so a FormSchema key that isn't consumed leaks onto the DOM and React logs + // "does not recognize the `fieldTabs` prop on a DOM element". + const Form = ComponentRegistry.get('form')!; + const { container } = render( + , + ); + const form = container.querySelector('form')!; + for (const attr of ['fieldtabs', 'defaultfieldtab', 'fieldtabsposition']) { + expect(form.hasAttribute(attr)).toBe(false); + } + }); + + it('renders fields no tab claims instead of dropping them', () => { + const { container } = renderTabbedForm({ + fieldTabs: [ + { key: 'basics', label: 'Basics', fields: ['subject'] }, + { key: 'detail', label: 'Detail', fields: ['description'] }, + ], + }); + // `status` belongs to no tab — it must still be in the DOM, outside the panels. + const status = container.querySelector('[data-field="status"]')!; + expect(status).toBeTruthy(); + expect(status.closest('[role="tabpanel"]')).toBeNull(); + }); + + it('falls back to a plain field list for a single tab', () => { + renderTabbedForm({ fieldTabs: [{ key: 'only', label: 'Only', fields: ['subject'] }] }); + expect(screen.queryAllByRole('tab')).toHaveLength(0); + // …and every field still renders, not just the one the lone tab claimed. + expect(document.body.querySelector('[data-field="description"]')).toBeTruthy(); + }); +}); diff --git a/packages/components/src/renderers/form/form.tsx b/packages/components/src/renderers/form/form.tsx index 17f1e6fd07..a78c03df26 100644 --- a/packages/components/src/renderers/form/form.tsx +++ b/packages/components/src/renderers/form/form.tsx @@ -7,7 +7,7 @@ */ import { ComponentRegistry, resolveFieldRuleState, evalFieldPredicate, resolveCascadingOptions, isValueStillOffered } from '@object-ui/core'; -import type { FormSchema, FormField as FormFieldConfig, ValidationRule, FieldCondition, SelectOption } from '@object-ui/types'; +import type { FormSchema, FormField as FormFieldConfig, FormFieldTab, ValidationRule, FieldCondition, SelectOption } from '@object-ui/types'; import { useForm } from 'react-hook-form'; import { Form, FormField, FormItem, FormLabel, FormControl, FormMessage, FormDescription } from '../../ui/form'; import { Button } from '../../ui/button'; @@ -23,6 +23,7 @@ import { SelectItem } from '../../ui/select'; import { renderChildren } from '../../lib/utils'; +import { Tabs, TabsContent, TabsList, TabsTrigger } from '../../ui/tabs'; import { Alert, AlertDescription } from '../../ui/alert'; import { toast } from '../../ui/sonner'; import { AlertCircle, ChevronDown, ChevronRight, Loader2, Maximize2, Check, X } from 'lucide-react'; @@ -34,14 +35,15 @@ import { createSafeTranslation } from '@object-ui/i18n'; /** Inline section header rendered as a virtual field inside a flat SchemaRenderer field list. * Collapsibility is controlled externally (collapsed state lives in DrawerForm). */ -function SectionDivider({ label, collapsible, collapsed, onToggle, className }: { +function SectionDivider({ label, description, collapsible, collapsed, onToggle, className }: { label?: string; + description?: string; collapsible?: boolean; collapsed?: boolean; onToggle?: () => void; className?: string; }) { - if (!label) return null; + if (!label && !description) return null; return (
-
- {collapsible && ( - collapsed - ? - : - )} - {label} -
+ {label && ( +
+ {collapsible && ( + collapsed + ? + : + )} + {label} +
+ )} + {/* A section's authored blurb. Carried on the divider because a sectioned + form is ONE form with inline headers — there is no per-section wrapper + left to render it (see FormSchema.fieldTabs / objectui#2959). */} + {description && ( +

{description}

+ )}
); } @@ -333,6 +343,14 @@ ComponentRegistry.register('form', const [isSubmitting, setIsSubmitting] = React.useState(false); const [submitError, setSubmitError] = React.useState(null); + // Active `fieldTabs` panel. Undefined until something picks one — the + // effective tab falls back to `defaultFieldTab`, then to the first tab. + // Which tab of an in-progress form is showing is presentational and dies + // with the form, so it stays component state (AGENTS.md #8). + const [pickedFieldTab, setPickedFieldTab] = React.useState(undefined); + // Field names the last submit attempt rejected (client rules or server + // `fields[]`), used to mark the tabs that hold them. + const [rejectedFieldNames, setRejectedFieldNames] = React.useState([]); // Live snapshot of all form values — subscribes to every change so // field-level CEL rules (visibleWhen/readonlyWhen/requiredWhen) re-evaluate @@ -408,6 +426,83 @@ ComponentRegistry.register('form', return m; }, [fields]); + // --- Tabbed field layout (#2959) --------------------------------------- + // `fieldTabs` spreads THIS form's fields across tab panels. Crucially there + // is still exactly ONE / react-hook-form instance: the panels are + // force-mounted and merely CSS-hidden, so a tab the user navigated away from + // keeps BOTH its values and its validation. (Rendering one form per tab lost + // every non-active tab's input — the footer submit button can only be + // associated with a single form — and unmounting a tab's fields makes + // react-hook-form skip their rules, which let a required field on a tab + // nobody opened sail past the client and return as a server 400.) + const fieldTabs = React.useMemo(() => { + const declared = schema.fieldTabs; + if (schema.children || !Array.isArray(declared)) return null; + const usable = declared.filter((t) => t && typeof t.key === 'string'); + return usable.length > 1 ? usable : null; + }, [schema.fieldTabs, schema.children]); + + /** Field name → the tab that owns it (first claim wins). */ + const tabKeyByFieldName = React.useMemo(() => { + const m = new Map(); + for (const tab of fieldTabs ?? []) { + for (const name of tab.fields ?? []) if (!m.has(name)) m.set(name, tab.key); + } + return m; + }, [fieldTabs]); + + const activeFieldTab = React.useMemo(() => { + if (!fieldTabs?.length) return undefined; + const keys = fieldTabs.map((t) => t.key); + if (pickedFieldTab && keys.includes(pickedFieldTab)) return pickedFieldTab; + if (schema.defaultFieldTab && keys.includes(schema.defaultFieldTab)) return schema.defaultFieldTab; + return keys[0]; + }, [fieldTabs, pickedFieldTab, schema.defaultFieldTab]); + + // Resolve each tab's declared field names against the form's field list. + const fieldTabGroups = React.useMemo(() => { + if (!fieldTabs) return null; + const byName = new Map(); + for (const f of fields as FormFieldConfig[]) { + if (f?.name && !byName.has(f.name)) byName.set(f.name, f); + } + return fieldTabs.map((tab) => ({ + key: tab.key, + label: tab.label || tab.key, + description: tab.description, + containerClass: tab.containerClass, + fields: (tab.fields ?? []) + .map((name) => byName.get(name)) + .filter((f): f is FormFieldConfig => Boolean(f)), + })); + }, [fieldTabs, fields]); + + // A field no tab claimed must not vanish — it renders above the tab strip, + // visible from every tab. + const untabbedFields = React.useMemo(() => { + if (!fieldTabGroups) return [] as FormFieldConfig[]; + const claimed = new Set(); + for (const group of fieldTabGroups) for (const f of group.fields) claimed.add(f.name); + return (fields as FormFieldConfig[]).filter((f) => f?.name && !claimed.has(f.name)); + }, [fieldTabGroups, fields]); + + // Tabs holding a field the LAST submit rejected, so their trigger can carry + // a marker — otherwise a rejection on a background tab is invisible and the + // user just sees a submit that does nothing. Fed from `announceFieldErrors`, + // which covers both referees (client rules and server `fields[]`), and reset + // when a submit gets past validation. Kept as state rather than derived from + // `formState.errors` because react-hook-form re-renders the erroring field's + // own Controller, not necessarily this component. + const erroredFieldTabs = React.useMemo(() => { + const out = new Set(); + if (!fieldTabs) return out; + for (const name of rejectedFieldNames) { + const key = tabKeyByFieldName.get(name); + if (key) out.add(key); + } + return out; + }, [fieldTabs, tabKeyByFieldName, rejectedFieldNames]); + // Cascade clear (#2284): when a select/radio's option list narrows — because a // controlling field changed or a role/context predicate flipped — a previously // chosen value may no longer be offered. Drop it so the form never submits a @@ -468,6 +563,8 @@ ComponentRegistry.register('form', lastDefaultsKey.current = key; form.reset(defaultValues); baselineRef.current = (defaultValues ?? {}) as Record; + // Fresh values, so last attempt's rejected-field markers no longer apply. + setRejectedFieldNames([]); // A fresh reset is by definition pristine — clear any stale dirty signal // (e.g. an edit-mode record that just finished loading). onDirtyChangeProp?.(false); @@ -502,20 +599,41 @@ ComponentRegistry.register('form', return () => subscription.unsubscribe(); }, [form, onDirtyChangeProp]); + /** + * Scroll a field into view and focus a control inside it. The field wrapper + * carries `data-field` (FormItem), so this reaches custom widgets that RHF's + * own focus cannot (#2793). + */ + const revealField = (name: string) => { + const root: ParentNode = formRef.current ?? document; + const escaped = + typeof CSS !== 'undefined' && typeof CSS.escape === 'function' + ? CSS.escape(name) + : name.replace(/["\\]/g, '\\$&'); + const target = root.querySelector(`[data-field="${escaped}"]`); + if (target) { + target.scrollIntoView?.({ behavior: 'smooth', block: 'center' }); + const focusable = target.querySelector( + 'input:not([type="hidden"]), select, textarea, button, [tabindex]:not([tabindex="-1"]), [contenteditable="true"]', + ); + focusable?.focus?.({ preventScroll: true }); + } + }; + /** * Tell the user WHICH fields are wrong, wherever the verdict came from. * - * Toast naming the offending fields, then scroll the FIRST of them into - * view (in declared/visual order, not the caller's key order) and focus a - * control inside it — the field wrapper carries `data-field` (FormItem), so - * this reaches custom widgets that RHF's own focus cannot (#2793). + * Toast naming the offending fields, then reveal the FIRST of them (in + * declared/visual order, not the caller's key order) — activating its tab + * first when it sits on a background one. * - * Extracted from the client-side invalid handler so the SERVER-rejection - * path gets identical treatment. The two failures are the same event as far - * as the person filling the form is concerned; only the referee differs. + * Shared by the client-side invalid handler and the SERVER-rejection path: + * the two failures are the same event as far as the person filling the form + * is concerned; only the referee differs. */ const announceFieldErrors = (names: string[]) => { if (names.length === 0) return; + setRejectedFieldNames(names); const labels = names.map((n) => fieldLabelByName[n] || n); const MAX = 3; const fieldsText = labels.slice(0, MAX).join('、') + (labels.length > MAX ? '…' : ''); @@ -526,25 +644,31 @@ ComponentRegistry.register('form', (fields as FormFieldConfig[]) .map((f) => f?.name) .find((n): n is string => Boolean(n) && errored.has(n as string)) ?? names[0]; - const root: ParentNode = formRef.current ?? document; - const escaped = - typeof CSS !== 'undefined' && typeof CSS.escape === 'function' - ? CSS.escape(firstName) - : firstName.replace(/["\\]/g, '\\$&'); - const target = root.querySelector(`[data-field="${escaped}"]`); - if (target) { - target.scrollIntoView?.({ behavior: 'smooth', block: 'center' }); - const focusable = target.querySelector( - 'input:not([type="hidden"]), select, textarea, button, [tabindex]:not([tabindex="-1"]), [contenteditable="true"]', - ); - focusable?.focus?.({ preventScroll: true }); + + // In a tabbed layout the offending field may sit on a tab the user is not + // looking at — naming it is useless if they can't see it. Activate its tab + // first, then reveal it once that panel has actually been painted (it is + // display:none until then, so scroll/focus would no-op). + const tabKey = tabKeyByFieldName.get(firstName); + if (tabKey && tabKey !== activeFieldTab) { + setPickedFieldTab(tabKey); + if (typeof requestAnimationFrame === 'function') { + requestAnimationFrame(() => revealField(firstName)); + } else { + setTimeout(() => revealField(firstName), 0); + } + return; } + revealField(firstName); }; // Handle form submission const handleSubmit = form.handleSubmit(async (data) => { setIsSubmitting(true); setSubmitError(null); + // This attempt cleared client validation — drop the previous attempt's + // tab markers (the server may re-add its own below). + setRejectedFieldNames([]); // Defensive check: If data is an Event, use getValues() let formData = data; @@ -684,10 +808,314 @@ ComponentRegistry.register('form', columns === 3 ? 'md:grid-cols-2 lg:grid-cols-3' : 'md:grid-cols-2 lg:grid-cols-3 xl:grid-cols-4'; - const gridClass = columns > 1 + const gridClass = columns > 1 ? cn('grid gap-4', gridColsClass) : 'space-y-4'; + // Every field container (flat, or one per tab panel) lays its fields out on + // the same grid, so per-field `colSpan` means the same thing on every tab. + const fieldGridClass = schema.fieldContainerClass || gridClass; + const fieldTabsPosition = schema.fieldTabsPosition || 'top'; + const isVerticalFieldTabs = fieldTabsPosition === 'left' || fieldTabsPosition === 'right'; + + // --- Field rendering --------------------------------------------------- + // Renders ONE field row (or a virtual section divider). Hoisted out of the + // field loop so a tabbed layout can place the very same row inside a tab + // panel — every field, on every tab, belongs to this one form instance. + const renderFormField = (field: FormFieldConfig): React.ReactNode => { + const { + name, + label, + description, + type = 'input', + required: staticRequired = false, + disabled: fieldDisabled = false, + validation = {}, + condition, + colSpan, + hidden, + widget, + visibleOn, + readonly: staticReadonly, + visibleWhen, + readonlyWhen, + requiredWhen, + ...fieldProps + } = field; + + // Skip hidden fields + if (hidden) return null; + + // Legacy `condition: { field, equals/notEquals/in }` — translated + // to CEL and evaluated on the canonical engine over the seeded + // live record (issue #1584), so it agrees with `visibleWhen` and + // the server. Fail-open (a broken predicate shows the field), + // matching the CEL rules below. + const legacyConditionCel = legacyConditionToCel(condition); + if (legacyConditionCel && !evalFieldPredicate(legacyConditionCel, ruleRecord, true)) { + return null; + } + + // Field-level CEL conditional rules (B2). Evaluated reactively + // against the live record via the canonical engine — same + // dialect the server enforces (requiredWhen / readonlyWhen), so + // the UX and the persisted verdict agree. A field with no rules + // resolves to its static flags unchanged. + const ruleState = resolveFieldRuleState( + { visibleWhen, readonlyWhen, requiredWhen }, + ruleRecord, + { required: staticRequired, readonly: staticReadonly === true }, + ); + if (!ruleState.visible) return null; + + // View-level conditional visibility — spec FormField.visibleOn, + // authored on the form view (not the object field). Same + // canonical CEL engine and record scope as visibleWhen; both + // the bare-string and `{ dialect, source }` wire shapes are + // accepted, and a broken predicate fails open (#2212). + if (visibleOn != null && !evalFieldPredicate(visibleOn, ruleRecord, true)) { + return null; + } + const required = ruleState.required; + const readonly = ruleState.readonly; + + // Section divider — renders a collapsible FormSection header inline + // so all fields share the same form instance (enables cross-section conditions). + if (type === 'section-divider') { + const fp = fieldProps as any; + return ( + + ); + } + + // Build validation rules + const rules: any = { + ...validation, + }; + + if (required) { + rules.required = typeof validation.required === 'string' + ? validation.required + : t('validation.required', { field: label || name }); + } + + // Localize the standard validation messages emitted by + // buildValidationRules. Each such rule carries a `messageKey` + // and leaves `message` undefined for the auto-generated case + // (a field-authored message is a string and is left untouched); + // we fill the blanks through i18n so they track the label's + // language. A fresh object avoids mutating the shared rule. + const localizeRule = ( + rule: any, + interp?: (r: any) => Record, + ) => { + if ( + rule && typeof rule === 'object' && + rule.message == null && typeof rule.messageKey === 'string' + ) { + return { + ...rule, + message: t(rule.messageKey, { field: label || name, ...(interp?.(rule)) }), + }; + } + return rule; + }; + if (rules.minLength) rules.minLength = localizeRule(rules.minLength, r => ({ min: r.value })); + if (rules.maxLength) rules.maxLength = localizeRule(rules.maxLength, r => ({ max: r.value })); + if (rules.min) rules.min = localizeRule(rules.min, r => ({ min: r.value })); + if (rules.max) rules.max = localizeRule(rules.max, r => ({ max: r.value })); + if (rules.pattern) rules.pattern = localizeRule(rules.pattern); + + // Use field.id or field.name for stable keys (never use index alone) + const fieldKey = field.id ?? name; + + // Resolve the component type: prefer an explicit form-config + // `widget`, then the object-schema field's own `widget` render + // hint (carried on the field metadata, e.g. `object-ref`, + // `filter-condition`, `recipient-picker`), then the bare `type`. + // The nested-metadata fallback matters because some field + // builders (e.g. the field-group/section layout path in + // ObjectForm) pass the field metadata through without hoisting + // its `widget` to the top-level form-config, which would + // otherwise degrade a picker field to its raw `type` input. + const resolvedType = widget || (fieldProps as any).field?.widget || type; + + // Cascading / role-gated option lists (#2284). For option fields, + // narrow the set by each option's `visibleWhen` (evaluated against + // the live record + `current_user`), and gate the whole control + // while a declared `dependsOn` parent is still empty — surfacing a + // "select the parent first" hint instead of an unfiltered list. + const isOptionField = + resolvedType === 'select' || + resolvedType === 'radio' || + resolvedType === 'multiselect' || + resolvedType === 'checkboxes'; + const rawOptions = (fieldProps as any).options as SelectOption[] | undefined; + // Resolve gating + `visibleWhen` filtering through the shared + // core helper so this pre-filter can't drift from the widgets' + // `useCascadingOptions` hook (#2715). Non-option fields pass + // their options through untouched. + const cascade = isOptionField + ? resolveCascadingOptions(rawOptions, ruleRecord, (field as any).dependsOn, predicateScope) + : null; + const optionGroupGated = cascade?.gated ?? false; + const dependsOnFields = cascade?.dependsOnFields ?? []; + const effectiveOptions = cascade ? cascade.options : rawOptions; + const gatedHint = optionGroupGated + ? `Select ${dependsOnFields.map((fn) => fieldLabelByName[fn] || fn).join(' / ')} first` + : undefined; + + // colSpan classes for grid layout. + // + // When the container uses container-query-based grid classes + // (e.g. `@md:grid-cols-2`), the grid's base is `grid-cols-1` + // on narrow containers. Applying a bare `col-span-2` in that + // state causes CSS grid to synthesize an implicit 2nd column + // track, distorting column widths. We must mirror the same + // container-query prefix on the col-span utilities so they + // only engage once the grid is actually multi-column. + // The effective container is whatever wraps the fields: either + // `fieldContainerClass` (overrides, typically container-query based) + // or the locally-computed `gridClass` (viewport-based). + const containerClass = schema.fieldContainerClass || gridClass; + // Match both container-query (`@md:`) and viewport (`md:`) prefixes. + // Return an explicit, statically-detectable class so Tailwind JIT + // can scan and include it. + const pickSpanClass = (targetCols: number): string => { + const re = /(@)?(sm|md|lg|xl|2xl|3xl|4xl|5xl|6xl|7xl):grid-cols-(\d+)/g; + const matches = Array.from(containerClass.matchAll(re)).map(m => ({ + at: m[1] || '', + bp: m[2], + cols: Number(m[3]), + })); + if (!matches.length) { + // No responsive/container prefix found — bare class is safe + // because the grid is already multi-column at all widths. + if (targetCols === 2) return 'col-span-2'; + if (targetCols === 3) return 'col-span-3'; + return 'col-span-4'; + } + const hit = matches.find(m => m.cols >= targetCols) || matches[matches.length - 1]; + const key = `${hit.at}${hit.bp}:${targetCols}`; + // Explicit literal map so Tailwind JIT discovers these classes. + const table: Record = { + '@sm:2': '@sm:col-span-2', + '@md:2': '@md:col-span-2', + '@lg:2': '@lg:col-span-2', + '@xl:2': '@xl:col-span-2', + '@2xl:2': '@2xl:col-span-2', + '@sm:3': '@sm:col-span-3', + '@md:3': '@md:col-span-3', + '@lg:3': '@lg:col-span-3', + '@xl:3': '@xl:col-span-3', + '@2xl:3': '@2xl:col-span-3', + '@4xl:3': '@4xl:col-span-3', + '@sm:4': '@sm:col-span-4', + '@md:4': '@md:col-span-4', + '@lg:4': '@lg:col-span-4', + '@xl:4': '@xl:col-span-4', + '@2xl:4': '@2xl:col-span-4', + '@4xl:4': '@4xl:col-span-4', + 'sm:2': 'sm:col-span-2', + 'md:2': 'md:col-span-2', + 'lg:2': 'lg:col-span-2', + 'xl:2': 'xl:col-span-2', + 'sm:3': 'sm:col-span-3', + 'md:3': 'md:col-span-3', + 'lg:3': 'lg:col-span-3', + 'xl:3': 'xl:col-span-3', + 'sm:4': 'sm:col-span-4', + 'md:4': 'md:col-span-4', + 'lg:4': 'lg:col-span-4', + 'xl:4': 'xl:col-span-4', + }; + return table[key] || (targetCols === 2 ? 'col-span-2' : targetCols === 3 ? 'col-span-3' : 'col-span-4'); + }; + + const colSpanClass = colSpan && colSpan > 1 + ? colSpan === 2 ? pickSpanClass(2) + : colSpan === 3 ? pickSpanClass(3) + : colSpan >= 4 ? pickSpanClass(4) + : '' + : ''; + + // Metadata-derived stable locator (ADR-0054 C4): the renderer + // emits it from the object + field name so every generated form + // inherits it with zero per-app work. Object prefix omitted when + // the form schema has no owning object. + const fieldTestId = `field:${schema.objectName ? `${schema.objectName}.` : ''}${name}`; + + return ( + ( + + {label && ( + + {label} + {required && ( + + * + + )} + + )} + + {/* Render the actual field component based on resolved type */} + {renderFieldComponent(resolvedType, { + ...fieldProps, + // specialized fields needs raw metadata, but we should traverse down if it exists + // field is the field configuration loop variable + field: (field as any).field || field, + ...formField, + inputType: fieldProps.inputType, + options: isOptionField ? effectiveOptions : fieldProps.options, + placeholder: fieldProps.placeholder ?? (resolvedType === 'select' ? t('common.selectOption') : undefined), + // `disabled` means "not interactive, muted"; `readonly` means + // "shown plainly, not editable" — keep them distinct so widgets + // that implement a real readonly display (e.g. EmailField's + // mailto link) actually receive it instead of always collapsing + // to the grayed-out disabled look. A dependency-gated option + // list (#2284) is disabled until its controlling field is set. + disabled: disabled || fieldDisabled || isSubmitting || optionGroupGated, + readonly, + // Gate hint shown when a dependent option list is still + // waiting on its controlling field (#2284). + emptyHint: gatedHint, + dataSource: contextDataSource, + // Live form values for dependent (cascading) lookups + // (#2215): the widget's `dependsOn` gate + filters must + // re-scope as the user picks the parent field in THIS + // form, not read a stale record snapshot. Forwarded to + // data-source widgets only (see stripRegisteredFieldProps). + dependentValues: ruleRecord, + })} + + {description && ( + {description} + )} + + + )} + /> + ); + }; + // Extract designer-related props and conflicting handlers const { 'data-obj-id': dataObjId, @@ -723,6 +1151,9 @@ ComponentRegistry.register('form', columns: _columns, validationMode: _validationMode, disabled: _disabledProp, + fieldTabs: _fieldTabs, + defaultFieldTab: _defaultFieldTab, + fieldTabsPosition: _fieldTabsPosition, ...formProps } = props; @@ -752,301 +1183,81 @@ ComponentRegistry.register('form',
{renderChildren(schema.children)}
+ ) : fieldTabGroups ? ( + // Tabbed field layout (#2959): one , one react-hook-form + // instance, N force-mounted panels. Inactive panels are CSS-hidden + // (`data-[state=inactive]:hidden`) rather than unmounted, which is + // what keeps their values and their validation alive. + <> + {untabbedFields.length > 0 && ( +
+ {untabbedFields.map(renderFormField)} +
+ )} + + + {fieldTabGroups.map((group) => ( + + {group.label} + {erroredFieldTabs.has(group.key) && ( + + ))} + +
+ {fieldTabGroups.map((group) => ( + + {group.description && ( +

{group.description}

+ )} +
+ {group.fields.map(renderFormField)} +
+ + ))} +
+ + ) : ( // Otherwise render fields from schema -
- {fields.map((field: FormFieldConfig) => { - const { - name, - label, - description, - type = 'input', - required: staticRequired = false, - disabled: fieldDisabled = false, - validation = {}, - condition, - colSpan, - hidden, - widget, - visibleOn, - readonly: staticReadonly, - visibleWhen, - readonlyWhen, - requiredWhen, - ...fieldProps - } = field; - - // Skip hidden fields - if (hidden) return null; - - // Legacy `condition: { field, equals/notEquals/in }` — translated - // to CEL and evaluated on the canonical engine over the seeded - // live record (issue #1584), so it agrees with `visibleWhen` and - // the server. Fail-open (a broken predicate shows the field), - // matching the CEL rules below. - const legacyConditionCel = legacyConditionToCel(condition); - if (legacyConditionCel && !evalFieldPredicate(legacyConditionCel, ruleRecord, true)) { - return null; - } - - // Field-level CEL conditional rules (B2). Evaluated reactively - // against the live record via the canonical engine — same - // dialect the server enforces (requiredWhen / readonlyWhen), so - // the UX and the persisted verdict agree. A field with no rules - // resolves to its static flags unchanged. - const ruleState = resolveFieldRuleState( - { visibleWhen, readonlyWhen, requiredWhen }, - ruleRecord, - { required: staticRequired, readonly: staticReadonly === true }, - ); - if (!ruleState.visible) return null; - - // View-level conditional visibility — spec FormField.visibleOn, - // authored on the form view (not the object field). Same - // canonical CEL engine and record scope as visibleWhen; both - // the bare-string and `{ dialect, source }` wire shapes are - // accepted, and a broken predicate fails open (#2212). - if (visibleOn != null && !evalFieldPredicate(visibleOn, ruleRecord, true)) { - return null; - } - const required = ruleState.required; - const readonly = ruleState.readonly; - - // Section divider — renders a collapsible FormSection header inline - // so all fields share the same form instance (enables cross-section conditions). - if (type === 'section-divider') { - const fp = fieldProps as any; - return ( - - ); - } - - // Build validation rules - const rules: any = { - ...validation, - }; - - if (required) { - rules.required = typeof validation.required === 'string' - ? validation.required - : t('validation.required', { field: label || name }); - } - - // Localize the standard validation messages emitted by - // buildValidationRules. Each such rule carries a `messageKey` - // and leaves `message` undefined for the auto-generated case - // (a field-authored message is a string and is left untouched); - // we fill the blanks through i18n so they track the label's - // language. A fresh object avoids mutating the shared rule. - const localizeRule = ( - rule: any, - interp?: (r: any) => Record, - ) => { - if ( - rule && typeof rule === 'object' && - rule.message == null && typeof rule.messageKey === 'string' - ) { - return { - ...rule, - message: t(rule.messageKey, { field: label || name, ...(interp?.(rule)) }), - }; - } - return rule; - }; - if (rules.minLength) rules.minLength = localizeRule(rules.minLength, r => ({ min: r.value })); - if (rules.maxLength) rules.maxLength = localizeRule(rules.maxLength, r => ({ max: r.value })); - if (rules.min) rules.min = localizeRule(rules.min, r => ({ min: r.value })); - if (rules.max) rules.max = localizeRule(rules.max, r => ({ max: r.value })); - if (rules.pattern) rules.pattern = localizeRule(rules.pattern); - - // Use field.id or field.name for stable keys (never use index alone) - const fieldKey = field.id ?? name; - - // Resolve the component type: prefer an explicit form-config - // `widget`, then the object-schema field's own `widget` render - // hint (carried on the field metadata, e.g. `object-ref`, - // `filter-condition`, `recipient-picker`), then the bare `type`. - // The nested-metadata fallback matters because some field - // builders (e.g. the field-group/section layout path in - // ObjectForm) pass the field metadata through without hoisting - // its `widget` to the top-level form-config, which would - // otherwise degrade a picker field to its raw `type` input. - const resolvedType = widget || (fieldProps as any).field?.widget || type; - - // Cascading / role-gated option lists (#2284). For option fields, - // narrow the set by each option's `visibleWhen` (evaluated against - // the live record + `current_user`), and gate the whole control - // while a declared `dependsOn` parent is still empty — surfacing a - // "select the parent first" hint instead of an unfiltered list. - const isOptionField = - resolvedType === 'select' || - resolvedType === 'radio' || - resolvedType === 'multiselect' || - resolvedType === 'checkboxes'; - const rawOptions = (fieldProps as any).options as SelectOption[] | undefined; - // Resolve gating + `visibleWhen` filtering through the shared - // core helper so this pre-filter can't drift from the widgets' - // `useCascadingOptions` hook (#2715). Non-option fields pass - // their options through untouched. - const cascade = isOptionField - ? resolveCascadingOptions(rawOptions, ruleRecord, (field as any).dependsOn, predicateScope) - : null; - const optionGroupGated = cascade?.gated ?? false; - const dependsOnFields = cascade?.dependsOnFields ?? []; - const effectiveOptions = cascade ? cascade.options : rawOptions; - const gatedHint = optionGroupGated - ? `Select ${dependsOnFields.map((fn) => fieldLabelByName[fn] || fn).join(' / ')} first` - : undefined; - - // colSpan classes for grid layout. - // - // When the container uses container-query-based grid classes - // (e.g. `@md:grid-cols-2`), the grid's base is `grid-cols-1` - // on narrow containers. Applying a bare `col-span-2` in that - // state causes CSS grid to synthesize an implicit 2nd column - // track, distorting column widths. We must mirror the same - // container-query prefix on the col-span utilities so they - // only engage once the grid is actually multi-column. - // The effective container is whatever wraps the fields: either - // `fieldContainerClass` (overrides, typically container-query based) - // or the locally-computed `gridClass` (viewport-based). - const containerClass = schema.fieldContainerClass || gridClass; - // Match both container-query (`@md:`) and viewport (`md:`) prefixes. - // Return an explicit, statically-detectable class so Tailwind JIT - // can scan and include it. - const pickSpanClass = (targetCols: number): string => { - const re = /(@)?(sm|md|lg|xl|2xl|3xl|4xl|5xl|6xl|7xl):grid-cols-(\d+)/g; - const matches = Array.from(containerClass.matchAll(re)).map(m => ({ - at: m[1] || '', - bp: m[2], - cols: Number(m[3]), - })); - if (!matches.length) { - // No responsive/container prefix found — bare class is safe - // because the grid is already multi-column at all widths. - if (targetCols === 2) return 'col-span-2'; - if (targetCols === 3) return 'col-span-3'; - return 'col-span-4'; - } - const hit = matches.find(m => m.cols >= targetCols) || matches[matches.length - 1]; - const key = `${hit.at}${hit.bp}:${targetCols}`; - // Explicit literal map so Tailwind JIT discovers these classes. - const table: Record = { - '@sm:2': '@sm:col-span-2', - '@md:2': '@md:col-span-2', - '@lg:2': '@lg:col-span-2', - '@xl:2': '@xl:col-span-2', - '@2xl:2': '@2xl:col-span-2', - '@sm:3': '@sm:col-span-3', - '@md:3': '@md:col-span-3', - '@lg:3': '@lg:col-span-3', - '@xl:3': '@xl:col-span-3', - '@2xl:3': '@2xl:col-span-3', - '@4xl:3': '@4xl:col-span-3', - '@sm:4': '@sm:col-span-4', - '@md:4': '@md:col-span-4', - '@lg:4': '@lg:col-span-4', - '@xl:4': '@xl:col-span-4', - '@2xl:4': '@2xl:col-span-4', - '@4xl:4': '@4xl:col-span-4', - 'sm:2': 'sm:col-span-2', - 'md:2': 'md:col-span-2', - 'lg:2': 'lg:col-span-2', - 'xl:2': 'xl:col-span-2', - 'sm:3': 'sm:col-span-3', - 'md:3': 'md:col-span-3', - 'lg:3': 'lg:col-span-3', - 'xl:3': 'xl:col-span-3', - 'sm:4': 'sm:col-span-4', - 'md:4': 'md:col-span-4', - 'lg:4': 'lg:col-span-4', - 'xl:4': 'xl:col-span-4', - }; - return table[key] || (targetCols === 2 ? 'col-span-2' : targetCols === 3 ? 'col-span-3' : 'col-span-4'); - }; - - const colSpanClass = colSpan && colSpan > 1 - ? colSpan === 2 ? pickSpanClass(2) - : colSpan === 3 ? pickSpanClass(3) - : colSpan >= 4 ? pickSpanClass(4) - : '' - : ''; - - // Metadata-derived stable locator (ADR-0054 C4): the renderer - // emits it from the object + field name so every generated form - // inherits it with zero per-app work. Object prefix omitted when - // the form schema has no owning object. - const fieldTestId = `field:${schema.objectName ? `${schema.objectName}.` : ''}${name}`; - - return ( - ( - - {label && ( - - {label} - {required && ( - - * - - )} - - )} - - {/* Render the actual field component based on resolved type */} - {renderFieldComponent(resolvedType, { - ...fieldProps, - // specialized fields needs raw metadata, but we should traverse down if it exists - // field is the field configuration loop variable - field: (field as any).field || field, - ...formField, - inputType: fieldProps.inputType, - options: isOptionField ? effectiveOptions : fieldProps.options, - placeholder: fieldProps.placeholder ?? (resolvedType === 'select' ? t('common.selectOption') : undefined), - // `disabled` means "not interactive, muted"; `readonly` means - // "shown plainly, not editable" — keep them distinct so widgets - // that implement a real readonly display (e.g. EmailField's - // mailto link) actually receive it instead of always collapsing - // to the grayed-out disabled look. A dependency-gated option - // list (#2284) is disabled until its controlling field is set. - disabled: disabled || fieldDisabled || isSubmitting || optionGroupGated, - readonly, - // Gate hint shown when a dependent option list is still - // waiting on its controlling field (#2284). - emptyHint: gatedHint, - dataSource: contextDataSource, - // Live form values for dependent (cascading) lookups - // (#2215): the widget's `dependsOn` gate + filters must - // re-scope as the user picks the parent field in THIS - // form, not read a stale record snapshot. Forwarded to - // data-source widgets only (see stripRegisteredFieldProps). - dependentValues: ruleRecord, - })} - - {description && ( - {description} - )} - - - )} - /> - ); - })} +
+ {fields.map(renderFormField)}
)} diff --git a/packages/plugin-form/README.md b/packages/plugin-form/README.md index 0ef4b2bbd6..607be1bd74 100644 --- a/packages/plugin-form/README.md +++ b/packages/plugin-form/README.md @@ -83,6 +83,37 @@ interface FormField { } ``` +### Tabbed field layout (`fieldTabs`) + +A sectioned form is **one** form. Instead of rendering a form per section — which +strands every section but the first outside the submit, and (in tabs) lets the +inactive panel unmount with its values — declare tabs on the single form and let +the renderer distribute the fields: + +```typescript +{ + type: 'form', + fields: [/* every tab's fields, in one flat list */], + fieldTabs: [ + { key: 'basics', label: 'Basics', fields: ['subject', 'status'] }, + { key: 'detail', label: 'Detail', description: 'Anything else', fields: ['description'] }, + ], + defaultFieldTab?: 'basics', // defaults to the first tab + fieldTabsPosition?: 'top', // 'top' | 'bottom' | 'left' | 'right' +} +``` + +- All panels are **force-mounted** and only CSS-hidden, so a tab the user leaves + keeps its values *and* its validation (react-hook-form skips unmounted fields, + which is how a required field on an unopened tab used to reach the server). +- A failed submit **activates the tab holding the first offending field** and + marks every tab with a rejected field (`data-error` on the trigger) — for both + client-side rules and server `fields[]` rejections. +- Fields no tab claims render above the tab strip rather than disappearing. +- Needs at least two tabs, and is ignored when the form uses `children`. + +`ModalForm` (`contentLayout: 'tabbed'`) and `TabbedForm` are built on this. + ## Examples ### Basic Form diff --git a/packages/plugin-form/src/ModalForm.tsx b/packages/plugin-form/src/ModalForm.tsx index c70f024a72..ea29d5f954 100644 --- a/packages/plugin-form/src/ModalForm.tsx +++ b/packages/plugin-form/src/ModalForm.tsx @@ -24,10 +24,6 @@ import { Skeleton, Button, cn, - Tabs, - TabsList, - TabsTrigger, - TabsContent, AlertDialog, AlertDialogContent, AlertDialogHeader, @@ -38,7 +34,6 @@ import { AlertDialogCancel, } from '@object-ui/components'; import { Loader2 } from 'lucide-react'; -import { FormSection } from './FormSection'; import { MasterDetailForm } from './MasterDetailForm'; import { SchemaRenderer, useSafeFieldLabel, usePreviewMode } from '@object-ui/react'; import { createSafeTranslation } from '@object-ui/i18n'; @@ -51,7 +46,6 @@ import { filterSystemFields, inferColumns, inferModalSize, - sectionFormLayout, CONTAINER_GRID_COLS, } from './autoLayout'; import { deriveFieldGroupSections } from './fieldGroups'; @@ -76,9 +70,13 @@ export interface ModalFormSectionConfig { description?: string; columns?: 1 | 2 | 3 | 4; fields: (string | FormField)[]; - /** Custom CSS class for the section's Card wrapper. */ + /** Custom CSS class for the section's header row (stacked layout). */ className?: string; - /** Custom CSS class for the section's field grid. */ + /** + * Custom CSS class for the section's field grid. Applied to the section's tab + * panel in the `tabbed` layout; in the stacked layout every section shares one + * grid inside the single form (#2959), so there is none to override. + */ gridClassName?: string; } @@ -515,14 +513,15 @@ export const ModalForm: React.FC = ({ ); } - // Explicit sections layout (curated form view). + // Explicit sections layout (curated form view) — ONE form for ALL sections. // - // KNOWN LIMITATION: each section renders its own SchemaRenderer, i.e. its - // own react-hook-form instance and element, all sharing `formId`. - // The footer submit button is associated with the FIRST form only, so with - // 2+ sections the later sections' values never reach the submit payload. - // The derived field-group branch below avoids this by rendering ONE form; - // fixing the explicit path is tracked in #2153. + // Every section's fields share a single SchemaRenderer, i.e. one + // react-hook-form instance and one element (#2153/#2959). Rendering a + // form PER section broke the modal in two ways: the footer submit button, + // associated by `form={formId}`, can only reach the FIRST form, so section + // 2+ silently dropped everything the user typed; and in the tabbed variant + // Radix unmounted the inactive panel, destroying that tab's form state + // outright. Same single-form pattern as ObjectForm / DrawerForm. if (schema.sections?.length) { const sections = schema.sections; const sectionKey = (sec: ModalFormSectionConfig, i: number) => sec.name || sec.label || String(i); @@ -530,65 +529,86 @@ export const ModalForm: React.FC = ({ // translated group label wins over the raw metadata label. const sectionTitle = (sec: ModalFormSectionConfig) => sec.name ? sectionLabel(schema.objectName, sec.name, sec.label || sec.name) : sec.label; - // Lay the section's fields out in `section.columns` columns INSIDE the - // form (columns + fieldContainerClass), not by wrapping the whole form in - // a grid — otherwise the single fills only the first grid cell and - // the remaining columns stay permanently empty. Actions live in the - // sticky footer, not inside sections. - const renderBody = (section: ModalFormSectionConfig) => ( - - ); + + // 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 (#2578), exactly like the + // full-page sectioned form. + 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 containerFieldClass = CONTAINER_GRID_COLS[formColumns]; + + // FLS first, so a section whose every field is non-readable drops out + // entirely (header included) instead of leaving an empty group. + const groups = sections + .map((section, index) => { + const body = applyFieldPerms(buildSectionFields(section)); + return { + key: sectionKey(section, index), + title: sectionTitle(section), + description: section.description, + className: section.className, + gridClassName: section.gridClassName, + fields: formColumns > 1 + ? applyAutoColSpan(body, formColumns, clampCol(section.columns)) + : body, + }; + }) + .filter((g) => g.fields.length > 0); + + const sharedFormSchema = { + ...baseFormSchema, + columns: formColumns, + ...(containerFieldClass ? { fieldContainerClass: containerFieldClass } : {}), + }; // ADR-0050 (#1890): a modal can host a tabbed layout — sections render as // tabs (label on the trigger) instead of a vertical stack, so a modal - // create/edit form composes with `tabbed`. - if (schema.contentLayout === 'tabbed' && sections.length > 1) { + // create/edit form composes with `tabbed`. The renderer owns the tab strip + // and panels (`fieldTabs`) so all tabs stay mounted inside the one form. + if (schema.contentLayout === 'tabbed' && groups.length > 1) { return ( - - - {sections.map((section, index) => ( - - {sectionTitle(section) || `Section ${index + 1}`} - - ))} - - {sections.map((section, index) => ( - - - {renderBody(section)} - - - ))} - + g.fields), + fieldTabs: groups.map((g, index) => ({ + key: g.key, + label: g.title || `Section ${index + 1}`, + description: g.description, + fields: g.fields.map((f) => f.name), + containerClass: g.gridClassName, + })), + }} + /> ); } - return ( -
- {sections.map((section, index) => ( - - {renderBody(section)} - - ))} -
- ); + // Stacked sections: a virtual `section-divider` field carries each group's + // header inline. (A per-section Card would need a per-section form, which + // is the defect above; `section.gridClassName` therefore has no per-section + // grid to override here.) + const allFields: FormField[] = []; + groups.forEach((g) => { + if (g.title || g.description) { + allFields.push({ + name: `__section_${g.key}`, + label: g.title, + description: g.description, + type: 'section-divider', + colSpan: 4, + className: g.className, + } as any); + } + allFields.push(...g.fields); + }); + + return ; } // Derived field-group sections (object `fieldGroups` metadata) — rendered diff --git a/packages/plugin-form/src/TabbedForm.tsx b/packages/plugin-form/src/TabbedForm.tsx index 771d8d958a..b4d764be96 100644 --- a/packages/plugin-form/src/TabbedForm.tsx +++ b/packages/plugin-form/src/TabbedForm.tsx @@ -15,11 +15,10 @@ import React, { useState, useCallback } from 'react'; import type { FormField, DataSource } from '@object-ui/types'; -import { Tabs, TabsContent, TabsList, TabsTrigger, cn } from '@object-ui/components'; -import { FormSection } from './FormSection'; +import { cn } from '@object-ui/components'; import { SchemaRenderer, useSafeFieldLabel } from '@object-ui/react'; import { buildSectionFields as buildSectionFieldsShared } from './sectionFields'; -import { sectionFormLayout } from './autoLayout'; +import { applyAutoColSpan, containerGridColsFor } from './autoLayout'; export interface FormSectionConfig { /** @@ -50,11 +49,15 @@ export interface FormSectionConfig { /** * Custom CSS class for the section's Card wrapper. + * + * Unused in the tabbed layout: all tabs share ONE form (#2959), so a tab's + * panel has no per-section Card to carry it. */ className?: string; /** - * Custom CSS class for the section's field grid. + * Custom CSS class for the section's field grid — applied to this tab's panel + * grid (overrides the shared column classes). */ gridClassName?: string; } @@ -182,9 +185,11 @@ export const TabbedForm: React.FC = ({ const [formData, setFormData] = useState>({}); const [loading, setLoading] = useState(true); const [error, setError] = useState(null); - const [activeTab, setActiveTab] = useState( - schema.defaultTab || schema.sections[0]?.name || schema.sections[0]?.label || 'tab-0' - ); + // Which tab opens first. The live tab state belongs to the form renderer from + // here on — it owns the panels, so only it can jump to the tab holding a + // rejected field on a failed submit (#2959). + const initialTab = + schema.defaultTab || schema.sections[0]?.name || schema.sections[0]?.label || 'tab-0'; // Fetch object schema React.useEffect(() => { @@ -303,81 +308,68 @@ export const TabbedForm: React.FC = ({ ); } - // Collect all fields across all sections for the form - const allFields: FormField[] = schema.sections.flatMap(section => buildSectionFields(section)); + // ONE form for ALL tabs (#2959). A SchemaRenderer per tab gave each tab its + // own react-hook-form instance, and Radix unmounted the inactive panel — so + // every tab the user left behind lost its input and only the visible tab's + // fields reached the submit payload. The renderer now owns the tab strip and + // panels (`fieldTabs`): all panels stay mounted inside a single , which + // is also what lets cross-tab conditions and validation see every field. + // + // Multi-column stays on the field container INSIDE the form: each tab's fields + // carry their own colSpan against the shared grid (sectionFormLayout parity), + // never a grid wrapped around the form — that would leave the extra columns + // empty (#2128). + const clampCol = (n: unknown): number | undefined => + typeof n === 'number' && n > 0 ? Math.min(Math.floor(n), 4) : undefined; + const declaredCols = schema.sections + .map((s) => clampCol(s.columns)) + .filter((c): c is number => c != null); + const formColumns = declaredCols.length ? Math.max(...declaredCols) : 1; + const containerFieldClass = containerGridColsFor(formColumns); - // Build the overall form schema - const formSchema = { - type: 'form' as const, - fields: allFields, - layout: 'vertical' as const, - defaultValues: formData, - submitLabel: schema.submitText || (schema.mode === 'create' ? 'Create' : 'Update'), - cancelLabel: schema.cancelText, - showSubmit: schema.showSubmit !== false && schema.mode !== 'view', - showCancel: schema.showCancel !== false, - onSubmit: handleSubmit, - onCancel: handleCancel, - }; + const tabGroups = schema.sections.map((section, index) => { + const body = buildSectionFields(section); + return { + key: getTabValue(section, index), + label: section.label || `Tab ${index + 1}`, + description: section.description, + containerClass: section.gridClassName, + fields: formColumns > 1 + ? applyAutoColSpan(body, formColumns, clampCol(section.columns)) + : body, + }; + }); - // Determine orientation based on tabPosition - const isVertical = schema.tabPosition === 'left' || schema.tabPosition === 'right'; + const allFields: FormField[] = tabGroups.flatMap((g) => g.fields); return ( -
- - - {schema.sections.map((section, index) => ( - - {section.label || `Tab ${index + 1}`} - - ))} - - -
- {schema.sections.map((section, index) => ( - - - {/* Render fields for this section. Multi-column is applied to - the field container inside the form (sectionFormLayout), not - by wrapping the form in a grid — which would leave the extra - columns empty. */} - - - - ))} -
- +
+ ({ + key: g.key, + label: g.label, + description: g.description, + fields: g.fields.map((f) => f.name), + containerClass: g.containerClass, + })), + defaultFieldTab: initialTab, + fieldTabsPosition: schema.tabPosition || 'top', + }} + />
); }; diff --git a/packages/plugin-form/src/sectionColumns.test.tsx b/packages/plugin-form/src/sectionColumns.test.tsx index e53e221aa1..f93d4cebd5 100644 --- a/packages/plugin-form/src/sectionColumns.test.tsx +++ b/packages/plugin-form/src/sectionColumns.test.tsx @@ -96,7 +96,10 @@ describe('ModalForm — grouped section multi-column layout (#2128)', () => { // Regression guard: the grid's direct children are the four fields — NOT a // single element (the old bug, which left the right column empty). - expect(grid!.children.length).toBe(4); + // The section's inline header row (a virtual `section-divider`, #2959) is + // the grid's only other child, so count the field wrappers rather than all + // children. + expect(grid!.querySelectorAll(':scope > [data-field]').length).toBe(4); expect(grid!.querySelector(':scope > form')).toBeNull(); // And no ancestor grid wraps the whole form in a multi-column grid anymore. diff --git a/packages/plugin-form/src/sectionedFormValues.test.tsx b/packages/plugin-form/src/sectionedFormValues.test.tsx new file mode 100644 index 0000000000..847e06146e --- /dev/null +++ b/packages/plugin-form/src/sectionedFormValues.test.tsx @@ -0,0 +1,283 @@ +/** + * 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. + */ + +/** + * A sectioned create/edit form must keep EVERY section's input (#2153, #2959). + * + * The explicit-`sections` path used to render one SchemaRenderer — i.e. one + * react-hook-form instance and one `` element — PER section, all sharing + * the same `formId`. Two independent failures followed: + * + * 1. the footer submit button (`form={formId}`) can only be associated with the + * FIRST of those forms, so section 2+ never reached the payload; and + * 2. in the `tabbed` variant Radix unmounted the inactive panel, destroying + * that tab's form state outright — so the reported HotCRM flow (fill tab 1, + * submit, server rejects a required field on tab 3, fill tab 3, submit + * again) came back with EVERY earlier value missing. + * + * These tests pin the contract that fixes both: one `` for all sections, + * with the tab panels force-mounted so a tab the user left keeps its values AND + * its validation. + */ + +import React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, fireEvent, waitFor, cleanup } from '@testing-library/react'; +import { registerAllFields } from '@object-ui/fields'; +import { ModalForm } from './ModalForm'; +import { TabbedForm } from './TabbedForm'; + +registerAllFields(); + +const makeDataSource = () => ({ + getObjectSchema: vi.fn().mockResolvedValue({ + name: 'case', + fields: { + subject: { type: 'text', label: 'Subject' }, + status: { type: 'text', label: 'Status' }, + priority: { type: 'text', label: 'Priority' }, + // The field that made the reported flow destructive: required, and parked + // on the last tab. + description: { type: 'text', label: 'Description', required: true }, + }, + }), + create: vi.fn().mockResolvedValue({ id: 'case-1' }), + update: vi.fn(), + findOne: vi.fn(), +}); + +const SECTIONS = [ + { name: 'basics', label: 'Basics', fields: ['subject'] }, + { name: 'triage', label: 'Triage', fields: ['status', 'priority'] }, + { name: 'detail', label: 'Detail', fields: ['description'] }, +]; + +/** Type into a field by its metadata locator, wherever it is laid out. */ +const fill = (name: string, value: string) => { + const input = document.body.querySelector(`[data-field="${name}"] input`); + if (!input) throw new Error(`field not rendered: ${name}`); + fireEvent.change(input, { target: { value } }); +}; + +const valueOf = (name: string) => + document.body.querySelector(`[data-field="${name}"] input`)?.value; + +const tab = (label: string) => screen.getByRole('tab', { name: new RegExp(label) }); + +const submitForm = () => { + const form = document.body.querySelector('form'); + if (!form) throw new Error('no rendered'); + // The footer button is associated by `form={formId}`; submitting the form + // element directly exercises the same handler without depending on the DOM + // implementation's support for out-of-form submit buttons. + fireEvent.submit(form); +}; + +beforeEach(() => { + vi.restoreAllMocks(); +}); + +afterEach(() => { + cleanup(); +}); + +describe('ModalForm tabbed sections — per-tab form state (#2959)', () => { + it('submits every tab’s values after the user moves between tabs', async () => { + const dataSource = makeDataSource(); + render( + , + ); + + await waitFor(() => expect(screen.getByTestId('modal-form-footer')).toBeTruthy()); + + // ONE form for all three sections — the shared-`formId` duplicate + // elements are what stranded section 2+ outside the submit (#2153). + expect(document.body.querySelectorAll('form')).toHaveLength(1); + + // Fill tab 1, walk to the last tab, fill it there. + fill('subject', 'Printer on fire'); + fireEvent.click(tab('Triage')); + fill('status', 'open'); + fill('priority', 'high'); + fireEvent.click(tab('Detail')); + fill('description', 'Smoke coming out of tray 2'); + + submitForm(); + + await waitFor(() => expect(dataSource.create).toHaveBeenCalledTimes(1)); + expect(dataSource.create.mock.calls[0][1]).toMatchObject({ + subject: 'Printer on fire', + status: 'open', + priority: 'high', + description: 'Smoke coming out of tray 2', + }); + }); + + it('keeps a tab’s input when the user leaves and comes back', async () => { + render( + , + ); + + await waitFor(() => expect(screen.getByTestId('modal-form-footer')).toBeTruthy()); + + fill('subject', 'Kept across tabs'); + fireEvent.click(tab('Triage')); + fireEvent.click(tab('Basics')); + + // Radix used to unmount the inactive panel, which reset the field to its + // defaultValue on the way back. + expect(valueOf('subject')).toBe('Kept across tabs'); + }); + + it('blocks the submit and activates the tab holding a missing required field', async () => { + const dataSource = makeDataSource(); + render( + , + ); + + await waitFor(() => expect(screen.getByTestId('modal-form-footer')).toBeTruthy()); + + // Only tab 1 is filled; `description` (tab 3) is required and empty. An + // unmounted field is skipped by react-hook-form, so this used to reach the + // server and come back as a 400 that named no tab. + fill('subject', 'Only the first tab'); + submitForm(); + + await waitFor(() => expect(tab('Detail')).toHaveAttribute('data-state', 'active')); + expect(dataSource.create).not.toHaveBeenCalled(); + // The offending tab is marked, so the failure is discoverable from any tab. + expect(tab('Detail')).toHaveAttribute('data-error', 'true'); + expect(tab('Basics')).not.toHaveAttribute('data-error'); + }); +}); + +describe('ModalForm stacked sections — one form for all sections (#2153)', () => { + it('submits the 2nd+ section’s values', async () => { + const dataSource = makeDataSource(); + render( + , + ); + + await waitFor(() => expect(screen.getByTestId('modal-form-footer')).toBeTruthy()); + expect(document.body.querySelectorAll('form')).toHaveLength(1); + + fill('subject', 'S1'); + fill('status', 'S2-status'); + fill('priority', 'S2-priority'); + fill('description', 'S3'); + + submitForm(); + + await waitFor(() => expect(dataSource.create).toHaveBeenCalledTimes(1)); + expect(dataSource.create.mock.calls[0][1]).toMatchObject({ + subject: 'S1', + status: 'S2-status', + priority: 'S2-priority', + description: 'S3', + }); + }); + + it('renders each section’s header and description inline', async () => { + render( + , + ); + + expect(await screen.findByText('Basics')).toBeTruthy(); + expect(screen.getByText('Triage')).toBeTruthy(); + // A section's authored blurb survives the move to inline headers. + expect(screen.getByText('Who is asking')).toBeTruthy(); + }); +}); + +describe('TabbedForm — one form for all tabs (#2959)', () => { + it('submits every tab’s values after the user moves between tabs', async () => { + const dataSource = makeDataSource(); + render( + , + ); + + await waitFor(() => expect(document.body.querySelector('form')).toBeTruthy()); + expect(document.body.querySelectorAll('form')).toHaveLength(1); + + fill('subject', 'T1'); + fireEvent.click(tab('Triage')); + fill('status', 'T2'); + fireEvent.click(tab('Detail')); + fill('description', 'T3'); + + submitForm(); + + await waitFor(() => expect(dataSource.create).toHaveBeenCalledTimes(1)); + expect(dataSource.create.mock.calls[0][1]).toMatchObject({ + subject: 'T1', + status: 'T2', + description: 'T3', + }); + }); +}); diff --git a/packages/types/src/form.ts b/packages/types/src/form.ts index 259ca7fa02..7898b807c4 100644 --- a/packages/types/src/form.ts +++ b/packages/types/src/form.ts @@ -779,6 +779,42 @@ export interface FieldCondition { custom?: (formData: any) => boolean; } +/** + * One tab of a tabbed field layout (see `FormSchema.fieldTabs`). + * + * A tab claims a SUBSET of the form's `fields` by name. The form renderer still + * renders exactly ONE `` element / react-hook-form instance and merely + * distributes the fields across tab panels that ALL STAY MOUNTED, so switching + * tabs can neither destroy the values nor skip the validation of the tabs the + * user is not currently looking at (objectui#2959). + */ +export interface FormFieldTab { + /** + * Stable tab key — the Radix `Tabs` value, also used for the panel's + * `data-testid` (`form-tab-panel:{key}`). + */ + key: string; + /** + * Tab trigger text. Falls back to `key`. + */ + label?: string; + /** + * Optional blurb rendered above the tab's fields. + */ + description?: string; + /** + * Names of the fields (as declared in `FormSchema.fields`) that belong to + * this tab, in render order. Unknown names are ignored; fields claimed by no + * tab render in a leading block above the tab strip (never dropped). + */ + fields: string[]; + /** + * Override the tab panel's field-grid classes (same role as + * `FormSchema.fieldContainerClass`, scoped to this tab). + */ + containerClass?: string; +} + /** * Form field configuration */ @@ -1014,6 +1050,27 @@ export interface FormSchema extends BaseSchema { * Field container CSS class */ fieldContainerClass?: string; + /** + * Tabbed field layout (objectui#2959): each entry claims a subset of `fields` + * and renders it in its own tab panel — inside the SAME `` / + * react-hook-form instance as every other tab. Panels are force-mounted (only + * CSS-hidden), so a tab the user has left keeps both its values and its + * validation, and a failed submit activates the tab holding the first + * offending field. + * + * Ignored when `children` is set or fewer than two tabs are given (the form + * then renders as a plain field list). + */ + fieldTabs?: FormFieldTab[]; + /** + * Initially active `fieldTabs` key. Defaults to the first tab. + */ + defaultFieldTab?: string; + /** + * Where the `fieldTabs` strip sits relative to the panels. + * @default 'top' + */ + fieldTabsPosition?: 'top' | 'bottom' | 'left' | 'right'; /** * Child components (alternative to fields array) */ diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index f0d9753316..6fe8961fdd 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -135,6 +135,7 @@ export type { ValidationRule, FieldCondition, FormField, + FormFieldTab, ComboboxSchema, CommandSchema, InputOTPSchema,