diff --git a/desktop/src/shared/theme/CommunityThemeController.tsx b/desktop/src/shared/theme/CommunityThemeController.tsx index ad8661d3f..c00222756 100644 --- a/desktop/src/shared/theme/CommunityThemeController.tsx +++ b/desktop/src/shared/theme/CommunityThemeController.tsx @@ -12,12 +12,12 @@ import { communityThemeScopeFallback, hasMigratedCommunityTheme, markCommunityThemeMigrated, - readCommunityThemeAppearanceSnapshot, readCommunityThemeOutbox, readCommunityThemePreference, sameCommunityThemePreference, writeCommunityThemeOutbox, writeCommunityThemePreference, + type CommunityThemeAppearance, type CommunityThemePreference, } from "./communityThemePreference"; import { @@ -37,6 +37,7 @@ export function CommunityThemeController() { const relayUrl = activeCommunity?.relayUrl; const managerRef = useRef(null); const scopeRef = useRef(""); + const appearanceSnapshotRef = useRef(null); const expectedAppliedRef = useRef(null); const scopedPreferenceRef = useRef(null); const lastRemoteRef = useRef({ createdAt: 0, eventId: "" }); @@ -89,6 +90,7 @@ export function CommunityThemeController() { pubkey, initialPreferenceRef.current, ); + appearanceSnapshotRef.current = snapshot; const appearanceFallback = communityThemeAppearanceFallback(snapshot); const scopeFallback: CommunityThemePreference = { ...communityThemeScopeFallback( @@ -126,7 +128,11 @@ export function CommunityThemeController() { useEffect(() => { if (!pubkey || !relayUrl) return; const scope = `${pubkey}:${relayUrl}`; - const snapshot = readCommunityThemeAppearanceSnapshot(pubkey); + // Reuse the snapshot captured by the layout effect above. Re-reading from + // storage here would collapse to defaults when the snapshot write was + // rejected (a full store), even though the layout effect already resolved + // the correct in-session value. + const snapshot = appearanceSnapshotRef.current; const appearanceFallback = communityThemeAppearanceFallback(snapshot); const local = readCommunityThemePreference( pubkey, @@ -280,9 +286,7 @@ export function CommunityThemeController() { const stored = readCommunityThemePreference( pubkey, relayUrl, - communityThemeAppearanceFallback( - readCommunityThemeAppearanceSnapshot(pubkey), - ), + communityThemeAppearanceFallback(appearanceSnapshotRef.current), ); if (stored && sameCommunityThemePreference(stored, preference)) return; scopedPreferenceRef.current = preference; diff --git a/desktop/src/shared/theme/communityThemePreference.test.mjs b/desktop/src/shared/theme/communityThemePreference.test.mjs index 92900e720..2cb852082 100644 --- a/desktop/src/shared/theme/communityThemePreference.test.mjs +++ b/desktop/src/shared/theme/communityThemePreference.test.mjs @@ -195,6 +195,48 @@ test("appearance snapshot is captured once and consumed per community", () => { ); }); +test("a full store still yields the correct in-session appearance fallback", () => { + // The controller captures the snapshot once, then reuses the returned value + // across its effects via a ref. This pins the contract that reuse depends on: + // even when the snapshot write is rejected (a full store), capture returns + // the live pre-migration appearance, so a legacy record still inherits it. + globalThis.window = { + localStorage: { + getItem: () => null, + setItem: () => { + throw new Error("quota exceeded"); + }, + removeItem: () => {}, + }, + }; + const inherited = { + ...DEFAULT_COMMUNITY_THEME, + glassBackground: true, + glassOpacity: 80, + prominentActiveTab: true, + }; + const snapshot = captureCommunityThemeAppearanceSnapshot("alice", inherited); + assert.deepEqual(snapshot, { + glassBackground: true, + glassOpacity: 80, + prominentActiveTab: true, + }); + // A re-read from storage collapses to defaults, which is exactly why the + // controller must reuse the captured value rather than re-reading it. + assert.equal(readCommunityThemeAppearanceSnapshot("alice"), null); + const legacy = { + version: 1, + theme: "houston", + accent: "#a855f7", + followSystem: false, + }; + const fallback = communityThemeAppearanceFallback(snapshot); + assert.deepEqual(parseCommunityThemePreference(legacy, fallback), { + ...legacy, + ...snapshot, + }); +}); + test("desktop appearance limits match the shared wire contract", () => { const contract = JSON.parse( readFileSync( diff --git a/mobile/lib/shared/theme/community_theme_preference.dart b/mobile/lib/shared/theme/community_theme_preference.dart index 52f6db7f0..52cd87619 100644 --- a/mobile/lib/shared/theme/community_theme_preference.dart +++ b/mobile/lib/shared/theme/community_theme_preference.dart @@ -130,6 +130,37 @@ class CommunityThemePreference { return findTheme(theme)?.isDark == true ? ThemeMode.dark : ThemeMode.light; } + /// Whether this preference carries a full desktop appearance opinion. A + /// preference parsed from a legacy three-field payload, or a fresh/legacy + /// mobile origin, omits these fields and must not replace a desktop record. + bool get includesDesktopAppearance => + includesGlassBackground && + includesGlassOpacity && + includesProminentActiveTab; + + /// Fold [source]'s desktop-only appearance into this preference. Used when + /// republishing a mobile edit that carries no desktop opinion so the relay + /// coordinate keeps the glass and prominent-tab values a desktop client + /// authored. A preference that already includes the fields is returned + /// unchanged; only the fields [source] actually carries are adopted. + CommunityThemePreference mergeDesktopAppearanceFrom( + CommunityThemePreference source, + ) { + if (includesDesktopAppearance) return this; + return CommunityThemePreference( + version: version, + theme: theme, + accent: accent, + followSystem: followSystem, + glassBackground: source.glassBackground, + glassOpacity: source.glassOpacity, + prominentActiveTab: source.prominentActiveTab, + includesGlassBackground: source.includesGlassBackground, + includesGlassOpacity: source.includesGlassOpacity, + includesProminentActiveTab: source.includesProminentActiveTab, + ); + } + @override bool operator ==(Object other) => other is CommunityThemePreference && diff --git a/mobile/lib/shared/theme/community_theme_sync.dart b/mobile/lib/shared/theme/community_theme_sync.dart index fdbe5ae3a..9e4cb0aad 100644 --- a/mobile/lib/shared/theme/community_theme_sync.dart +++ b/mobile/lib/shared/theme/community_theme_sync.dart @@ -60,6 +60,7 @@ class CommunityThemeSyncManager { int _publishRetryAttempt = 0; bool _publishInFlight = false; bool _publishRequestedWhileInFlight = false; + bool _hydrationObserved = false; bool _disposed = false; CommunityThemeSyncManager({ @@ -124,6 +125,11 @@ class CommunityThemeSyncManager { CommunityThemeRemoteStatus.unavailable, ); } + if (result.status == CommunityThemeRemoteStatus.absent) { + // A confirmed absence means the coordinate has been observed: there is no + // desktop record to preserve, so a gated incomplete edit may publish. + _observeHydration(); + } return result; } @@ -184,6 +190,10 @@ class CommunityThemeSyncManager { if (_disposed) return; if (result.status == CommunityThemeRemoteStatus.valid) { _accept(result.remote!); + } else if (result.status == CommunityThemeRemoteStatus.absent) { + // A confirmed absence here observes the coordinate too, releasing a gated + // incomplete edit if the initial fetch had been unavailable. + _observeHydration(); } } @@ -204,6 +214,25 @@ class CommunityThemeSyncManager { _publishTimer = null; } + /// Record that the relay coordinate has been observed at least once (a valid + /// record or a confirmed absence) and release an edit held by the + /// pre-hydration gate in [flush]. Only an incomplete edit is ever gated, so + /// a complete edit's own scheduled publish is left untouched. + void _observeHydration() { + if (_hydrationObserved) return; + _hydrationObserved = true; + final pending = _pending; + if (!_disposed && + pending != null && + !pending.includesDesktopAppearance && + !_publishInFlight) { + // Accelerate the held edit now that the coordinate is known. Any pending + // debounce timer is cancelled by _schedulePublish. A publish in flight is + // left to its own finally-block, which reschedules the pending edit. + _schedulePublish(Duration.zero); + } + } + void publish(CommunityThemePreference preference) { stage(preference); publishStaged(preference); @@ -238,6 +267,13 @@ class CommunityThemeSyncManager { } final preference = _pending; if (_disposed || preference == null) return; + // Hold an edit that carries no desktop appearance opinion until the relay + // coordinate has been observed, so a pre-hydration edit cannot replace a + // desktop-authored record before we have seen it. _observeHydration + // releases it. A complete edit is never gated. + if (!preference.includesDesktopAppearance && !_hydrationObserved) { + return; + } if (preference == _lastPublished) { _pending = null; onPublished(preference); @@ -245,7 +281,15 @@ class CommunityThemeSyncManager { } _publishInFlight = true; try { - final content = crypto.encrypt(jsonEncode(preference.toJson())); + // Preserve the desktop-only fields the relay already holds when the local + // edit omits them, so publishing the replaceable coordinate does not strip + // a desktop client's glass and prominent-tab choices. Bookkeeping stays on + // the local `preference` so the outbox ack contract is unaffected. + final remote = _lastRemote?.preference; + final outgoing = remote == null + ? preference + : preference.mergeDesktopAppearanceFrom(remote); + final content = crypto.encrypt(jsonEncode(outgoing.toJson())); if (_disposed) return; final createdAt = max( DateTime.now().millisecondsSinceEpoch ~/ 1000, @@ -336,6 +380,7 @@ class CommunityThemeSyncManager { _lastCreatedAt = remote.createdAt; _lastEventId = remote.eventId; _lastRemote = remote; + _observeHydration(); if (_pending != null) { _lastPublished = null; return; diff --git a/mobile/test/shared/theme/community_theme_sync_test.dart b/mobile/test/shared/theme/community_theme_sync_test.dart index 3242c0e0d..6ae83ea80 100644 --- a/mobile/test/shared/theme/community_theme_sync_test.dart +++ b/mobile/test/shared/theme/community_theme_sync_test.dart @@ -388,6 +388,93 @@ void main() { expect(manager.pending, isNull); }, ); + + test( + 'gated incomplete edit holds until hydration then merges remote glass', + () async { + // A desktop client already published glass settings the mobile parser + // omits. A mobile edit made before hydration must not strip them. + const desktop = CommunityThemePreference( + theme: 'buzz', + accent: '#3b82f6', + followSystem: true, + glassBackground: true, + glassOpacity: 80, + prominentActiveTab: true, + ); + const incompleteEdit = CommunityThemePreference( + theme: 'dracula', + accent: '#ef4444', + followSystem: false, + includesGlassBackground: false, + includesGlassOpacity: false, + includesProminentActiveTab: false, + ); + final history = Completer>(); + final session = _FakeSession(historyFuture: history.future); + final relay = _FakeSignedRelay(); + final manager = _manager(session, relay); + + final initializing = manager.initialize(); + // Edit lands before the coordinate is observed: publishing is held. + manager.publish(incompleteEdit); + await manager.flush(); + expect(relay.submissions, isEmpty); + expect(manager.pending, incompleteEdit); + + // Hydration reveals the desktop record; the gate releases and the edit + // publishes with the desktop-only fields merged back in. + history.complete([ + _event( + id: 'desktop', + createdAt: 100, + content: jsonEncode(desktop.toJson()), + ), + ]); + await initializing; + await _waitUntil(() => relay.submissions.isNotEmpty); + + final published = + jsonDecode(relay.submissions.single.content) as Map; + expect(published['theme'], 'dracula'); + expect(published['glassBackground'], true); + expect(published['glassOpacity'], 80); + expect(published['prominentActiveTab'], true); + }, + ); + + test('confirmed absence releases a gated incomplete edit', () async { + const incompleteEdit = CommunityThemePreference( + theme: 'dracula', + accent: '#ef4444', + followSystem: false, + includesGlassBackground: false, + includesGlassOpacity: false, + includesProminentActiveTab: false, + ); + final history = Completer>(); + final session = _FakeSession(historyFuture: history.future); + final relay = _FakeSignedRelay(); + final manager = _manager(session, relay); + + final initializing = manager.initialize(); + manager.publish(incompleteEdit); + await manager.flush(); + expect(relay.submissions, isEmpty); + + // No remote record exists; a confirmed absence is enough to release the + // gate, and with no desktop values to preserve the edit publishes as-is. + history.complete([]); + await initializing; + await _waitUntil(() => relay.submissions.isNotEmpty); + + final published = + jsonDecode(relay.submissions.single.content) as Map; + expect(published['theme'], 'dracula'); + expect(published.containsKey('glassBackground'), isFalse); + expect(published.containsKey('glassOpacity'), isFalse); + expect(published.containsKey('prominentActiveTab'), isFalse); + }); } CommunityThemeSyncManager _manager(