fix(webui): replace native SettingsField controls with shared components (#8021)

* fix(webui): use shared settings field controls * fix(webui): stabilize settings control widths * fix(webui): preserve select menu accessibility semantics * test(e2e): target select menus by combobox role
2026-09-02 23:56:24 +08:00 · 2026-09-02 07:58:42 +00:00
parent 4ba6a668af
commit 99457e152e
7 changed files with 214 additions and 42 deletions
--- a/crates/product/ironclaw_webui/frontend/src/design-system/select-menu.dom.test.tsx
+++ b/crates/product/ironclaw_webui/frontend/src/design-system/select-menu.dom.test.tsx
@@ -0,0 +1,80 @@
+// @vitest-environment happy-dom
+
+import assert from "node:assert/strict";
+import React, { act, useState } from "react";
+import { createRoot } from "react-dom/client";
+import { test } from "vitest";
+
+import { SelectMenu } from "./select-menu";
+
+globalThis.IS_REACT_ACT_ENVIRONMENT = true;
+
+test("SelectMenu exposes a valid select-only combobox relationship in the DOM", () => {
+  const container = document.createElement("div");
+  document.body.append(container);
+  const root = createRoot(container);
+
+  function Harness() {
+    const [value, setValue] = useState("sandbox");
+    return (
+      <SelectMenu
+        value={value}
+        onChange={setValue}
+        aria-label="Execution policy"
+        options={[
+          { value: "sandbox", label: "Sandbox policy" },
+          { value: "fusion", label: "Fusion strategy" },
+          { value: "tunnel", label: "Tunnel provider" },
+        ]}
+      />
+    );
+  }
+
+  try {
+    act(() => root.render(<Harness />));
+
+    const combobox = container.querySelector<HTMLButtonElement>(
+      'button[role="combobox"]',
+    );
+    assert.ok(combobox, "expected the trigger to expose the combobox role");
+    assert.equal(combobox.getAttribute("aria-label"), "Execution policy");
+    assert.match(combobox.textContent ?? "", /Sandbox policy/);
+    const closedListboxId = combobox.getAttribute("aria-controls");
+    assert.ok(closedListboxId, "expected the combobox to always reference its popup");
+    const closedListbox = document.getElementById(closedListboxId);
+    assert.equal(closedListbox?.getAttribute("role"), "listbox");
+    assert.equal(closedListbox?.hidden, true);
+
+    act(() => {
+      combobox.focus();
+      combobox.dispatchEvent(
+        new KeyboardEvent("keydown", { key: "ArrowDown", bubbles: true }),
+      );
+    });
+
+    assert.equal(document.activeElement, combobox);
+    assert.equal(combobox.getAttribute("aria-expanded"), "true");
+    assert.equal(combobox.getAttribute("aria-controls"), closedListboxId);
+    const openListbox = document.getElementById(closedListboxId);
+    assert.equal(openListbox?.getAttribute("role"), "listbox");
+    assert.equal(openListbox?.hidden, false);
+
+    const activeOptionId = combobox.getAttribute("aria-activedescendant");
+    assert.ok(activeOptionId, "expected the open combobox to expose its active option");
+    const activeOption = document.getElementById(activeOptionId);
+    assert.equal(activeOption?.getAttribute("role"), "option");
+    assert.match(activeOption?.textContent ?? "", /Fusion strategy/);
+
+    act(() => {
+      combobox.dispatchEvent(
+        new KeyboardEvent("keydown", { key: "Enter", bubbles: true }),
+      );
+    });
+
+    assert.equal(combobox.getAttribute("aria-expanded"), "false");
+    assert.match(combobox.textContent ?? "", /Fusion strategy/);
+  } finally {
+    act(() => root.unmount());
+    container.remove();
+  }
+});
--- a/crates/product/ironclaw_webui/frontend/src/design-system/select-menu.stories.tsx
+++ b/crates/product/ironclaw_webui/frontend/src/design-system/select-menu.stories.tsx
@@ -59,7 +59,7 @@ export const Placeholder: Story = { args: { initialValue: "", placeholder: "Sele
 export const Selecting: Story = {
  // The listbox open/close + selection is the behavior worth proving.
  play: async ({ canvas, userEvent }) => {
-    const trigger = canvas.getByRole("button", { name: /model provider/i });
+    const trigger = canvas.getByRole("combobox", { name: /model provider/i });
    await expect(trigger).toHaveAttribute("aria-expanded", "false");
    await userEvent.click(trigger);
    await expect(trigger).toHaveAttribute("aria-expanded", "true");
--- a/crates/product/ironclaw_webui/frontend/src/design-system/select-menu.test.ts
+++ b/crates/product/ironclaw_webui/frontend/src/design-system/select-menu.test.ts
@@ -254,14 +254,18 @@ test("SelectMenu renders a closed custom trigger with the selected label", () =>
  assert.match(collectTemplateText(rendered), /aria-haspopup="listbox"/);
  assert.equal(firstValueAfter(rendered, "aria-expanded="), "false");
  assert.equal(collectObjects(rendered).some((value) => "aria-owns" in value), false);
-  assert.equal(collectObjects(rendered).some((value) => "aria-controls" in value), false);
+  assert.match(
+    firstObjectWith(rendered, "aria-controls")["aria-controls"],
+    /^v2-select-menu-\d+-listbox$/,
+  );
  assert.equal(
    collectObjects(rendered).some((value) => "aria-activedescendant" in value),
    false
  );
  assert.ok(collectScalars(rendered).includes("Follow global"));
  assert.doesNotMatch(collectTemplateText(rendered), /<select/);
-  assert.doesNotMatch(collectTemplateText(rendered), /role="listbox"/);
+  assert.match(collectTemplateText(rendered), /role="listbox"/);
+  assert.match(collectTemplateText(rendered), /hidden="true"/);
 });

 test("SelectMenu renders a generic option adornment in the trigger and listbox", () => {
--- a/crates/product/ironclaw_webui/frontend/src/design-system/select-menu.tsx
+++ b/crates/product/ironclaw_webui/frontend/src/design-system/select-menu.tsx
@@ -286,7 +286,7 @@ export function SelectMenu({
  const optionsKey = optionsIdentity(visibleOptions);
  const rootPassthroughProps = safeRootProps(rest);
  const buttonListboxProps = {
-    ...(open ? { "aria-controls": listboxId } : {}),
+    "aria-controls": listboxId,
    ...(activeOptionId && !searchable
      ? { "aria-activedescendant": activeOptionId }
      : {}),
@@ -417,6 +417,7 @@ export function SelectMenu({
      <button
        ref={buttonRef}
        type="button"
+        role={searchable ? undefined : "combobox"}
        aria-haspopup="listbox"
        aria-expanded={open ? "true" : "false"}
        aria-label={effectiveAriaLabel}
@@ -464,6 +465,8 @@ export function SelectMenu({
        </span>
      )}

+      {!open && <div id={listboxId} role="listbox" hidden />}
+
      {open && (
        <div
          className={cn(
--- a/crates/product/ironclaw_webui/frontend/src/pages/settings/components/settings-field.test.ts
+++ b/crates/product/ironclaw_webui/frontend/src/pages/settings/components/settings-field.test.ts
@@ -43,14 +43,18 @@ function component(name) {
 }

 function renderSettingsField() {
+  const Input = component("Input");
+  const SelectMenu = component("SelectMenu");
  const Switch = component("Switch");
  const context = {
    Card: component("Card"),
+    Input,
    React: {
      useCallback: (callback) => callback,
      useEffect: () => {},
      useState: (initialValue) => [initialValue, () => {}],
    },
+    SelectMenu,
    Switch,
    useT: () => (key) => key,
  };
@@ -60,7 +64,7 @@ function renderSettingsField() {
    context,
    import.meta.url
  );
-  return { exports, Switch };
+  return { exports, Input, SelectMenu, Switch };
 }

 test("SettingsField uses the compact shared Switch and preserves string persistence", () => {
@@ -87,3 +91,75 @@ test("SettingsField uses the compact shared Switch and preserves string persiste
  props.onChange(false);
  assert.deepEqual(saved, [{ key: "agent.use_planning", value: "false" }]);
 });
+
+test("SettingsField uses the shared Input and preserves numeric constraints and commit behavior", () => {
+  const { exports, Input } = renderSettingsField();
+  const saved = [];
+  const rendered = exports.SettingsField({
+    field: {
+      key: "search.weight",
+      label: "Search weight",
+      type: "float",
+      min: 0,
+      max: 1,
+      step: 0.1,
+    },
+    value: 0.5,
+    onSave: (key, value) => saved.push({ key, value }),
+    isSaved: false,
+  });
+  const inputNode = findComponentNode(rendered, Input);
+
+  assert.ok(inputNode, "expected numeric settings field to render the shared Input");
+  const props = componentProps(inputNode, Input);
+  assert.equal(props.type, "number");
+  assert.equal(props.size, "sm");
+  assert.equal(props.className, "text-right font-mono");
+  assert.equal(props.step, "0.1");
+  assert.equal(props.min, "0");
+  assert.equal(props.max, "1");
+  assert.equal(props.placeholder, "tools.default");
+  assert.equal(props["aria-label"], "Search weight");
+
+  props.onBlur({ currentTarget: { value: "0.75" } });
+  props.onKeyDown({ key: "Escape", currentTarget: { value: "0.9" } });
+  props.onKeyDown({ key: "Enter", currentTarget: { value: "0.9" } });
+  assert.deepEqual(saved, [
+    { key: "search.weight", value: 0.75 },
+    { key: "search.weight", value: 0.9 },
+  ]);
+});
+
+test("SettingsField uses the shared SelectMenu and preserves the default option", () => {
+  const { exports, SelectMenu } = renderSettingsField();
+  const saved = [];
+  const rendered = exports.SettingsField({
+    field: {
+      key: "search.fusion_strategy",
+      label: "Fusion strategy",
+      type: "select",
+      options: ["rrf", "weighted"],
+    },
+    value: "rrf",
+    onSave: (key, value) => saved.push({ key, value }),
+    isSaved: false,
+  });
+  const selectNode = findComponentNode(rendered, SelectMenu);
+
+  assert.ok(selectNode, "expected select settings field to render the shared SelectMenu");
+  const props = componentProps(selectNode, SelectMenu);
+  assert.equal(props.ariaLabel, "Fusion strategy");
+  assert.equal(props.className, "!min-w-0 w-36");
+  assert.deepEqual(JSON.parse(JSON.stringify(props.options)), [
+    { label: "tools.default", value: "" },
+    { label: "rrf", value: "rrf" },
+    { label: "weighted", value: "weighted" },
+  ]);
+
+  props.onChange("weighted");
+  props.onChange("");
+  assert.deepEqual(saved, [
+    { key: "search.fusion_strategy", value: "weighted" },
+    { key: "search.fusion_strategy", value: null },
+  ]);
+});
--- a/crates/product/ironclaw_webui/frontend/src/pages/settings/components/settings-field.tsx
+++ b/crates/product/ironclaw_webui/frontend/src/pages/settings/components/settings-field.tsx
@@ -1,6 +1,8 @@
 import React from "react";
 import { useT } from "../../../lib/i18n";
 import { Card } from "../../../design-system/card";
+import { Input } from "../../../design-system/input";
+import { SelectMenu } from "../../../design-system/select-menu";
 import { Switch } from "../../../design-system/switch";

 function SavedIndicator({ visible }) {
@@ -21,6 +23,12 @@ export function SettingsField({ field, value, onSave, isSaved }) {
  const [localValue, setLocalValue] = React.useState("");
  const label = field.labelKey ? t(field.labelKey) : field.label || "";
  const description = field.descKey ? t(field.descKey) : field.description || "";
+  const selectOptions = field.type === "select"
+    ? [
+        { label: t("tools.default"), value: "" },
+        ...field.options.map((option) => ({ label: option, value: option })),
+      ]
+    : [];

  React.useEffect(() => {
    if (field.type !== "boolean") {
@@ -65,35 +73,34 @@ export function SettingsField({ field, value, onSave, isSaved }) {
            )
          : field.type === "select"
          ? (
-              <select
+              <SelectMenu
                value={localValue}
-                onChange={(e) => {
-                  setLocalValue(e.currentTarget.value);
-                  handleCommit(e.currentTarget.value);
+                options={selectOptions}
+                onChange={(nextValue) => {
+                  setLocalValue(nextValue);
+                  handleCommit(nextValue);
                }}
-                aria-label={label}
-                className="v2-select h-9 rounded-md border border-white/12 bg-white/[0.04] px-3 text-sm text-iron-100 outline-none focus:border-signal/45"
-              >
-                <option value="">{t("tools.default")}</option>
-                {field.options.map(
-                  (opt) => (<option key={opt} value={opt}>{opt}</option>)
-                )}
-              </select>
+                ariaLabel={label}
+                className="!min-w-0 w-36"
+              />
            )
          : (
-              <input
-                type={field.type === "float" || field.type === "number" ? "number" : "text"}
-                value={localValue}
-                onChange={(e) => setLocalValue(e.currentTarget.value)}
-                onBlur={(e) => handleCommit(e.currentTarget.value)}
-                onKeyDown={(e) => e.key === "Enter" && handleCommit(e.currentTarget.value)}
-                step={field.step !== undefined ? String(field.step) : field.type === "float" ? "any" : "1"}
-                min={field.min !== undefined ? String(field.min) : undefined}
-                max={field.max !== undefined ? String(field.max) : undefined}
-                placeholder={t("tools.default")}
-                aria-label={label}
-                className="h-9 w-36 rounded-md border border-white/12 bg-white/[0.04] px-3 text-right font-mono text-sm text-iron-100 outline-none placeholder:text-iron-700 focus:border-signal/45"
-              />
+              <div className="w-36">
+                <Input
+                  type={field.type === "float" || field.type === "number" ? "number" : "text"}
+                  value={localValue}
+                  onChange={(e) => setLocalValue(e.currentTarget.value)}
+                  onBlur={(e) => handleCommit(e.currentTarget.value)}
+                  onKeyDown={(e) => e.key === "Enter" && handleCommit(e.currentTarget.value)}
+                  step={field.step !== undefined ? String(field.step) : field.type === "float" ? "any" : "1"}
+                  min={field.min !== undefined ? String(field.min) : undefined}
+                  max={field.max !== undefined ? String(field.max) : undefined}
+                  placeholder={t("tools.default")}
+                  aria-label={label}
+                  size="sm"
+                  className="text-right font-mono"
+                />
+              </div>
            )}
        <SavedIndicator visible={isSaved} />
      </div>
--- a/tests/e2e/scenarios/test_reborn_webui_v2_smoke.py
+++ b/tests/e2e/scenarios/test_reborn_webui_v2_smoke.py
@@ -1850,9 +1850,9 @@ async def test_reborn_v2_model_capability_tags_persist_after_policy_reload(

        selector = page.locator(SEL_V2["settings_model_selector"])
        await expect(
-            selector.get_by_role("button").locator("[data-capability='text']")
+            selector.get_by_role("combobox").locator("[data-capability='text']")
        ).to_have_attribute("title", "Text")
-        await selector.get_by_role("button").click()
+        await selector.get_by_role("combobox").click()
        vision_option = page.get_by_role("option").filter(has_text=vision_model)
        await expect(
            vision_option.locator("[data-capability='image-input']")
@@ -1919,11 +1919,11 @@ async def _choose_model_preference(
    await expect(page.get_by_role("button", name="Add provider")).to_have_count(0)
    await expect(page.locator(SEL_V2["settings_model_policy_editor"])).to_have_count(0)
    selector = page.locator(SEL_V2["settings_model_selector"])
-    button = selector.get_by_role("button")
-    await expect(button).to_be_enabled(timeout=15000)
-    await button.click()
+    combobox = selector.get_by_role("combobox")
+    await expect(combobox).to_be_enabled(timeout=15000)
+    await combobox.click()
    await page.get_by_role("option", name=selected_model, exact=True).click()
-    await expect(button).to_contain_text(selected_model)
+    await expect(combobox).to_contain_text(selected_model)
    description = page.get_by_text(
        "Used for future messages in all conversations.", exact=True
    )
@@ -1972,11 +1972,11 @@ async def _assert_model_preference_permissions(
        "/settings/inference",
        SEL_V2["settings_model_selector"],
    )
-    button = default_page.locator(SEL_V2["settings_model_selector"]).get_by_role(
-        "button"
+    combobox = default_page.locator(SEL_V2["settings_model_selector"]).get_by_role(
+        "combobox"
    )
-    await expect(button).to_contain_text("mock-model")
-    await expect(button).not_to_contain_text(selected_model)
+    await expect(combobox).to_contain_text("mock-model")
+    await expect(combobox).not_to_contain_text(selected_model)


 async def _send_model_preference_turn(
@@ -2107,7 +2107,9 @@ async def test_reborn_v2_settings_model_preference_reaches_provider(
            SEL_V2["settings_model_selector"],
        )
        await expect(
-            selected_page.locator(SEL_V2["settings_model_selector"]).get_by_role("button")
+            selected_page.locator(SEL_V2["settings_model_selector"]).get_by_role(
+                "combobox"
+            )
        ).to_contain_text(selected_model)


@@ -3637,7 +3639,7 @@ async def test_reborn_v2_logs_page_passes_scope_to_api_and_renders_context(
    ).to_contain_text("run-ui")

    level_filter = reborn_v2_page.locator(SEL_V2["logs_level_filter"])
-    level_trigger = level_filter.get_by_role("button")
+    level_trigger = level_filter.get_by_role("combobox")
    await expect(level_trigger).to_have_attribute("aria-haspopup", "listbox")
    await expect(level_filter.locator("select")).to_have_count(0)
    await reborn_v2_page.locator(SEL_V2["logs_target_filter"]).fill("ironclaw::ui")