fix(plugin-grid,components): 行动作的 visible 门按「已声明」判定,visible: false 不再渲染 (#3758)

.changeset/row-action-declared-visible-gate-3758.md

@@ -0,0 +1,33 @@
+---
+"@object-ui/plugin-grid": patch
+"@object-ui/components": patch
+---
+
+Row actions declaring `visible: false` are now hidden instead of rendered
+
+A custom row action's visibility **gate** was detected by truthiness, so
+`visible: false` — the most explicit way an author can say "never show this" —
+fell into the "no gate declared" branch and the action rendered for every row.
+Both surfaces of the ObjectGrid row cell (the "⋮" overflow item and the inline
+`variant:'primary'` button) and the data-table's row overflow menu read the same
+gate, so all three rendered it; the `#3562` emptiness guard counts with that same
+gate, so a row whose only action was `visible: false` also grew a "⋮" it could
+not fill.
+
+The gate now detects a **declared** gate by `!= null && !== ''` and lets the
+declaration itself decide — a boolean short-circuits to its own verdict rather
+than being handed to the CEL engine. This is the invariant objectui#3492 already
+established for the selection bar, whose `hasVisibilityGate` spells out why
+truthiness cannot answer the question, and the same `!= null` posture the
+built-in `visibleWhen` gate has always had. `visible: true` still renders,
+`''` and an absent `visible` are still no gate at all, and no expression-valued
+`visible` changes verdict.
+
+Behaviour change surface, deliberately narrow: only an action whose `visible` is
+the literal boolean `false` (or another falsy non-empty value) changes — it goes
+from rendered to hidden, which is what the declaration asked for.
+`ActionSchema.visible` is `ExpressionInputSchema` with no boolean member, so
+`objectstack build` cannot emit this shape; hand-written view JSON and
+in-process callers constructing defs can, and did. The three row surfaces now
+reach the same verdict as the selection bar and the record page header for every
+non-expression shape, which `predicate-surface-parity` pins.

packages/components/src/renderers/complex/__tests__/data-table-row-action-visible.test.tsx

@@ -114,3 +114,36 @@ describe('data-table row action — visible / disabled CEL evaluation', () => {
     expect(screen.getByTestId('row-action-locked_action')).toHaveAttribute('data-disabled');
   });
 });
+
+/**
+ * objectui#3758 — a DECLARED boolean `visible` is a verdict, not a missing gate.
+ *
+ * `isCustomRowActionVisible` used to ask truthiness (`if (!action?.visible)
+ * return true`), so `visible: false` — the most explicit way to say "never show
+ * this" — answered "no gate declared" and the item rendered for every row.
+ * Declaration is now detected by `!= null && !== ''`, the invariant
+ * objectui#3492 established for the selection bar (`hasVisibilityGate`) and the
+ * one the built-in `visibleWhen` gate has always used, so a declared boolean
+ * reaches the evaluator and decides.
+ *
+ * The `visible: true` and empty-string cases are asserted alongside on purpose:
+ * they are what separates "detect the declaration" from "hide unconditionally".
+ */
+describe('data-table row action — declared boolean `visible` (objectui#3758)', () => {
+  it('hides an action declaring `visible: false`', () => {
+    renderRowActionItem({ name: 'ghost', label: 'Ghost', visible: false }, { id: '2', role: 'member' });
+    expect(screen.queryByTestId('row-action-ghost')).toBeNull();
+    expect(screen.queryByText('Ghost')).toBeNull();
+  });
+
+  it('renders an action declaring `visible: true`', () => {
+    renderRowActionItem({ name: 'always', label: 'Always', visible: true }, { id: '2', role: 'member' });
+    expect(screen.getByTestId('row-action-always')).toBeInTheDocument();
+    expect(screen.getByText('Always')).toBeInTheDocument();
+  });
+
+  it('treats an empty-string `visible` as no gate at all, matching `hasVisibilityGate`', () => {
+    renderRowActionItem({ name: 'compiled_away', label: 'Compiled Away', visible: '' }, { id: '2', role: 'member' });
+    expect(screen.getByTestId('row-action-compiled_away')).toBeInTheDocument();
+  });
+});

packages/components/src/renderers/complex/__tests__/data-table-row-menu-empty-guard.test.tsx

