From b7c96657020a9469d984c3936ba370ab591fc132 Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Mon, 27 Jul 2026 20:28:09 +0800 Subject: [PATCH] fix(action): honor the spec disabled predicate on every action-rendering surface (#1885 follow-through) Spec Action field is `disabled` (boolean | CEL, disabled when TRUE); the schema has no `enabled` key. #1885 wired action:button only. Browser dogfooding found five more surfaces where an authored `disabled` silently did nothing: - components: action:group leaves (inline + dropdown), action:icon, action:menu read the legacy non-spec `enabled` only. `disabled` is now primary (same evaluation scope as `visible`), `enabled` stays as a deprecated fallback. - app-shell: DeclaredActionsBar read neither; gains `disabled` (no legacy fallback -- declared actions are spec-shaped). - plugin-detail: record:quick_actions HAD a disabled implementation but its `typeof === 'string'` split dropped the {dialect:'cel', source} envelope the server compiles authored CEL into (#2661), so it never fired on real metadata. Now feeds toPredicateInput's result to useCondition whole. Pinned by new DropdownActionItem tests (4 cases); browser-verified against the showcase showcase_archive_task specimen: greyed on an in-progress task, clickable on a done one. Co-Authored-By: Claude Fable 5 --- .changeset/action-disabled-all-renderers.md | 32 +++++++++++++++ .../src/views/DeclaredActionsBar.tsx | 8 +++- .../action-group-dropdown-visible.test.tsx | 40 +++++++++++++++++++ .../src/renderers/action/action-group.tsx | 22 +++++++++- .../src/renderers/action/action-icon.tsx | 12 +++++- .../src/renderers/action/action-menu.tsx | 10 ++++- .../src/renderers/record-quick-actions.tsx | 14 ++++--- 7 files changed, 127 insertions(+), 11 deletions(-) create mode 100644 .changeset/action-disabled-all-renderers.md diff --git a/.changeset/action-disabled-all-renderers.md b/.changeset/action-disabled-all-renderers.md new file mode 100644 index 000000000..a8363b542 --- /dev/null +++ b/.changeset/action-disabled-all-renderers.md @@ -0,0 +1,32 @@ +--- +"@object-ui/components": patch +"@object-ui/app-shell": patch +"@object-ui/plugin-detail": patch +--- + +fix(action): honor the spec `disabled` predicate on every action-rendering surface (#1885 follow-through) + +The spec Action field is `disabled` (boolean | CEL — disabled when TRUE); the +schema has no `enabled` key. #1885 wired it in `action:button` only. Browser +dogfooding against the showcase found FIVE more surfaces where a spec-authored +`disabled` silently did nothing: + +- **components** — the `action:group` leaves (inline + dropdown), `action:icon` + and `action:menu` still read the legacy non-spec `enabled`. They now consume + `disabled` as the primary control (evaluated in the same scope as `visible`), + with `enabled` kept as a deprecated fallback. +- **app-shell** — `DeclaredActionsBar` (server-declared action bar) read + neither; it gains `disabled` (no legacy fallback: declared actions are + spec-shaped and never carried `enabled`). +- **plugin-detail** — `record:quick_actions` HAD a `disabled` implementation, + but its `typeof === 'string'` split dropped the `{dialect:'cel', source}` + envelope the server compiles authored CEL into (#2661 routes envelopes to the + canonical formula engine), so the predicate never fired on real metadata. It + now feeds `toPredicateInput`'s result to `useCondition` whole, like every + other surface. + +Pinned by new `DropdownActionItem` tests (disabled-when-TRUE, false-stays- +clickable, disabled-wins-over-enabled, boolean literal) and browser-verified +end-to-end against the showcase `showcase_archive_task` specimen: greyed on an +in-progress task, clickable on a done one (with `visible` hiding Mark Done on +the same screen — the hide-vs-grey contrast). diff --git a/packages/app-shell/src/views/DeclaredActionsBar.tsx b/packages/app-shell/src/views/DeclaredActionsBar.tsx index b1b285854..c73962f50 100644 --- a/packages/app-shell/src/views/DeclaredActionsBar.tsx +++ b/packages/app-shell/src/views/DeclaredActionsBar.tsx @@ -112,6 +112,12 @@ const DeclaredActionButton: React.FC<{ throwOnError: true, label: `declared action "${action.name ?? action.label ?? 'action'}" (visible)`, }); + // Spec `disabled` (boolean | CEL — disabled when TRUE), evaluated against the + // same record context as `visible`. #1885 wired it in action-button only; + // this bar ignored it, so a spec-authored `disabled` guard on a declared + // action did nothing here. (No legacy `enabled` fallback: server-declared + // actions are spec-shaped and never carried the non-spec key.) + const isDisabledPred = useCondition(toPredicateInput((action as any).disabled), recordData); const handleClick = useCallback(async () => { if (loading) return; @@ -219,7 +225,7 @@ const DeclaredActionButton: React.FC<{ type="button" size="sm" variant={variant as any} - disabled={loading} + disabled={((action as any).disabled != null ? isDisabledPred : false) || loading} onClick={handleClick} data-testid={`declared-action-${action.name}`} > diff --git a/packages/components/src/renderers/action/__tests__/action-group-dropdown-visible.test.tsx b/packages/components/src/renderers/action/__tests__/action-group-dropdown-visible.test.tsx index b1578d3ed..835755d4f 100644 --- a/packages/components/src/renderers/action/__tests__/action-group-dropdown-visible.test.tsx +++ b/packages/components/src/renderers/action/__tests__/action-group-dropdown-visible.test.tsx @@ -70,3 +70,43 @@ describe('action:group dropdown item — visible / enabled CEL', () => { expect(item).toHaveAttribute('data-disabled'); }); }); + +// Spec field is `disabled` (boolean | CEL — disabled when TRUE). #1885 wired it +// in action-button only; the group/icon/menu leaves kept reading the legacy +// non-spec `enabled`, so a spec-authored `disabled` guard silently did nothing +// there. These pin the follow-through: `disabled` is the primary control and +// wins over the deprecated `enabled` fallback. +describe('action:group dropdown item — spec `disabled` CEL', () => { + it('disables an action whose `disabled` predicate is TRUE', () => { + renderItem( + { name: 'close', label: 'Close', disabled: 'record.status == "closed"' }, + { record: { status: 'closed' } }, + ); + const item = screen.getByText('Close').closest('[role="menuitem"]'); + expect(item).toHaveAttribute('data-disabled'); + }); + + it('keeps an action clickable when `disabled` is FALSE', () => { + renderItem( + { name: 'close', label: 'Close', disabled: 'record.status == "closed"' }, + { record: { status: 'open' } }, + ); + const item = screen.getByText('Close').closest('[role="menuitem"]'); + expect(item).not.toHaveAttribute('data-disabled'); + }); + + it('`disabled` wins over the legacy `enabled` fallback when both are present', () => { + renderItem( + { name: 'close', label: 'Close', disabled: 'record.locked == true', enabled: 'true' }, + { record: { locked: true } }, + ); + const item = screen.getByText('Close').closest('[role="menuitem"]'); + expect(item).toHaveAttribute('data-disabled'); + }); + + it('supports the boolean literal form (`disabled: true`)', () => { + renderItem({ name: 'close', label: 'Close', disabled: true }); + const item = screen.getByText('Close').closest('[role="menuitem"]'); + expect(item).toHaveAttribute('data-disabled'); + }); +}); diff --git a/packages/components/src/renderers/action/action-group.tsx b/packages/components/src/renderers/action/action-group.tsx index 31d3e9100..f9b1b148f 100644 --- a/packages/components/src/renderers/action/action-group.tsx +++ b/packages/components/src/renderers/action/action-group.tsx @@ -69,6 +69,11 @@ const InlineActionButton: React.FC<{ }> = ({ action, variant, size, onExecute }) => { const [loading, setLoading] = useState(false); const isVisible = useCondition(toPredicateInput(action.visible)); + // Spec field is `disabled` (boolean | CEL — disabled when TRUE). #1885 wired + // it in action-button only; this leaf kept reading the legacy non-spec + // `enabled`, so a spec-authored `disabled` guard did nothing here. `disabled` + // is now the primary control; `enabled` stays as a deprecated fallback. + const isDisabledPred = useCondition(toPredicateInput((action as any).disabled)); const isEnabled = useCondition(toPredicateInput(action.enabled)); const Icon = resolveIcon(action.icon); @@ -93,7 +98,13 @@ const InlineActionButton: React.FC<{ variant={btnVariant as any} size={btnSize as any} className={action.className} - disabled={(action.enabled ? !isEnabled : false) || loading} + disabled={( + (action as any).disabled != null + ? isDisabledPred + : action.enabled != null + ? !isEnabled + : false + ) || loading} onClick={handleClick} > {loading && } @@ -122,10 +133,17 @@ export const DropdownActionItem: React.FC<{ onSelect: (action: ActionSchema) => void | Promise; }> = ({ action, index, onSelect }) => { const isVisible = useCondition(toPredicateInput(action.visible)); + // Spec `disabled` primary, legacy non-spec `enabled` fallback (see + // InlineActionButton above — #1885 follow-through). + const isDisabledPred = useCondition(toPredicateInput((action as any).disabled)); const isEnabled = useCondition(toPredicateInput(action.enabled)); if (action.visible && !isVisible) return null; const Icon = resolveIcon(action.icon); - const isDisabled = action.enabled ? !isEnabled : false; + const isDisabled = (action as any).disabled != null + ? isDisabledPred + : action.enabled != null + ? !isEnabled + : false; const showSeparator = action.tags?.includes('separator-before') && index > 0; return ( <> diff --git a/packages/components/src/renderers/action/action-icon.tsx b/packages/components/src/renderers/action/action-icon.tsx index 3b489c760..0b5ac7e1f 100644 --- a/packages/components/src/renderers/action/action-icon.tsx +++ b/packages/components/src/renderers/action/action-icon.tsx @@ -43,6 +43,10 @@ const ActionIconRenderer = forwardRef( const [loading, setLoading] = useState(false); const isVisible = useCondition(toPredicateInput(schema.visible)); + // Spec `disabled` (boolean | CEL — disabled when TRUE) primary, legacy + // non-spec `enabled` fallback (#1885 follow-through — only action-button + // was wired; this renderer ignored a spec-authored `disabled`). + const isDisabledPred = useCondition(toPredicateInput((schema as any).disabled)); const isEnabled = useCondition(toPredicateInput(schema.enabled)); const Icon = resolveIcon(schema.icon); @@ -85,7 +89,13 @@ const ActionIconRenderer = forwardRef( variant={variant as any} size={size} className={cn('h-8 w-8', schema.className, className)} - disabled={(schema.enabled ? !isEnabled : false) || loading} + disabled={( + (schema as any).disabled != null + ? isDisabledPred + : schema.enabled != null + ? !isEnabled + : false + ) || loading} onClick={handleClick} aria-label={schema.label || schema.name} {...rest} diff --git a/packages/components/src/renderers/action/action-menu.tsx b/packages/components/src/renderers/action/action-menu.tsx index e5e7bd4a0..abd00ba9e 100644 --- a/packages/components/src/renderers/action/action-menu.tsx +++ b/packages/components/src/renderers/action/action-menu.tsx @@ -69,6 +69,10 @@ const ActionMenuItem: React.FC<{ throwOnError: true, label: `action "${action.name ?? action.label ?? 'action:menu item'}" (visible)`, }); + // Spec `disabled` (boolean | CEL — disabled when TRUE) primary, legacy + // non-spec `enabled` fallback (#1885 follow-through — only action-button + // was wired; this renderer ignored a spec-authored `disabled`). + const isDisabledPred = useCondition(toPredicateInput((action as any).disabled)); const isEnabled = useCondition(toPredicateInput(action.enabled)); const iconElement = useMemo(() => { @@ -81,7 +85,11 @@ const ActionMenuItem: React.FC<{ return ( { e.preventDefault(); onExecute(action); diff --git a/packages/plugin-detail/src/renderers/record-quick-actions.tsx b/packages/plugin-detail/src/renderers/record-quick-actions.tsx index 4f28084f8..2c917214e 100644 --- a/packages/plugin-detail/src/renderers/record-quick-actions.tsx +++ b/packages/plugin-detail/src/renderers/record-quick-actions.tsx @@ -198,12 +198,14 @@ function QuickActionButton({ }) { const [running, setRunning] = React.useState(false); const recordCtx = React.useMemo(() => ({ record: record || {} }), [record]); - // `disabled` may be a boolean or a CEL predicate. Evaluate the predicate form - // against the record; useCondition must be called unconditionally (hook). - const disabledPred = toPredicateInput((action as any).disabled); - const condDisabled = useCondition(typeof disabledPred === 'string' ? disabledPred : undefined, recordCtx); - const isDisabled = - (typeof disabledPred === 'string' ? condDisabled : disabledPred === true) || running; + // `disabled` may be a boolean or a CEL predicate (disabled when TRUE). Feed + // toPredicateInput's result to useCondition WHOLE — since #2661 a CEL-dialect + // `{dialect, source}` envelope (the shape the server compiles authored CEL + // into) must reach useCondition intact to route to the canonical formula + // engine. The previous `typeof === 'string'` split dropped the envelope, so + // a spec-authored `disabled` never disabled anything on this surface. + const isDisabledPred = useCondition(toPredicateInput((action as any).disabled), recordCtx); + const isDisabled = ((action as any).disabled != null ? isDisabledPred : false) || running; return (