From f1f3fa6efd7a86a385e760a1ef5b0909c99effa4 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 02:07:19 +0000 Subject: [PATCH] fix(components): read SchemaRenderer's evaluated `disabled` verdict, never the raw key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `disabled` on a schema node is `boolean | string` — the string being a predicate. `SchemaRenderer` evaluates `disabled` / `disabledOn`, strips the raw key from the props it spreads, and forwards the answer as a real `disabled` prop. Eight widgets re-read the raw key beside that verdict, and an expression string is truthy however it evaluates. One carrier for one question (AGENTS.md #0.1); the in-tree precedent is `plugin-chatbot`'s renderer. Two runtime defects, both measured through the real renderer and the real registry before the fix: * `ui:form` destructured `disabled` off `FormSchema` and discarded the host prop as `_disabledProp`. A FALSE predicate greyed out every field, the submit button and the cancel button. * `ui:button` computed `schema.disabled || props.disabled || isLoading` and then let `toFormControlDomProps` re-declare `disabled` in the spread that follows — the key is kept even when the value is `undefined`, so the computed state was overwritten and `loading: true` rendered a spinner on a live, clickable button. The six DOM pass-throughs (`input`, `textarea`, `checkbox`, `select`, `combobox`, `collapsible`) were already correct AT RUNTIME through the host, because the forwarded prop was spread after the raw one; they now take the verdict by name so the behaviour no longer depends on spread order, and the raw read that the `disabled?: boolean` narrowings were load-bearing for is gone. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01NRRumy89BYdW9ogbcdHTho --- .changeset/7238-disabled-one-carrier.md | 14 ++ .../disabled-verdict-one-carrier.test.tsx | 206 ++++++++++++++++++ .../src/__tests__/form-renderers.test.tsx | 19 +- .../src/renderers/disclosure/collapsible.tsx | 7 +- .../components/src/renderers/form/button.tsx | 21 +- .../src/renderers/form/checkbox.tsx | 7 +- .../src/renderers/form/combobox.tsx | 7 +- .../components/src/renderers/form/form.tsx | 22 +- .../components/src/renderers/form/input.tsx | 11 +- .../components/src/renderers/form/select.tsx | 7 +- .../src/renderers/form/textarea.tsx | 7 +- 11 files changed, 304 insertions(+), 24 deletions(-) create mode 100644 .changeset/7238-disabled-one-carrier.md create mode 100644 packages/components/src/__tests__/disabled-verdict-one-carrier.test.tsx diff --git a/.changeset/7238-disabled-one-carrier.md b/.changeset/7238-disabled-one-carrier.md new file mode 100644 index 0000000000..2961511fc5 --- /dev/null +++ b/.changeset/7238-disabled-one-carrier.md @@ -0,0 +1,14 @@ +--- +"@object-ui/components": patch +--- + +fix(components): eight widgets now consume `SchemaRenderer`'s evaluated `disabled` verdict instead of re-reading the raw authored key + +`disabled` on a schema node is `boolean | string` — the string being a predicate. `SchemaRenderer` evaluates `disabled` / `disabledOn`, strips the raw key from the props it spreads, and forwards the answer as a real `disabled` prop. `ui:form`, `ui:button`, `ui:input`, `ui:textarea`, `ui:checkbox`, `ui:select`, `ui:combobox` and `ui:collapsible` re-read the raw key beside that verdict, and an expression string is truthy however it evaluates. + +Two user-visible defects go away: + +- `ui:form` — a form declaring `disabled: "${...}"` greyed out every field, the submit button and the cancel button even when the predicate was FALSE. +- `ui:button` — `loading: true` did not disable the button when it was rendered through `SchemaRenderer`: the computed state was overwritten by the forwarded verdict arriving through the DOM pass-through spread, so the spinner ran on a live control. + +The six DOM pass-throughs kept their behaviour through `SchemaRenderer` and now read the verdict by name rather than depending on spread order. diff --git a/packages/components/src/__tests__/disabled-verdict-one-carrier.test.tsx b/packages/components/src/__tests__/disabled-verdict-one-carrier.test.tsx new file mode 100644 index 0000000000..98c92467e0 --- /dev/null +++ b/packages/components/src/__tests__/disabled-verdict-one-carrier.test.tsx @@ -0,0 +1,206 @@ +/** + * 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. + */ + +/** + * One carrier for one question: the widgets read `SchemaRenderer`'s EVALUATED + * `disabled` verdict, never the raw authored key beside it (objectui#7238, + * AGENTS.md #0.1; the in-tree precedent is `plugin-chatbot`'s renderer, + * objectui#6169). + * + * `disabled` on a `BaseSchema` node is `boolean | string` — the string being a + * predicate. `SchemaRenderer` evaluates `disabled` / `disabledOn`, STRIPS the + * raw key from the props it spreads, and forwards the verdict as a real + * `disabled` prop (`disabled: __disabled || undefined`). A widget that re-reads + * `schema.disabled` is reading an expression string, which is truthy however it + * evaluates. + * + * Every case here renders through the REAL `SchemaRenderer` and the real + * registry, so the verdict path is the one under test. Each predicate is + * exercised in BOTH polarities against the same node, which is what separates + * "the widget ignores the verdict" from "the widget is never disabled at all" — + * a one-polarity pin would go green on a widget that dropped `disabled` + * entirely. + */ + +import { describe, it, expect } from 'vitest'; +import { render, screen } from '@testing-library/react'; +import React from 'react'; +import { SchemaRenderer, SchemaRendererContext } from '@object-ui/react'; +// Registers the renderers at module scope, NOT inside a `beforeAll` — there the +// cold transform is billed to `hookTimeout` (objectui#3010/#3021). +import '../renderers'; + +/** `${data.locked}` resolves against this — the `dataSource` on the context. */ +function renderNode(schema: Record, locked: boolean) { + return render( + + + , + ); +} + +/** A predicate string — the shape that is truthy however it evaluates. */ +const PREDICATE = '${data.locked}'; + +/* ──────────────────────────────────────────────────────────────────────────── + * (b) `form` — the raw read that dropped the host verdict entirely + * ───────────────────────────────────────────────────────────────────────── */ + +function formSchema() { + return { + type: 'form', + disabled: PREDICATE, + submitLabel: 'Save', + showCancel: true, + cancelLabel: 'Cancel', + fields: [{ name: 'notes', label: 'Notes', type: 'input' }], + }; +} + +describe('`form` consumes the evaluated verdict, not the raw predicate (objectui#7238)', () => { + it('a FALSE predicate leaves the fields and the action bar interactive', () => { + renderNode(formSchema(), false); + + expect((screen.getByLabelText(/notes/i) as HTMLInputElement).disabled).toBe(false); + expect((screen.getByRole('button', { name: 'Save' }) as HTMLButtonElement).disabled).toBe(false); + expect((screen.getByRole('button', { name: 'Cancel' }) as HTMLButtonElement).disabled).toBe(false); + }); + + it('a TRUE predicate still greys the whole form out', () => { + renderNode(formSchema(), true); + + expect((screen.getByLabelText(/notes/i) as HTMLInputElement).disabled).toBe(true); + expect((screen.getByRole('button', { name: 'Save' }) as HTMLButtonElement).disabled).toBe(true); + expect((screen.getByRole('button', { name: 'Cancel' }) as HTMLButtonElement).disabled).toBe(true); + }); + + it('a literal `disabled: true` is unchanged — the verdict says the same thing', () => { + renderNode({ ...formSchema(), disabled: true }, false); + + expect((screen.getByLabelText(/notes/i) as HTMLInputElement).disabled).toBe(true); + expect((screen.getByRole('button', { name: 'Save' }) as HTMLButtonElement).disabled).toBe(true); + }); +}); + +/* ──────────────────────────────────────────────────────────────────────────── + * (c) `ui:button` — `isDisabled` was computed from the raw key, and then + * overwritten by the forwarded verdict arriving through the DOM spread. The + * `loading` leg of that OR was the casualty: it never reached the element. + * ───────────────────────────────────────────────────────────────────────── */ + +const button = () => screen.getByRole('button') as HTMLButtonElement; + +describe('`ui:button` consumes the evaluated verdict, not the raw predicate (objectui#7238)', () => { + it('a FALSE predicate leaves the button clickable', () => { + renderNode({ type: 'button', label: 'Go', disabled: PREDICATE }, false); + expect(button().disabled).toBe(false); + }); + + it('a TRUE predicate disables it', () => { + renderNode({ type: 'button', label: 'Go', disabled: PREDICATE }, true); + expect(button().disabled).toBe(true); + }); + + it('`loading` disables the button — the leg the raw read used to lose', () => { + // `isDisabled` was `schema.disabled || props.disabled || isLoading`, applied + // BEFORE the DOM spread that carries the host's `disabled` prop. With no + // predicate authored the verdict is `undefined`, and `{...toFormControlDomProps(rest)}` + // re-declared `disabled` — key present, value `undefined` (`pickDomProps` + // iterates `Object.keys`) — so it overwrote the computed value and the + // spinner spun on a live button. + renderNode({ type: 'button', label: 'Go', loading: true }, false); + expect(button().disabled).toBe(true); + expect(document.querySelector('svg.animate-spin')).not.toBeNull(); + }); + + it('`loading` and a TRUE predicate agree', () => { + renderNode({ type: 'button', label: 'Go', loading: true, disabled: PREDICATE }, true); + expect(button().disabled).toBe(true); + }); +}); + +/* ──────────────────────────────────────────────────────────────────────────── + * (a) the six DOM pass-throughs. Their runtime was already correct THROUGH the + * renderer (the forwarded prop was spread after the raw one), so these are the + * regression guard on removing the raw read: they must not move. + * ───────────────────────────────────────────────────────────────────────── */ + +const PASSTHROUGHS: Array<{ type: string; node: Record; control: () => HTMLElement }> = [ + { + type: 'input', + node: { type: 'input', id: 'i1', label: 'Notes' }, + control: () => document.querySelector('input')!, + }, + { + type: 'textarea', + node: { type: 'textarea', id: 't1', label: 'Notes' }, + control: () => document.querySelector('textarea')!, + }, + { + type: 'checkbox', + node: { type: 'checkbox', id: 'c1', label: 'Agree' }, + control: () => document.querySelector('[role="checkbox"]')!, + }, + { + type: 'select', + node: { type: 'select', id: 's1', options: [{ label: 'A', value: 'a' }] }, + control: () => document.querySelector('[role="combobox"]')!, + }, + { + type: 'combobox', + node: { type: 'combobox', id: 'cb1', options: [{ label: 'A', value: 'a' }] }, + control: () => document.querySelector('button')!, + }, +]; + +describe.each(PASSTHROUGHS)('`$type` consumes the evaluated verdict (objectui#7238)', ({ node, control }) => { + it('a FALSE predicate leaves the control interactive', () => { + renderNode({ ...node, disabled: PREDICATE }, false); + const el = control(); + expect(el).toBeTruthy(); + expect(el.hasAttribute('disabled')).toBe(false); + }); + + it('a TRUE predicate disables the control', () => { + renderNode({ ...node, disabled: PREDICATE }, true); + expect(control().hasAttribute('disabled')).toBe(true); + }); + + it('a literal `disabled: true` disables the control', () => { + renderNode({ ...node, disabled: true }, false); + expect(control().hasAttribute('disabled')).toBe(true); + }); +}); + +describe('`collapsible` consumes the evaluated verdict (objectui#7238)', () => { + const node = { + type: 'collapsible', + id: 'col1', + trigger: [{ type: 'text', content: 'Toggle' }], + content: [{ type: 'text', content: 'Body' }], + }; + // Radix's Collapsible root stamps `data-disabled` from the same prop it + // hands the trigger, so this reads the root's own verdict rather than a + // nested widget's. + const root = () => document.querySelector('[data-state]')!; + + it('a FALSE predicate leaves it enabled', () => { + renderNode({ ...node, disabled: PREDICATE }, false); + expect(root().hasAttribute('data-disabled')).toBe(false); + }); + + it('a TRUE predicate disables it', () => { + renderNode({ ...node, disabled: PREDICATE }, true); + expect(root().hasAttribute('data-disabled')).toBe(true); + }); + + it('a literal `disabled: true` disables it', () => { + renderNode({ ...node, disabled: true }, false); + expect(root().hasAttribute('data-disabled')).toBe(true); + }); +}); diff --git a/packages/components/src/__tests__/form-renderers.test.tsx b/packages/components/src/__tests__/form-renderers.test.tsx index 50f6300b32..cdf7ecca00 100644 --- a/packages/components/src/__tests__/form-renderers.test.tsx +++ b/packages/components/src/__tests__/form-renderers.test.tsx @@ -7,7 +7,8 @@ */ import { describe, it, expect } from 'vitest'; -import { screen } from '@testing-library/react'; +import { render, screen } from '@testing-library/react'; +import { SchemaRenderer } from '@object-ui/react'; import { renderComponent, validateComponentRegistration, @@ -158,12 +159,18 @@ describe('Form Renderers - Display Issue Detection', () => { expect(error).toBeTruthy(); }); + // Through the REAL renderer, not `renderComponent`. An authored `disabled` + // is `boolean | string` and is EVALUATED by `SchemaRenderer`, which strips + // the raw key and forwards the verdict as a `disabled` prop; the widget + // consumes that prop and no longer re-reads `schema.disabled` beside it + // (objectui#7238). `renderComponent` mounts the registration with no host in + // front of it, so it can only ever assert the raw read this card removed — + // the predicate half of the same key is pinned in + // `disabled-verdict-one-carrier.test.tsx`. it('should handle disabled state', () => { - const { container } = renderComponent({ - type: 'input', - label: 'Field', - disabled: true, - }); + const { container } = render( + , + ); const input = container.querySelector('input'); expect(input?.hasAttribute('disabled')).toBe(true); diff --git a/packages/components/src/renderers/disclosure/collapsible.tsx b/packages/components/src/renderers/disclosure/collapsible.tsx index b29285d4c0..ee999dc2ab 100644 --- a/packages/components/src/renderers/disclosure/collapsible.tsx +++ b/packages/components/src/renderers/disclosure/collapsible.tsx @@ -16,8 +16,11 @@ import { import { renderChildren } from '../../lib/utils'; ComponentRegistry.register('collapsible', - ({ schema, className, ...props }: { schema: CollapsibleSchema; className?: string; [key: string]: any }) => ( - + // `hostDisabled` is `SchemaRenderer`'s EVALUATED verdict on `disabled` / + // `disabledOn`, not the raw authored key — which may be a predicate STRING, + // truthy however it evaluates (objectui#7238, precedent objectui#6169). + ({ schema, className, disabled: hostDisabled, ...props }: { schema: CollapsibleSchema; className?: string; disabled?: boolean; [key: string]: any }) => ( + {renderChildren(schema.trigger)} diff --git a/packages/components/src/renderers/form/button.tsx b/packages/components/src/renderers/form/button.tsx index d1d0a27806..f5f1a0a0d5 100644 --- a/packages/components/src/renderers/form/button.tsx +++ b/packages/components/src/renderers/form/button.tsx @@ -19,7 +19,14 @@ import { resolveIcon } from '../action/resolve-icon'; // argument — mechanism note on `action:bar` (objectui#4422), pinned by // `__tests__/forwardref-props-annotation.guard.test.ts`. const ButtonRenderer = forwardRef( - ({ schema, ...props }: { schema: ButtonSchema; [key: string]: any }, ref) => { + // `disabled` is the host-EVALUATED verdict, taken by name. `SchemaRenderer` + // evaluates the node's `disabled` / `disabledOn` (either may be a predicate + // STRING), strips the raw key from the props it spreads, and forwards the + // verdict as a real `disabled` prop. Consuming it here rather than re-reading + // `schema.disabled` keeps one carrier for one question (AGENTS.md #0.1, + // objectui#7238; precedent `plugin-chatbot`, objectui#6169) — and taking it + // OFF `props` is the load-bearing half, see `isDisabled` below. + ({ schema, disabled: hostDisabled, ...props }: { schema: ButtonSchema; disabled?: boolean; [key: string]: any }, ref) => { // Extract designer-related props const { 'data-obj-id': dataObjId, @@ -38,8 +45,16 @@ const ButtonRenderer = forwardRef( // Determine loading state const isLoading = schema.loading || props.loading; - // Determine disabled state - const isDisabled = schema.disabled || props.disabled || isLoading; + // Determine disabled state. This used to be + // `schema.disabled || props.disabled || isLoading`, and the `loading` leg of + // that OR never reached the element: `disabled` also rode `buttonProps` into + // `toFormControlDomProps` below, which forwards it BY NAME and keeps the key + // even when the value is `undefined` (`pickDomProps` iterates `Object.keys`). + // Spread after `disabled={isDisabled}`, it overwrote the computed value with + // the host's verdict — so a `loading` button with no authored predicate + // rendered its spinner on a live, clickable control. Destructuring `disabled` + // out of `props` above removes that second writer; this is now the only one. + const isDisabled = hostDisabled || isLoading; return (