From e718aaf8d3faf449435156a864de4c7f41ea4e31 Mon Sep 17 00:00:00 2001 From: thepoy Date: Tue, 20 Oct 2026 10:33:14 +0800 Subject: [PATCH] feat(settings): let the content area separate itself from the title bar The title bar is painted with the window's own background colour, so with nothing between them it is hard to tell where it ends - and where to grab it. The appearance section gains a switch that draws a border along the top of the content. It is only offered, and only drawn, while the title bar really is painted like the content: once it follows the Windows theme it has a colour of its own and the border would be decoration. Gating what is drawn as well as what is offered means a preference saved earlier cannot stay active, invisibly, behind a switch that is no longer there. The choice is remembered in the webview's storage rather than config.toml, because it is presentation only and that file is the daemon's too. It is read synchronously, so the first paint already has it. --- src/components/settings/AppearanceSection.tsx | 20 ++- src/i18n/en-US.ts | 3 + src/i18n/ja-JP.ts | 3 + src/i18n/ko-KR.ts | 3 + src/i18n/zh-CN.ts | 3 + src/i18n/zh-HK.ts | 3 + src/i18n/zh-TW.ts | 3 + src/layout/AppShell.tsx | 15 ++- src/store/ui.selectors.test.ts | 91 ++++++++++++++ src/store/ui.selectors.ts | 29 +++++ src/store/ui.store.test.ts | 115 ++++++++++++++++++ src/store/ui.store.ts | 57 ++++++++- 12 files changed, 339 insertions(+), 6 deletions(-) create mode 100644 src/store/ui.selectors.test.ts create mode 100644 src/store/ui.selectors.ts create mode 100644 src/store/ui.store.test.ts diff --git a/src/components/settings/AppearanceSection.tsx b/src/components/settings/AppearanceSection.tsx index 095d0bd1..bd5b1714 100644 --- a/src/components/settings/AppearanceSection.tsx +++ b/src/components/settings/AppearanceSection.tsx @@ -1,13 +1,15 @@ -// Responsibility: Appearance & language group — three options + palette + title bar + language. Note: "follow the system" is the "System" option among the three. +// Responsibility: Appearance & language group — three options + palette + title bar + its separator + language. Note: "follow the system" is the "System" option among the three. import { SettingsGroup } from "./SettingsGroup"; import { SettingsRow } from "./SettingsRow"; import { ThemeSegmented } from "./ThemeSegmented"; import { LanguageRow } from "./LanguageRow"; import { Switch } from "@/components/ui/switch"; import { patch, save, settingsStore } from "@/store/settings.store"; +import { setTitlebarBorder, uiStore } from "@/store/ui.store"; +import { titleBarMatchesContent } from "@/store/ui.selectors"; import { t } from "@/i18n"; import { Sparkles } from "lucide-solid"; -import { createSignal } from "solid-js"; +import { createSignal, Show } from "solid-js"; export function AppearanceSection() { const [flash, setFlash] = createSignal(null); @@ -45,6 +47,20 @@ export function AppearanceSection() { /> } /> + {/* Only while the title bar has no colour of its own: once it follows the Windows + theme, the two areas are visibly different and the switch would be decoration. */} + + setTitlebarBorder(checked)} + /> + } + /> + -
+ {/* The window's title bar is painted with the app's own background colour, so it is + invisible when the content sits directly below it. This border marks where it + ends; it is only inside the box (Tailwind's preflight is border-box), so it does + not add a pixel of overflow. */} +
diff --git a/src/store/ui.selectors.test.ts b/src/store/ui.selectors.test.ts new file mode 100644 index 00000000..8321905a --- /dev/null +++ b/src/store/ui.selectors.test.ts @@ -0,0 +1,91 @@ +import { beforeEach, describe, expect, test, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + followsWindows: undefined as boolean | undefined, +})); + +vi.mock("./settings.store", () => ({ + settingsStore: { + get config() { + return mocks.followsWindows === undefined + ? null + : { title_bar_color_follows_windows_theme: mocks.followsWindows }; + }, + }, +})); + +const mocks2 = vi.hoisted(() => ({ + border: false, +})); + +vi.mock("./ui.store", () => ({ + uiStore: { + get titlebarBorder() { + return mocks2.border; + }, + }, +})); + +async function freshSelectors() { + vi.resetModules(); + + return await import("./ui.selectors"); +} + +beforeEach(() => { + vi.clearAllMocks(); + mocks.followsWindows = undefined; + mocks2.border = false; +}); + +describe("ui selectors: the title bar separator", () => { + test("is offered while the title bar is painted like the content", async () => { + mocks.followsWindows = false; + const selectors = await freshSelectors(); + + expect(selectors.titleBarMatchesContent()).toBe(true); + }); + + test("is not offered once the title bar follows the Windows theme", async () => { + mocks.followsWindows = true; + const selectors = await freshSelectors(); + + expect(selectors.titleBarMatchesContent()).toBe(false); + }); + + test("is not offered before the configuration has loaded", async () => { + // Undefined means "not read yet", and the window opens before the settings do. + const selectors = await freshSelectors(); + + expect(selectors.titleBarMatchesContent()).toBe(false); + }); + + test("is drawn when the preference is set and the title bar has no colour of its own", async () => { + mocks.followsWindows = false; + mocks2.border = true; + const selectors = await freshSelectors(); + + expect(selectors.showsTitlebarBorder()).toBe(true); + }); + + /** + * The case a hidden row alone would leave broken: a preference saved while the border + * made sense must not keep drawing it, invisibly, after the title bar gains a colour of + * its own — the switch that could turn it off is gone by then. + */ + test("is not drawn once the title bar follows the Windows theme, even if it was asked for", async () => { + mocks.followsWindows = true; + mocks2.border = true; + const selectors = await freshSelectors(); + + expect(selectors.showsTitlebarBorder()).toBe(false); + }); + + test("is not drawn when the preference is off", async () => { + mocks.followsWindows = false; + mocks2.border = false; + const selectors = await freshSelectors(); + + expect(selectors.showsTitlebarBorder()).toBe(false); + }); +}); diff --git a/src/store/ui.selectors.ts b/src/store/ui.selectors.ts new file mode 100644 index 00000000..55f9d99e --- /dev/null +++ b/src/store/ui.selectors.ts @@ -0,0 +1,29 @@ +// Responsibility: derived answers about the window chrome, read from the settings and ui stores. +import { settingsStore } from "./settings.store"; +import { uiStore } from "./ui.store"; + +/** + * Whether the title bar is painted with Dwall's own background colour. + * + * That is what the "title bar follows the Windows theme" switch being off means, and it is + * the only case where the bar has no colour of its own to set it apart from the content. + * + * Explicitly `false`, not "falsy": the window renders before the configuration has been + * read, and `!config?.field` would claim the title bar matches the content during that + * moment — briefly offering a switch whose setting is not known yet. + */ +const titleBarMatchesContent = () => + settingsStore.config?.title_bar_color_follows_windows_theme === false; + +/** + * Whether the separator is actually drawn. + * + * The saved preference alone is not enough: once the title bar follows the Windows theme + * it has a colour of its own, so the border would be decoration. Keeping the condition here + * means a preference saved while the border was meaningful cannot stay active, invisibly, + * after the setting that made it meaningful changed. + */ +const showsTitlebarBorder = () => + titleBarMatchesContent() && uiStore.titlebarBorder; + +export { titleBarMatchesContent, showsTitlebarBorder }; diff --git a/src/store/ui.store.test.ts b/src/store/ui.store.test.ts new file mode 100644 index 00000000..5a762184 --- /dev/null +++ b/src/store/ui.store.test.ts @@ -0,0 +1,115 @@ +import { beforeEach, describe, expect, test, vi } from "vitest"; + +/** + * A stand-in for the webview's storage. The store reads it at module load, so each test + * resets the modules and installs its own before importing. + */ +function fakeStorage(initial: Record = {}) { + const items = new Map(Object.entries(initial)); + + return { + items, + getItem: vi.fn((key: string) => items.get(key) ?? null), + setItem: vi.fn((key: string, value: string) => { + items.set(key, value); + }), + }; +} + +async function freshStore(storage: unknown) { + vi.resetModules(); + vi.stubGlobal("localStorage", storage); + + return await import("./ui.store"); +} + +beforeEach(() => { + vi.unstubAllGlobals(); +}); + +describe("ui store: the title bar separator", () => { + test("is off when nothing was saved", async () => { + const store = await freshStore(fakeStorage()); + + expect(store.uiStore.titlebarBorder).toBe(false); + }); + + test("is restored from a saved choice", async () => { + const store = await freshStore( + fakeStorage({ "dwall.ui.titlebar-border": "true" }), + ); + + expect(store.uiStore.titlebarBorder).toBe(true); + }); + + /** + * Anything but the literals this module writes is treated as off: storage can hold a + * value from another build, or something a user typed. + */ + test("ignores a value it did not write", async () => { + for (const value of ["1", "yes", "TRUE", ""]) { + const store = await freshStore( + fakeStorage({ "dwall.ui.titlebar-border": value }), + ); + + expect(store.uiStore.titlebarBorder).toBe(false); + } + }); + + test("setting it updates the window and remembers the choice", async () => { + const storage = fakeStorage(); + const store = await freshStore(storage); + + store.setTitlebarBorder(true); + + expect(store.uiStore.titlebarBorder).toBe(true); + expect(storage.items.get("dwall.ui.titlebar-border")).toBe("true"); + + store.setTitlebarBorder(false); + + expect(store.uiStore.titlebarBorder).toBe(false); + expect(storage.items.get("dwall.ui.titlebar-border")).toBe("false"); + }); + + /** A webview without storage still has to open and still has to toggle. */ + test("works when storage is unavailable", async () => { + const store = await freshStore(undefined); + + expect(store.uiStore.titlebarBorder).toBe(false); + + store.setTitlebarBorder(true); + + expect(store.uiStore.titlebarBorder).toBe(true); + }); + + test("works when storage throws", async () => { + const throwing = { + getItem: () => { + throw new Error("denied"); + }, + setItem: () => { + throw new Error("denied"); + }, + }; + + const store = await freshStore(throwing); + + expect(store.uiStore.titlebarBorder).toBe(false); + + store.setTitlebarBorder(true); + + expect(store.uiStore.titlebarBorder).toBe(true); + }); +}); + +describe("ui store: view switching", () => { + test("switching to settings collapses the library drawer", async () => { + const store = await freshStore(fakeStorage()); + + store.setLibOpen(true); + store.toggleView(); + + expect(store.uiStore.view).toBe("settings"); + expect(store.uiStore.libOpen).toBe(false); + }); +}); diff --git a/src/store/ui.store.ts b/src/store/ui.store.ts index adaa1ed2..5e070215 100644 --- a/src/store/ui.store.ts +++ b/src/store/ui.store.ts @@ -1,15 +1,42 @@ -// Responsibility: view routing state — current view + theme-library drawer toggle. Switching to the settings view also collapses the drawer. +// Responsibility: view routing state — current view + theme-library drawer toggle — and the window's own +// cosmetic preferences (persisted in the webview). Switching to the settings view also collapses the drawer. import type { View } from "@/models/types"; import { createStore } from "solid-js/store"; interface UiState { view: View; libOpen: boolean; // whether the theme library drawer is expanded at the smallest breakpoint (ignored at xl+) + /** Whether the content area draws a border under the title bar. */ + titlebarBorder: boolean; } +/** + * Where the window's own preferences are remembered. + * + * Not `config.toml`: that file is the daemon's too, and a presentation flag would put a + * field for one line of CSS into the model both binaries share. + */ +const TITLEBAR_BORDER_KEY = "dwall.ui.titlebar-border"; + +/** + * The saved value, read synchronously so the first paint already has it — reading it in + * an effect would show the window without the border for a frame. + * + * Anything other than the literals this module writes counts as "off", and a webview + * where storage is unavailable keeps the window working instead of failing to load. + */ +const readTitlebarBorder = () => { + try { + return globalThis.localStorage?.getItem(TITLEBAR_BORDER_KEY) === "true"; + } catch { + return false; + } +}; + const [uiStore, setUiStore] = createStore({ view: "main", libOpen: false, + titlebarBorder: readTitlebarBorder(), }); const toggleView = () => @@ -23,4 +50,30 @@ const setLibOpen = (libOpen: boolean) => setUiStore("libOpen", libOpen); const toggleLib = () => setUiStore("libOpen", (prev) => !prev); -export { uiStore, toggleView, setLibOpen, toggleLib }; +/** + * Turns the border on or off and remembers the choice. + * + * The store changes first: if the webview refuses the write the toggle still applies for + * this session, and the next start falls back to the saved value. + */ +const setTitlebarBorder = (titlebarBorder: boolean) => { + setUiStore("titlebarBorder", titlebarBorder); + + try { + globalThis.localStorage?.setItem( + TITLEBAR_BORDER_KEY, + String(titlebarBorder), + ); + } catch { + // See above: remembering it is a convenience, not part of the toggle working. + } +}; + +export { + TITLEBAR_BORDER_KEY, + uiStore, + toggleView, + setLibOpen, + toggleLib, + setTitlebarBorder, +};