the account stores a theme, not a colour mode
Your theme now follows the account instead of living in one browser's localStorage.
Choosing Plain on a laptop reaches the phone.
`appearance.colorMode` is superseded by `appearance.theme`. Along the way this
resolves a mismatch that had been sitting there: the server's default was
`appearance: { theme: 'light' }` while the client read `appearance.colorMode`, so that
default had never once been read by anything. Both sides say `theme` now.
The Light/Dark item is gone from the user menu. Light and dark are two themes, both
already in the list above it, and that toggle was the last writer of the legacy key.
── Two bugs found by testing this, both mine, both silent ──
**A default that looked like a choice.** `useSettings` returns `DEFAULT_SETTINGS`
until the request lands — `settings` is never null, it is a complete valid object. So
"wait for settings" fired on the first render against the defaults, marked itself
done, applied the default theme and never saw the real one. Then the persist side
wrote that default over the account's actual preference. It overwrote the owner's own
`dark` while I was testing it; restored by hand from the `colorMode` it had not yet
destroyed. `useSettings` now exposes `settingsLoaded` so the difference is expressible.
**Defaults masking a migration.** With that fixed, an account holding only
`colorMode: 'dark'` STILL came up light. The client merges `DEFAULT_SETTINGS` over
whatever arrives, so a missing `theme` is filled in with the default before any
consumer sees it — and a default is indistinguishable from a choice. The migration
therefore belongs on the SERVER, which sees the stored object before anything is
merged over it. It is read-only: a GET that rewrites the row on every call is a
surprise, and the client persists on its next change anyway.
Verified by putting the account into the pre-themes shape and asking the new handler
directly, rather than by restarting to find out:
stored: {"colorMode":"dark"} -> returned: {"colorMode":"dark","theme":"duck-dark"}
── Needs a restart ──
The first server-side change in this work. Everything until now was frontend and a
rebuild was enough.
950 pass / 7 fail unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -11,7 +11,9 @@ const DEFAULT_SETTINGS = {
|
||||
defaultPwd: '~',
|
||||
},
|
||||
appearance: {
|
||||
theme: 'light',
|
||||
// A theme id, not a colour mode. `'light'` sat here for a long time while the client read
|
||||
// `appearance.colorMode`, so this default was never once read by anything.
|
||||
theme: 'duck-light',
|
||||
},
|
||||
};
|
||||
|
||||
@@ -22,6 +24,33 @@ export const settingsRouter = createRouter();
|
||||
settingsRouter.route('/', selfPermissionsRouter);
|
||||
|
||||
// GET /settings — return user settings from DB, default if empty
|
||||
/**
|
||||
* An account from before themes existed, expressed in the vocabulary that replaced it.
|
||||
*
|
||||
* Themes superseded `appearance.colorMode`, and an existing account still has one. The migration has to
|
||||
* happen HERE rather than in the client, and that is the whole point of this function: the client merges
|
||||
* `DEFAULT_SETTINGS` over whatever arrives, so by the time it sees the data, a missing `theme` has already
|
||||
* been filled in with the default — and a default is indistinguishable from a choice. Verified the wrong
|
||||
* way round first: an account holding only `colorMode: 'dark'` came up light, because the client's own
|
||||
* default `theme` masked it.
|
||||
*
|
||||
* The server sees the stored object before anything is merged over it, so it is the only place that can
|
||||
* tell "never chose a theme" from "chose the default one".
|
||||
*
|
||||
* Read-only: nothing is written back here. A GET that rewrites the row on every call is a surprise, and
|
||||
* the client persists the resolved theme on its next change anyway.
|
||||
*/
|
||||
function withThemeMigrated(settings: Record<string, unknown>): Record<string, unknown> {
|
||||
const appearance = settings.appearance as { theme?: unknown; colorMode?: unknown } | undefined;
|
||||
if (!appearance || typeof appearance.theme === 'string') return settings;
|
||||
|
||||
const migrated =
|
||||
appearance.colorMode === 'dark' ? 'duck-dark' : appearance.colorMode === 'light' ? 'duck-light' : undefined;
|
||||
if (!migrated) return settings;
|
||||
|
||||
return { ...settings, appearance: { ...appearance, theme: migrated } };
|
||||
}
|
||||
|
||||
settingsRouter.get('/settings', async (ctx) => {
|
||||
const userId = ctx.get('user').id;
|
||||
const settings = await getUserSettings(userId);
|
||||
@@ -31,7 +60,7 @@ settingsRouter.get('/settings', async (ctx) => {
|
||||
return ctx.json(DEFAULT_SETTINGS);
|
||||
}
|
||||
|
||||
return ctx.json(settings);
|
||||
return ctx.json(withThemeMigrated(settings as Record<string, unknown>));
|
||||
});
|
||||
|
||||
// PUT /settings — full replacement
|
||||
|
||||
Reference in New Issue
Block a user