COndense comments, better prefcache use for theme

web/index.html

@@ -45,10 +45,8 @@
     <!-- PWA -->
     <link rel="manifest" href="/manifest.webmanifest" />
 
-    <!-- Splash screen: painted instantly (before the JS bundle loads) and faded out by the app
-       once the initial data is ready (see src/app/splash.js). Avoids the white flash + spinner
-       flicker on first load. Background colors match MUI's grey[100]/grey[900] (the app's main
-       background) so the handoff to the app is seamless. -->
+    <!-- Splash: painted before the JS bundle loads, faded out by the app once ready (see
+       src/app/splash.js). Background matches MUI's grey[100]/grey[900] for a seamless handoff. -->
     <style>
       html {
         background-color: #f5f5f5;
@@ -108,19 +106,20 @@
       }
     </style>
 
-    <!-- Decide light/dark before first paint, reading the theme preference mirrored to
-       localStorage by Prefs.js. Avoids the light/dark flash caused by the async IndexedDB read. -->
+    <!-- Decide light/dark before first paint from the "prefcache" blob (written by PrefCache.jsx) --
+       avoids the async-IndexedDB theme flash. Keep the key in sync with PrefCache.jsx. -->
     <script>
       (function () {
         try {
-          var stored = localStorage.getItem("theme");
+          var cache = JSON.parse(localStorage.getItem("prefcache"));
+          var theme = cache && cache.theme;
           var prefersDark = window.matchMedia && window.matchMedia("(prefers-color-scheme: dark)").matches;
-          var dark = stored === "dark" || ((!stored || stored === "system") && prefersDark);
+          var dark = theme === "dark" || ((!theme || theme === "system") && prefersDark);
           if (dark) {
             document.documentElement.classList.add("dark");
           }
         } catch (e) {
-          /* localStorage/matchMedia unavailable -- fall back to the default light splash */
+          /* localStorage/matchMedia/JSON unavailable -- fall back to the default light splash */
         }
       })();
     </script>
@@ -132,8 +131,7 @@
       subscribe.
     </noscript>
 
-    <!-- Static splash, removed by src/app/splash.js once the app is ready. The logo is a same-origin
-     SVG precached by the service worker, so it paints right after the (instant) background. -->
+    <!-- Static splash, removed by src/app/splash.js once ready. Logo is a same-origin SVG (precached by the SW). -->
     <div id="splash" aria-hidden="true">
       <img src="/static/images/ntfy-splash.svg" alt="" />
     </div>

web/src/app/Prefs.js

@@ -6,12 +6,7 @@ export const THEME = {
   SYSTEM: "system",
 };
 