@@ -243,3 +243,46 @@ describe('planDataTableRowMenu', () => {
     expect(plan).toMatchObject({ edit: false, count: 0 });
   });
 });
+
+/**
+ * objectui#3758 reaching the guard: `planDataTableRowMenu` counts with the same
+ * `isCustomRowActionVisible` the item gates itself on, so correcting the gate
+ * propagates to whether the row gets a "⋮" at all — no separate change, and no
+ * way for the trigger and its contents to disagree about a boolean `visible`.
+ */
+describe('planDataTableRowMenu — declared boolean `visible` (objectui#3758)', () => {
+  const scope = {};
+  const row = MIXED_ROWS[1];
+
+  it('drops a custom action declaring `visible: false`, reaching zero items', () => {
+    const plan = planDataTableRowMenu({
+      customActions: [{ name: 'ghost', visible: false }] as any,
+      row,
+      scope,
+    });
+    expect(plan.custom).toEqual([]);
+    expect(plan.count).toBe(0);
+  });
+
+  // Keeps the assertion above from passing for the empty reason: a gate
+  // rewritten to "always hide" would also reach `count: 0`.
+  it('keeps a custom action declaring `visible: true` — declaration detection, not "always hide"', () => {
+    const plan = planDataTableRowMenu({
+      customActions: [{ name: 'always', visible: true }] as any,
+      row,
+      scope,
+    });
+    expect(plan.custom.map((a) => a.name)).toEqual(['always']);
+    expect(plan.count).toBe(1);
+  });
+
+  it('treats an empty-string `visible` as no gate at all, matching `hasVisibilityGate`', () => {
+    const plan = planDataTableRowMenu({
+      customActions: [{ name: 'compiled_away', visible: '' }] as any,
+      row,
+      scope,
+    });
+    expect(plan.custom.map((a) => a.name)).toEqual(['compiled_away']);
+    expect(plan.count).toBe(1);
+  });
+});

packages/components/src/renderers/complex/data-table.tsx

