fix(app-shell): InspectorComboField 的 label 关联 trigger,无名 combo 不再可编译 (#3997)

.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.

packages/app-shell/src/views/metadata-admin/i18n.ts

@@ -328,6 +328,7 @@ const ENGINE_STRINGS_EN: Record<string, string> = {
     '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<string, string> = {
     '把每个仪表盘级过滤器映射到本组件自己的字段；取消勾选「应用」可让本组件不受该过滤器影响。留空表示使用过滤器自身的字段。',
   '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': '节点',

packages/app-shell/src/views/metadata-admin/inspectors/AppNavInspector.tsx

@@ -235,6 +235,10 @@ function FiltersEditor({
         <div key={`${field}-${i}`} className="flex items-center gap-1">
           <div className="min-w-0 flex-1">
             <InspectorComboField
+              // No visible per-row label — the `field = value` shape is read
+              // from the row itself — so the trigger carries its own name
+              // instead of being an anonymous combobox (objectui#3997).
+              ariaLabel={t('engine.inspector.appNav.filtersField', locale)}
               value={field}
               onCommit={(v) => update(i, v, value)}
               options={fieldOptions}

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
+    // `<Label htmlFor={id}>` and expects the wrapped control to carry the same
+    // id. Every other field honoured it (`Input id`, `SelectTrigger id`); the
+    // dataset combo could not, because `InspectorComboField` took no id — so
+    // `htmlFor="widget-dataset"` pointed at an id nothing carried and the
+    // picker was an anonymous combobox. This is the call-site half of the fix:
+    // the atom's own pins live in `_shared.labels.test.tsx`.
+    renderWidget({ dataset: 'sales_pipeline' });
+
+    const label = screen.getByText('Dataset');
+    const forId = label.getAttribute('for');
+    expect(forId).toBeTruthy();
+
+    const trigger = screen.getByLabelText('Dataset');
+    expect(trigger).toBe(document.getElementById(forId!));
+    expect(trigger.tagName).toBe('BUTTON');
+    expect(trigger).toHaveAttribute('role', 'combobox');
+    expect(trigger).toHaveAccessibleName('Dataset');
+    // Exactly one element answers that id — the trigger, not a wrapper.
+    expect(document.querySelectorAll(`[id="${forId}"]`)).toHaveLength(1);
+
+    // Scoped to this `Field` deliberately: a panel-wide "no dangling for" sweep
+    // would also catch `widget-color`, whose `ColorVariantPicker` accepts no id
+    // either. That is a different component and out of this issue's scope — it
+    // is filed separately rather than fixed here or asserted broken here.
+  });
+
   it('disables every picker when readOnly', () => {
     renderWidget({ dataset: 'sales_pipeline', object: 'crm_opportunity' }, { readOnly: true });
     const combos = screen.getAllByRole('combobox');

packages/app-shell/src/views/metadata-admin/inspectors/DashboardWidgetInspector.tsx

@@ -259,6 +259,12 @@ export function DashboardWidgetInspector({
         </div>
         <Field id="widget-dataset" label={t('engine.inspector.widget.dataset', locale)}>
           <InspectorComboField
+            // `Field` above renders `<Label htmlFor="widget-dataset">`; 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<DashboardWidgetSchema>)}
             options={datasetComboOptions}
@@ -346,6 +352,15 @@ export function DashboardWidgetInspector({
                   <div className="flex items-center gap-1">
                     <div className="min-w-0 flex-1">
                       <InspectorComboField
+                        // The filter's name above is a heading for the whole row
+                        // (it also captions the Apply checkbox) and disappears
+                        // from the association when the row is opted out, so the
+                        // trigger carries its own name rather than borrowing it
+                        // (objectui#3997). Includes the filter name because a
+                        // dashboard has several of these rows.
+                        ariaLabel={tFormat('engine.inspector.widget.filterBindingField', locale, {
+                          filter: def.label || def.name,
+                        })}
                         value={override}
                         onCommit={(v) => setBinding(v ? v : undefined)}
                         options={fieldComboOptions}

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) => (
             <div key={i} className="flex items-center gap-1.5">
               <InspectorComboField
+                // One row per join under the "Included relationships" heading;
+                // no per-row visible label, so the trigger is named directly
+                // rather than left anonymous (objectui#3997).
+                ariaLabel="Included relationship"
                 value={rel}
                 onCommit={(v) => onPatch({ include: include.map((r, idx) => (idx === i ? v : r)) })}
                 options={relationshipComboOptions}

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 extends true> = T;
+type Extends<A, B> = [A] extends [B] ? true : false;
+type IsAny<T> = 0 extends 1 & T ? true : false;
+type Equal<A, B> = (<T>() => T extends A ? 1 : 2) extends <T>() => 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<Equal<IsAny<InspectorComboFieldProps>, false>>;
+
+    // ── the three legal spellings ────────────────────────────────────────────
+    // The atom renders the visible label and owns the `htmlFor` ⇄ `id` pair.
+    type _LabelIsANaming = Assert<Extends<Base & { label: string }, InspectorComboFieldProps>>;
+    // No visible label exists (repeated rows); the trigger names itself.
+    type _AriaLabelIsANaming = Assert<Extends<Base & { ariaLabel: string }, InspectorComboFieldProps>>;
+    // An external `<Label htmlFor={id}>` owns the naming; the id must reach the
+    // trigger or that `for` dangles.
+    type _IdIsANaming = Assert<Extends<Base & { id: string }, InspectorComboFieldProps>>;
+
+    // ── zero naming channels is not authorable ───────────────────────────────
+    // The regression this file exists for. Five call sites shipped this shape.
+    type _AnonymousRejected = Assert<Equal<Extends<Base, InspectorComboFieldProps>, false>>;
+
+    // ── two naming channels is not authorable either ─────────────────────────
+    type _LabelPlusAriaRejected = Assert<
+      Equal<Extends<Base & { label: string; ariaLabel: string }, InspectorComboFieldProps>, false>
+    >;
+    type _LabelPlusIdRejected = Assert<
+      Equal<Extends<Base & { label: string; id: string }, InspectorComboFieldProps>, false>
+    >;
+    type _AriaPlusIdRejected = Assert<
+      Equal<Extends<Base & { ariaLabel: string; id: string }, InspectorComboFieldProps>, false>
+    >;
+
+    // ── the same two rejections as a caller actually writes them ─────────────
+    // Assignability above is the contract; JSX is the authoring surface, and
+    // excess-property checking makes them differ often enough to pin both.
+    const anonymous = (
+      // @ts-expect-error objectui#3997 — a combo with no name must not compile.
+      <InspectorComboField value="" options={OPTIONS} onCommit={noop} />
+    );
+    const doublyNamed = (
+      // @ts-expect-error objectui#3997 — `label` and `ariaLabel` are exclusive.
+      <InspectorComboField label="Group" ariaLabel="Group" value="" options={OPTIONS} onCommit={noop} />
+    );
+
+    expect([anonymous, doublyNamed].every(React.isValidElement)).toBe(true);
+  });
+
+  it('accepts each legal spelling as a real element', () => {
+    // Keeps the three positive cases honest: an over-tight union that rejected a
+    // shape a call site needs would fail here rather than only in CI's build.
+    const labelled = <InspectorComboField label="Group" value="" options={OPTIONS} onCommit={noop} />;
+    const ariaLabelled = <InspectorComboField ariaLabel="Group" value="" options={OPTIONS} onCommit={noop} />;
+    const externallyLabelled = (
+      <InspectorComboField id="widget-dataset" value="" options={OPTIONS} onCommit={noop} />
+    );
+
+    expect([labelled, ariaLabelled, externallyLabelled].every(React.isValidElement)).toBe(true);
+  });
+});