diff --git a/.changeset/inspector-combo-field-label-3997.md b/.changeset/inspector-combo-field-label-3997.md new file mode 100644 index 000000000..a50a70cdc --- /dev/null +++ b/.changeset/inspector-combo-field-label-3997.md @@ -0,0 +1,19 @@ +--- +'@object-ui/app-shell': patch +--- + +Name the `InspectorComboField` trigger: the visible label now owns it, and an anonymous combo no longer compiles (objectui#3997). + +This is the fourth inspector field atom with the shape PR #3996 fixed for the three in `_shared.tsx` — a `Label` rendered as a plain sibling of the control, with no `htmlFor`, no `id` and no `aria-label`. It lives in its own module, so it stayed broken after the other three were closed. The label and the `button[role=combobox]` were adjacent only visually: assistive tech announced an anonymous combobox with the field name floating above it as unowned text, `getByLabelText` could not reach it, and clicking the visible label did nothing. It renders at eighteen call sites across the object-field, dataset, dashboard-widget, app-nav and view-variant inspectors (lookup display/description fields, `lookupFilters` rows, summary aggregates, dataset dimensions and measures, nav targets), so it is on screen the moment any of those panels opens. + +The labelled branch closes the pair the same way the other atoms do: `React.useId()` mints the id inside the atom, `Label` gets the `htmlFor`, and the id lands on the trigger `Button` that `PopoverTrigger asChild` renders. Never on `Popover` — Radix's `Popover.Root` is a context provider that renders no DOM element, so an id handed to it is dropped silently and the `for` dangles, which is the objectui#3976 / #3994 mistake this repo has now paid for twice. + +`label` was optional, and the un-labelled branch was the same defect one notch worse: a combobox with no name at all. Five of the eighteen call sites had authored exactly that. Rather than adding a lenient fallback (synthesising a name from the placeholder would have produced "Select…" as the announced name), naming became a type-level requirement of exactly one of three channels: + +- `label` — the atom renders the visible label and owns the association. Unchanged for the thirteen call sites that already passed one. +- `ariaLabel` — for repeated rows where no visible label exists and one would break the grid: an app-nav URL filter's `field = value` pair, a dataset's list of joined relationships, the dependent-lookup "add a field" picker. +- `id` — for when an external `Label htmlFor` already owns the naming. `DashboardWidgetInspector` wraps its controls in a `Field` that renders `Label htmlFor={id}` and hands the same id to the control; every other field honoured it (`Input id`, `SelectTrigger id`) but the dataset combo could not, because the atom accepted no id. That `for` pointed at an id nothing carried — a dangling IDREF, worse than an unnamed control, because tooling reports an association that resolves to nothing. + +Zero channels and two channels are now both unauthorable: zero is anonymous, and two is the double-announcement failure objectui#3961/#3978 exists to avoid. Neither has a runtime symptom the component could detect and report — an unnamed combobox renders, lays out and commits values perfectly, and is wrong only for the users who cannot see it — so the check is compile-time or nothing. It is pinned in `InspectorComboField.naming.types.test.tsx`, listed in `tsconfig.typetests.json` so a compiler actually reads it. + +One new pair of strings (`engine.inspector.widget.filterBindingField`, en-US + zh-CN) names the per-filter binding combo in the dashboard widget inspector, which sits under a heading that captions its whole row rather than the combo alone. diff --git a/packages/app-shell/src/views/metadata-admin/i18n.ts b/packages/app-shell/src/views/metadata-admin/i18n.ts index b5d638426..00ed90875 100644 --- a/packages/app-shell/src/views/metadata-admin/i18n.ts +++ b/packages/app-shell/src/views/metadata-admin/i18n.ts @@ -328,6 +328,7 @@ const ENGINE_STRINGS_EN: Record = { 'Map each dashboard-level filter to one of this widget’s own fields, or untick Apply to opt the widget out. Empty = the filter’s own field.', 'engine.inspector.widget.filterBindingApply': 'Apply', 'engine.inspector.widget.filterBindingDefault': 'Default ({field})', + 'engine.inspector.widget.filterBindingField': 'Bound field for {filter}', 'engine.inspector.widget.filterBindingReset': 'Reset', // Flow node inspector 'engine.inspector.flowNode.kind': 'Node', @@ -2074,6 +2075,7 @@ const ENGINE_STRINGS_ZH: Record = { '把每个仪表盘级过滤器映射到本组件自己的字段;取消勾选「应用」可让本组件不受该过滤器影响。留空表示使用过滤器自身的字段。', 'engine.inspector.widget.filterBindingApply': '应用', 'engine.inspector.widget.filterBindingDefault': '默认({field})', + 'engine.inspector.widget.filterBindingField': '{filter} 绑定的字段', 'engine.inspector.widget.filterBindingReset': '恢复默认', // Flow node inspector 'engine.inspector.flowNode.kind': '节点', diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/AppNavInspector.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/AppNavInspector.tsx index 11f69af61..d0eaf80af 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/AppNavInspector.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/AppNavInspector.tsx @@ -235,6 +235,10 @@ function FiltersEditor({
update(i, v, value)} options={fieldOptions} diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/DashboardWidgetInspector.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/DashboardWidgetInspector.test.tsx index bbbe26437..5e9e52bb3 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/DashboardWidgetInspector.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/DashboardWidgetInspector.test.tsx @@ -95,6 +95,34 @@ describe('DashboardWidgetInspector — dataset binding', () => { expect(screen.getByText('维度')).toBeInTheDocument(); }); + it('the Dataset label resolves to the combo trigger, not to nothing (#3997)', () => { + // This panel labels its controls through a `Field` wrapper that renders + // `
`; the id + // has to reach the trigger or that `for` dangles (objectui#3997). + // Every other `Field` in this file already hands its id to the + // control it wraps (`Input id`, `SelectTrigger id`) — this one could + // not, because the combo took no id at all. + id="widget-dataset" value={datasetName} onCommit={(v) => patchWidget({ dataset: v || undefined } as Partial)} options={datasetComboOptions} @@ -346,6 +352,15 @@ export function DashboardWidgetInspector({
setBinding(v ? v : undefined)} options={fieldComboOptions} diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/DatasetDefaultInspector.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/DatasetDefaultInspector.tsx index dea37a25b..be9ac9076 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/DatasetDefaultInspector.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/DatasetDefaultInspector.tsx @@ -412,6 +412,10 @@ export function DatasetDefaultInspector({ draft, onPatch, readOnly, name }: Meta include.map((rel, i) => (
onPatch({ include: include.map((r, idx) => (idx === i ? v : r)) })} options={relationshipComboOptions} diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/InspectorComboField.naming.types.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/InspectorComboField.naming.types.test.tsx new file mode 100644 index 000000000..8ca29a83c --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/inspectors/InspectorComboField.naming.types.test.tsx @@ -0,0 +1,117 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `InspectorComboField`'s naming contract is enforced by the TYPE, so it is + * pinned at compile time (objectui#3997). + * + * The atom accepts exactly one of `label` / `ariaLabel` / `id` — never zero, + * never two. Zero is an anonymous `button[role=combobox]`: assistive tech reads + * "combobox" with no idea which field it edits. Two is the double-announcement + * failure objectui#3961/#3978 exists to avoid. Both were authorable before this + * change, and five of the eleven live call sites had in fact authored zero. + * + * The reason this is a TYPE and not a runtime guard is that neither mistake has + * a runtime symptom the component could detect and report: an unnamed combobox + * renders, lays out and commits values perfectly. It is only wrong for the users + * who cannot see it. So the check has to happen at authoring time or not at all. + * + * ## Why this file exists separately, and why it is listed + * + * The assertions below are its entire point, so it is listed in + * `packages/app-shell/tsconfig.typetests.json`. That listing is the difference + * between a pin and a decoration: the package's build tsconfig excludes + * `**\/*.test.tsx`, and vitest erases types before running, so a + * `@ts-expect-error` written in an ordinary `*.test.tsx` file in this directory + * is read by NO compiler — it neither fails when the error disappears nor when + * the error was never there. This was drafted that way first, and the mutation + * run said so: making naming optional again produced a completely green + * `tsc --noEmit`. That is objectui#3009's failure verbatim (assertions that + * never ran, under a header calling them the real enforcement), and + * `tsconfig.typetests.json`'s own header warns about it — so the type-level + * cases moved here, out of `_shared.labels.test.tsx`, where they are compiled. + * + * The runtime `expect` at the bottom is deliberately thin: the DOM consequences + * (which element carries the id, what the accessible name resolves to, that the + * id never lands on the Radix `Popover` root) are pinned in + * `_shared.labels.test.tsx`, which renders. This file only has to be a file + * vitest can run without complaining that it holds no tests. + */ + +import * as React from 'react'; +import { describe, it, expect } from 'vitest'; +import { InspectorComboField, type InspectorComboFieldProps } from './InspectorComboField'; + +type Assert = T; +type Extends = [A] extends [B] ? true : false; +type IsAny = 0 extends 1 & T ? true : false; +type Equal = (() => T extends A ? 1 : 2) extends () => T extends B ? 1 : 2 + ? true + : false; + +const OPTIONS = [{ value: 'profile', label: 'Profile' }]; +const noop = (_v: string) => {}; + +/** Everything the combo needs EXCEPT a name. */ +type Base = { + value: string; + onCommit: (v: string) => void; + options: Array<{ value: string; label: string }>; +}; + +describe('InspectorComboField — naming is required, and singular (#3997)', () => { + it('is pinned at compile time', () => { + // Guard against the probe lying: were the props `any`, every assignability + // assertion below would pass while proving nothing. + type _PropsNotAny = Assert, false>>; + + // ── the three legal spellings ──────────────────────────────────────────── + // The atom renders the visible label and owns the `htmlFor` ⇄ `id` pair. + type _LabelIsANaming = Assert>; + // No visible label exists (repeated rows); the trigger names itself. + type _AriaLabelIsANaming = Assert>; + // An external `