Merge pull request #162 from Coffey-Labs/i18n-first-paint-language

Keep a settings change made before the first read, and wait for it

web/src/App.tsx

@@ -17,7 +17,7 @@ import { AppShell } from "@/views/AppShell";
 import { MailView } from "@/views/mail/MailView";
 import { ComposerDock } from "@/views/compose/ComposerDock";
 import { setUnreadBadge } from "@/lib/notify";
-import { useSettings, syncedPart } from "@/store/settings";
+import { PAINTED_FROM_CACHE, useSettings, syncedPart } from "@/store/settings";
 import { armSettingsSync, loadRemoteSettings, queueSettingsPush, settingsAlreadyLoadedFor, settingsSyncAvailable } from "@/lib/settingsSync";
 import { listenForVerification, renewWebPush } from "@/lib/webpushEnable";
 import { useLanguageVersion, whenLanguageReady } from "@/lib/i18n";
@@ -82,21 +82,45 @@ function AuthedApp() {
   const accountId = useSession((s) => s.accountId);
   const [location] = useLocation();
 
-  // Settings that live with the account rather than the browser. The cached
+  /*
-  // ones have already painted, so this only has to correct them (issue #54).
+   * Settings that live with the account rather than the browser.
-  //
+   *
-  // Once per account, not once per mount: this subtree is keyed on the
+   * When this browser has them cached they have already painted, and this only
-  // language version, so picking a language throws it away and builds it
+   * has to correct them (issue #54). When it does not -- an untrusted device,
-  // again. Re-reading the settings file there would apply a copy written
+   * or the sign-out that every deploy causes -- the first frame is the
-  // before the change and undo it.
+   * defaults, and the defaults are English. Rendering then means anything
+   * computed before the settings land is computed in the wrong language: not
+   * the interface, which is rebuilt when the catalogue arrives, but a string
+   * emitted once, like a toast. That is why the stale-folder toast came out
+   * in English on an otherwise German screen.
+   *
+   * So without a cache the tree waits, which costs nothing: there was nothing
+   * worth painting yet. With one it does not wait, and the screen is as quick
+   * as it was.
+   *
+   * Once per account, not once per mount: this subtree is keyed on the
+   * language version, so picking a language throws it away and builds it
+   * again. Re-reading the settings file there would apply a copy written
+   * before the change and undo it.
+   */
+  const [ready, setReady] = useState(PAINTED_FROM_CACHE);
   useEffect(() => {
-    if (settingsAlreadyLoadedFor(accountId)) return;
+    if (settingsAlreadyLoadedFor(accountId)) {
+      setReady(true);
+      return;
+    }
     let cancelled = false;
     void (async () => {
       const remote = await loadRemoteSettings();
       if (cancelled) return;
       if (remote) useSettings.getState().hydrate(remote);
-      // Pushes were held back until now so they could not race the load.
+      // The catalogue for whatever language that turned out to be. Hydrating
+      // asks for it; this is waiting for the answer.
+      await whenLanguageReady();
+      if (cancelled) return;
+      setReady(true);
+      // Pushes were held back until now so they could not race the load. A
+      // change made while it was in flight was kept, and goes out here.
       armSettingsSync();
       // No file yet — seed one from what this browser has, so the next device
       // to sign in starts from these rather than from the defaults.
@@ -188,6 +212,16 @@ function AuthedApp() {
     if (notif) void import("@/lib/notify").then((m) => m.requestNotificationPermission());
   }, [notif]);
 
+  // Nothing worth painting until the account's settings are in force; see the
+  // comment on `ready` above. With a cache this was true from the first frame.
+  if (!ready) {
+    return (
+      <div className="center" style={{ height: "100%" }}>
+        <Spinner size="lg" />
+      </div>
+    );
+  }
+
   return (
     <AppShell>
       <Suspense fallback={<Spinner size="lg" />}>

web/src/lib/settingsSync.ts

@@ -85,6 +85,8 @@ export function settingsAlreadyLoadedFor(accountId: string | null | undefined):
 export function armSettingsSync(): void {
   armed = true;
   bindFlushListeners();
+  // A change made while the read was in flight has been waiting for this.
+  if (pending) void flushSettingsPush();
 }
 
 /** Stop syncing and drop anything queued (logout). */
@@ -104,8 +106,22 @@ export function stopSettingsSync(): void {
  * one request goes out once the changes stop.
  */
 export function queueSettingsPush(synced: Record<string, unknown>): void {
-  if (!armed || !settingsSyncAvailable()) return;
+  if (!settingsSyncAvailable()) return;
+  /*
+   * Held, not dropped, before the first load has settled.
+   *
+   * This used to return here, which silently threw the change away: a
+   * language picked in the second or so before the settings file came back
+   * was never written, so it survived until the next reload and no further.
+   * That is the other half of "sometimes it takes several clicks" -- the
+   * click that stuck was one made after the read had finished.
+   *
+   * Keeping it is safe because `hydrate` refuses to overwrite a key that is
+   * still queued, so the newer local change wins over the older file rather
+   * than racing it. `armSettingsSync` writes whatever is waiting.
+   */
   pending = synced;
+  if (!armed) return;
   if (timer !== null) window.clearTimeout(timer);
   timer = window.setTimeout(() => {
     timer = null;

web/src/lib/storage.ts

@@ -107,6 +107,25 @@ export function loadJson<T>(key: string, fallback: T): T {
   }
 }
 
+/**
+ * Did `loadJson` have something to return, or did it hand back the fallback?
+ *
+ * The difference decides whether a first paint is worth anything. With a
+ * cached value the screen can be right immediately and the account's copy only
+ * has to correct it; without one -- an untrusted device, or the sign-out that
+ * every deploy causes -- the first paint is the defaults, and painting it
+ * before the account's settings arrive shows English to somebody who chose
+ * otherwise.
+ */
+export function hasCachedJson(key: string): boolean {
+  if (!trusted) return false;
+  try {
+    return localStorage.getItem(PREFIX + key) != null;
+  } catch {
+    return false;
+  }
+}
+
 export function loadRaw<T>(key: string, fallback: T): T {
   if (!trusted) return fallback;
   try {

web/src/store/settings.ts

@@ -1,6 +1,6 @@
 import { useEffect, useState } from "react";
 import { create } from "zustand";
-import { loadJson, saveJson } from "@/lib/storage";
+import { hasCachedJson, loadJson, saveJson } from "@/lib/storage";
 import { pendingSettingsKeys, queueSettingsPush } from "@/lib/settingsSync";
 import { setDateTimePrefs, setUiLanguageForFormatting, type DateFormat, type TimeFormat } from "@/lib/datetime";
 import type { SwipeAction } from "@/lib/swipe";
@@ -311,6 +311,17 @@ interface SettingsState {
 }
 
 const initialSettings = loadJson<Settings>("settings", DEFAULT_SETTINGS);
+
+/**
+ * Whether the first frame is this account's settings or merely the defaults.
+ *
+ * False after every deploy, because deploys sign everyone out and sign-out
+ * clears the cache -- and false on any untrusted device, where the cache is
+ * never read. In that state `uiLanguage` starts as English and only becomes
+ * the account's choice once the settings file lands, which is why the
+ * authenticated tree waits for it.
+ */
+export const PAINTED_FROM_CACHE = hasCachedJson("settings");
 applyDateTimePrefs(initialSettings);
 
 export const useSettings = create<SettingsState>((set, get) => ({