From e76640c8c458a724279b83823248c97b418307d7 Mon Sep 17 00:00:00 2001 From: Kalvin C Date: Fri, 1 May 2026 14:08:48 -0700 Subject: [PATCH] fix: respect goose2 interface density settings (#8953) --- ui/goose2/index.html | 12 +- .../ui/__tests__/AppearanceSettings.test.tsx | 42 ++++++ ui/goose2/src/shared/styles/globals.css | 10 ++ .../src/shared/theme/ThemeProvider.test.tsx | 120 ++++++++++++++++++ ui/goose2/src/shared/theme/ThemeProvider.tsx | 42 +++--- 5 files changed, 201 insertions(+), 25 deletions(-) create mode 100644 ui/goose2/src/features/settings/ui/__tests__/AppearanceSettings.test.tsx diff --git a/ui/goose2/index.html b/ui/goose2/index.html index de8ca95e..d75202b5 100644 --- a/ui/goose2/index.html +++ b/ui/goose2/index.html @@ -62,11 +62,6 @@ getDefaultAccent(resolvedTheme); const foreground = getContrastColor(accent); const density = localStorage.getItem("goose-density") || "comfortable"; - const spacingScale = { - compact: "0.75", - comfortable: "1", - spacious: "1.25", - }; root.classList.add(resolvedTheme === "dark" ? "dark" : "light"); root.style.colorScheme = resolvedTheme === "dark" ? "dark" : "light"; @@ -75,10 +70,9 @@ root.style.setProperty("--color-brand", accent); root.style.setProperty("--color-brand-foreground", foreground); root.style.accentColor = accent; - root.style.setProperty( - "--density-spacing", - spacingScale[density] || spacingScale.comfortable, - ); + if (density === "compact" || density === "spacious") { + root.dataset.density = density; + } } catch { // ThemeProvider applies the canonical theme state after React mounts. } diff --git a/ui/goose2/src/features/settings/ui/__tests__/AppearanceSettings.test.tsx b/ui/goose2/src/features/settings/ui/__tests__/AppearanceSettings.test.tsx new file mode 100644 index 00000000..398b3c10 --- /dev/null +++ b/ui/goose2/src/features/settings/ui/__tests__/AppearanceSettings.test.tsx @@ -0,0 +1,42 @@ +import { screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { beforeEach, describe, expect, it } from "vitest"; + +import { ThemeProvider } from "@/shared/theme/ThemeProvider"; +import { renderWithProviders } from "@/test/render"; +import { AppearanceSettings } from "../AppearanceSettings"; + +describe("AppearanceSettings", () => { + beforeEach(() => { + localStorage.clear(); + document.documentElement.removeAttribute("data-density"); + document.documentElement.style.removeProperty("--density-spacing"); + document.documentElement.style.removeProperty("--spacing"); + }); + + it("updates interface density from the appearance controls", async () => { + const user = userEvent.setup(); + + renderWithProviders( + + + , + ); + + const compact = screen.getByRole("radio", { name: "Compact" }); + + await user.click(compact); + + await waitFor(() => { + expect(localStorage.getItem("goose-density")).toBe("compact"); + expect(document.documentElement.dataset.density).toBe("compact"); + expect( + document.documentElement.style.getPropertyValue("--density-spacing"), + ).toBe(""); + expect(document.documentElement.style.getPropertyValue("--spacing")).toBe( + "", + ); + }); + expect(compact).toHaveAttribute("data-state", "on"); + }); +}); diff --git a/ui/goose2/src/shared/styles/globals.css b/ui/goose2/src/shared/styles/globals.css index fa5af0cc..caa4d215 100644 --- a/ui/goose2/src/shared/styles/globals.css +++ b/ui/goose2/src/shared/styles/globals.css @@ -254,6 +254,16 @@ --density-spacing: 1; } +[data-density="compact"] { + --density-spacing: 0.75; + --spacing: 0.1875rem; +} + +[data-density="spacious"] { + --density-spacing: 1.25; + --spacing: 0.3125rem; +} + .dark { /* theming accents */ --brand: var(--color-white); diff --git a/ui/goose2/src/shared/theme/ThemeProvider.test.tsx b/ui/goose2/src/shared/theme/ThemeProvider.test.tsx index c0d1cbda..d1cb8cec 100644 --- a/ui/goose2/src/shared/theme/ThemeProvider.test.tsx +++ b/ui/goose2/src/shared/theme/ThemeProvider.test.tsx @@ -1,8 +1,19 @@ import { render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; +import { readFileSync } from "node:fs"; +import { dirname, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; import { describe, it, expect, beforeEach } from "vitest"; import { ThemeProvider, useTheme } from "./ThemeProvider"; +const testDirname = dirname(fileURLToPath(import.meta.url)); + +function rootCssVariable(name: string) { + return getComputedStyle(document.documentElement) + .getPropertyValue(name) + .trim(); +} + function ThemeConsumer() { const { theme, @@ -12,6 +23,7 @@ function ThemeConsumer() { setAccentColor, resetAccentColor, density, + setDensity, } = useTheme(); return (
@@ -37,6 +49,9 @@ function ThemeConsumer() { +
); } @@ -45,6 +60,7 @@ describe("ThemeProvider", () => { beforeEach(() => { localStorage.clear(); document.documentElement.classList.remove("light", "dark"); + document.documentElement.removeAttribute("data-density"); document.documentElement.removeAttribute("style"); }); @@ -195,6 +211,23 @@ describe("ThemeProvider", () => { ).toBe("#000000"); }); + it("falls back to default accent color when storage is invalid", () => { + localStorage.setItem("goose-accent-color", "blue"); + + render( + + + , + ); + + expect(screen.getByTestId("accent")).toHaveTextContent("#1a1a1a"); + expect(screen.getByTestId("accent-preference")).toHaveTextContent( + "default", + ); + expect(rootCssVariable("--color-brand")).toBe("#1a1a1a"); + expect(rootCssVariable("--color-brand-foreground")).toBe("#ffffff"); + }); + it("provides default density", () => { render( @@ -202,5 +235,92 @@ describe("ThemeProvider", () => { , ); expect(screen.getByTestId("density")).toHaveTextContent("comfortable"); + expect(document.documentElement).not.toHaveAttribute("data-density"); + expect( + document.documentElement.style.getPropertyValue("--density-spacing"), + ).toBe(""); + expect(document.documentElement.style.getPropertyValue("--spacing")).toBe( + "", + ); + }); + + it("falls back to default theme when storage is invalid", () => { + localStorage.setItem("goose-theme", "sepia"); + + render( + + + , + ); + + expect(screen.getByTestId("theme")).toHaveTextContent("system"); + }); + + it("reads persisted density", () => { + localStorage.setItem("goose-density", "compact"); + + render( + + + , + ); + + expect(screen.getByTestId("density")).toHaveTextContent("compact"); + expect(document.documentElement.dataset.density).toBe("compact"); + expect( + document.documentElement.style.getPropertyValue("--density-spacing"), + ).toBe(""); + expect(document.documentElement.style.getPropertyValue("--spacing")).toBe( + "", + ); + }); + + it("persists density and updates spacing tokens", async () => { + const user = userEvent.setup(); + render( + + + , + ); + + await user.click(screen.getByText("Set Spacious")); + + expect(screen.getByTestId("density")).toHaveTextContent("spacious"); + expect(localStorage.getItem("goose-density")).toBe("spacious"); + expect(document.documentElement.dataset.density).toBe("spacious"); + expect( + document.documentElement.style.getPropertyValue("--density-spacing"), + ).toBe(""); + expect(document.documentElement.style.getPropertyValue("--spacing")).toBe( + "", + ); + }); + + it("falls back to comfortable density when storage is invalid", () => { + localStorage.setItem("goose-density", "tiny"); + + render( + + + , + ); + + expect(screen.getByTestId("density")).toHaveTextContent("comfortable"); + expect(document.documentElement).not.toHaveAttribute("data-density"); + }); + + it("keeps density spacing values in CSS", () => { + const css = readFileSync( + resolve(testDirname, "../styles/globals.css"), + "utf8", + ); + + expect(css).toContain('[data-density="compact"]'); + expect(css).toContain("--density-spacing: 0.75;"); + expect(css).toContain("--spacing: 0.1875rem;"); + expect(css).toContain('[data-density="spacious"]'); + expect(css).toContain("--density-spacing: 1.25;"); + expect(css).toContain("--spacing: 0.3125rem;"); + expect(css).toContain("padding: calc(0.5rem * var(--density-spacing));"); }); }); diff --git a/ui/goose2/src/shared/theme/ThemeProvider.tsx b/ui/goose2/src/shared/theme/ThemeProvider.tsx index 690c71f8..c81a8a35 100644 --- a/ui/goose2/src/shared/theme/ThemeProvider.tsx +++ b/ui/goose2/src/shared/theme/ThemeProvider.tsx @@ -4,6 +4,9 @@ type ThemePreference = "light" | "dark" | "system"; type ResolvedTheme = "light" | "dark"; type Density = "compact" | "comfortable" | "spacious"; +const THEME_PREFERENCES = ["light", "dark", "system"] as const; +const DENSITIES = ["compact", "comfortable", "spacious"] as const; + type ThemeProviderProps = { children: React.ReactNode; defaultTheme?: ThemePreference; @@ -29,6 +32,14 @@ const DEFAULT_ACCENT_COLOR_PREFERENCE = "default"; const DEFAULT_LIGHT_ACCENT_COLOR = "#1a1a1a"; const DEFAULT_DARK_ACCENT_COLOR = "#ffffff"; +function isDensity(value: string | null): value is Density { + return DENSITIES.includes(value as Density); +} + +function isThemePreference(value: string | null): value is ThemePreference { + return THEME_PREFERENCES.includes(value as ThemePreference); +} + function resolveTheme(preference: ThemePreference): ResolvedTheme { if (preference === "system") { return window.matchMedia("(prefers-color-scheme: dark)").matches @@ -93,15 +104,21 @@ function applyAccentColor(root: HTMLElement, color: string) { root.style.accentColor = color; } +function applyDensityAttribute(root: HTMLElement, density: Density) { + if (density === "comfortable") { + root.removeAttribute("data-density"); + } else { + root.dataset.density = density; + } +} + export function ThemeProvider({ children, defaultTheme = "system", }: ThemeProviderProps) { const [theme, setThemeState] = React.useState(() => { - const stored = localStorage.getItem( - "goose-theme", - ) as ThemePreference | null; - return stored ?? defaultTheme; + const stored = localStorage.getItem("goose-theme"); + return isThemePreference(stored) ? stored : defaultTheme; }); const [resolvedTheme, setResolvedTheme] = React.useState(() => @@ -117,8 +134,8 @@ export function ThemeProvider({ }); const [density, setDensityState] = React.useState(() => { - const stored = localStorage.getItem("goose-density") as Density | null; - return stored ?? "comfortable"; + const stored = localStorage.getItem("goose-density"); + return isDensity(stored) ? stored : "comfortable"; }); const accentColor = React.useMemo(() => { @@ -178,18 +195,11 @@ export function ThemeProvider({ }, [theme]); React.useEffect(() => { - const root = window.document.documentElement; - applyAccentColor(root, accentColor); + applyAccentColor(window.document.documentElement, accentColor); }, [accentColor]); - React.useEffect(() => { - const root = window.document.documentElement; - const spacingScale: Record = { - compact: "0.75", - comfortable: "1", - spacious: "1.25", - }; - root.style.setProperty("--density-spacing", spacingScale[density]); + React.useLayoutEffect(() => { + applyDensityAttribute(window.document.documentElement, density); }, [density]); const value = React.useMemo(