Skin is a launch-time choice; stop applying it to a live window
Switching Modern -> Win7 at runtime broke the UI. SettingsService.applySkin toggled the `.win7` class the moment the setting changed — harmless when there was one component tree, and a broken window as soon as there were two. The tree is bootstrapped once from the preload-baked __APP_SKIN, so the live flip painted 7.css onto shadlike markup that has no .window.glass, no .title-bar, no menu[role=tablist] and no <table> ListView. The CSS landed on a tree that cannot receive it. A reload would not have helped, and neither would anything short of a restart: - The preload is a persistent WebKitUserScript injected at DOCUMENT_START, so its string is frozen at webview construction — loadURL() re-runs it with the OLD __APP_SKIN. updatePreloadScriptToWebView exists in the FFI but is reachable only from electrobun's internal <electrobun-webview> handler. - titleBarStyle/transparent have no setters; a live window cannot be re-framed. win7 -> modern is the dangerous direction — the modern tree draws no title bar, so it would land in a frameless transparent window with no way to move, minimize or close it. - Recreating the main window is also out: exitOnLastWindowClosed is true, the main rpc is captured by six push closures, the tray captures `win`, and listeners cannot be removed. So: applySkin is deleted outright, from setSkin AND from applyAll — the latter matters independently, since applyAll runs on the settingsChanged push and was therefore breaking every open editor and manager window too. Clicking a skin now only persists it. Also fixed a second leak found while tracing this: manager-window and editor-windows build from `currentSettings()`, the LIVE snapshot, so a window opened after a pending change would boot the new tree and frame while main still ran the old one. `initSessionSkin` now freezes the skin at startup before any window exists, and buildPreload/chromeOptions read that pinned value — chromeOptions loses its parameter so no call site can get it wrong. The options row shows the saved value as pending (skinPending() compares settings.skin() against APP_SKIN) with the hint retexted in all 7 locales; it said "Reopen the window to change its frame", which described the deleted design. Verified: tsc + build:ui clean; both skins boot correctly from a persisted setting (win7 frameless, modern with the native frame). The live flip is now impossible by construction, which is the check that matters here — the UI bundle contains no classList write for win7 at all, the only writer is buildPreload, and sessionSkin has exactly one assignment site. Not verified: the click itself and the pending hint appearing, which need an interaction pass — no window automation is available here. Both are present in the built bundle. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
+19
-11
@@ -315,15 +315,23 @@ A fifth theming axis, and the only one that is a **class** (`.win7` on
|
||||
`<html>`) rather than a `data-*` attribute — `7.css/dist/7.scoped.css` is
|
||||
precompiled against a literal `.win7`, so the name is not ours to pick.
|
||||
Default `modern` = class absent, matching the delete-when-default rule.
|
||||
`SettingsService.applySkin` owns the class once the page is live;
|
||||
`buildPreload` writes it for the first paint.
|
||||
`buildPreload` is the **only** writer of that class, ever — there is
|
||||
deliberately no `SettingsService.applySkin`.
|
||||
|
||||
Unlike the other four axes, this one is not only CSS: it selects a whole
|
||||
component tree (see "UI layout"). So a skin change is **two-phase** —
|
||||
`applySkin` flips the CSS immediately, but the tree and the native frame are
|
||||
both fixed at window construction (`main.ts` reads the preload-baked
|
||||
`__APP_SKIN`; `titleBarStyle`/`transparent` are `BrowserWindow` constructor
|
||||
options). The options row carries the reopen hint as its tooltip.
|
||||
**Unlike the other four axes, this one is launch-time only.** It selects a
|
||||
whole component tree (see "UI layout") as well as CSS, and neither half can
|
||||
change on a live window: the tree is bootstrapped once from the preload-baked
|
||||
`__APP_SKIN`, and `titleBarStyle`/`transparent` are `BrowserWindow` constructor
|
||||
options with no setters. Clicking a skin therefore only persists it; the
|
||||
options row shows the saved value as pending (`skinPending()` compares
|
||||
`settings.skin()` against `APP_SKIN`) and the running UI is untouched until the
|
||||
next app start.
|
||||
|
||||
The bun side pins this: `initSessionSkin` freezes the skin at startup, before
|
||||
any window exists, and both `buildPreload` and `chromeOptions` read that pinned
|
||||
value rather than the live settings snapshot. Otherwise an editor or manager
|
||||
opened after a pending change would boot the *new* tree and frame while the
|
||||
main window still ran the old one.
|
||||
|
||||
**Stylesheet split.** `ui/src/styles.css` holds only what BOTH skins consume —
|
||||
the `@layer` statement, the Tailwind import, OKLCH tokens, `@theme inline`,
|
||||
@@ -427,9 +435,9 @@ without decorations (custom titlebar)" (verified on GNOME/Wayland), so the
|
||||
frameless Aero chrome is not a mac/win-only feature. The webview then draws the whole
|
||||
frame via `ui/src/app/skins/7like/title-bar/title-bar.ts` (7.css's `.title-bar` >
|
||||
`.title-bar-text` + `.title-bar-controls`, buttons keyed by `aria-label`),
|
||||
mounted by all three window roots. Because `titleBarStyle` is fixed at
|
||||
construction, **a skin change only reaches the frame on the next window open**;
|
||||
the CSS half flips live, and the options row says so. `IS_WIN7_CHROME` in
|
||||
mounted by all three of that skin's window roots. Because `titleBarStyle` is
|
||||
fixed at construction, **a skin change only reaches the frame on the next app
|
||||
start** — and nothing about the skin flips live, deliberately. `IS_WIN7_CHROME` in
|
||||
`platform.ts` reads the preload-baked `__APP_SKIN` (settings resolve too late
|
||||
for layout) and forces `IS_MAC_CHROME` false, since there are no traffic
|
||||
lights to reserve `pl-24` for.
|
||||
|
||||
Reference in New Issue
Block a user