@@ -210,6 +210,11 @@ function evalRowActionVisibility(
   label: string,
 ): boolean {
   if (typeof pred === 'boolean') return pred;
+  // Not dead, and not the place that decides "was a gate declared?" — the two
+  // callers below answer that first, each by its own rule. `''` still arrives
+  // here from `isBuiltinRowActionVisible`, whose gate is `!= null` alone; a
+  // nothing-to-evaluate predicate that reached an evaluator fails CLOSED like
+  // any other unevaluable one.
   if (pred == null || pred === '') return false;
   return evalRowPredicate(pred as never, row ?? {}, {
     fallback: false,
@@ -245,19 +250,28 @@ export function isBuiltinRowActionVisible(
  * Does this schema-driven custom row action render for THIS row? Same
  * single-definition rule as the built-ins above.
  *
- * The gate is truthiness, which is what the item has always applied: a
- * `visible: false` def renders (the objectui#3492 shape). Preserved verbatim —
- * the contract this function exists for is that the guard and the item agree,
- * and re-deciding `visible: false` would change WHICH items render, a separate
- * question tracked on its own issue.
+ * A gate counts as DECLARED by `!= null && !== ''`, never by truthiness
+ * (objectui#3758). Truthiness cannot answer the question: `visible: false` is a
+ * declared gate that excludes every row, and testing `!action.visible`
+ * classified it as *ungated* — so the most explicit way to say "never show this"
+ * rendered the item for everyone, and counted toward the "⋮" guard. This is the
+ * invariant objectui#3492 established for the selection bar (plugin-grid's
+ * `hasVisibilityGate`), and the same `!= null` posture the built-in `visibleWhen`
+ * gate above has always had. The boolean then decides in
+ * {@link evalRowActionVisibility}, which short-circuits it instead of handing it
+ * to the engine.
+ *
+ * `''` is grouped with `null` deliberately: an empty predicate is nothing to
+ * evaluate, so it must not hide the item from everyone either.
  */
 export function isCustomRowActionVisible(
   action: { name?: string; visible?: unknown } | undefined,
   row: any,
   scope: Record<string, unknown>,
 ): boolean {
-  if (!action?.visible) return true;
-  return evalRowActionVisibility(action.visible, row, scope, action.name ?? 'row-action');
+  const pred = action?.visible;
+  if (pred == null || pred === '') return true;
+  return evalRowActionVisibility(pred, row, scope, action?.name ?? 'row-action');
 }
 
 /** What the row overflow menu will actually render for ONE row. */

packages/plugin-grid/src/__tests__/predicate-surface-parity.test.tsx

@@ -21,11 +21,14 @@
  * "⋮" guard share) and the selection bar (`partitionBulkRows`), and depends on
  * `@object-ui/components`, which owns `page:header`.
  *
- * Scope of the claim, deliberately: these fixtures are all non-empty predicate
- * STRINGS / envelopes, i.e. the dialect question. Boolean and empty `visible`
- * are a separate, still-open divergence — the kebab renders a `visible: false`
- * def (objectui#3758) while the header hides it — and pinning them here would
- * read as blessing a difference this issue did not fix.
+ * Scope of the claim: most fixtures are non-empty predicate STRINGS / envelopes,
+ * i.e. the dialect question #3521 asked. The boolean and empty-string shapes were
+ * excluded while they still diverged — the kebab rendered a `visible: false` def
+ * while the bar and the header hid it — and pinning them then would have read as
+ * blessing a difference #3521 did not fix. objectui#3758 closed that divergence
+ * by making the kebab's gate detect a DECLARED gate (`!= null && !== ''`) instead
+ * of a truthy one, so they belong in the table now: the shapes an author can
+ * write with no expression at all reach one verdict on all three surfaces too.
  */
 
 import { describe, it, expect } from 'vitest';
@@ -203,6 +206,13 @@ const CASES: Case[] = [
   },
   // A predicate that cannot be evaluated: fail-CLOSED on all three.
   { what: 'faulting predicate fails closed', name: 'par_fault', visible: 'no_such_var_par == 1', expected: false },
+  // The non-expression shapes, parity-checked since objectui#3758. A declared
+  // BOOLEAN is a verdict every surface short-circuits rather than evaluating;
+  // `''` and an absent `visible` are no gate at all on every surface.
+  { what: 'declared boolean false excludes every row', name: 'par_bool_false', visible: false, expected: false },
+  { what: 'declared boolean true offers every row', name: 'par_bool_true', visible: true, expected: true },
+  { what: 'empty-string `visible` is no gate', name: 'par_empty_string', visible: '', expected: true },
+  { what: 'absent `visible` is no gate', name: 'par_absent', visible: undefined, expected: true },
 ];
 
 describe('one predicate, one verdict on all three action surfaces (#3521)', () => {

packages/plugin-grid/src/components/RowActionMenu.tsx

@@ -132,6 +132,11 @@ function evalRowActionVisibility(
   fields: unknown,
 ): boolean {
   if (typeof pred === 'boolean') return pred;
+  // Not dead, and not the place that decides "was a gate declared?" — the two
+  // callers below answer that first, each by its own rule. `''` still arrives
+  // here from `isBuiltinRowActionVisible`, whose gate is `!= null` alone; a
+  // nothing-to-evaluate predicate that reached an evaluator fails CLOSED like
+  // any other unevaluable one.
   if (pred == null || pred === '') return false;
   return evalRowPredicate(pred as never, row ?? {}, {
     fallback: false,
@@ -175,20 +180,29 @@ export function isBuiltinRowActionVisible(
  * ({@link RowActionInlineButton}). One function, so a def cannot be visible on
  * one surface and hidden on the other.
  *
- * The gate is truthiness, which is what both items have always applied: a
- * `visible: false` def RENDERS (the objectui#3492 shape). Preserved verbatim —
- * the contract this function exists for is that the guard and the items agree,
- * and re-deciding `visible: false` would change WHICH items render, a separate
- * question tracked as objectui#3758.
+ * A gate counts as DECLARED by `!= null && !== ''`, never by truthiness
+ * (objectui#3758). Truthiness cannot answer the question: `visible: false` is a
+ * declared gate that excludes every row, and testing `!def.visible` classified
+ * it as *ungated* — so the most explicit way to say "never show this" rendered
+ * the action for everyone, on both surfaces and in the "⋮" guard's count. This
+ * is `bulkEligibility`'s `hasVisibilityGate` verbatim, the invariant
+ * objectui#3492 established for the selection bar, and the same `!= null`
+ * posture the built-in `visibleWhen` gate above has always had. The boolean then
+ * decides in {@link evalRowActionVisibility}, which short-circuits it instead of
+ * handing it to the engine.
+ *
+ * `''` is grouped with `null` deliberately: an empty predicate is nothing to
+ * evaluate, so it must not hide the action from everyone either.
  */
 export function isCustomRowActionVisible(
   def: RowActionDef | undefined,
   row: any,
   scope: Record<string, unknown>,
   fields?: unknown,
 ): boolean {
-  if (!def?.visible) return true;
-  return evalRowActionVisibility(def.visible, row, scope, def.name ?? 'row-action', fields);
+  const pred = def?.visible;
+  if (pred == null || pred === '') return true;
+  return evalRowActionVisibility(pred, row, scope, def?.name ?? 'row-action', fields);
 }
 
 /** What this row's action cell will actually render. */

packages/plugin-grid/src/components/__tests__/RowActionMenu.emptyGuard.test.tsx

@@ -410,17 +410,60 @@ describe('planRowActionMenu', () => {
     }
   });
 
-  it('preserves the `visible: false` truthy gate verbatim (objectui#3758, not this issue)', () => {
-    // `!def.visible` reads `false` as "ungated", so the def renders and counts —
-    // exactly what the item components have always done. Re-deciding this would
-    // change WHICH items render, which is out of #3562's scope; the point of the
-    // shared function is only that the guard and the items agree.
+  // objectui#3758 replaced the fixture that used to live here. It pinned the
+  // truthiness gate verbatim — `!def.visible` read `false` as "ungated", so the
+  // def rendered and counted — because re-deciding `visible: false` changes
+  // WHICH items render and was out of #3562's scope. #3758 then decided it, the
+  // other way: a declared boolean is a verdict (the #3492 invariant), so the
+  // fixture's expectations (`['ghost']` / `1`) are now the wrong verdicts and
+  // are replaced rather than re-spelled.
+  it('a declared `visible: false` excludes the def and leaves no trigger (objectui#3758)', () => {
     const plan = planRowActionMenu({
       ...base,
       row: FROZEN,
       menuDefs: [{ name: 'ghost', visible: false }],
     });
-    expect(plan.custom.map((a) => a.name)).toEqual(['ghost']);
+    expect(plan.custom).toEqual([]);
+    expect(plan.menuCount).toBe(0);
+  });
+
+  // The counterpart that keeps the assertion above from passing for the empty
+  // reason: a gate rewritten to "always hide" would also reach `menuCount: 0`.
+  it('a declared `visible: true` still counts — declaration detection, not "always hide"', () => {
+    const plan = planRowActionMenu({
+      ...base,
+      row: FROZEN,
+      menuDefs: [{ name: 'always', visible: true }],
+    });
+    expect(plan.custom.map((a) => a.name)).toEqual(['always']);
     expect(plan.menuCount).toBe(1);
   });
+
+  it('an empty-string `visible` is no gate at all, matching `hasVisibilityGate`', () => {
+    const plan = planRowActionMenu({
+      ...base,
+      row: FROZEN,
+      menuDefs: [{ name: 'compiled_away', visible: '' }],
+    });
+    expect(plan.custom.map((a) => a.name)).toEqual(['compiled_away']);
+    expect(plan.menuCount).toBe(1);
+  });
+});
+
+/**
+ * The DOM half of objectui#3758 on this surface: a row whose only custom action
+ * declares `visible: false` renders no "⋮" at all. This is the #3562 guard and
+ * the #3758 gate meeting — the guard counts what the items will render, so
+ * correcting the gate propagates to the trigger with no separate change.
+ */
+describe('declared `visible: false` reaches the "⋮" guard (objectui#3758)', () => {
+  it('renders no trigger when the row\'s only custom action declares `visible: false`', () => {
+    renderMenu({ rowActionDefs: [{ name: 'ghost', label: 'Ghost', variant: 'secondary', visible: false }] });
+    expect(trigger()).not.toBeInTheDocument();
+  });
+
+  it('keeps the trigger when that same action declares `visible: true`', () => {
+    renderMenu({ rowActionDefs: [{ name: 'ghost', label: 'Ghost', variant: 'secondary', visible: true }] });
+    expect(trigger()).toBeInTheDocument();
+  });
 });