fix(fields): BooleanField 使用 host 下发的控件 id,可见 label 不再悬空 (#3952)

.changeset/boolean-field-host-owns-control-id.md

@@ -0,0 +1,9 @@
+---
+'@object-ui/fields': patch
+---
+
+fix(fields): `BooleanField` uses the control id its host hands down, so a boolean field's visible label is really associated with the switch
+
+A `boolean` / `checkbox` field inside a form emitted **two** labels with the same text, and the visible one pointed at nothing. `<FormControl>` (a Radix `Slot`) hands the control the id its `<FormLabel htmlFor>` already references, and the widget replaced it with the field name — so `label for="_r_3_-form-item"` had no target while the switch carried `id="notifications"`. Clicking the visible label, the normal affordance for a switch/checkbox row, toggled nothing on every generated form in every app; the accessible name survived only through the widget's own `sr-only` label.
+
+The widget now honours the id it was handed (`id` is a declared key of the widget contract, forwarded by `toDomProps`) and stops emitting its own label when a host supplied one. Standalone rendering is unchanged: with no host id the id still falls back to the field name and then to `useId()`, and the `sr-only` label — the only accessible name the inline grid editor and the console's action-param dialog have — is still emitted.

packages/fields/src/__tests__/boolean-label-association-e2e.test.tsx

@@ -0,0 +1,298 @@
+/**
+ * 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.
+ */
+
+/**
+ * End-to-end: a boolean field's VISIBLE label is associated with the control
+ * the widget renders (objectui#3952).
+ *
+ * The defect: `<FormControl>` is a Radix `Slot` that hands its child the id
+ * `<FormLabel htmlFor>` already points at (`_r_N_-form-item`), and
+ * `BooleanField` REPLACED it with the field name. So the form's visible label
+ * referenced an id no element carried — clicking it toggled nothing — while the
+ * widget emitted a SECOND, `sr-only` label of its own carrying the same text.
+ * The accessible name survived only by that accident.
+ *
+ * Two scenarios, pinned separately, because the widget has two hosts with
+ * opposite needs:
+ *
+ *  - **hosted** (the form renderer, `ObjectForm`, any `<FormControl>`): the host
+ *    owns the id AND the label. The widget must use the id it was handed and
+ *    must NOT emit a label of its own — that is the duplicate half of the bug.
+ *  - **standalone** (`FieldEditWidget`'s inline grid editor, a bare SDUI node):
+ *    nobody hands an id and nobody renders a label, so the widget's own
+ *    `sr-only` label is the control's ONLY accessible name and must stay.
+ *
+ * The widgets are registered raw rather than through `registerAllFields()`,
+ * which wraps every loader in `React.lazy`: an unbounded module load inside a
+ * bounded `findBy`/`waitFor` window is the repo's known flake generator
+ * (AGENTS.md 测试纪律, objectui#3010). Same component, no Suspense boundary.
+ */
+
+import { describe, it, expect, beforeAll, beforeEach, afterEach, vi } from 'vitest';
+import { render, screen, fireEvent, cleanup } from '@testing-library/react';
+import '@testing-library/jest-dom';
+import { ComponentRegistry } from '@object-ui/core';
+// Module scope: pulls in the form renderer's registration side effect.
+import '@object-ui/components';
+
+import { BooleanField } from '../widgets/BooleanField';
+
+beforeAll(() => {
+  ComponentRegistry.register('boolean', BooleanField as any, {
+    namespace: 'field',
+    skipFallback: true,
+  });
+}, 30000);
+
+beforeEach(() => {
+  if (!(Element.prototype as any).scrollIntoView) {
+    (Element.prototype as any).scrollIntoView = () => {};
+  }
+});
+
+afterEach(() => {
+  cleanup();
+  vi.restoreAllMocks();
+});
+
+/** The real form renderer hosting one field — the #3952 reproduction. */
+function renderForm(field: any, defaultValue: unknown) {
+  const Form = ComponentRegistry.get('form')!;
+  return render(
+    <Form
+      schema={{
+        type: 'form',
+        mode: 'create',
+        showSubmit: true,
+        showCancel: false,
+        submitLabel: 'Create',
+        defaultValues: { [field.name]: defaultValue },
+        fields: [field],
+        onSubmit: () => {},
+      }}
+    />,
+  );
+}
+
+/** Every `<label for=…>` in the document, paired with its resolved target. */
+function labelTargets(): Array<{ text: string; forId: string; resolves: boolean }> {
+  return Array.from(document.querySelectorAll('label'))
+    .map((el) => ({
+      text: (el.textContent ?? '').trim(),
+      forId: el.getAttribute('for') ?? '',
+      resolves: !!el.getAttribute('for') && !!document.getElementById(el.getAttribute('for')!),
+    }))
+    .filter((l) => l.forId !== '');
+}
+
+const switchOf = (name: string): Element => {
+  const el = document.querySelector(`[data-field="${name}"] [role="switch"]`);
+  if (!el) throw new Error(`no switch rendered for field "${name}"`);
+  return el;
+};
+
+const BOOLEAN_FIELD = {
+  name: 'notifications',
+  label: 'Email Notifications',
+  type: 'boolean',
+};
+
+describe('a form-hosted boolean field is labelled by its HOST (objectui#3952)', () => {
+  it('every label the form emits points at an element that exists', () => {
+    renderForm(BOOLEAN_FIELD, false);
+
+    const labels = labelTargets();
+    expect(labels.length).toBeGreaterThan(0);
+    // A `for` naming an id nothing carries is the defect verbatim: the form's
+    // visible label used to reference `_r_N_-form-item` while the switch
+    // carried `notifications`.
+    expect(labels.filter((l) => !l.resolves)).toEqual([]);
+  });
+
+  it('the visible label names the switch — one label, associated with the control', () => {
+    renderForm(BOOLEAN_FIELD, false);
+
+    // `getAllByLabelText` resolves label→control through `for`/id, so it guards
+    // the ID half: restore the override and it does not merely return the wrong
+    // node, it throws "found a label … however no form control was found
+    // associated to that label" — the switch left with NO accessible name,
+    // which is exactly what #3952 warned would happen the day the widget's own
+    // label went away. It does NOT guard the duplicate half: two labels
+    // pointing at ONE control still resolve to one element (measured), so the
+    // duplicate is pinned by the label-text count below.
+    const labelled = screen.getAllByLabelText('Email Notifications');
+    expect(labelled).toHaveLength(1);
+    expect(labelled[0]).toBe(switchOf('notifications'));
+    expect(labelled[0]).toHaveAttribute('role', 'switch');
+  });
+
+  it('the label text appears exactly once in the DOM', () => {
+    renderForm(BOOLEAN_FIELD, false);
+
+    // The duplicate half of the defect (and why a naive `getByText` on a
+    // boolean field returned two nodes).
+    expect(screen.getAllByText('Email Notifications')).toHaveLength(1);
+  });
+
+  it('activating the visible label toggles the switch', () => {
+    renderForm(BOOLEAN_FIELD, false);
+
+    const control = switchOf('notifications');
+    expect(control).toHaveAttribute('aria-checked', 'false');
+
+    // The user-facing symptom of #3952: the normal affordance for a switch row
+    // is clicking its label. It reaches the control only through the `for`/id
+    // association the form set up.
+    fireEvent.click(screen.getByText('Email Notifications'));
+
+    expect(switchOf('notifications')).toHaveAttribute('aria-checked', 'true');
+  });
+
+  it('the control keeps the HOST id verbatim — the widget does not rename it', () => {
+    renderForm(BOOLEAN_FIELD, false);
+
+    const control = switchOf('notifications');
+    const id = control.getAttribute('id') ?? '';
+    // The host's id is a `useId()`-derived form-item id; the field name is what
+    // the widget used to substitute for it.
+    expect(id).not.toBe('');
+    expect(id).not.toBe('notifications');
+    expect(id).toMatch(/-form-item$/);
+    // …and it is the id the visible label points at.
+    const visible = screen.getByText('Email Notifications');
+    expect(visible.getAttribute('for')).toBe(id);
+  });
+
+  it('two boolean fields in one form each get their own label', () => {
+    const Form = ComponentRegistry.get('form')!;
+    render(
+      <Form
+        schema={{
+          type: 'form',
+          mode: 'create',
+          showSubmit: false,
+          showCancel: false,
+          defaultValues: { notifications: false, digest: false },
+          fields: [BOOLEAN_FIELD, { name: 'digest', label: 'Weekly Digest', type: 'boolean' }],
+          onSubmit: () => {},
+        }}
+      />,
+    );
+
+    expect(labelTargets().filter((l) => !l.resolves)).toEqual([]);
+    // Each label reaches its OWN switch (a shared/colliding id would make both
+    // resolve to the first one).
+    fireEvent.click(screen.getByText('Weekly Digest'));
+    expect(switchOf('digest')).toHaveAttribute('aria-checked', 'true');
+    expect(switchOf('notifications')).toHaveAttribute('aria-checked', 'false');
+  });
+});
+
+describe('a STANDALONE boolean widget keeps its own accessible name (objectui#3952)', () => {
+  // `FieldEditWidget` (the grid's inline cell editor) renders these widgets with
+  // no `id` and no label of its own, so removing the widget's `sr-only` label
+  // unconditionally would leave the control unnamed. These pin the fallback.
+
+  it('switch variant: the sr-only label names the control', () => {
+    render(
+      <BooleanField
+        value={false}
+        onChange={() => {}}
+        field={{ name: 'notifications', label: 'Email Notifications', type: 'boolean' } as any}
+      />,
+    );
+
+    expect(labelTargets().filter((l) => !l.resolves)).toEqual([]);
+    const labelled = screen.getAllByLabelText('Email Notifications');
+    expect(labelled).toHaveLength(1);
+    expect(labelled[0]).toHaveAttribute('role', 'switch');
+  });
+
+  it('checkbox variant: the sr-only label names the control', () => {
+    render(
+      <BooleanField
+        value={false}
+        onChange={() => {}}
+        field={
+          {
+            name: 'agreed',
+            label: 'I agree',
+            type: 'boolean',
+            widget: 'checkbox',
+          } as any
+        }
+      />,
+    );
+
+    expect(labelTargets().filter((l) => !l.resolves)).toEqual([]);
+    const labelled = screen.getAllByLabelText('I agree');
+    expect(labelled).toHaveLength(1);
+    expect(labelled[0]).toHaveAttribute('role', 'checkbox');
+  });
+});
+
+describe('a host that labels by field NAME keeps its association (objectui#3952)', () => {
+  // `@object-ui/app-shell`'s `ActionParamDialog` boolean branch renders
+  // `<Label htmlFor={param.name}>` and — unlike its own generic branch, which
+  // passes `id={param.name}` — hands the widget NO id. It works only because
+  // the fallback chain's second term derives the id from the field name
+  // (`paramToField` sets `name: param.name`).
+  //
+  // So the chain is `hostId || config.name || useId()`, in that order, and the
+  // middle term is NOT dead weight to be "cleaned up" into a bare `useId()`:
+  // that would silently unlink every boolean action param in the console. This
+  // pin is what makes that attempt fail loudly instead.
+  it('falls back to the field name when no id was handed down', () => {
+    render(
+      <div>
+        <BooleanField
+          value={false}
+          onChange={() => {}}
+          field={{ name: 'confirmed', label: 'Confirmed', type: 'boolean' } as any}
+        />
+        <label htmlFor="confirmed">Send anyway</label>
+      </div>,
+    );
+
+    const control = document.querySelector('[role="switch"]')!;
+    expect(control).toHaveAttribute('id', 'confirmed');
+    expect(labelTargets().filter((l) => !l.resolves)).toEqual([]);
+    // The host's OWN label reaches the control through that id.
+    expect(screen.getAllByLabelText(/Send anyway/)).toHaveLength(1);
+  });
+});
+
+describe('a host-supplied id wins for BOTH variants (objectui#3952)', () => {
+  // The checkbox variant is unreachable through the standalone form renderer —
+  // `resolvedType = widget || field.widget || type` turns `widget: 'checkbox'`
+  // into the renderer's own BUILTIN `checkbox` branch before the registry is
+  // consulted — so its host contract is pinned at the prop boundary, exactly
+  // as `<FormControl>`'s Slot delivers it.
+  it.each([
+    ['switch', undefined, 'switch'],
+    ['checkbox', 'checkbox', 'checkbox'],
+  ] as const)('%s variant uses the id the host handed it', (_name, widget, role) => {
+    render(
+      <label htmlFor="host-item">
+        Host Label
+        <BooleanField
+          value={false}
+          onChange={() => {}}
+          id="host-item"
+          field={{ name: 'agreed', label: 'I agree', type: 'boolean', widget } as any}
+        />
+      </label>,
+    );
+
+    const control = document.querySelector(`[role="${role}"]`)!;
+    expect(control).toHaveAttribute('id', 'host-item');
+    // And the widget adds no second label of its own.
+    expect(screen.queryByText('I agree')).toBeNull();
+    expect(document.querySelectorAll('label')).toHaveLength(1);
+  });
+});

packages/fields/src/widgets/BooleanField.tsx

@@ -13,8 +13,42 @@ export function BooleanField({ value, onChange, field, readonly, ...props }: Fie
   const widget = config?.widget;
   // Generate unique ID using React's useId hook - must be before early returns (rules of hooks)
   const generatedId = useId();
-  const id = config?.name || generatedId;
+  /**
+   * The HOST owns the control's id (objectui#3952).
+   *
+   * `<FormControl>` is a Radix `Slot` that hands its child the id its
+   * `<FormLabel htmlFor>` already points at (`_r_N_-form-item`). This widget
+   * used to REPLACE it with the field name, so the form's visible label
+   * referenced an id no element carried: `label for="_r_3_-form-item"` against
+   * `all ids in form = ["notifications"]`. Clicking the label — the normal
+   * affordance for a switch/checkbox row — toggled nothing, on every generated
+   * form in every app.
+   *
+   * `id` is a DECLARED key of the widget contract (`FieldWidgetDomProps`) and
+   * `toDomProps` forwards it, so honouring it here is the contract's own
+   * direction: the fix belongs at the consumer that discarded a delivered
+   * value, not in the form renderer (AGENTS.md #0.1).
+   *
+   * The fallback chain stays for the STANDALONE hosts that hand down no id —
+   * `FieldEditWidget` (the grid's inline cell editor) and a bare SDUI node.
+   */
+  const hostId = props.id;
+  const id = hostId || config?.name || generatedId;
   const label = config?.label || 'Checkbox';
+  /**
+   * A host that supplies the id is a host that renders its own label for it
+   * (that is what the id is FOR), so the widget's `sr-only` label would be the
+   * second copy of one name — the other half of #3952, and why a naive
+   * `getByText(label)` on a boolean field returned two nodes.
+   *
+   * Standalone it is the control's ONLY accessible name (`FieldEditWidget`
+   * renders no label of its own), so there it stays. Deliberately keyed on the
+   * host id rather than on `config.label`: inside a form the naming is the
+   * host's job for EVERY widget — a text field in a label-less form row is
+   * equally unnamed — and the generic `'Checkbox'` fallback emitted there was
+   * an actively WRONG accessible name, not a safety net.
+   */
+  const emitOwnLabel = !hostId;
 
   if (readonly) {
     return <span className="text-sm">{value ? 'Yes' : 'No'}</span>;
@@ -25,28 +59,28 @@ export function BooleanField({ value, onChange, field, readonly, ...props }: Fie
   if (widget === 'checkbox') {
      return (
         <div className="flex items-center space-x-2">
-            <Checkbox 
+            <Checkbox
                 {...domProps}
                 id={id}
                 checked={!!value}
                 onCheckedChange={(checked) => onChange(!!checked)}
                 disabled={readonly || domProps.disabled}
             />
-            <Label htmlFor={id} className="sr-only">{label}</Label>
+            {emitOwnLabel && <Label htmlFor={id} className="sr-only">{label}</Label>}
         </div>
      )
   }
 
   return (
     <div className="flex items-center space-x-2">
-        <Switch 
+        <Switch
             {...domProps}
-            id={id} 
-            checked={!!value} 
+            id={id}
+            checked={!!value}
             onCheckedChange={onChange}
             disabled={readonly || domProps.disabled}
         />
-        <Label htmlFor={id} className="sr-only">{label}</Label>
+        {emitOwnLabel && <Label htmlFor={id} className="sr-only">{label}</Label>}
     </div>
   );
 }