From bbffe43ecc73a3245d15bf08522967cb0dbbb41f Mon Sep 17 00:00:00 2001 From: kenny lopez Date: Fri, 14 Aug 2026 08:10:31 +0100 Subject: [PATCH] fix(theme): snapshot pre-migration appearance for per-community carry-forward The appearance-widening migration read the live global glass keys, which this feature now rewrites on every community switch, and gated the carry-forward on a single per-profile latch. A brand-new or unreachable community visited first therefore reset glassBackground/glassOpacity/prominentActiveTab to defaults and republished the loss, and the one-shot latch only ever carried settings forward into whichever community hydrated first. Capture the profile's pre-migration appearance once into a durable snapshot, before the first mount rewrites the global keys, and derive both the empty-scope and legacy-record fallbacks from that snapshot. Every community now inherits the same former glass and prominent-tab choices regardless of hydration order or relay reachability. Replaces the per-profile appearance-migrated latch. On mobile, defaultCommunityTheme and legacyPreference() were constructor-built with the appearance presence flags defaulting to true, so a pre-hydration edit serialized false/65/false over a real desktop record. Mark those two origins as carrying no desktop appearance opinion so they omit the desktop-only fields, matching how a three-field payload parses. Co-authored-by: kenny lopez Reviewed-by: Mongo Signed-off-by: kenny lopez --- .../shared/theme/CommunityThemeController.tsx | 49 +++++----- .../theme/communityThemePreference.test.mjs | 93 +++++++++++-------- .../shared/theme/communityThemePreference.ts | 93 +++++++++++++++---- .../theme/community_theme_preference.dart | 10 ++ .../community_theme_preference_test.dart | 25 +++++ .../theme/community_theme_provider_test.dart | 6 ++ 6 files changed, 197 insertions(+), 79 deletions(-) diff --git a/desktop/src/shared/theme/CommunityThemeController.tsx b/desktop/src/shared/theme/CommunityThemeController.tsx index 90f93a0d0f..ad8661d3f6 100644 --- a/desktop/src/shared/theme/CommunityThemeController.tsx +++ b/desktop/src/shared/theme/CommunityThemeController.tsx @@ -4,15 +4,15 @@ import { relayClient } from "@/shared/api/relayClient"; import { useIdentityQuery } from "@/shared/api/hooks"; import { cacheAndApplyCommunityTheme, + captureCommunityThemeAppearanceSnapshot, clearCommunityThemeOutbox, communityThemeAppearanceFallback, communityThemeApplyExpectation, communityThemePersistenceAction, communityThemeScopeFallback, hasMigratedCommunityTheme, - hasMigratedCommunityThemeAppearance, - markCommunityThemeAppearanceMigrated, markCommunityThemeMigrated, + readCommunityThemeAppearanceSnapshot, readCommunityThemeOutbox, readCommunityThemePreference, sameCommunityThemePreference, @@ -82,14 +82,21 @@ export function CommunityThemeController() { useLayoutEffect(() => { if (!pubkey || !relayUrl) return; - const scopeFallback = communityThemeScopeFallback( - hasMigratedCommunityTheme(pubkey), - initialPreferenceRef.current, - ); - const appearanceFallback = communityThemeAppearanceFallback( - hasMigratedCommunityThemeAppearance(pubkey), + // Capture the profile's pre-migration appearance once, before this mount + // rewrites the global glass keys. Every community then inherits the same + // former settings regardless of hydration order. + const snapshot = captureCommunityThemeAppearanceSnapshot( + pubkey, initialPreferenceRef.current, ); + const appearanceFallback = communityThemeAppearanceFallback(snapshot); + const scopeFallback: CommunityThemePreference = { + ...communityThemeScopeFallback( + hasMigratedCommunityTheme(pubkey), + initialPreferenceRef.current, + ), + ...snapshot, + }; const local = readCommunityThemePreference( pubkey, relayUrl, @@ -119,10 +126,8 @@ export function CommunityThemeController() { useEffect(() => { if (!pubkey || !relayUrl) return; const scope = `${pubkey}:${relayUrl}`; - const appearanceFallback = communityThemeAppearanceFallback( - hasMigratedCommunityThemeAppearance(pubkey), - initialPreferenceRef.current, - ); + const snapshot = readCommunityThemeAppearanceSnapshot(pubkey); + const appearanceFallback = communityThemeAppearanceFallback(snapshot); const local = readCommunityThemePreference( pubkey, relayUrl, @@ -134,13 +139,16 @@ export function CommunityThemeController() { appearanceFallback, ); // A value already cached for this relay is the safest fallback for a later - // incomplete event. Without one, use the one-time appearance migration - // seed, which is never taken from a previous community after migration. + // incomplete event. Without one, use the durable appearance snapshot, which + // holds the profile's pre-migration glass and tab choices for every scope. const legacyFallback = durablePending ?? local ?? appearanceFallback; - const scopeFallback = communityThemeScopeFallback( - hasMigratedCommunityTheme(pubkey), - initialPreferenceRef.current, - ); + const scopeFallback: CommunityThemePreference = { + ...communityThemeScopeFallback( + hasMigratedCommunityTheme(pubkey), + initialPreferenceRef.current, + ), + ...snapshot, + }; scopeRef.current = scope; lastRemoteRef.current = { createdAt: 0, eventId: "" }; const manager = new CommunityThemeSyncManager( @@ -210,7 +218,6 @@ export function CommunityThemeController() { if (remote) { applyRemote(remote); markCommunityThemeMigrated(pubkey); - markCommunityThemeAppearanceMigrated(pubkey); } else if (shouldSeedCommunityTheme(result)) { const seed = readCommunityThemeOutbox(pubkey, relayUrl, legacyFallback) ?? @@ -220,7 +227,6 @@ export function CommunityThemeController() { writeCommunityThemePreference(pubkey, relayUrl, seed); writeCommunityThemeOutbox(pubkey, relayUrl, seed); markCommunityThemeMigrated(pubkey); - markCommunityThemeAppearanceMigrated(pubkey); manager.publish(seed); } // Invalid or unavailable hydration keeps the already-applied fallback @@ -275,8 +281,7 @@ export function CommunityThemeController() { pubkey, relayUrl, communityThemeAppearanceFallback( - hasMigratedCommunityThemeAppearance(pubkey), - initialPreferenceRef.current, + readCommunityThemeAppearanceSnapshot(pubkey), ), ); if (stored && sameCommunityThemePreference(stored, preference)) return; diff --git a/desktop/src/shared/theme/communityThemePreference.test.mjs b/desktop/src/shared/theme/communityThemePreference.test.mjs index f3804a6196..92900e7205 100644 --- a/desktop/src/shared/theme/communityThemePreference.test.mjs +++ b/desktop/src/shared/theme/communityThemePreference.test.mjs @@ -4,6 +4,7 @@ import test from "node:test"; import { DEFAULT_COMMUNITY_THEME, cacheAndApplyCommunityTheme, + captureCommunityThemeAppearanceSnapshot, clearCommunityThemeOutbox, communityThemeAppearanceFallback, communityThemeApplyExpectation, @@ -11,11 +12,8 @@ import { communityThemePersistenceAction, communityThemeScopeFallback, communityThemeStorageKey, - hasMigratedCommunityTheme, - hasMigratedCommunityThemeAppearance, - markCommunityThemeAppearanceMigrated, - markCommunityThemeMigrated, parseCommunityThemePreference, + readCommunityThemeAppearanceSnapshot, readCommunityThemeOutbox, readCommunityThemePreference, sameCommunityThemePreference, @@ -105,13 +103,7 @@ test("older theme records inherit the pre-migration appearance controls", () => }); }); -test("completed appearance migration uses stable defaults", () => { - const priorCommunity = { - ...DEFAULT_COMMUNITY_THEME, - glassBackground: true, - glassOpacity: 42, - prominentActiveTab: true, - }; +test("appearance fallback uses the durable snapshot, else stable defaults", () => { const olderRecord = { version: 1, theme: "houston", @@ -119,16 +111,32 @@ test("completed appearance migration uses stable defaults", () => { followSystem: false, }; + // No snapshot: an upgrading record resolves to the stable defaults. assert.deepEqual( parseCommunityThemePreference( olderRecord, - communityThemeAppearanceFallback(true, priorCommunity), + communityThemeAppearanceFallback(null), ), { ...DEFAULT_COMMUNITY_THEME, ...olderRecord }, ); + + // With a snapshot: the record inherits the profile's pre-migration glass + // and prominent-tab choices, never a previous community's live appearance. + const snapshot = { + glassBackground: true, + glassOpacity: 42, + prominentActiveTab: true, + }; + assert.deepEqual( + parseCommunityThemePreference( + olderRecord, + communityThemeAppearanceFallback(snapshot), + ), + { ...olderRecord, ...snapshot }, + ); }); -test("appearance widening is independent from community scoping", () => { +test("appearance snapshot is captured once and consumed per community", () => { globalThis.window = { localStorage: localStorageStub() }; const inherited = { ...DEFAULT_COMMUNITY_THEME, @@ -143,38 +151,47 @@ test("appearance widening is independent from community scoping", () => { followSystem: false, }; window.localStorage.setItem( - communityThemeStorageKey("alice", "wss://relay.example"), + communityThemeStorageKey("alice", "wss://a.example"), + JSON.stringify(legacy), + ); + window.localStorage.setItem( + communityThemeStorageKey("alice", "wss://b.example"), JSON.stringify(legacy), ); - markCommunityThemeMigrated("alice"); - assert.equal(hasMigratedCommunityTheme("alice"), true); - assert.equal(hasMigratedCommunityThemeAppearance("alice"), false); + // Snapshot is empty until first captured. + assert.equal(readCommunityThemeAppearanceSnapshot("alice"), null); + const snapshot = captureCommunityThemeAppearanceSnapshot("alice", inherited); + assert.deepEqual(snapshot, { + glassBackground: true, + glassOpacity: 80, + prominentActiveTab: true, + }); + + // A later community switch drifts the live appearance to defaults, but the + // snapshot stays pinned so every legacy record inherits the same values. + const drifted = { + ...DEFAULT_COMMUNITY_THEME, + glassBackground: false, + glassOpacity: 65, + prominentActiveTab: false, + }; assert.deepEqual( - readCommunityThemePreference( - "alice", - "wss://relay.example", - communityThemeAppearanceFallback( - hasMigratedCommunityThemeAppearance("alice"), - inherited, - ), - ), - { - ...legacy, - glassBackground: true, - glassOpacity: 80, - prominentActiveTab: true, - }, + captureCommunityThemeAppearanceSnapshot("alice", drifted), + snapshot, ); - markCommunityThemeAppearanceMigrated("alice"); - assert.equal(hasMigratedCommunityThemeAppearance("alice"), true); + const expected = { ...legacy, ...snapshot }; + const fallback = communityThemeAppearanceFallback( + readCommunityThemeAppearanceSnapshot("alice"), + ); assert.deepEqual( - communityThemeAppearanceFallback( - hasMigratedCommunityThemeAppearance("alice"), - inherited, - ), - DEFAULT_COMMUNITY_THEME, + readCommunityThemePreference("alice", "wss://a.example", fallback), + expected, + ); + assert.deepEqual( + readCommunityThemePreference("alice", "wss://b.example", fallback), + expected, ); }); diff --git a/desktop/src/shared/theme/communityThemePreference.ts b/desktop/src/shared/theme/communityThemePreference.ts index f3457b5723..0b13b50402 100644 --- a/desktop/src/shared/theme/communityThemePreference.ts +++ b/desktop/src/shared/theme/communityThemePreference.ts @@ -12,8 +12,8 @@ import { SYNTAX_THEMES, type SyntaxThemeName } from "./theme-loader"; const STORAGE_KEY_PREFIX = "buzz-community-theme.v1"; const OUTBOX_KEY_PREFIX = "buzz-community-theme-outbox.v1"; const MIGRATION_KEY_PREFIX = "buzz-community-theme-migrated.v1"; -const APPEARANCE_MIGRATION_KEY_PREFIX = - "buzz-community-theme-appearance-migrated.v1"; +const APPEARANCE_SNAPSHOT_KEY_PREFIX = + "buzz-community-theme-appearance-snapshot.v1"; export type CommunityThemePreference = { version: 1; @@ -25,6 +25,13 @@ export type CommunityThemePreference = { prominentActiveTab: boolean; }; +// The appearance fields added to the existing v1 payload. Older records and +// brand-new communities predate them and inherit these from the snapshot. +export type CommunityThemeAppearance = Pick< + CommunityThemePreference, + "glassBackground" | "glassOpacity" | "prominentActiveTab" +>; + export const DEFAULT_COMMUNITY_THEME: CommunityThemePreference = Object.freeze({ version: 1, theme: "buzz", @@ -183,27 +190,73 @@ export function markCommunityThemeMigrated(pubkey: string): void { } } -export function hasMigratedCommunityThemeAppearance(pubkey: string): boolean { +export function communityThemeAppearanceSnapshotKey(pubkey: string): string { + return `${APPEARANCE_SNAPSHOT_KEY_PREFIX}:${pubkey}`; +} + +function parseCommunityThemeAppearance( + value: unknown, +): CommunityThemeAppearance | null { + if (typeof value !== "object" || value === null || Array.isArray(value)) { + return null; + } + const candidate = value as Record; + const { glassBackground, glassOpacity, prominentActiveTab } = candidate; + if ( + typeof glassBackground !== "boolean" || + typeof glassOpacity !== "number" || + !Number.isInteger(glassOpacity) || + glassOpacity < GLASS_OPACITY_MIN || + glassOpacity > GLASS_OPACITY_MAX || + typeof prominentActiveTab !== "boolean" + ) { + return null; + } + return { glassBackground, glassOpacity, prominentActiveTab }; +} + +export function readCommunityThemeAppearanceSnapshot( + pubkey: string, +): CommunityThemeAppearance | null { try { - return ( - window.localStorage.getItem( - `${APPEARANCE_MIGRATION_KEY_PREFIX}:${pubkey}`, - ) === "true" + const raw = window.localStorage.getItem( + communityThemeAppearanceSnapshotKey(pubkey), ); + return raw ? parseCommunityThemeAppearance(JSON.parse(raw)) : null; } catch { - return false; + return null; } } -export function markCommunityThemeAppearanceMigrated(pubkey: string): void { +/** + * Persist the profile's pre-migration appearance the first time it is seen and + * return the durable snapshot. Writing once fixes the value while the live + * global appearance keys are still authoritative, so later community switches — + * which now rewrite those keys per community — cannot corrupt the seed the + * per-record migration inherits from. Subsequent calls return the stored + * snapshot unchanged; a full storage falls back to the live appearance for the + * current session without pinning a wrong value. + */ +export function captureCommunityThemeAppearanceSnapshot( + pubkey: string, + appearance: CommunityThemeAppearance, +): CommunityThemeAppearance { + const existing = readCommunityThemeAppearanceSnapshot(pubkey); + if (existing) return existing; + const snapshot: CommunityThemeAppearance = { + glassBackground: appearance.glassBackground, + glassOpacity: appearance.glassOpacity, + prominentActiveTab: appearance.prominentActiveTab, + }; try { window.localStorage.setItem( - `${APPEARANCE_MIGRATION_KEY_PREFIX}:${pubkey}`, - "true", + communityThemeAppearanceSnapshotKey(pubkey), + JSON.stringify(snapshot), ); } catch { - // The expanded preference remains usable in memory when storage is full. + // The live appearance still seeds this session even when storage is full. } + return snapshot; } export function writeCommunityThemePreference( @@ -240,16 +293,18 @@ export function communityThemeScopeFallback( } /** - * Choose values for appearance fields missing from the original v1 payload. - * This migration is independent from the older per-community scope migration: - * profiles that already completed scoping still need one chance to retain - * their former global glass and prominent-tab preferences. + * Build the fallback appearance for records that predate the widened fields. + * The snapshot is the profile's durable pre-migration appearance, captured + * once while the global keys were still authoritative, so every community's + * legacy record inherits the same former glass and prominent-tab choices no + * matter which community hydrates first. */ export function communityThemeAppearanceFallback( - migrated: boolean, - inherited: CommunityThemePreference, + snapshot: CommunityThemeAppearance | null, ): CommunityThemePreference { - return migrated ? DEFAULT_COMMUNITY_THEME : inherited; + return snapshot + ? { ...DEFAULT_COMMUNITY_THEME, ...snapshot } + : DEFAULT_COMMUNITY_THEME; } export function sameCommunityThemePreference( diff --git a/mobile/lib/shared/theme/community_theme_preference.dart b/mobile/lib/shared/theme/community_theme_preference.dart index 3b065b5cc8..52f6db7f01 100644 --- a/mobile/lib/shared/theme/community_theme_preference.dart +++ b/mobile/lib/shared/theme/community_theme_preference.dart @@ -17,6 +17,11 @@ const defaultCommunityTheme = CommunityThemePreference( theme: 'buzz', accent: '#3b82f6', followSystem: true, + // A fresh community carries no desktop appearance opinion, so it must not + // serialize the desktop-only fields and overwrite a real desktop record. + includesGlassBackground: false, + includesGlassOpacity: false, + includesProminentActiveTab: false, ); class CommunityThemePreference { @@ -245,6 +250,11 @@ class CommunityThemeStorage { theme: resolvedTheme, accent: legacyAccentWireValue(legacyAccent), followSystem: mode == ThemeMode.system, + // The pre-per-community mobile theme has no desktop appearance opinion, + // so it must not publish desktop-only defaults over a real desktop record. + includesGlassBackground: false, + includesGlassOpacity: false, + includesProminentActiveTab: false, ); } } diff --git a/mobile/test/shared/theme/community_theme_preference_test.dart b/mobile/test/shared/theme/community_theme_preference_test.dart index b1982b6050..a180093a8a 100644 --- a/mobile/test/shared/theme/community_theme_preference_test.dart +++ b/mobile/test/shared/theme/community_theme_preference_test.dart @@ -158,6 +158,23 @@ void main() { } }); + test('a fresh community default omits desktop-only appearance fields', () { + // Regression: a pre-hydration mobile edit built from the default must not + // publish false/65/false over a real desktop record on the relay. + expect(defaultCommunityTheme.toJson(), { + 'version': 1, + 'theme': 'buzz', + 'accent': '#3b82f6', + 'followSystem': true, + }); + expect(defaultCommunityTheme.copyWith(accent: '#ef4444').toJson(), { + 'version': 1, + 'theme': 'buzz', + 'accent': '#ef4444', + 'followSystem': true, + }); + }); + test('supported edits preserve appearance values mobile cannot render', () { const desktopPreference = CommunityThemePreference( theme: 'buzz', @@ -241,8 +258,16 @@ void main() { theme: 'dracula', accent: 'neutral', followSystem: false, + // The legacy mobile theme carries no desktop appearance opinion, so it + // must not serialize desktop-only defaults over a real desktop record. + includesGlassBackground: false, + includesGlassOpacity: false, + includesProminentActiveTab: false, ), ); + expect(preference.toJson(), isNot(contains('glassBackground'))); + expect(preference.toJson(), isNot(contains('glassOpacity'))); + expect(preference.toJson(), isNot(contains('prominentActiveTab'))); expect(preference.mode, ThemeMode.dark); expect(legacyAccentWireValue(6), '#a855f7'); expect(legacyAccentWireValue(7), '#6366f1'); diff --git a/mobile/test/shared/theme/community_theme_provider_test.dart b/mobile/test/shared/theme/community_theme_provider_test.dart index 7f0474ec02..0c2b99e3a1 100644 --- a/mobile/test/shared/theme/community_theme_provider_test.dart +++ b/mobile/test/shared/theme/community_theme_provider_test.dart @@ -227,8 +227,14 @@ void main() { theme: 'dracula', accent: '#3b82f6', followSystem: true, + // A fresh-community edit carries no desktop appearance opinion, so it + // must not serialize desktop-only fields over a real desktop record. + includesGlassBackground: false, + includesGlassOpacity: false, + includesProminentActiveTab: false, ); expect(container.read(communityThemeProvider), local); + expect(local.toJson(), isNot(contains('glassBackground'))); session.emit(session.remoteEvent(theme: 'houston', id: 'remote-z')); expect(container.read(communityThemeProvider), local);