From c6e97a309f14dc601fc0774d2c0adcfd7d44f017 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 29 Aug 2026 03:21:56 +0000 Subject: [PATCH] fix(plugin-form): report an inert section visibleWhen on the tabbed and wizard arms ObjectForm rebuilds each section key by key when delegating to a layout, so a key the map does not copy never reaches a renderer. The split/drawer/modal maps copy visibleWhen (#6111) and the flat arm carries it on the section-divider pseudo-field; the tabbed and wizard maps copy nothing, so an authored section predicate did nothing at all, with no signal anywhere. Those two arms now warn, naming the layout and the sections being dropped, through one shared message builder. No rendering behaviour changes and no authorable key is added: this is the interim half of the 2026-08-29 ruling that the real repair is one section/group predicate contract designed once for every layout arm. Deliberately silent on the four arms that honour the key, and on the master-detail branch, which re-enters ObjectForm where the real layout is decided. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49 --- ...237-section-predicate-layout-diagnostic.md | 39 +++ content/docs/plugins/plugin-form.mdx | 15 + packages/plugin-form/README.md | 8 + packages/plugin-form/src/ObjectForm.tsx | 33 ++- ...ionPredicateLayoutDiagnostic-6237.test.tsx | 264 ++++++++++++++++++ .../src/sectionPredicateDiagnostic.ts | 50 ++++ 6 files changed, 407 insertions(+), 2 deletions(-) create mode 100644 .changeset/6237-section-predicate-layout-diagnostic.md create mode 100644 packages/plugin-form/src/__tests__/sectionPredicateLayoutDiagnostic-6237.test.tsx create mode 100644 packages/plugin-form/src/sectionPredicateDiagnostic.ts 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. + */