fix(settings): close the review findings in the validator and the theme cache
Four defects the existing tests did not reach. The web theme resolver compared the server's value against the appearance cache and fell back when they agreed, but the mirroring effect writes the server's value into that same cache — so the comparison held on the first render and stopped holding on the second, reverting an explicitly chosen theme to the default. The server's value is this account's own stored choice, so it now simply wins. The regression test re-renders rather than asserting on the first paint, which is why the original one passed. golangci-lint's exclusions.paths is a path regex, not a directory list, so a bare `web` also excluded internal/jellycompat/web_component.go, internal/webhooksync/, internal/notifications/webhook*.go and eleven other non-test files that were being linted before. Anchored. json.Number is a string kind, so `"1.5"` unmarshalled into it happily and Float64 parsed the quoted digits: a numeric setting validated as a JSON string and NormalizeValue stored the quoted form into jsonb. Rejected. The lone-surrogate check ran only on the object branch, so a lone surrogate in ui.custom_css decoded to U+FFFD on SQLite and was refused outright by Postgres jsonb — the two backends disagreeing about whether the same value could be stored. Hoisted to cover every type. The strict language-tag validation this branch added is correct, but it rejects what the shipped Android client sends; the companion fix is silo-android 4aeb78b4. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -186,6 +186,31 @@ describe("appearance cache ownership", () => {
|
||||
expect(captured.custom.customCss).toBe("body { color: blue; }");
|
||||
});
|
||||
|
||||
it("keeps applying the server's theme once the mirror has written it back", () => {
|
||||
signedInAs(2);
|
||||
mocks.useSettings.mockReturnValue({
|
||||
data: { ui_theme: "oxblood-noir", ui_text_scale: "x-large" },
|
||||
});
|
||||
|
||||
const view = renderAppearance();
|
||||
expect(view.captured.theme.theme).toBe("oxblood-noir");
|
||||
|
||||
// The mirror effect writes the server's theme into the same namespace the
|
||||
// resolver reads. A resolver that compared the two would see them agree
|
||||
// here and fall back to the default from the second render on, so this
|
||||
// re-renders rather than trusting the first paint.
|
||||
act(() => {
|
||||
view.rerender();
|
||||
});
|
||||
act(() => {
|
||||
view.rerender();
|
||||
});
|
||||
|
||||
expect(view.captured.theme.theme).toBe("oxblood-noir");
|
||||
expect(view.captured.theme.textScale).toBe("x-large");
|
||||
expect(document.documentElement.getAttribute("data-theme")).toBe("oxblood-noir");
|
||||
});
|
||||
|
||||
it("mirrors the server's appearance so the next cold start paints it", () => {
|
||||
signedInAs(2);
|
||||
mocks.useSettings.mockReturnValue({
|
||||
|
||||
@@ -110,10 +110,12 @@ export function ThemeProvider({ children }: { children: ReactNode }) {
|
||||
const fallbackTheme: ThemeId =
|
||||
!hasStoredThemeChoice && isValidTheme(adminDefaultTheme) ? adminDefaultTheme : localTheme;
|
||||
|
||||
const theme =
|
||||
loadApiTheme && apiTheme
|
||||
? getInitialThemeFromApi(apiTheme, fallbackTheme, cacheOwner)
|
||||
: fallbackTheme;
|
||||
// The server's value is this account's own stored choice, so it wins outright
|
||||
// whenever it is present and valid. It is deliberately not compared against
|
||||
// the local cache: the effect below mirrors the server's value into that very
|
||||
// cache, so any such comparison stops holding after the first render and the
|
||||
// theme silently reverts to the default on the second.
|
||||
const theme = loadApiTheme && isValidTheme(apiTheme) ? apiTheme : fallbackTheme;
|
||||
const textScale = loadApiTheme ? parseTextScale(apiTextScale ?? localTextScale) : localTextScale;
|
||||
const textWeight = loadApiTheme
|
||||
? parseTextWeight(apiTextWeight ?? localTextWeight)
|
||||
@@ -243,12 +245,3 @@ export function useTheme(): ThemeContextValue {
|
||||
if (!ctx) throw new Error("useTheme must be used within ThemeProvider");
|
||||
return ctx;
|
||||
}
|
||||
|
||||
function getInitialThemeFromApi(
|
||||
apiTheme: string | null,
|
||||
fallback: ThemeId,
|
||||
cacheOwner: string | null,
|
||||
): ThemeId {
|
||||
if (!apiTheme || !isValidTheme(apiTheme)) return fallback;
|
||||
return appearanceCache.get(storage.KEYS.THEME, cacheOwner) !== apiTheme ? apiTheme : fallback;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user