diff --git a/.changeset/6237-section-predicate-layout-diagnostic.md b/.changeset/6237-section-predicate-layout-diagnostic.md new file mode 100644 index 0000000000..97d9c70221 --- /dev/null +++ b/.changeset/6237-section-predicate-layout-diagnostic.md @@ -0,0 +1,39 @@ +--- +'@object-ui/plugin-form': patch +--- + +An authored section `visibleWhen` on `formType: 'tabbed'` or `formType: 'wizard'` now +**reports** that the layout cannot honour it, instead of being silently dropped +(objectui#6237). + +`ObjectForm` rebuilds each section key by key when it delegates to a layout, so a key +the map does not copy never reaches a renderer at all. Three of those maps copy +`visibleWhen` (`split` / `drawer` / `modal`, objectui#6111) and the flat arm carries it +on the `section-divider` pseudo-field — but the `tabbed` and `wizard` maps copy nothing, +so an author writing the key on those two arms watched it do exactly nothing, with no +signal anywhere. That silence is the defect this ships against. + +The two arms now log a warning naming the layout and the sections whose predicate is +being dropped, through one shared message builder so they cannot drift apart. + +**This changes no rendering behaviour** — the predicate is still not evaluated on those +arms. It is the interim half of a maintainer ruling (2026-08-29) that the real repair is +a **design** task: one renderer-side section/group contract with a predicate slot, +designed once for every layout arm (tabbed / TabbedForm / WizardForm / flat) rather than +patched arm by arm. The ruling requires the diagnostic to land first, so the gap stops +being invisible while that contract is designed. + +Deliberately silent on the arms that work, so the warning stays worth reading: + +- `split` / `drawer` / `modal`, and the flat layout — all honour a section `visibleWhen`. +- `ModalForm` with `contentLayout: 'tabbed'` — honours it through the real + `FormFieldTab.visibleWhen` slot that landed in objectui#6619. "Tabbed" names two + different things on this card; only `formType: 'tabbed'` (`TabbedForm`) is inert. +- A master-detail parent, which re-enters `ObjectForm` through its own parent schema — + the report is left to that inner pass, where the real layout is decided (a + master-detail `wizard` parent renders `simple`, which honours the key). Reporting at + both would double-report the tabbed parent and false-report the wizard one. + +No authorable key is added anywhere: declaring `visibleWhen` on a type whose renderer +ignores it is the defect this card family exists to close, and the shared +`FormSectionConfig` that `WizardForm` uses for its steps makes that trap concrete. diff --git a/content/docs/plugins/plugin-form.mdx b/content/docs/plugins/plugin-form.mdx index 476bc74de8..5c5467703c 100644 --- a/content/docs/plugins/plugin-form.mdx +++ b/content/docs/plugins/plugin-form.mdx @@ -251,6 +251,21 @@ surface whose renderer ignores it would make the metadata lie (objectui#6111), so the key stops at the boundary until each surface enforces it. Track objectui#6237 for both. +Both **No** rows now **report themselves** rather than failing silently. Authoring +a section `visibleWhen` on `formType: 'tabbed'` or `formType: 'wizard'` logs a +console warning naming the layout and the sections whose predicate is being +dropped: + +> `[ObjectForm] Section \`visibleWhen\` is not yet supported on this layout: the +> \`tabbed\` layout's tabs drop the predicate, so section(s) pay render +> unconditionally. …` + +This is an interim diagnostic, ruled 2026-08-29 alongside the decision to design +the real repair as **one** section/group predicate contract shared by every layout +arm instead of patching them one at a time. It changes no behaviour — the +predicate is still dropped on those two arms — it only stops the drop from being +invisible. Nothing warns on the four arms that honour the key. + ### Wizard steps and `allowSkip` `allowSkip` lets the user jump to any step from the indicator instead of walking diff --git a/packages/plugin-form/README.md b/packages/plugin-form/README.md index 88d72f5ba2..8d49a015cb 100644 --- a/packages/plugin-form/README.md +++ b/packages/plugin-form/README.md @@ -450,6 +450,14 @@ surface whose renderer ignores it would make the metadata lie (#6111), so the key stops at the boundary until each surface enforces it. Track #6237 for both. +Both **No** rows now **report themselves** rather than failing silently. Authoring +a section `visibleWhen` on `formType: 'tabbed'` or `formType: 'wizard'` logs a +console warning naming the layout and the sections whose predicate is being +dropped. This is an interim diagnostic (ruled 2026-08-29, alongside the decision +to design the real repair as **one** section/group predicate contract shared by +every layout arm): it changes no behaviour, it only stops the drop from being +invisible. Nothing warns on the four arms that honour the key. + ### Wizard steps and `allowSkip` `allowSkip` lets the user jump to any step from the indicator instead of walking diff --git a/packages/plugin-form/src/ObjectForm.tsx b/packages/plugin-form/src/ObjectForm.tsx index 1b57df05ab..de30e38123 100644 --- a/packages/plugin-form/src/ObjectForm.tsx +++ b/packages/plugin-form/src/ObjectForm.tsx @@ -26,6 +26,7 @@ import { type PendingSubmitRedirect, } from './submitRedirectNavigation'; import { usePermissions } from '@object-ui/permissions'; +import { sectionPredicateUnsupportedWarning } from './sectionPredicateDiagnostic'; import { TabbedForm } from './TabbedForm'; import { WizardForm, NAVIGATE_ON_SUCCESS_REFUSED_NOTE } from './WizardForm'; import { SplitForm } from './SplitForm'; @@ -199,8 +200,36 @@ export const ObjectForm: React.FC = ({ // page. Skipped in view mode (read-only detail uses related lists instead). // For drawer/modal formTypes we fall through to DrawerForm/ModalForm, which // host the master-detail form INSIDE their envelope. - if ((schema as any).subforms?.length && schema.mode !== 'view' - && schema.formType !== 'drawer' && schema.formType !== 'modal') { + const routesToMasterDetail = !!(schema as any).subforms?.length && schema.mode !== 'view' + && schema.formType !== 'drawer' && schema.formType !== 'modal'; + + // ── objectui#6237 interim diagnostic (maintainer ruling, 2026-08-29) ──────── + // `tabbed` and `wizard` are the two routes below that drop an authored section + // `visibleWhen` (see `sectionPredicateUnsupportedWarning`). Report the gap + // instead of dropping it in silence. This declares no key and hides nothing — + // making these arms honour the predicate is the ruled design task's job. + // + // Deliberately NOT reported for the master-detail branch: that branch re-enters + // `ObjectForm` through `MasterDetailForm`'s parent schema, which is where the + // real layout is decided (a master-detail `wizard` parent renders `simple`, + // which DOES honour the predicate). Reporting here as well would double-report + // the tabbed parent and false-report the wizard one. + const inertPredicateLayout = !routesToMasterDetail + && (schema.formType === 'tabbed' || schema.formType === 'wizard') + ? schema.formType + : null; + // Joined to a string on purpose: the effect's deps must be primitives, or a + // fresh array identity each render would re-report on every keystroke. + const inertPredicateSections = (schema.sections ?? []) + .filter((s: any) => s?.visibleWhen != null) + .map((s: any) => s?.name || s?.label || '(unnamed)') + .join(', '); + useEffect(() => { + if (!inertPredicateLayout || !inertPredicateSections) return; + console.warn(sectionPredicateUnsupportedWarning(inertPredicateLayout, inertPredicateSections)); + }, [inertPredicateLayout, inertPredicateSections]); + + if (routesToMasterDetail) { return ( ({ dialect: 'cel', source }); +const GATE = cel("'sales_manager' in current_user.positions"); + +function hostScope(positions: string[]) { + const user = { id: 'u1', name: 'Kim', positions }; + return { current_user: user, user, ctx: { user }, os: { user }, app: {}, data: {}, features: {} }; +} +/** + * DENIED on purpose: the predicate resolves FALSE, which is the only state in + * which an author would notice the arm is inert. The diagnostic is deliberately + * verdict-INDEPENDENT (it reports that nothing will evaluate the key at all), and + * the ALLOWED row below pins exactly that. + */ +const DENIED = hostScope(['sales']); +const ALLOWED = hostScope(['sales_manager']); + +const objectSchema = { + name: 'crm_case', + fields: { + subject: { type: 'text', label: 'Subject' }, + salary: { type: 'text', label: 'Salary' }, + }, +}; + +let dataSource: any; +let warnSpy: ReturnType; + +beforeEach(() => { + dataSource = { + getObjectSchema: vi.fn().mockResolvedValue(objectSchema), + findOne: vi.fn(), + create: vi.fn(), + update: vi.fn(), + }; + warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); +}); + +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +/** `Always` is the un-gated control section; `Compensation` carries the gate. */ +const sections = (gate: unknown = GATE) => [ + { name: 'always', label: 'Always', fields: ['subject'] }, + { name: 'pay', label: 'Compensation', visibleWhen: gate, fields: ['salary'] }, +]; + +const renderObjectForm = async ( + scope: Record, + extra: Record, + sectionList: unknown = sections(), +) => { + const view = render( + + + , + ); + // The un-gated sibling is the readiness signal AND the proof the form mounted. + await waitFor(() => expect(screen.getAllByText('Always').length).toBeGreaterThan(0)); + return view; +}; + +/** Only the #6237 diagnostic — other console.warn traffic is not this pin's subject. */ +const diagnosticCalls = () => + warnSpy.mock.calls + .map((c: unknown[]) => String(c[0])) + .filter((m: string) => m.includes('Section `visibleWhen` is not yet supported on this layout')); + +describe('#6237 — the inert arms REPORT instead of dropping the predicate in silence', () => { + it('formType `tabbed`: reports, naming the ruled phrase, the surface and the section', async () => { + await renderObjectForm(DENIED, { formType: 'tabbed' }); + await waitFor(() => expect(diagnosticCalls().length).toBe(1)); + const message = diagnosticCalls()[0]; + expect(message).toContain('not yet supported on this layout'); + expect(message).toContain("the `tabbed` layout's tabs"); + // The section is named, so an author with ten sections knows which one. + expect(message).toContain('pay'); + // The remedy names an arm that genuinely works today. + expect(message).toContain('objectui#6237'); + }); + + it('formType `wizard`: reports too — steps are the second silently-inert arm', async () => { + await renderObjectForm(DENIED, { formType: 'wizard' }); + await waitFor(() => expect(diagnosticCalls().length).toBe(1)); + expect(diagnosticCalls()[0]).toContain("the `wizard` layout's steps"); + }); + + it('the report does not depend on the VERDICT — an admitting predicate is just as inert', async () => { + // Nothing evaluates the key on these arms, so a TRUE predicate is dropped + // exactly as a FALSE one is. A diagnostic that only fired on the denied + // scope would leave the author of an allow-rule believing it worked. + await renderObjectForm(ALLOWED, { formType: 'tabbed' }); + await waitFor(() => expect(diagnosticCalls().length).toBe(1)); + }); + + it('every section carrying a predicate is named, not just the first', async () => { + await renderObjectForm(DENIED, { formType: 'tabbed' }, [ + { name: 'always', label: 'Always', fields: ['subject'] }, + { name: 'pay', label: 'Compensation', visibleWhen: GATE, fields: ['salary'] }, + { name: 'extra', label: 'Extra', visibleWhen: GATE, fields: ['subject'] }, + ]); + await waitFor(() => expect(diagnosticCalls().length).toBe(1)); + expect(diagnosticCalls()[0]).toContain('pay, extra'); + }); +}); + +describe('#6237 — the boundary: arms that HONOUR the predicate stay silent', () => { + // Each row authors the identical predicate on the identical section. The only + // variable is the layout, so a warning here would be a false alarm about a + // feature #6111 / #6619 already landed. + const honouring: Array<[string, Record]> = [ + ['simple (flat, section-divider pseudo-field)', { formType: 'simple' }], + ['modal (ObjectForm modal map copies it)', { formType: 'modal', open: true }], + ['modal + contentLayout tabbed (#6619 FormFieldTab.visibleWhen)', + { formType: 'modal', open: true, contentLayout: 'tabbed' }], + ['drawer (ObjectForm drawer map copies it)', { formType: 'drawer', open: true }], + ['split (ObjectForm split map copies it)', { formType: 'split' }], + ]; + + for (const [label, extra] of honouring) { + it(`${label}: silent`, async () => { + await renderObjectForm(DENIED, extra); + expect(diagnosticCalls()).toEqual([]); + }); + } + + it('an inert arm with NO authored predicate is silent — the gap, not the layout, is reported', async () => { + await renderObjectForm(DENIED, { formType: 'tabbed' }, [ + { name: 'always', label: 'Always', fields: ['subject'] }, + { name: 'pay', label: 'Compensation', fields: ['salary'] }, + ]); + expect(diagnosticCalls()).toEqual([]); + }); +}); + +describe('#6237 — the report is once per mount, not once per render', () => { + it('re-rendering the same schema does not re-report', async () => { + const view = await renderObjectForm(DENIED, { formType: 'tabbed' }); + await waitFor(() => expect(diagnosticCalls().length).toBe(1)); + // A fresh element with the same authored content: the effect's deps are + // primitives (layout + joined names), so identity churn must not re-fire it. + // Authoring a form re-renders on every keystroke; a per-render warning would + // bury the console it is trying to speak into. + view.rerender( + + + , + ); + await waitFor(() => expect(screen.getAllByText('Always').length).toBeGreaterThan(0)); + expect(diagnosticCalls().length).toBe(1); + }); +}); + +describe('#6237 — the master-detail branch reports through its INNER pass, exactly once', () => { + // `ObjectForm` routes a subforms-bearing schema to `MasterDetailForm` FIRST, + // and that component builds a parent schema which re-enters `ObjectForm`. The + // real layout is decided on that inner pass — a master-detail `wizard` parent + // is rendered `simple`, which HONOURS the predicate. Reporting at the outer + // call as well would double-report the tabbed parent and, worse, false-report + // the wizard one about a layout it never actually renders. + const subforms = [{ childObject: 'crm_case_line', relationshipField: 'case' }]; + + it('master-detail `wizard`: silent — the parent renders `simple`, which honours the key', async () => { + await renderObjectForm(DENIED, { formType: 'wizard', subforms }); + expect(diagnosticCalls()).toEqual([]); + }); + + it('master-detail `tabbed`: reported ONCE, not once per ObjectForm pass', async () => { + await renderObjectForm(DENIED, { formType: 'tabbed', subforms }); + await waitFor(() => expect(diagnosticCalls().length).toBe(1)); + expect(diagnosticCalls()[0]).toContain("the `tabbed` layout's tabs"); + }); +}); + +describe('#6237 — the message is single-sourced', () => { + it('both arms speak through one builder, so the two cannot drift apart', () => { + expect(sectionPredicateUnsupportedWarning('tabbed', 'pay')) + .toContain('not yet supported on this layout'); + expect(sectionPredicateUnsupportedWarning('wizard', 'pay')) + .toContain('not yet supported on this layout'); + // The one thing that differs is the surface noun. + expect(sectionPredicateUnsupportedWarning('tabbed', 'pay')) + .not.toEqual(sectionPredicateUnsupportedWarning('wizard', 'pay')); + }); +}); diff --git a/packages/plugin-form/src/sectionPredicateDiagnostic.ts b/packages/plugin-form/src/sectionPredicateDiagnostic.ts new file mode 100644 index 0000000000..6d789465c6 --- /dev/null +++ b/packages/plugin-form/src/sectionPredicateDiagnostic.ts @@ -0,0 +1,50 @@ +/** + * 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 layout arms that DROP an authored `FormSection.visibleWhen` (objectui#6237). + * + * Of the five layout routes in `ObjectForm`, three rebuild each section key by key + * and DO copy the predicate (`split` / `drawer` / `modal`), and the flat arm carries + * it on the `section-divider` pseudo-field — those four honour it. `tabbed` + * (`TabbedForm`) and `wizard` (`WizardForm`) rebuild the section the same way but + * copy no predicate, so an authored key never reaches a renderer at all. + * + * Making those two arms actually honour it is a DESIGN task, ruled 2026-08-29 + * (option A): ONE renderer-side section/group contract with a predicate slot, + * designed once for every layout arm rather than patched arm by arm. Ruled as part + * of that option, this diagnostic lands FIRST so the gap stops being silent — an + * author who writes the key on one of these arms is told it is not yet supported + * here instead of watching it do nothing. + * + * Single-sourced so both inert arms report the gap in one voice, and so a test can + * pin the wording without restating it. + */ +export function sectionPredicateUnsupportedWarning( + layout: 'tabbed' | 'wizard', + sectionNames: string, +): string { + const surface = layout === 'tabbed' + ? "the `tabbed` layout's tabs" + : "the `wizard` layout's steps"; + return '[ObjectForm] Section `visibleWhen` is not yet supported on this layout: ' + + `${surface} drop the predicate, so section(s) ${sectionNames} render ` + + 'unconditionally. Support is being designed as ONE grouping contract across ' + + 'every layout arm (objectui#6237); until it lands, use ' + + "`formType: 'modal' | 'drawer' | 'split'` or the flat layout — each honours a " + + 'section `visibleWhen` — or move the predicate onto the individual fields, ' + + 'whose own `visibleWhen` is evaluated on every layout.'; +} + +/* + * Lives in its own module rather than in `ObjectForm.tsx`: exporting a + * non-component from a component file costs that file Fast Refresh + * (`react-refresh/only-export-components`), and `ObjectForm` is edited often + * enough for that to be a real tax. Deliberately NOT re-exported from the + * package barrel — this is an internal diagnostic, not published surface. + */