diff --git a/.changeset/action-member-declared-disabled-gate-3849.md b/.changeset/action-member-declared-disabled-gate-3849.md new file mode 100644 index 0000000000..f66b66875c --- /dev/null +++ b/.changeset/action-member-declared-disabled-gate-3849.md @@ -0,0 +1,50 @@ +--- +"@object-ui/components": patch +"@object-ui/plugin-detail": patch +--- + +`disabled: ''` no longer greys out the remaining five action surfaces (objectui#3849) + +objectui#3842 / PR #3851 fixed the "is a `disabled` gate DECLARED?" test on +`action:button` and app-shell's `DeclaredActionsBar`. Five same-shaped sites were +outside that PR's scope and stayed on `!= null`, so within one component the +`visible` gate asked `hasDeclaredVisibilityGate` while the `disabled` gate on the +next line asked `!= null` — two spellings of one question: + +- `@object-ui/components` — `action:icon`, `action:group`'s inline button + (`InlineActionButton`) and dropdown item (`DropdownActionItem`), and + `action:menu`'s item (`ActionMenuItem`). +- `@object-ui/plugin-detail` — `record:quick_actions`' `QuickActionButton`. + +Why the missing `!== ''` half is a defect on this key and not on `visible`: +`toPredicateInput('')` is `undefined` and `evaluateCondition(undefined)` is +`true`. On `visible` that `true` means SHOW, so an over-broad "declared" test and +a permissive empty predicate cancel out. On `disabled` it means DISABLE, so they +compound — `disabled: ''` (an empty predicate: nothing declared) rendered a +permanently greyed-out control, with nothing the author could write to un-grey +it. Unlike #3842's approvals inbox, these five are the general action face +(toolbars, dropdowns, record quick actions), so the reach is wider even though no +single high-value host owns them. + +**Behaviour change surface, deliberately narrow.** Only `disabled: ''` changes — +from disabled to clickable, which is what "no predicate" asked for. `disabled: +true` still disables, `disabled: false` and an absent `disabled` still do not, and +no expression-valued `disabled` changes verdict. On the four sites that also carry +the legacy non-spec `enabled` fallback, one consequence follows: an empty +`disabled` now falls THROUGH to that leg instead of short-circuiting on the empty +predicate, so an action spelling both (`disabled: ''` + `enabled: true`) becomes +clickable. `record:quick_actions` has no `enabled` leg, so its chain is the single +gate. + +Routing those legacy `enabled` legs through the same definition is +behaviour-preserving by derivation rather than a fix: the leg is negated +(`disabled = !isEnabled`), so an empty predicate's `true` already arrived as "not +disabled" — the verdict "no gate declared" produces. #3842's four-shape derivation +table is reproduced next to the new pins, together with the statement that no +`enabled` case can go red by reverting that leg. + +`hasDeclaredVisibilityGate` keeps its historic name (the objectui#3842 ruling): the +predicate is key-neutral, and one implementation behind two names is how a repo +grows dialects. The three `@object-ui/components` sites import it relatively; +`record:quick_actions` takes it from the package barrel, the cross-package route +objectui#3835 opened. Every call site says so in a comment. diff --git a/packages/components/src/renderers/action/__tests__/action-member-disabled-declared-gate.test.tsx b/packages/components/src/renderers/action/__tests__/action-member-disabled-declared-gate.test.tsx new file mode 100644 index 0000000000..0cc3880a03 --- /dev/null +++ b/packages/components/src/renderers/action/__tests__/action-member-disabled-declared-gate.test.tsx @@ -0,0 +1,236 @@ +/** + * 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. + */ + +/** + * objectui#3849 — the declared-`disabled` gate on the four remaining action-face + * sites. objectui#3842 / PR #3851 fixed `action:button` (and app-shell's + * `DeclaredActionsBar`); these four stayed on `!= null`, so the same component + * asked "is a gate DECLARED?" one way for `visible` and another for `disabled`. + * + * The mechanism is #3842's, verbatim: `toPredicateInput('')` is `undefined` and + * `evaluateCondition(undefined)` is `true`. On `visible` that `true` means SHOW, + * so an over-broad "declared" test and a permissive empty predicate cancel out. + * On `disabled` the same `true` means DISABLE, so they compound — `disabled: ''` + * (an empty predicate, i.e. nothing declared) became "greyed out forever", with + * nothing the author could write to un-grey it. All four now read + * `hasDeclaredVisibilityGate` (`!= null && !== ''`), imported rather than + * re-spelled; historic name kept per the #3842 ruling. + * + * ## What each case detects + * + * • `disabled: ''` → NOT disabled. THE defect, and a genuine mutation + * detector on this key: restore `!= null` at one site and that site's `''` + * case alone goes red (the reverse-verification this PR ran). + * • `disabled: true` → disabled; `disabled: false` / undeclared → not + * disabled. Anti-mutation guards: "never disable anything" satisfies three + * of the four shapes on its own, and `true` is what refuses it. + * • expression-valued `disabled` → the verdict still decides, both ways. The + * gate narrowed; evaluation did not change. + * + * ## The legacy `enabled` leg — four cases that are documentation, one that moves + * + * The leg is NEGATED (`disabled = !isEnabled`), so an empty predicate's `true` + * arrives as `!true` = "not disabled", which is exactly what "no gate declared" + * produces. Every shape reaches the same verdict under either test, so the + * tightening is behaviour-preserving by derivation (#3842's table): + * + * | `enabled` | `!= null` (old) | `hasDeclaredVisibilityGate` (new) | + * |-------------|---------------------------|-----------------------------------| + * | `''` | gate → `!true` = enabled | no gate → `false` = enabled | + * | `true` | gate → `!true` = enabled | gate → `!true` = enabled | + * | `false` | gate → `!false` = DISABLED| gate → `!false` = DISABLED | + * | undeclared | no gate → `false` | no gate → `false` | + * + * Stated plainly rather than dressed up as coverage: no `enabled` case here can + * go red by reverting the `enabled` leg. They are kept because they pin the + * semantics the derivation asserts (`enabled: false` must still disable), which + * a future rewrite of this chain would otherwise break silently. The case that + * DOES move is precedence: with `disabled: ''` no longer a gate, the chain falls + * through to the legacy leg instead of short-circuiting on an empty predicate. + */ + +import { describe, it, expect } from 'vitest'; +import { render, screen } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import React from 'react'; +import { ComponentRegistry } from '@object-ui/core'; +import { PredicateScopeProvider } from '@object-ui/react'; +// Module-scope side-effect imports so the two hosts are in the registry when +// `ComponentRegistry.get` runs — the light `dom` project deliberately does not +// load the `@object-ui/components` graph. Module scope, not a `beforeAll`, per +// AGENTS.md §测试纪律: the cost lands in the import phase, unbounded by any +// hook timeout. +import '../action-icon'; +import { DropdownActionItem } from '../action-group'; +import { ActionMenuItem } from '../action-menu'; +import { DropdownMenu, DropdownMenuContent, DropdownMenuTrigger } from '../../../ui'; + +const LABEL = 'Act'; +const ACT = { name: 'act', label: LABEL, type: 'script' }; + +/** An ungated companion — a passing assertion must not mean "the host vanished". */ +const COMPANION = { name: 'view', label: 'View', type: 'script' }; + +function getRenderer(type: string) { + const R = ComponentRegistry.get(type); + if (!R) throw new Error(`${type} is not registered`); + return R; +} + +/** + * Site 1 — `action:icon`, mounted the way `action:bar` mounts a member: the + * whole action spread onto the leaf's own `schema` (`action-bar.tsx` resolves + * the renderer from the registry itself, so this gate is the only one on that + * path — the reachability #3823 established for the `visible` half of the same + * two lines). + */ +function mountIcon(action: any, scope: Record = {}) { + const Renderer = getRenderer('action:icon'); + return render( + + + , + ); +} + +/** + * Site 2 — `InlineActionButton`, through the real `action:group` host so the + * member gate is observed where it runs (the group `.map()`s its own `actions`; + * neither `SchemaRenderer` nor `ActionEngine` is in this path). + */ +function mountInlineGroup(action: any, scope: Record = {}) { + const Group = getRenderer('action:group'); + return render( + + + , + ); +} + +/** + * Sites 3 and 4 — the two dropdown leaves, each inside a controlled-open menu so + * the portal content mounts deterministically (Radix opens on pointerdown, flaky + * to synthesize in happy-dom). Same harness as + * `action-group-dropdown-visible.test.tsx` and `action-member-visible-gate.test.tsx`. + */ +function mountInMenu(node: React.ReactNode, scope: Record = {}) { + return render( + + + menu + {node} + + , + ); +} + +/** A `