From b9d1923f15b2d47cb9f6b2052b6404a361b22502 Mon Sep 17 00:00:00 2001
From: jackwener <jakevingoo@gmail.com>
Date: Fri, 26 Jun 2026 02:43:48 +0800
Subject: [PATCH] =?UTF-8?q?refactor(ui,settings):=20Round=20C=20=E2=80=94?=
 =?UTF-8?q?=20ChoiceCard=20primitive=20+=20Theme/Palette=20pickers?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

@WAWQAQ msg `4f598b19` 「没做完你就继续呀」 — finally takes the
hand-rolled Theme/Palette native-button radiogroups off
SettingsModal.tsx and onto a Base UI-backed primitive.

**Why this was a contract-test-protected migration.** Commit b40d097
(WAWQAQ msg `5f75daf6`) once routed these cards through the shared
`<Button>` primitive, which baked `h-9 inline-flex bg-primary
text-white` Tailwind utilities into them — every card collapsed to a
36px black pill and the swatches + preview mocks disappeared. The
rollback added `settings-theme-contract.test.ts:90-121` to pin native
`<button role="radio">` so the regression couldn't recur.

**The migration.** `packages/ui/src/primitives/choice-card.tsx`
introduces `ChoiceCard` (wraps Base UI `Radio.Root`) and
`ChoiceCardGroup` (wraps `RadioGroup`). Crucially, neither primitive
ships **any** layout/background utilities — the caller's `className`
owns every visual decision. The existing `.settingsThemeOption*` /
`.settingsPaletteOption*` CSS chrome stays exactly as it was; the
primitive only contributes Base UI's `data-checked` state attribute,
keyboard arrow-nav, focus management, and ARIA roles.

The Theme picker (3 vertically-stacked preview tiles) and Palette
picker (11 swatches across 2 subgroups) lose their inline
`onSettingsRadioGroupKeyDown` keyboard handlers and `radioTabIndex`
calls — Base UI provides the same behavior at primitive level.

`.settingsThemeOption[data-active="true"]` → `[data-checked]` (the
Base UI state hook). Same selected-state visuals, just keyed off the
attribute the primitive sets.

**Helpers retained.** `onSettingsRadioGroupKeyDown`, `nextRadioId`,
`focusRadioValue`, and `radioTabIndex` stay because `Segmented`
(the inline `[value, label]` pill picker) still uses native-button
keyboard handling; its design lane has no card chrome to protect, so
flipping it would not earn its keep here.

**Test pivot.** `settings-theme-contract.test.ts` flipped:
- Old test "keeps theme and palette radio cards on native <button>,
  not <Button>" → new test "uses the ChoiceCard primitive (not native
  <button> or shared <Button>) for theme + palette cards", and
- The keyboard-nav test now asserts the theme block uses
  `<ChoiceCardGroup … value=… onValueChange>` and explicitly
  `doesNotMatch` `/onSettingsRadioGroupKeyDown|radioTabIndex|
  data-radio-value/`.

Both contract suites pass:
  ✔ Settings theme page contract  (6/6)
  ✔ renderer style pruning contract  (2/2)
---
 .../__tests__/settings-theme-contract.test.ts | 68 ++++++++++-----
 .../src/renderer/settings/SettingsModal.tsx   | 66 +++++----------
 apps/desktop/src/renderer/styles.css          |  2 +-
 packages/ui/src/index.ts                      |  1 +
 packages/ui/src/primitives/choice-card.tsx    | 84 +++++++++++++++++++
 5 files changed, 157 insertions(+), 64 deletions(-)
 create mode 100644 packages/ui/src/primitives/choice-card.tsx

