diff --git a/.changeset/8450-inspector-select-placeholder.md b/.changeset/8450-inspector-select-placeholder.md new file mode 100644 index 0000000000..415678f5e3 --- /dev/null +++ b/.changeset/8450-inspector-select-placeholder.md @@ -0,0 +1,27 @@ +--- +'@object-ui/app-shell': patch +--- + +Make `InspectorSelectField`'s `placeholder` reach the rendered trigger +(objectui#8450). Every empty select in the metadata-admin designer drew a BLANK +trigger instead of its hint. + +The field bridges a caller's `''` through an internal sentinel so a "— None —" +option can exist at all (Radix `` throws). That bridge also +guaranteed the value handed to Radix was never `''` or `undefined` — the only two +values for which Radix renders `SelectValue`'s placeholder — and a controlled +value matching no `SelectItem` renders as nothing. So the declared `'—'` default +was unreachable at all 45 call sites, none of which passes a placeholder of its +own. + +The trigger now renders the placeholder itself, on the narrow state that means +"nothing is selected": no value AND no option standing for none. Where the caller +DOES offer a `''` row, that row is a selection and its label still wins, +unchanged. A non-empty value matching no option also still renders blank — +that is a stale value, not an empty one, and is out of this change's scope. + +Measured effect on the designer: 13 of the 45 call sites can be empty without +offering a "none" row, and those now show `'—'` where they showed nothing — the +flow-node config selects, the app-nav item type, the curated page-block props, +the report dataset/chart-axis pickers and the action target/variant/mode/component +pickers. The other 32 render exactly as before. diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.declaredDefault.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.declaredDefault.test.tsx index ca747dc66b..6e44f03cfd 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.declaredDefault.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.declaredDefault.test.tsx @@ -24,10 +24,13 @@ * * 1. On a BOOLEAN control a declared default now seeds the value: an unset key * draws the declared state, a stored value beats it, and nothing is written - * to the node. On a SELECT it still reaches nothing — an unset key draws a - * blank trigger, not the declared option and not even a placeholder, - * because the field always passes a controlled value and Radix renders its - * placeholder only for an undefined one (objectui#8450). + * to the node. On a SELECT it still reaches nothing — an unset key draws + * `InspectorSelectField`'s own em-dash PLACEHOLDER, which says "nothing is + * selected" and is not anything the table declared. (objectui#8450 made + * that placeholder reachable at all; before it the trigger was blank, + * because the sentinel the field bridges `''` through kept Radix from ever + * recognising the empty state. Blank and em-dash are the same fact about + * `defaultValue`, told twice.) * 2. Both writers of the property feed the repaired boolean — the hand-written * table here and the engine-published `configSchema` that * `json-schema-to-fields` converts (`default: true` -> `defaultValue: @@ -166,15 +169,16 @@ describe('select: a declared defaultValue still does not reach the control (obje renderInspector(draftWith('http_request', { config: {} })); - // Measured, not assumed: the trigger is EMPTY — it shows neither the - // declared default nor `InspectorSelectField`'s own em-dash placeholder. - // Radix renders a placeholder only for an UNCONTROLLED/undefined value; the - // field always passes a controlled one (the `''` -> sentinel bridge), and a - // controlled value matching no `SelectItem` renders as nothing at all. + // Measured, not assumed: the trigger shows `InspectorSelectField`'s own + // em-dash placeholder — the "nothing is selected" mark — and NOT the + // declared default. (It rendered blank before objectui#8450, which fixed + // the placeholder's own unreachability in the shared primitive. That was a + // defect in the primitive, not evidence about `defaultValue`; this card's + // subject is the assertion below, which is unchanged either way.) expect( triggerText('Method'), - 'the Method trigger renders empty — no declared default, not even the placeholder', - ).toBe(''); + 'the Method trigger renders its placeholder — the declared default seeds nothing', + ).toBe('—'); expect( screen.queryByText('GET'), 'the declared default "GET" reaches no part of the rendered inspector', @@ -315,8 +319,9 @@ describe('the online writer of defaultValue hits the same dead end', () => { expect( triggerText('Method'), - 'the server-derived default is not seeded into the control either', - ).toBe(''); + 'the server-derived default is not seeded into the control either — the ' + + 'trigger sits on its placeholder, same as the hand-written half', + ).toBe('—'); expect(screen.queryByText('POST'), 'the derived default reaches no rendered node').toBeNull(); }); }); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/_shared.select.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/_shared.select.test.tsx index 1ddaf35d44..c0762f85fd 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/_shared.select.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/_shared.select.test.tsx @@ -1,11 +1,47 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. /** - * Regression: InspectorSelectField must accept an option whose value is - * the empty string (a "— None —" choice). Radix `` - * throws on render; the field bridges "" through an internal sentinel. - * This is exactly what the object field "Group" selector relies on once - * field groups exist, so a regression here crashes the whole inspector. + * Two contracts of `InspectorSelectField`'s trigger, pinned together because + * they are the same question asked twice: what does the trigger render when the + * caller's value is EMPTY? + * + * 1. Regression: the field must accept an option whose value is the empty + * string (a "— None —" choice). Radix `` throws on + * render; the field bridges "" through an internal sentinel. This is exactly + * what the object field "Group" selector relies on once field groups exist, + * so a regression here crashes the whole inspector. + * + * 2. objectui#8450: when the caller offers NO such option, the empty state must + * draw the `placeholder`. It used not to. The sentinel that makes (1) work + * is also what broke (2): Radix renders `SelectValue`'s placeholder only for + * a value of `''` or `undefined` (`shouldShowPlaceholder`), the bridge + * guarantees the value is neither, and a controlled value matching no + * `SelectItem` renders as nothing at all. Measured across the tree: 45 + * non-test call sites, 0 of which passed an explicit `placeholder` — so what + * never rendered was the declared `'—'` default, at every empty select in + * the designer. + * + * ⚠️ The two pull against each other, which is why they live in one file. A + * repair that shows the placeholder whenever the value is empty breaks (1): the + * "— No group —" row IS a selection and its label must win. A repair that keeps + * quiet whenever the value is empty is the bug in (2). The predicate that + * satisfies both is narrow — no value AND no option standing for "none". + * + * ## The non-regression half (objectui#8350's lesson) + * + * Every positive case below is also satisfied by a field that renders the + * placeholder ALWAYS — an implementation strictly worse than the bug, since it + * would hide the selected value too. The two "does not render" cases are what + * fail on it, and they assert the trigger's exact text rather than the + * placeholder's absence, so an implementation that renders both also fails. + * + * ## Reverse verification + * + * Ablating the READ SITE — restoring `` + * inside an unconditional trigger, i.e. the pre-fix source — turns the three + * `renders …` rows red and leaves the two `does not render …` rows green. That + * asymmetry is the point: the pin fails for the bug's own reason and for no + * other. Run recorded on the PR. */ import { describe, it, expect, vi, afterEach } from 'vitest'; @@ -20,6 +56,16 @@ const OPTIONS = [ { value: 'meta', label: 'Metadata' }, ]; +/** The same roster minus the "none" row — the shape 35 of the 45 call sites use. */ +const NO_NONE_OPTIONS = OPTIONS.filter((o) => o.value !== ''); + +/** The trigger Radix renders — `button[role=combobox]`, named by the `