refactor(ui,settings): Round C — ChoiceCard primitive + Theme/Palette migration

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"/);

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>
  ))}
 

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);
 }

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';

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}
+ />
+ );
+ },
+);