-// Key under which the theme preference is mirrored to localStorage. The inline script in
-// index.html reads it synchronously to pick the splash background before first paint.
-export const THEME_LOCALSTORAGE_KEY = "theme";
-
-// Default values returned by the getters below when a preference hasn't been set. Single source of
-// truth, also used by the PrefCache (see PrefCache.jsx) for its loading-state placeholder.
+// Default values the getters return when a pref is unset; also used by PrefCache (PrefCache.jsx).
 export const PREF_DEFAULTS = {
   sound: "ding",
   minPriority: 1,
@@ -20,15 +15,6 @@ export const PREF_DEFAULTS = {
   webPushEnabled: false,
 };
 
-const mirrorThemeToLocalStorage = (value) => {
-  try {
-    localStorage.setItem(THEME_LOCALSTORAGE_KEY, value);
-  } catch (e) {
-    // localStorage may be unavailable (private mode, disabled cookies); the splash just falls
-    // back to the system color scheme in that case.
-  }
-};
-
 class Prefs {
   constructor(dbImpl) {
     this.db = dbImpl;
@@ -72,17 +58,11 @@ class Prefs {
 
   async theme() {
     const theme = await this.db.prefs.get("theme");
-    const value = theme?.value ?? PREF_DEFAULTS.theme;
-    // Mirror to localStorage so the inline script in index.html can pick the splash background
-    // synchronously before first paint. Self-heals for users who set their theme before the
-    // mirror existed.
-    mirrorThemeToLocalStorage(value);
-    return value;
+    return theme?.value ?? PREF_DEFAULTS.theme;
   }
 
   async setTheme(mode) {
     await this.db.prefs.put({ key: "theme", value: mode });
-    mirrorThemeToLocalStorage(mode);
   }
 }
 

web/src/app/Session.js

@@ -44,9 +44,8 @@ class Session {
   }
 
   async resetAndRedirect(url, { fade = false } = {}) {
-    // For user-initiated exits (logout, account deletion) fade the app out first -- while the data
-    // is still intact -- so the wipe + reload below doesn't flash a broken/empty UI. Error-driven
-    // redirects (e.g. an expired session) pass no options and cut straight to the target page.
+    // User-initiated exits (logout, account deletion) fade out first, while data's intact, so the
+    // wipe + reload doesn't flash a broken UI. Error redirects pass no options and cut straight through.
     if (fade) {
       await fadeOut();
     }

web/src/app/splash.js

@@ -1,15 +1,11 @@
-// Fades out and removes the static splash screen baked into index.html (see web/index.html).
-// The splash paints before the JS bundle loads to avoid the white flash + spinner flicker on
-// first load; the app calls this once it has mounted and the initial data is ready. Idempotent --
-// safe to call from multiple routes/effects.
+// Fades out and removes the static splash (see web/index.html), called once the app has mounted and
+// its data is ready. Idempotent.
 
-// Keep the splash up for at least this long so it doesn't flash-and-vanish on fast (warm-cache)
-// loads -- the logo gets a beat to be seen (and to pulse) before fading out.
+// Minimum time the splash stays up, so it doesn't flash-and-vanish on warm-cache loads.
 const MIN_VISIBLE_MS = 1000;
 
-// Hide in two phases: first fade the (pulsing) logo all the way out, then fade the background away
-// to reveal -- "fade in" -- the app underneath. APP_FADE_MS must match the #splash opacity
-// transition in index.html.
+// Hide in two phases: fade the logo out, then fade the background away to reveal the app.
+// APP_FADE_MS must match the #splash opacity transition in index.html.
 const LOGO_FADE_MS = 300;
 const APP_FADE_MS = 100;
 
@@ -21,24 +17,23 @@ const fadeOutAndRemove = () => {
     return;
   }
 
-  // Phase 1: fade the logo out completely. Freeze the pulse at its current opacity first, then
-  // transition to 0 -- otherwise stopping the animation would snap the logo to full opacity.
+  // Phase 1: freeze the pulse at its current opacity (else stopping the animation snaps to full),
+  // then fade the logo to 0.
   const img = splash.querySelector("img");
   if (img) {
     const current = getComputedStyle(img).opacity;
     img.style.opacity = current;
     img.style.animation = "none";
-    img.getBoundingClientRect(); // force reflow so the fade starts from `current`, not the snapped value
+    img.getBoundingClientRect(); // force reflow so the fade starts from `current`
     img.style.transition = `opacity ${LOGO_FADE_MS}ms ease-out`;
     img.style.opacity = "0";
   }
 
-  // Phase 2: once the logo is gone, lift the background to fade the app in, then remove the node.
+  // Phase 2: lift the background to fade the app in, then remove the node.
   setTimeout(() => {
     splash.classList.add("splash-hidden");
     const remove = () => splash.remove();
-    // Only react to the background's own opacity transition -- the logo's transitionend bubbles up
-    // here too, and would otherwise remove the splash before the app has finished fading in.
+    // Ignore the logo's bubbling transitionend; only the background's own fade should remove it.
     const onEnd = (event) => {
       if (event.target === splash) {
         splash.removeEventListener("transitionend", onEnd);
@@ -55,8 +50,7 @@ const hideSplash = () => {
     return;
   }
   removed = true;
-  // performance.now() is the time since the page started loading, i.e. roughly how long the splash
-  // has been visible. Hold it until MIN_VISIBLE_MS has elapsed before fading out.
+  // performance.now() ~= how long the splash has been visible; hold until MIN_VISIBLE_MS elapses.
   const remaining = Math.max(0, MIN_VISIBLE_MS - performance.now());
   setTimeout(fadeOutAndRemove, remaining);
 };

web/src/app/transition.js

@@ -1,12 +1,10 @@
-// Fade transitions for navigating between the main app and the auth pages (login/signup/reset).
-// We fade the whole app (#root) out, then either navigate client-side and fade back in, or do a
-// full reload (where the splash screen in index.html fades the next page in).
+// Fade transitions between the app and the auth pages (login/signup/reset): fade #root out, then
+// navigate client-side (fading back in) or full-reload (the splash fades the next page in).
 
 const FADE_MS = 150;
 
-// Fade #root out over FADE_MS, resolving once the fade has finished (immediately if #root isn't in
-// the document). Exported so callers that need to run their own teardown between the fade-out and
-// the reload -- e.g. session.resetAndRedirect() wiping IndexedDB -- can await it first.
+// Fade #root out, resolving when done. Exported so callers can await it before their own
+// teardown + reload (e.g. resetAndRedirect wiping IndexedDB).
 export const fadeOut = () =>
   new Promise((resolve) => {
     const node = document.getElementById("root");
@@ -19,10 +17,8 @@ export const fadeOut = () =>
     setTimeout(resolve, FADE_MS);
   });
 
-// Fade #root back in, then remove the inline styles we added so nothing is left behind on the
-// element -- otherwise the lingering `transition` would silently animate any future opacity change
-// to #root. Uses setTimeout (not requestAnimationFrame) so a backgrounded tab can't strand #root
-// at opacity 0; rAF can be paused entirely in background tabs, while timers still fire.
+// Fade #root back in, then strip the inline styles (a lingering `transition` would animate future
+// opacity changes). setTimeout, not rAF, so a backgrounded tab can't strand #root at opacity 0.
 const fadeInRoot = () => {
   const node = document.getElementById("root");
   if (!node) {
@@ -35,8 +31,7 @@ const fadeInRoot = () => {
   }, FADE_MS);
 };
 
-// Fade the app out, run a client-side navigation, then fade the new page back in. Used for
-// app -> login/signup, which stay within the same document (no reload).
+// Fade out, navigate client-side, fade the new page in (app -> login/signup, no reload).
 export const fadeNavigate = (navigate, to) => {
   fadeOut().then(() => {
     navigate(to);
@@ -44,8 +39,8 @@ export const fadeNavigate = (navigate, to) => {
   });
 };
 
-// Fade the app out, then do a full page reload to `url`. Used for login/signup -> app, which must
-// reload (the per-user IndexedDB changes). The splash screen fades the reloaded app back in.
+// Fade out, then full-reload to `url` (login/signup -> app needs a reload for the per-user DB).
+// The splash fades it back in.
 export const fadeReload = (url) => {
   fadeOut().then(() => {
     window.location.href = url;

web/src/components/App.jsx

@@ -48,9 +48,8 @@ const App = () => {
     document.dir = languageDir;
   }, [i18n.language, languageDir]);
 
-  // Keep the <html> background (visible behind the app, e.g. overscroll) in sync with the resolved
-  // theme once the stored preference has loaded. The inline script in index.html sets the initial
-  // class before first paint; don't override it while the preference is still loading (undefined).
+  // Keep the <html> background in sync with the theme once loaded. The splash script sets the
+  // initial class; don't override it while the preference is still loading.
   useEffect(() => {
     if (themePreference === undefined) {
       return;
@@ -58,7 +57,7 @@ const App = () => {
     document.documentElement.classList.toggle("dark", isDark);
   }, [isDark, themePreference]);
 
-  // Safety net: never let the splash trap the UI if no route hides it (e.g. an unmatched path).
+  // Safety net: hide the splash even if no route does (e.g. an unmatched path).
   useEffect(() => {
     const timer = setTimeout(() => hideSplash(), 3000);
     return () => clearTimeout(timer);
@@ -109,8 +108,7 @@ const updateTitle = (newNotificationsCount) => {
   updateFavicon(newNotificationsCount);
 };
 
-// Wraps the auth pages (login, signup, ...). They render synchronously with no async data, so the
-// splash can be removed as soon as the page mounts.
+// Auth pages render synchronously, so the splash can be removed on mount.
 const AuthLayout = () => {
   useEffect(() => hideSplash(), []);
   return <Outlet />;
@@ -123,8 +121,8 @@ const Layout = () => {
   const [sendDialogOpenMode, setSendDialogOpenMode] = useState("");
   const users = useLiveQuery(() => userManager.all());
   const subscriptions = useLiveQuery(() => subscriptionManager.all());
-  // Preloaded here (not in AllSubscriptions) so the "All notifications" view has its data ready on
-  // mount -- otherwise switching from a topic to All flashes an empty frame while the query runs.
+  // Preloaded here so the All view (and single topics, via filter) have data on mount -- no empty
+  // frame when switching.
   const allNotifications = useLiveQuery(() => subscriptionManager.getAllNotifications());
   const webPushTopics = useWebPushTopics();
   const subscriptionsWithoutInternal = subscriptions?.filter((s) => !s.internal);
@@ -140,8 +138,7 @@ const Layout = () => {
   useBackgroundProcesses();
   useEffect(() => updateTitle(newNotificationsCount), [newNotificationsCount]);
 
-  // Reveal the app only once the subscriptions have loaded from IndexedDB, so the navigation and
-  // message list don't pop in empty-then-filled behind the splash.
+  // Hide the splash only once subscriptions have loaded, so the nav/list don't pop in empty-then-filled.
   useEffect(() => {
     if (subscriptions !== undefined) {
       hideSplash();

web/src/components/Notifications.jsx

@@ -54,8 +54,7 @@ const priorityFiles = {
 };
 
 export const AllSubscriptions = () => {
-  // allNotifications is preloaded in Layout (App.jsx) so this view has its data ready on mount and
-  // doesn't flash an empty frame when switching to it from a topic.
+  // allNotifications is preloaded in Layout, so this view has its data on mount (no empty frame on switch).
   const { subscriptions, allNotifications } = useOutletContext();
   if (!subscriptions || allNotifications === null || allNotifications === undefined) {
     return <DeferredLoading />;
@@ -85,9 +84,8 @@ const AllSubscriptionsList = (props) => {
 
 const SingleSubscriptionList = (props) => {
   const { subscription, allNotifications } = props;
-  // Derived from the preloaded allNotifications by filtering -- getNotifications(id) is exactly
-  // getAllNotifications() filtered by subscriptionId, so this is the same data with no per-topic
-  // IndexedDB query, making switches to/between topics instant (no empty frame on mount).
+  // Filter the preloaded allNotifications instead of a per-topic query (getNotifications(id) ==
+  // getAllNotifications() filtered by id), so topic switches are instant.
   const notifications = useMemo(
     () => allNotifications.filter((notification) => notification.subscriptionId === subscription.id),
     [allNotifications, subscription.id]
@@ -670,11 +668,8 @@ const Loading = () => {
   );
 };
 
-// Reading notifications from IndexedDB takes only tens of milliseconds, but switching topics (or
-// going from "All notifications" to a single topic) remounts the list and briefly re-runs the
-// query, which would flash the centered Loading spinner each time. Render nothing until the load
-// has taken at least `delayMs`, so the spinner only appears for genuinely slow loads (large DB,
-// slow device) and normal switches just swap content directly.
+// Render nothing until a load takes at least `delayMs`, so the centered spinner only shows on
+// genuinely slow loads -- normal sub-frame IndexedDB reads don't flash it on every remount.
 const DeferredLoading = ({ delayMs = 250 }) => {
   const [show, setShow] = useState(false);
   useEffect(() => {

web/src/components/PrefCache.jsx

@@ -1,15 +1,31 @@
 import * as React from "react";
-import { createContext, useContext } from "react";
+import { createContext, useContext, useEffect } from "react";
 import { useLiveQuery } from "dexie-react-hooks";
 import prefs, { PREF_DEFAULTS } from "../app/Prefs";
 
-// A render-time CACHE of the user's preferences -- NOT the source of truth. The source of truth is
-// the `prefs` table in IndexedDB, always read/written via Prefs.js. This context preloads all prefs
-// once (mounted by Layout, above the routed content), so the Settings page renders its controls
-// with the right values immediately instead of each one flickering in as its own IndexedDB read
-// resolves. By the time you navigate to Settings the query has already resolved.
+// A CACHE of the user's prefs -- not the source of truth (that's the `prefs` IndexedDB table, via
+// Prefs.js). Preloaded once in Layout so Settings renders instantly, and written through to
+// localStorage on every change so (a) Settings is instant even on a cold load and (b) the inline
+// splash script in index.html can read the theme synchronously before the bundle loads. The
+// "prefcache" key is duplicated in that script -- keep them in sync.
+const PREFCACHE_LOCALSTORAGE_KEY = "prefcache";
+
 const PrefCacheContext = createContext(undefined);
 
+// Synchronous fallback before the live query resolves. Merged over PREF_DEFAULTS so a newly-added
+// pref still has a value.
+const readPersistedCache = () => {
+  try {
+    const raw = localStorage.getItem(PREFCACHE_LOCALSTORAGE_KEY);
+    if (raw) {
+      return { ...PREF_DEFAULTS, ...JSON.parse(raw) };
+    }
+  } catch (e) {
+    // malformed or unavailable storage -- fall back to defaults
+  }
+  return PREF_DEFAULTS;
+};
+
 export const PrefCacheProvider = ({ children }) => {
   const cache = useLiveQuery(async () => ({
     sound: await prefs.sound(),
@@ -18,9 +34,21 @@ export const PrefCacheProvider = ({ children }) => {
     theme: await prefs.theme(),
     webPushEnabled: await prefs.webPushEnabled(),
   }));
+
+  // Write through to localStorage on change (prefs change rarely).
+  useEffect(() => {
+    if (cache !== undefined) {
+      try {
+        localStorage.setItem(PREFCACHE_LOCALSTORAGE_KEY, JSON.stringify(cache));
+      } catch (e) {
+        // localStorage may be unavailable (private mode) -- the cache just isn't persisted
+      }
+    }
+  }, [cache]);
+
   return <PrefCacheContext.Provider value={cache}>{children}</PrefCacheContext.Provider>;
 };
 
-// While the cache is still loading (only on the very first paint, mostly hidden by the splash) fall
-// back to the same defaults the Prefs getters use, so controls never render with no value.
-export const usePrefCache = () => useContext(PrefCacheContext) ?? PREF_DEFAULTS;
+// Live context once resolved; else the synchronous localStorage snapshot (instant on cold load);
+// else defaults.
+export const usePrefCache = () => useContext(PrefCacheContext) ?? readPersistedCache();