Skip to content

Commit eea0431

Browse files
committed
Refine theme settings picker
1 parent 2023dac commit eea0431

2 files changed

Lines changed: 39 additions & 119 deletions

File tree

packages/web/src/features/settings/components/settings-page.test.tsx

Lines changed: 24 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1008,7 +1008,7 @@ describe("SettingsPage", () => {
10081008
});
10091009
});
10101010

1011-
it("renders appearance option groups through shared pills with group semantics", async () => {
1011+
it("renders appearance theme and language controls with shared semantics", async () => {
10121012
const sendCommand = vi.fn().mockImplementation(async (op: string) => {
10131013
if (op === "settings.get") {
10141014
return {
@@ -1022,31 +1022,21 @@ describe("SettingsPage", () => {
10221022
renderSettingsPage(store);
10231023
fireEvent.click(screen.getByRole("button", { name: "外观" }));
10241024

1025-
const mintThemePill = await screen.findByRole("button", { name: "Mint" });
1026-
const darkVariantPill = screen.getByRole("button", { name: "深色" });
1027-
const lightVariantPill = screen.getByRole("button", { name: "浅色" });
1025+
const themePicker = await screen.findByRole("combobox", { name: "主题" });
10281026
const chineseLanguagePill = screen.getByRole("button", { name: "中文" });
10291027

1030-
expect(
1031-
screen.getByRole("group", {
1032-
name: "主题系列",
1033-
})
1034-
).toHaveAccessibleDescription("选择应用主题");
1035-
expect(
1036-
screen.getByRole("group", {
1037-
name: "明暗模式",
1038-
})
1039-
).toHaveAccessibleDescription("选择应用主题");
1028+
expect(themePicker).toHaveAccessibleDescription("选择应用主题");
1029+
expect(screen.getByRole("option", { name: "Mint 深色" })).toBeInTheDocument();
1030+
expect(screen.getByRole("option", { name: "Mint 浅色" })).toBeInTheDocument();
1031+
expect(screen.getByRole("option", { name: "高对比深色" })).toBeInTheDocument();
10401032
expect(
10411033
screen.getByRole("group", {
10421034
name: "语言",
10431035
})
10441036
).toHaveAccessibleDescription("选择界面语言");
10451037
expect(screen.queryByRole("group", { name: "终端渲染器" })).not.toBeInTheDocument();
10461038
expect(screen.queryByRole("switch", { name: "选中自动复制" })).not.toBeInTheDocument();
1047-
expect(mintThemePill).toHaveAttribute("aria-pressed", "true");
1048-
expect(darkVariantPill).toHaveAttribute("aria-pressed", "true");
1049-
expect(lightVariantPill).toHaveAttribute("aria-pressed", "false");
1039+
expect(themePicker).toHaveValue("mint-dark");
10501040
expect(chineseLanguagePill).toHaveAttribute("aria-pressed", "true");
10511041
});
10521042

@@ -1112,7 +1102,7 @@ describe("SettingsPage", () => {
11121102
expect(screen.queryByText("选中自动复制")).not.toBeInTheDocument();
11131103
});
11141104

1115-
it("updates theme family and variant through the shared appearance pills", async () => {
1105+
it("updates theme through a single shared appearance picker", async () => {
11161106
window.localStorage.setItem("ui.locale", JSON.stringify("en"));
11171107
const sendCommand = vi.fn().mockImplementation(async (op: string) => {
11181108
if (op === "settings.get") {
@@ -1125,14 +1115,13 @@ describe("SettingsPage", () => {
11251115
renderSettingsPage(store);
11261116
fireEvent.click(screen.getByRole("button", { name: "Appearance" }));
11271117

1128-
expect(await screen.findByRole("button", { name: "Mint" })).toBeInTheDocument();
1129-
expect(screen.getByRole("button", { name: "Graphite" })).toBeInTheDocument();
1130-
expect(screen.getByRole("button", { name: "Nord" })).toBeInTheDocument();
1131-
expect(screen.getByRole("button", { name: "High Contrast" })).toBeInTheDocument();
1132-
expect(screen.getByRole("button", { name: "Dark" })).toBeInTheDocument();
1133-
expect(screen.getByRole("button", { name: "Light" })).toBeInTheDocument();
1118+
const picker = await screen.findByRole("combobox", { name: "Theme" });
1119+
expect(screen.getByRole("option", { name: "Mint Dark" })).toBeInTheDocument();
1120+
expect(screen.getByRole("option", { name: "Graphite Dark" })).toBeInTheDocument();
1121+
expect(screen.getByRole("option", { name: "Graphite Light" })).toBeInTheDocument();
1122+
expect(screen.getByRole("option", { name: "Nord Light" })).toBeInTheDocument();
11341123

1135-
fireEvent.click(screen.getByRole("button", { name: "Graphite" }));
1124+
fireEvent.change(picker, { target: { value: "graphite-dark" } });
11361125

11371126
await waitFor(() => {
11381127
expect(sendCommand).toHaveBeenCalledWith(
@@ -1150,7 +1139,7 @@ describe("SettingsPage", () => {
11501139

11511140
expect(document.documentElement).toHaveAttribute("data-theme", "graphite-dark");
11521141

1153-
fireEvent.click(screen.getByRole("button", { name: "Light" }));
1142+
fireEvent.change(picker, { target: { value: "graphite-light" } });
11541143

11551144
await waitFor(() => {
11561145
expect(sendCommand).toHaveBeenCalledWith(
@@ -1167,15 +1156,10 @@ describe("SettingsPage", () => {
11671156
});
11681157

11691158
expect(document.documentElement).toHaveAttribute("data-theme", "graphite-light");
1170-
expect(screen.getByRole("button", { name: "Graphite" })).toHaveAttribute(
1171-
"aria-pressed",
1172-
"true"
1173-
);
1174-
expect(screen.getByRole("button", { name: "Light" })).toHaveAttribute("aria-pressed", "true");
1175-
expect(screen.getByRole("button", { name: "Dark" })).toHaveAttribute("aria-pressed", "false");
1159+
expect(picker).toHaveValue("graphite-light");
11761160
});
11771161

1178-
it("hydrates theme family and variant from settings.get themeId", async () => {
1162+
it("hydrates the single theme picker from settings.get themeId", async () => {
11791163
window.localStorage.setItem("ui.locale", JSON.stringify("en"));
11801164
const sendCommand = vi.fn().mockImplementation(async (op: string) => {
11811165
if (op === "settings.get") {
@@ -1191,9 +1175,7 @@ describe("SettingsPage", () => {
11911175
fireEvent.click(screen.getByRole("button", { name: "Appearance" }));
11921176

11931177
await waitFor(() => {
1194-
expect(screen.getByRole("button", { name: "Nord" })).toHaveAttribute("aria-pressed", "true");
1195-
expect(screen.getByRole("button", { name: "Light" })).toHaveAttribute("aria-pressed", "true");
1196-
expect(screen.getByRole("button", { name: "Dark" })).toHaveAttribute("aria-pressed", "false");
1178+
expect(screen.getByRole("combobox", { name: "Theme" })).toHaveValue("nord-light");
11971179
});
11981180
});
11991181

@@ -1213,8 +1195,7 @@ describe("SettingsPage", () => {
12131195
fireEvent.click(screen.getByRole("button", { name: "Appearance" }));
12141196

12151197
await waitFor(() => {
1216-
expect(screen.getByRole("button", { name: "Mint" })).toHaveAttribute("aria-pressed", "true");
1217-
expect(screen.getByRole("button", { name: "Light" })).toHaveAttribute("aria-pressed", "true");
1198+
expect(screen.getByRole("combobox", { name: "Theme" })).toHaveValue("mint-light");
12181199
expect(document.documentElement).toHaveAttribute("data-theme", "mint-light");
12191200
});
12201201
});
@@ -1235,11 +1216,7 @@ describe("SettingsPage", () => {
12351216
fireEvent.click(screen.getByRole("button", { name: "Appearance" }));
12361217

12371218
await waitFor(() => {
1238-
expect(screen.getByRole("button", { name: "Graphite" })).toHaveAttribute(
1239-
"aria-pressed",
1240-
"true"
1241-
);
1242-
expect(screen.getByRole("button", { name: "Light" })).toHaveAttribute("aria-pressed", "true");
1219+
expect(screen.getByRole("combobox", { name: "Theme" })).toHaveValue("graphite-light");
12431220
expect(document.documentElement).toHaveAttribute("data-theme", "graphite-light");
12441221
});
12451222
});
@@ -1260,7 +1237,9 @@ describe("SettingsPage", () => {
12601237

12611238
renderSettingsPage(store);
12621239
fireEvent.click(screen.getByRole("button", { name: "Appearance" }));
1263-
fireEvent.click(await screen.findByRole("button", { name: "Graphite" }));
1240+
fireEvent.change(await screen.findByRole("combobox", { name: "Theme" }), {
1241+
target: { value: "graphite-dark" },
1242+
});
12641243

12651244
await waitFor(() => {
12661245
expect(sendCommand).toHaveBeenCalledWith(
@@ -1283,11 +1262,7 @@ describe("SettingsPage", () => {
12831262
await settingsGetPromise;
12841263
});
12851264

1286-
expect(screen.getByRole("button", { name: "Graphite" })).toHaveAttribute(
1287-
"aria-pressed",
1288-
"true"
1289-
);
1290-
expect(screen.getByRole("button", { name: "Dark" })).toHaveAttribute("aria-pressed", "true");
1265+
expect(screen.getByRole("combobox", { name: "Theme" })).toHaveValue("graphite-dark");
12911266
expect(document.documentElement).toHaveAttribute("data-theme", "graphite-dark");
12921267
});
12931268

packages/web/src/features/settings/components/settings-page.tsx

Lines changed: 15 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -32,17 +32,10 @@ import {
3232
serverInfoAtom,
3333
} from "../../../atoms/connection";
3434
import { resolvedActiveWorkspaceIdAtom } from "../../../atoms/workspaces";
35-
import { Input, Notice, Pill, Switch } from "../../../components/ui";
35+
import { Input, Notice, Pill, Select, Switch } from "../../../components/ui";
3636
import { useViewport } from "../../../hooks/use-viewport";
3737
import { useTranslation } from "../../../lib/i18n";
38-
import {
39-
getThemeById,
40-
getThemeFamily,
41-
getThemeIdForFamilyVariant,
42-
getThemeVariant,
43-
resolveStoredThemeId,
44-
type ThemeFamily,
45-
} from "../../../theme";
38+
import { getThemeById, resolveStoredThemeId, THEMES } from "../../../theme";
4639
import { notificationPreferencesAtom } from "../../notifications/atoms";
4740
import { MobilePageHeader } from "../../shared/components/mobile-page-header";
4841
import {
@@ -73,8 +66,6 @@ type SettingsNavigationState =
7366
type SettingsContentLayoutMode = "default" | "fill-height";
7467

7568
const DEFAULT_SETTINGS_SECTION: SettingsSection = SETTINGS_SECTIONS[0].id;
76-
const THEME_FAMILIES: ReadonlyArray<ThemeFamily> = ["mint", "graphite", "nord", "hc"];
77-
const THEME_VARIANTS = ["dark", "light"] as const;
7869

7970
function isStandaloneWebApp(): boolean {
8071
if (typeof window === "undefined") {
@@ -1210,14 +1201,15 @@ function AppearanceSettings({ locale, setLocale, theme, setTheme }: AppearanceSe
12101201
const t = useTranslation();
12111202
const themeTitleId = useId();
12121203
const themeDescId = useId();
1213-
const themeFamilyTitleId = useId();
1214-
const themeVariantTitleId = useId();
1204+
const themeSelectId = useId();
12151205
const languageTitleId = useId();
12161206
const languageDescId = useId();
12171207
const dispatch = useAtomValue(dispatchCommandAtom);
12181208
const currentThemeId = resolveStoredThemeId(theme);
1219-
const currentThemeFamily = getThemeFamily(currentThemeId);
1220-
const currentThemeVariant = getThemeVariant(currentThemeId);
1209+
const themeOptions = THEMES.map((registeredTheme) => ({
1210+
value: registeredTheme.id,
1211+
label: t(registeredTheme.labelKey),
1212+
}));
12211213

12221214
const saveSettings = async (settings: Record<string, unknown>) => {
12231215
await dispatch("settings.update", { settings });
@@ -1234,20 +1226,6 @@ function AppearanceSettings({ locale, setLocale, theme, setTheme }: AppearanceSe
12341226
void saveSettings({ appearance: { themeId: resolvedTheme.id } });
12351227
};
12361228

1237-
const handleThemeFamilyChange = (family: ThemeFamily) => {
1238-
const nextThemeId = getThemeIdForFamilyVariant(family, currentThemeVariant);
1239-
if (nextThemeId) {
1240-
handleThemeChange(nextThemeId);
1241-
}
1242-
};
1243-
1244-
const handleThemeVariantChange = (variant: (typeof THEME_VARIANTS)[number]) => {
1245-
const nextThemeId = getThemeIdForFamilyVariant(currentThemeFamily, variant);
1246-
if (nextThemeId) {
1247-
handleThemeChange(nextThemeId);
1248-
}
1249-
};
1250-
12511229
return (
12521230
<div className="settings-section">
12531231
<div className="settings-group">
@@ -1257,48 +1235,15 @@ function AppearanceSettings({ locale, setLocale, theme, setTheme }: AppearanceSe
12571235
<p className="settings-group-desc" id={themeDescId}>
12581236
{t("settings.theme.hint")}
12591237
</p>
1260-
1261-
<h3 className="settings-group-title" id={themeFamilyTitleId}>
1262-
{t("settings.theme.family")}
1263-
</h3>
1264-
<div
1238+
<Select
1239+
id={themeSelectId}
12651240
aria-describedby={themeDescId}
1266-
aria-labelledby={themeFamilyTitleId}
1267-
className="settings-pills"
1268-
role="group"
1269-
>
1270-
{THEME_FAMILIES.map((family) => (
1271-
<Pill
1272-
key={family}
1273-
leadingIcon={currentThemeFamily === family ? <Check size={12} /> : undefined}
1274-
onClick={() => handleThemeFamilyChange(family)}
1275-
active={currentThemeFamily === family}
1276-
>
1277-
{t(`settings.theme.family_${family}`)}
1278-
</Pill>
1279-
))}
1280-
</div>
1281-
1282-
<h3 className="settings-group-title" id={themeVariantTitleId}>
1283-
{t("settings.theme.variant")}
1284-
</h3>
1285-
<div
1286-
aria-describedby={themeDescId}
1287-
aria-labelledby={themeVariantTitleId}
1288-
className="settings-pills"
1289-
role="group"
1290-
>
1291-
{THEME_VARIANTS.map((variant) => (
1292-
<Pill
1293-
key={variant}
1294-
leadingIcon={currentThemeVariant === variant ? <Check size={12} /> : undefined}
1295-
onClick={() => handleThemeVariantChange(variant)}
1296-
active={currentThemeVariant === variant}
1297-
>
1298-
{t(`settings.theme.variant_${variant}`)}
1299-
</Pill>
1300-
))}
1301-
</div>
1241+
aria-label={t("settings.theme.title")}
1242+
className="settings-input-compact"
1243+
options={themeOptions}
1244+
value={currentThemeId}
1245+
onValueChange={handleThemeChange}
1246+
/>
13021247
</div>
13031248

13041249
<div className="settings-group">

0 commit comments

Comments
 (0)