diff --git a/.changeset/action-param-dialog-option-visiblewhen-4758.md b/.changeset/action-param-dialog-option-visiblewhen-4758.md new file mode 100644 index 000000000..c0335d9f7 --- /dev/null +++ b/.changeset/action-param-dialog-option-visiblewhen-4758.md @@ -0,0 +1,26 @@ +--- +'@object-ui/components': patch +--- + +`ActionParamDialog` (the `custom` barrel's published dialog) now resolves each +`select` param's options through `@object-ui/core`'s shared option evaluator, so a +per-option `visibleWhen` narrows the offered list here exactly as it does on the +app-shell action dialog and in the object form (objectui#4758). + +This surface is the repo's second action-param dialog, and its `select` branch +rendered `param.options?.map(...)` straight into Radix items. A per-option +`visibleWhen` was not evaluated wrongly — it was not evaluated at all, so an option +gated on `record.*` (a sibling param) or on `current_user.*` was offered +unconditionally, while the app-shell dialog filtered the identical field metadata. +Triage ruled the governed side authoritative; the dialog rebinds to +`resolveVisibleOptions`, resolving predicates against the dialog's own in-progress +values (the objectui#3765 Option B ruling) plus the ambient predicate scope. + +Rebind, not removal: the component stays a published export, its props and every +other branch are untouched, and retiring it remains a separate decision. + +A selection the predicate stops offering is now cleared rather than kept as a hidden +value — the same `isValueStillOffered` clear `SelectField` already performs. Without +it, filtering alone would let a picked-then-gated-out option vanish from the trigger +while still riding in the submitted payload. Params whose options declare no +predicate are untouched. diff --git a/packages/components/src/__tests__/action-param-dialog-option-visible-when.test.tsx b/packages/components/src/__tests__/action-param-dialog-option-visible-when.test.tsx new file mode 100644 index 000000000..b347bb768 --- /dev/null +++ b/packages/components/src/__tests__/action-param-dialog-option-visible-when.test.tsx @@ -0,0 +1,265 @@ +/** + * 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. + */ + +/** + * `components/custom`'s ActionParamDialog — per-option `visibleWhen` reaches + * this surface too (objectui#4758). + * + * The `select` branch rendered `param.options?.map(...)` straight into Radix + * `SelectItem`s, bypassing `@object-ui/core`'s option evaluator entirely. On + * this published dialog a per-option `visibleWhen` was not evaluated WRONG — it + * was not evaluated at all, so every option was offered unconditionally while + * the app-shell dialog (objectui#3765 / PR #4756) filtered the same metadata. + * Triage ruled the governed side authoritative and this one rebinds to it + * ("rebind, don't delete" — the component is a published export). + * + * ## What each case is for + * + * Two cases below are NON-VACUITY CONTROLS and pass on the unfixed code: they + * prove the harness mounts the dialog and really opens the Radix listbox, so a + * red result from the other cases is the defect rather than broken setup. + * Marked inline. + * + * ## Reverse verification + * + * Restoring the bare `param.options?.map(...)` turns the four defect cases red + * (an unfiltered list offers every option); the two controls stay green because + * neither depends on filtering happening. + */ + +import React from 'react'; +import { describe, it, expect, vi, beforeAll, afterEach } from 'vitest'; +import { render, screen, fireEvent, waitFor, cleanup } from '@testing-library/react'; +import type { ActionParamDef } from '@object-ui/core'; +import { PredicateScopeProvider } from '@object-ui/react'; +import { ActionParamDialog } from '../custom/action-param-dialog'; + +// Radix Select opens on pointer events happy-dom does not implement — same +// shim the in-form select tests use (`renderers/form/__tests__/ +// option-value-round-trip.test.tsx`). +beforeAll(() => { + class MockPointerEvent extends Event { + button: number; + ctrlKey: boolean; + pointerType: string; + constructor(type: string, props: any = {}) { + super(type, props); + this.button = props.button ?? 0; + this.ctrlKey = props.ctrlKey ?? false; + this.pointerType = props.pointerType ?? 'mouse'; + } + } + (window as any).PointerEvent = MockPointerEvent; + (HTMLElement.prototype as any).hasPointerCapture = vi.fn(); + (HTMLElement.prototype as any).releasePointerCapture = vi.fn(); + (HTMLElement.prototype as any).scrollIntoView = vi.fn(); +}); + +afterEach(cleanup); + +/** The controlling param — a plain select with no predicates of its own. */ +const tierParam: ActionParamDef = { + name: 'tier', + label: 'Tier', + type: 'select', + options: [ + { label: 'Silver tier', value: 'silver' }, + { label: 'Gold tier', value: 'gold' }, + ], +}; + +/** The dependent param: its second option is gated on the SIBLING param. */ +const planParam: ActionParamDef = { + name: 'plan', + label: 'Plan', + type: 'select', + options: [ + { label: 'Basic plan', value: 'basic' }, + { label: 'Premium plan', value: 'premium', visibleWhen: "record.tier == 'gold'" }, + ], +}; + +function dialog(params: ActionParamDef[], onSubmit = vi.fn()) { + return ( + + ); +} + +/** Open a param's Radix select by its host-owned control id. */ +function openSelect(name: string) { + const trigger = document.getElementById(name); + expect(trigger).toBeTruthy(); + fireEvent.pointerDown(trigger!, { button: 0 }); +} + +/** The option labels a user can actually see in the open listbox. */ +function offeredLabels(): string[] { + return screen.getAllByRole('option').map((o) => (o.textContent ?? '').trim()); +} + +/** Pick an option by its visible label. */ +async function pick(label: string) { + const option = await screen.findByRole('option', { name: label }); + fireEvent.click(option); + await waitFor(() => expect(screen.queryByRole('option', { name: label })).toBeNull()); +} + +describe('custom ActionParamDialog — per-option visibleWhen (objectui#4758)', () => { + it('CONTROL: mounts the dialog and really opens the select', async () => { + render(dialog([planParam])); + + // The label a user reads, from the real render tree. + expect(screen.getByText('Plan')).toBeInTheDocument(); + + openSelect('plan'); + // If this resolves, the listbox opened — the precondition every case below + // depends on. It passes with or without the fix. + expect(await screen.findByRole('option', { name: 'Basic plan' })).toBeInTheDocument(); + }); + + it('does not offer an option whose visibleWhen is FALSE against the dialog values', async () => { + render(dialog([tierParam, planParam])); + + openSelect('plan'); + await screen.findByRole('option', { name: 'Basic plan' }); + + // `tier` is still unset, so `record.tier == 'gold'` is false and the gold- + // only option must not be in the list the user sees. + expect(offeredLabels()).toEqual(['Basic plan']); + expect(screen.queryByRole('option', { name: 'Premium plan' })).toBeNull(); + }); + + it("resolves the predicate against the DIALOG's own in-progress values", async () => { + render(dialog([tierParam, planParam])); + + openSelect('tier'); + await pick('Gold tier'); + + openSelect('plan'); + await screen.findByRole('option', { name: 'Basic plan' }); + // The record IS this dialog's values (the ruled Option B semantics): once + // the sibling param says gold, the gated option is offered. A fix that + // resolved against a constant `{}` would leave it hidden here. + expect(offeredLabels()).toEqual(['Basic plan', 'Premium plan']); + }); + + it('honours role/context gating supplied by the predicate scope', async () => { + const roleParam: ActionParamDef = { + name: 'grant', + label: 'Grant', + type: 'select', + options: [ + { label: 'Read only', value: 'read' }, + { label: 'Admin only', value: 'admin', visibleWhen: "'admin' in current_user.positions" }, + ], + }; + + render( + + {dialog([roleParam])} + , + ); + + openSelect('grant'); + await screen.findByRole('option', { name: 'Read only' }); + expect(offeredLabels()).toEqual(['Read only']); + + cleanup(); + + render( + + {dialog([roleParam])} + , + ); + + openSelect('grant'); + await screen.findByRole('option', { name: 'Read only' }); + // CONTROL half: an admin still sees both — the filter narrows, it does not + // blank the list. Green with or without the fix. + expect(offeredLabels()).toEqual(['Read only', 'Admin only']); + }); + + it('fails OPEN: a broken predicate keeps its option offered', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const brokenParam: ActionParamDef = { + name: 'broken', + label: 'Broken', + type: 'select', + options: [ + { label: 'Kept', value: 'kept' }, + { label: 'Also kept', value: 'also', visibleWhen: 'record.tier ===' }, + ], + }; + + render(dialog([brokenParam])); + openSelect('broken'); + await screen.findByRole('option', { name: 'Kept' }); + expect(offeredLabels()).toEqual(['Kept', 'Also kept']); + } finally { + warn.mockRestore(); + } + }); + + it('drops a selection the predicate stopped offering, instead of submitting it unseen', async () => { + const onSubmit = vi.fn(); + render(dialog([tierParam, planParam], onSubmit)); + + openSelect('tier'); + await pick('Gold tier'); + openSelect('plan'); + await pick('Premium plan'); + + // What the user sees right now: the trigger reads back their choice. + await waitFor(() => + expect(document.getElementById('plan')!.textContent).toContain('Premium plan'), + ); + + // Now the controlling param moves away, so the gold-only option is no + // longer offered. + openSelect('tier'); + await pick('Silver tier'); + + // The trigger must not keep displaying an option the list no longer holds, + // and the submit must not carry it. + await waitFor(() => + expect(document.getElementById('plan')!.textContent).not.toContain('Premium plan'), + ); + + fireEvent.click(screen.getByRole('button', { name: 'Continue' })); + await waitFor(() => expect(onSubmit).toHaveBeenCalled()); + expect(onSubmit.mock.calls[0][0].plan).not.toBe('premium'); + expect(onSubmit.mock.calls[0][0].tier).toBe('silver'); + }); + + it('leaves an option list that declares no predicate completely untouched', async () => { + const onSubmit = vi.fn(); + const plainParam: ActionParamDef = { + name: 'plain', + label: 'Plain', + type: 'select', + // A default that matches no option — nothing here may clear it, because + // no option on this param declares a predicate. + defaultValue: 'orphaned', + options: [ + { label: 'One', value: 'one' }, + { label: 'Two', value: 'two' }, + ], + }; + + render(dialog([plainParam], onSubmit)); + openSelect('plain'); + await screen.findByRole('option', { name: 'One' }); + expect(offeredLabels()).toEqual(['One', 'Two']); + + fireEvent.keyDown(document.body, { key: 'Escape' }); + fireEvent.click(screen.getByRole('button', { name: 'Continue' })); + await waitFor(() => expect(onSubmit).toHaveBeenCalled()); + expect(onSubmit.mock.calls[0][0].plain).toBe('orphaned'); + }); +}); diff --git a/packages/components/src/custom/action-param-dialog.tsx b/packages/components/src/custom/action-param-dialog.tsx index d27374931..0472c4937 100644 --- a/packages/components/src/custom/action-param-dialog.tsx +++ b/packages/components/src/custom/action-param-dialog.tsx @@ -9,8 +9,10 @@ * Used by the ActionRunner when an action defines params to collect. */ -import React, { useState, useCallback } from 'react'; +import React, { useState, useCallback, useEffect, useMemo } from 'react'; import type { ActionParamDef } from '@object-ui/core'; +import { isValueStillOffered, resolveVisibleOptions, type OptionLike } from '@object-ui/core'; +import { usePredicateScope } from '@object-ui/react'; import { createSafeTranslation } from '@object-ui/i18n'; import { Dialog, @@ -130,6 +132,87 @@ export const ActionParamDialog: React.FC = ({ const [errors, setErrors] = useState>({}); + // ── Per-option `visibleWhen`, through the SHARED evaluator (objectui#4758) ── + // + // This file is the repo's SECOND action-param dialog, and its `select` branch + // used to render `param.options?.map(...)` straight into Radix `SelectItem`s. + // A per-option `visibleWhen` was not evaluated WRONGLY here — it was not + // evaluated at all, so an option the field metadata gates on `record.*` or + // `current_user.*` was offered unconditionally on this published surface, + // while the app-shell dialog (objectui#3765 / PR #4756) filtered the identical + // metadata. Triage ruled the governed side authoritative and this one rebinds + // to it. Rebind, NOT delete: `ActionParamDialog` is a published export of this + // package's `custom` barrel, so retiring it is a separate maintainer decision. + // + // `resolveVisibleOptions` is the same entry the governed path reaches: + // app-shell renders params through `@object-ui/fields`' widgets, whose + // `useCascadingOptions` calls `resolveCascadingOptions`, which delegates the + // filtering half to this very function. + // + // The `dependsOn` GATING half of `resolveCascadingOptions` ("select the parent + // first") is deliberately not reproduced, and that is measured rather than + // assumed: `paramToField()` copies `depends_on` onto the field only for + // `EXPANDABLE_FIELD_TYPES` (lookup / reference / user), so a `select` param + // reaches the governed widget carrying no `dependsOn` at all and + // `resolveCascadingOptions` reduces to exactly the call below. Gating this + // branch would make the two surfaces DIVERGE, not converge. + // + // The record is this dialog's own in-progress `values` — the ruling on + // objectui#3765 (maintainer 2026-08-11, Option B: "the dialog is a small + // form"), which app-shell implements as `dependentValues={values}` on the + // widget. `current_user` / `features` / `app` come from the ambient predicate + // scope, `{}` when no host mounted a provider, which is the same source the + // object form reads (`renderers/form/form.tsx`). + // + // The cast is the seam `@object-ui/core`'s `ActionParamDef.options.test.ts` + // documents: `ActionParamOption`'s catch-all types every key other than + // `label` / `value` as `unknown`, so it is not STATICALLY an `OptionLike` + // even though every value it carries is one. Written out rather than narrowing + // the param option type, which would re-open objectui#3559. + const predicateScope = usePredicateScope(); + const optionState = useMemo(() => { + const byParam = new Map(); + for (const p of params) { + if (p.type !== 'select') continue; + const raw = (p.options ?? []) as OptionLike[]; + byParam.set(p.name, { + offered: resolveVisibleOptions(raw, values, predicateScope), + predicated: raw.some((o) => o?.visibleWhen != null), + }); + } + return byParam; + }, [params, values, predicateScope]); + + // A selection the predicate stopped offering must not survive as a hidden + // value. Filtering alone would introduce a state the unfiltered code could not + // reach: the user picks an option, then changes the sibling param that gated + // it, and the trigger falls back to its placeholder (Radix renders no label + // for a value with no matching item) while `values` still holds the choice and + // `handleSubmit` still submits it — gone from the screen, present in the + // payload. The governed side already answers this with the same shared helper + // (`fields/src/widgets/SelectField.tsx` clears when + // `!isValueStillOffered(value, options)`), so this is the sibling's shape, not + // a new rule invented here. + // + // Confined to option lists that actually declare a predicate: with no + // `visibleWhen` anywhere on the param the offered set IS the authored set, + // this effect can never fire, and the change stays inert for every param that + // rendered correctly before. (The governed widget also clears a value matching + // no option at all — a different question, about unmatched authored defaults, + // left exactly as it was.) + useEffect(() => { + const stale = params.filter((p) => { + const state = optionState.get(p.name); + return state?.predicated === true && !isValueStillOffered(values[p.name], state.offered); + }); + if (stale.length === 0) return; + setValues((prev) => { + const next = { ...prev }; + for (const p of stale) next[p.name] = ''; + return next; + }); + }, [params, values, optionState]); + const handleChange = useCallback((name: string, value: any) => { setValues((prev) => ({ ...prev, [name]: value })); // Clear error on change @@ -254,8 +337,15 @@ export const ActionParamDialog: React.FC = ({ - {param.options?.map((opt) => ( - + {/* The offered set, not the authored one — see `optionState` + above for why this branch may not read `param.options` + directly (objectui#4758). `String(...)` because the shared + reader types an option value `string | number | boolean` + (core#3090) while a Radix item speaks strings; a resolved + param option's `value` is already declared `string`, so this + is a type bridge and not a conversion. */} + {(optionState.get(param.name)?.offered ?? []).map((opt) => ( + {opt.label} ))}