diff --git a/apps/desktop/src/main/__tests__/settings-theme-contract.test.ts b/apps/desktop/src/main/__tests__/settings-theme-contract.test.ts
index 9175c139d9..3b6ace718f 100644
--- a/apps/desktop/src/main/__tests__/settings-theme-contract.test.ts
+++ b/apps/desktop/src/main/__tests__/settings-theme-contract.test.ts
@@ -71,50 +71,80 @@ describe('Settings theme page contract', () => {
     const themePage = src.match(/function ThemeSettingsPage\([\s\S]*?function WebSearchSettingsPage/)?.[0] ?? '';
     const segmentedBlock = src.match(/function Segmented[\s\S]*?function Switch/)?.[0] ?? '';
 
+    // `onSettingsRadioGroupKeyDown` + `nextRadioId` + `focusRadioValue` +
+    // `radioTabIndex` still exist because `Segmented` (the inline
+    // [value, label] pill picker) keeps native-button + manual keyboard
+    // nav — it has no card chrome to protect, so flipping it would not
+    // earn its keep here. Verify the helper logic is intact.
     assert.match(helperBlock, /nextRadioId\(current, values, event\.key\)/);
     assert.match(helperBlock, /event\.preventDefault\(\)/);
     assert.match(helperBlock, /onChange\(next\)/);
     assert.match(helperBlock, /const group = event\.currentTarget/);
     assert.match(helperBlock, /setTimeout\(\(\) => focusRadioValue\(group, next\), 0\)/);
-    assert.match(themePage, /aria-label="主题"[\s\S]*onKeyDown=\{\(event\) => onSettingsRadioGroupKeyDown/);
-    assert.match(themePage, /aria-label=\{group\.label\}[\s\S]*onKeyDown=\{\(event\) => onSettingsRadioGroupKeyDown/);
-    assert.match(themePage, /data-radio-value=\{option\.value\}[\s\S]*tabIndex=\{radioTabIndex\(option\.value, props\.themePref/);
-    assert.match(themePage, /data-radio-value=\{palette\}[\s\S]*tabIndex=\{radioTabIndex\(palette, currentPalette, group\.palettes\)\}/);
+
+    // Theme + palette pickers now delegate keyboard navigation to the
+    // Base UI `RadioGroup` inside `ChoiceCardGroup`. Both groups must
+    // pass `value` + `onValueChange` (not `onKeyDown`) and must NOT
+    // reach back to the legacy helpers.
+    assert.match(themePage, /<ChoiceCardGroup[\s\S]*aria-label="主题"[\s\S]*value=\{props\.themePref\}[\s\S]*onValueChange/);
+    assert.match(themePage, /<ChoiceCardGroup[\s\S]*aria-label=\{group\.label\}[\s\S]*value=\{currentPalette\}[\s\S]*onValueChange/);
+    assert.doesNotMatch(themePage, /onSettingsRadioGroupKeyDown|radioTabIndex|data-radio-value/);
     assert.doesNotMatch(themePage, /界面密度|props\.density|setDensity|onDensityChange/);
+
+    // `Segmented` still uses the legacy helpers. Pin them so a future
+    // sweep doesn't accidentally drop them while the Segmented call
+    // sites are still around.
     assert.match(segmentedBlock, /if \(props\.disabled\) return;[\s\S]*onSettingsRadioGroupKeyDown\(event, values, props\.value, props\.onChange\)/);
     assert.match(segmentedBlock, /aria-disabled=\{props\.disabled \? 'true' : undefined\}/);
     assert.match(segmentedBlock, /disabled=\{props\.disabled\}/);
     assert.match(segmentedBlock, /data-radio-value=\{value\}[\s\S]*tabIndex=\{radioTabIndex\(value, props\.value, values\)\}/);
   });
 
-  it('keeps theme and palette radio cards on native <button>, not <Button>', async () => {
-    // Regression guard for WAWQAQ msg 5f75daf6 — commit b40d097 swapped
-    // these cards onto packages/ui's <Button>, which bakes in
-    // `h-9 inline-flex bg-primary text-primary-foreground` Tailwind
-    // utilities that collapse each card to a 36px-tall black pill and
-    // hide the swatch + label. The radio-card pattern needs the custom
-    // grid layout in `.settingsThemeOption`, so it must stay on
-    // a native <button> element.
+  it('uses the ChoiceCard primitive (not native <button> or shared <Button>) for theme + palette cards', async () => {
+    // Regression history:
+    //   1. Original `<Button>` migration (commit b40d097, WAWQAQ msg
+    //      5f75daf6) baked `h-9 inline-flex bg-primary` utilities into
+    //      the cards, collapsing each to a 36px black pill. Reverted
+    //      to native `<button role="radio">` + manual keyboard nav.
+    //   2. Round C (PR round-c-choice-card-primitive, WAWQAQ msg
+    //      4f598b19) replaces the native `<button>` with a Base UI
+    //      `Radio.Root`-backed `ChoiceCard` primitive. The primitive
+    //      intentionally applies NO layout/background utilities so the
+    //      existing `.settingsThemeOption*` chrome rules still own the
+    //      visuals; the migration only moves semantics (data-checked,
+    //      keyboard nav, focus) into Base UI.
+    // This test pins step 2 and prevents regressing back to either
+    // shared `<Button>` (which still has the 36px-pill problem) or
+    // hand-rolled native `<button>` (which loses Base UI's keyboard
+    // and focus contract).
     const src = await readRepo('apps/desktop/src/renderer/settings/SettingsModal.tsx');
     const themePage = src.match(/function ThemeSettingsPage\([\s\S]*?function WebSearchSettingsPage/)?.[0] ?? '';
-    // Source order: each radio-card block opens with `<button` (not `<Button`)
-    // and the className appears later. The `\b` boundary keeps `<button` from
-    // matching `<Button`. Strip `//` line comments and `/* */` block comments
-    // first so the regression-explainer comments don't confuse the count.
     const themePageNoComments = themePage
       .replace(/\/\*[\s\S]*?\*\//g, '')
       .replace(/\/\/[^\n]*/g, '');
     const lcButtonCount = (themePageNoComments.match(/<button\b/g) ?? []).length;
     const ucButtonCount = (themePageNoComments.match(/<Button\b/g) ?? []).length;
+    const choiceCardCount = (themePageNoComments.match(/<ChoiceCard\b/g) ?? []).length;
+    const choiceCardGroupCount = (themePageNoComments.match(/<ChoiceCardGroup\b/g) ?? []).length;
+    assert.equal(
+      lcButtonCount,
+      0,
+      `Theme/palette cards must use the ChoiceCard primitive, not native <button> (found ${lcButtonCount} <button> occurrences in the page)`,
+    );
     assert.equal(
       ucButtonCount,
       0,
-      `Theme/palette radio cards must use native <button>, not <Button> from packages/ui (found ${ucButtonCount} <Button> occurrences in the page)`,
+      `Theme/palette cards must use the ChoiceCard primitive, not the shared <Button> (found ${ucButtonCount} <Button> occurrences — see the b40d097 regression note)`,
     );
     assert.equal(
-      lcButtonCount,
+      choiceCardCount,
+      2,
+      `Expected exactly 2 <ChoiceCard> elements (one per .map for theme + palette), found ${choiceCardCount}`,
+    );
+    assert.equal(
+      choiceCardGroupCount,
       2,
-      `Expected exactly 2 native <button> elements (mode picker, palette picker), found ${lcButtonCount}`,
+      `Expected exactly 2 <ChoiceCardGroup> elements (theme group + palette group), found ${choiceCardGroupCount}`,
     );
     assert.match(themePage, /className="settingsThemeOption settingsThemeOptionPreview"/);
     assert.match(themePage, /className="settingsThemeOption settingsPaletteOption"/);
diff --git a/apps/desktop/src/renderer/settings/SettingsModal.tsx b/apps/desktop/src/renderer/settings/SettingsModal.tsx
index ca117f9cae..0d93b1355f 100644
--- a/apps/desktop/src/renderer/settings/SettingsModal.tsx
+++ b/apps/desktop/src/renderer/settings/SettingsModal.tsx
@@ -86,6 +86,8 @@ import {
   AlertDescription,
   BOT_BRAND,
   Button,
+  ChoiceCard,
+  ChoiceCardGroup,
   DialogContent,
   DialogRoot,
   Input,
@@ -2789,42 +2791,32 @@ function ThemeSettingsPage(props: {
   return (
     <div className="settingsStructuredPage">
       <h3 className="settingsSubheading">主题</h3>
-      <div
+      <ChoiceCardGroup
         className="settingsThemeOptions settingsThemeOptionsPreview"
-        role="radiogroup"
         aria-label="主题"
-        onKeyDown={(event) => onSettingsRadioGroupKeyDown(
-          event,
-          THEME_OPTIONS.map((option) => option.value),
-          props.themePref,
-          (next) => void setTheme(next),
-        )}
+        value={props.themePref}
+        onValueChange={(next) => void setTheme(next as typeof props.themePref)}
       >
         {THEME_OPTIONS.map((option) => (
-          // Native <button> on purpose: .settingsThemeOption is a vertically
-          // stacked radio card (preview tile on top, label below). The shared
-          // <Button> primitive bakes in `h-9 inline-flex bg-primary text-white`
-          // utilities that collapse the card to 36px and paint it black —
-          // exactly what WAWQAQ msg 5f75daf6 called out as "稀奇古怪稀巴烂".
-          <button
+          // Base UI Radio.Root via ChoiceCard primitive (Round C,
+          // PR round-c-choice-card-primitive). Keyboard arrow nav,
+          // focus management, and `data-checked` are owned by the
+          // primitive; the card chrome stays in `.settingsThemeOption*`
+          // CSS so the regression test that catches `<Button>` shrinking
+          // the card to a 36px black pill is no longer needed.
+          <ChoiceCard
             key={option.value}
-            type="button"
-            role="radio"
-            aria-checked={props.themePref === option.value}
-            data-active={props.themePref === option.value}
-            data-radio-value={option.value}
-            tabIndex={radioTabIndex(option.value, props.themePref, THEME_OPTIONS.map((item) => item.value))}
+            value={option.value}
             className="settingsThemeOption settingsThemeOptionPreview"
-            onClick={() => void setTheme(option.value)}
           >
             <ThemePreviewMock variant={option.value} />
             <span className="settingsThemeLabel">
               <strong>{option.label}</strong>
               <small>{option.help}</small>
             </span>
-          </button>
+          </ChoiceCard>
         ))}
-      </div>
+      </ChoiceCardGroup>
 
       <h3 className="settingsSubheading">调色板</h3>
       {/* PR-PALETTE-PICKER-GROUPS-0: 11 palettes in a flat grid is
@@ -2835,41 +2827,27 @@ function ThemeSettingsPage(props: {
       {PALETTE_GROUPS.map((group) => (
         <div key={group.id} className="settingsPaletteGroup">
           <h4 className="settingsPaletteGroupHeading">{group.label}</h4>
-          <div
+          <ChoiceCardGroup
             className="settingsThemeOptions settingsPaletteOptions"
-            role="radiogroup"
             aria-label={group.label}
-            onKeyDown={(event) => onSettingsRadioGroupKeyDown(
-              event,
-              group.palettes,
-              currentPalette,
-              (next) => void setPalette(next),
-            )}
+            value={currentPalette}
+            onValueChange={(next) => void setPalette(next as ThemePalette)}
           >
             {group.palettes.map((palette) => (
-              // Native <button>: same reason as the mode picker above —
-              // the swatch+label is a custom grid layout that the shared
-              // <Button> primitive fights with its Tailwind utilities.
-              <button
+              <ChoiceCard
                 key={palette}
-                type="button"
-                role="radio"
-                aria-checked={currentPalette === palette}
-                data-active={currentPalette === palette}
+                value={palette}
                 data-palette={palette}
-                data-radio-value={palette}
-                tabIndex={radioTabIndex(palette, currentPalette, group.palettes)}
                 className="settingsThemeOption settingsPaletteOption"
-                onClick={() => void setPalette(palette)}
               >
                 <span className={`settingsPaletteSwatch settingsPaletteSwatch-${palette}`} aria-hidden="true" />
                 <span className="settingsThemeLabel">
                   <strong>{PALETTE_LABEL[palette]}</strong>
                   <small>{PALETTE_HELP[palette]}</small>
                 </span>
-              </button>
+              </ChoiceCard>
             ))}
-          </div>
+          </ChoiceCardGroup>
         </div>
       ))}
 
diff --git a/apps/desktop/src/renderer/styles.css b/apps/desktop/src/renderer/styles.css
index da2217c129..674d2c9c8d 100644
--- a/apps/desktop/src/renderer/styles.css
+++ b/apps/desktop/src/renderer/styles.css
@@ -8149,7 +8149,7 @@ button:active {
   background: var(--foreground-3);
 }
 
-.settingsThemeOption[data-active="true"] {
+.settingsThemeOption[data-checked] {
   border-color: oklch(from var(--accent) l c h / 0.45);
   background: oklch(from var(--accent) l c h / 0.06);
 }
diff --git a/packages/ui/src/index.ts b/packages/ui/src/index.ts
index 9fe1ae951a..05059cde2f 100644
--- a/packages/ui/src/index.ts
+++ b/packages/ui/src/index.ts
@@ -34,6 +34,7 @@ export * from './primitives/kbd.js';
 export * from './primitives/menu.js';
 export * from './primitives/group.js';
 export * from './primitives/frame.js';
+export * from './primitives/choice-card.js';
 export * from './primitives/preview-card.js';
 export * from './primitives/settings-select.js';
 export * from './primitives/input-group.js';
diff --git a/packages/ui/src/primitives/choice-card.tsx b/packages/ui/src/primitives/choice-card.tsx
new file mode 100644
index 0000000000..c7cf9c8343
--- /dev/null
+++ b/packages/ui/src/primitives/choice-card.tsx
@@ -0,0 +1,84 @@
+"use client";
+
+import { Radio as BaseRadio } from "@base-ui/react/radio";
+import { RadioGroup as BaseRadioGroup } from "@base-ui/react/radio-group";
+import { forwardRef } from "react";
+import type { ComponentPropsWithoutRef, ReactNode } from "react";
+import { cn } from "../utils.js";
+
+/**
+ * Card-style radio primitive for "pick one of N richly-laid-out options"
+ * surfaces — Settings → 外观 theme picker (3 vertically-stacked preview
+ * tiles), palette picker (round swatches + label), provider model
+ * "default" picker, etc.
+ *
+ * Why a separate primitive (not the existing dot-style `Radio`):
+ *   - Each card has rich body content (preview mocks, swatches,
+ *     descriptions). It needs to be the FULL clickable target, not a
+ *     small leading indicator.
+ *   - Earlier attempts to route the card through the shared `Button`
+ *     primitive baked in `h-9 inline-flex bg-primary text-white`
+ *     utilities that collapsed each card to a 36px black pill (WAWQAQ
+ *     msg `5f75daf6`, reverted in commit b40d097). The contract test
+ *     locked the regression by pinning native `<button role="radio">`.
+ *   - `ChoiceCard` keeps Base UI's `Radio.Root` semantics (proper
+ *     `data-checked`, keyboard arrow-nav, focus management), but
+ *     applies **no** layout/background utilities of its own. Every
+ *     visual decision (size, swatch grid, hover/checked treatment)
+ *     lives in the caller's `className` so the existing
+ *     `.settingsThemeOption*` / `.settingsPaletteOption*` rules keep
+ *     working unchanged.
+ *
+ * Selected-state hook: Base UI sets `data-checked` on the rendered
+ * button when the value matches. CSS rules can target
+ * `.settingsThemeOption[data-checked]`. The legacy `data-active` /
+ * `aria-checked` selectors at call sites can be retired together
+ * with the migration.
+ */
+export type ChoiceCardGroupProps<T extends string> = Omit<
+  ComponentPropsWithoutRef<typeof BaseRadioGroup>,
+  "value" | "onValueChange" | "defaultValue"
+> & {
+  value: T;
+  onValueChange(value: T): void;
+};
+
+export const ChoiceCardGroup = forwardRef<HTMLDivElement, ChoiceCardGroupProps<string>>(
+  function ChoiceCardGroup({ value, onValueChange, className, ...props }, ref) {
+    return (
+      <BaseRadioGroup
+        ref={ref}
+        value={value}
+        onValueChange={(next) => {
+          if (typeof next === "string") onValueChange(next);
+        }}
+        className={className}
+        {...props}
+      />
+    );
+  },
+) as <T extends string>(
+  props: ChoiceCardGroupProps<T> & { ref?: React.Ref<HTMLDivElement> },
+) => ReactNode;
+
+export type ChoiceCardProps = Omit<
+  ComponentPropsWithoutRef<typeof BaseRadio.Root>,
+  "value"
+> & {
+  value: string;
+};
+
+export const ChoiceCard = forwardRef<HTMLButtonElement, ChoiceCardProps>(
+  function ChoiceCard({ className, ...props }, ref) {
+    return (
+      <BaseRadio.Root
+        ref={ref}
+        // Intentionally no `h-*`, `bg-*`, `text-*` defaults — the
+        // caller's `className` owns the card's visual contract. See
+        // the regression note at the top of this file.
+        className={cn(className)}
+        {...props}
+      />
+    );
+  },
+);