Skip to content

fix: stop the white flash when entering the app - #320

Open
zxch3n wants to merge 5 commits into
mainfrom
fix/login-white-flash
Open

fix: stop the white flash when entering the app#320
zxch3n wants to merge 5 commits into
mainfrom
fix/login-white-flash

Conversation

@zxch3n

@zxch3n zxch3n commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Fixes the few-seconds white screen after signing in. Two independent causes, both landing before React can correct them.

A. The lazy layout rendered nothing

RouteSuspense was <Suspense fallback={null}>, and it wraps the app's largest chunk (lazy(() => import('@/components/main-layout'))). While that chunk is fetched, React renders nothing at all and the window falls through to the bare <body> canvas — white, and on a cold start or a slow connection for a long time.

  • The fallback now paints bg-background (the same token the layout root uses) on frame one.
  • The spinner and label are held back for 300ms and then fade in, by CSS animate-in fade-in delay-300 fill-mode-both ease-out — no timer, no React state. A chunk that arrives quickly shows only the canvas, so we do not trade a white flash for a spinner flash. Under prefers-reduced-motion the global reset collapses the duration but keeps the delay, so the indicator simply appears at 300ms without the fade.
  • RouteSuspense gained a scope. viewport is for a boundary that owns the window; content (the Archive route) fills its pane so it cannot paint over the already-mounted sidebar.
  • login-page.tsx warms the main-layout chunk on an idle callback while the page waits on the user, so the post-sign-in route swap does not begin with a fetch.

B. .dark landed after the first paint (Electron)

The renderer CSP (script-src 'self') deliberately rejects inline scripts, so next-themes' blocking pre-paint script is neutered and the theme class could only be applied once React mounted. Meanwhile the window backgroundColor came from nativeTheme.shouldUseDarkColors. A user on an explicit dark theme under a light system therefore opened a #FFFFFF window with a light canvas and watched it turn black on mount.

  • theme-settings.ts mirrors the committed theme into the main process (the renderer keeps it in localStorage, which main cannot read). getInitialMainWindowThemeSource feeds it to nativeTheme.themeSource before the BrowserWindow is constructed, so the background color and the win32 caption overlay are right on frame one. Onboarding stays pinned to Light.
  • Preload — the only renderer-side code that runs before the document is parsed — applies the resolved .dark/.light class from a --lody-initial-window-theme launch argument, mirroring exactly what theme-provider does on mount. If the parser has not produced <html> yet it observes the document and applies the class the instant it appears, which is still a microtask during parsing. DOMContentLoaded would be too late: the app is a module script, so Chromium is free to paint the parsed body first.
  • A theme preview (hovering a theme in Settings) still retints live chrome via app.setNativeTheme, but never reaches the persisted startup theme — only app.setStartupThemeSource writes it.
  • CSP is unchanged.

Design review

Ran better-ui over the fallback. Notes that shaped the result:

  • A layout-shaped skeleton was rejected: a fake sidebar replaced by the real one is a second structural flash, and the actual defect is the canvas colour, which bg-background fixes at frame one.
  • An immediate spinner was rejected under motion restraint — animation on a sub-300ms, zero-information event.
  • A minimum display time was rejected: it would mean delaying real content. The 300ms delay plus the fade ramp already means a 400ms resolve peaks at roughly a third opacity.

Tests

  • route-suspense.test.tsx: the fallback renders (not null), carries the right scope, defers by CSS, and yields to the loaded route. The chunk is an explicitly resolved promise — no timers, no scheduler luck.
  • theme-provider.test.ts: the committed theme is mirrored to main; a preview is not.
  • onboarding-theme-lifecycle.test.tsx: onboarding's forced Light and restored System both reach the startup theme.
  • window-theme.test.mjs, initial-window-theme-argument.test.mjs, initial-window-theme.test.mjs (preload, with an injected observer).
  • Storybook: LoadingPlaceholder gained DeferredIndicator and DeferredIndicatorInContentPane.

pnpm check and pnpm format pass.

Out of scope

The Capacitor mobile shell needs the same pre-paint contract for its WebView background and splash colour, but that project is outside this repository. The invariant is recorded in packages/components/AGENTS.md rather than faked here.

🤖 Generated with Claude Code

@zxch3n @claude
fix: stop the white flash when entering the app
ddc23eb 
Two independent sources of white, both before React can correct them.

`RouteSuspense` rendered `fallback={null}`, so nothing at all was on screen
while the lazy `main-layout` chunk was fetched — the window fell through to
the bare `<body>` canvas for the length of that fetch, which on a cold start
or a slow connection is the white screen a user sees right after signing in.
It now paints `bg-background` on the first frame, with the spinner and label
held back 300ms by CSS (no timer) so a fast chunk shows only the canvas and
never flashes an indicator. A `scope` prop keeps a nested boundary inside its
pane instead of covering the mounted sidebar. The login page also warms the
`main-layout` chunk while it waits on the user, so the post-sign-in route
swap does not begin with a fetch.

On Electron the theme class itself landed late. The renderer CSP rejects
next-themes' inline pre-paint script, so `.dark` could only be applied after
React mounted, and the window background followed `nativeTheme` — meaning a
user on an explicit dark theme under a light system opened a white window
and watched it turn black. The committed theme is now mirrored into the main
process, drives the window `backgroundColor` and win32 caption overlay before
the window exists, and reaches preload as a launch argument so the class is
on `<html>` before the first paint. A theme preview retints live chrome but
is never persisted.

Model: claude-opus-5[1m]

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@zxch3n zxch3n mentioned this pull request Sep 3, 2026
Merged
@zxch3n @claude
refactor: drop ablated dead paths and snapshot tests from the flash fix
566dcb3 
Ablated every mechanism the fix added. Three turned out not to carry weight.

An Electron 39 probe showed that at preload's first line the document is
`{ readyState: 'loading', documentElement: null, childCount: 0 }`, so the
synchronous "apply now if `<html>` exists" branch never runs and observing the
document is the only path rather than a fallback. That branch, its unreachable
"a class is already there" guard, and the test that covered it are gone, and
the module is now one function. The `typeof document !== 'undefined'` guard in
preload goes with them: preload only ever runs in a renderer.

`prewarm-main-layout.ts` existed for a module-level "already started" flag that
ablation showed changes nothing — the module registry already dedupes the
dynamic import — so the flag is gone and the remaining single idle-scheduled
import is inlined at its one call site.

Measuring the two `LoadingPlaceholder` variants in Chromium corrected the
reason `scope` exists: a `100dvh` placeholder does not cover the sidebar (a
flex sibling), it overflows its pane by exactly the top safe-area inset and
drops the spinner half an inset below centre, where `overflow-hidden` clips it.
Identical without an inset, so the comments, AGENTS entry and test now say
that instead.

Tests: removed a class-token snapshot that asserted `delay-300` and
`fill-mode-both` while missing `fade-in`, the one token whose removal silently
disables the whole deferral — it could not catch the regression it existed for,
so the fragile set is named in the code comment instead. Also folded four
overlapping cases into two and dropped two `className` assertions and one
duplicate onboarding assertion.

Kept despite no failing test: the `setStartupThemeSource` input guard (foreign
IPC input, and the predicate itself is covered) and `deferIndicator` (jsdom has
no CSS engine; the two Storybook stories cover it).

Model: claude-opus-5[1m]

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@zxch3n LodyAI

zxch3n commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Ablation results

Every mechanism, constant, branch, prop and early return the fix added was removed one at a time. Electron modules were checked with pnpm --dir apps/electron test and components with the three affected vitest files (both seconds-long); the full pnpm check ran once at the end and is green.

Two questions could not be settled by removing code, so they were measured directly with a throwaway Electron 39 harness (deleted afterwards, nothing committed):

  • Preload timing. At preload's first line the document is { readyState: "loading", hasDocumentElement: false, childCount: 0 }. The MutationObserver then fired while readyState was still loading, and by DOMContentLoaded the computed body background was rgb(16, 16, 16). So the observer is the only live path, not a fallback — the opposite of how I had written it.
  • viewport vs content placeholder. In a real Chromium layout matching web-workspace-layout.tsx: with no safe-area inset both variants measure 768px in a 768px pane, spinner dead centre — identical. With a 48px top inset (the iPad shell) the viewport variant measures 768px in a 720px pane, overflowing by 48px and putting the spinner 24px below the pane centre, clipped by overflow-hidden. So scope is load-bearing, but not for the reason I wrote: the sidebar is a flex sibling and is never covered.
# Ablated Result Verdict Reason
A1 RouteSuspense scope prop 1 test fails Keep Load-bearing, but only where the pane is shorter than the viewport (measured above). Rationale corrected in code, AGENTS and the test.
A2 deferIndicator entirely 1 test fails (that test is now deleted, see below) Keep, untested The anti-flash behaviour. jsdom has no CSS engine and pnpm check does not run browser tests, so it is covered by the two Storybook stories instead of a fake unit test.
A3 fade-in token only nothing fails Keep + comment The token that actually sets the from opacity to 0. Its removal silently disables the whole deferral and the test claiming to guard the deferral did not notice — see test deletions.
A4 data-loading-placeholder-deferred nothing fails Delete Existed only for the deleted test.
A5 prewarmStarted guard nothing fails Delete Genuinely redundant: the module registry already dedupes the dynamic import, so the flag saved nothing. The remaining one-line idle import is now inlined and lib/prewarm-main-layout.ts is gone.
A6 .catch(() => {}) on the prewarm import nothing fails Keep An offline prewarm would otherwise reject unhandled and be reported as a renderer error.
A7 exported RouteSuspenseScope type nothing fails Un-export No consumer outside the module.
A11 isNativeWindowThemeSource in setStartupThemeSource nothing fails Keep, gap noted Foreign IPC input, which apps/electron/AGENTS.md requires validating at the class boundary — not deleted for lack of coverage. The predicate is covered; the uncovered part is one call-site wiring that would need an Electron-runtime harness.
A12 getInitialMainWindowThemeSource stored-source param 1 test fails Keep Core of the window-colour fix.
A13 launch-argument value validation 1 test fails Keep Foreign argv input; an unvalidated value would become a class name on <html>.
A14 preload typeof document !== "undefined" nothing fails Delete Unreachable: preload only runs in a renderer, and the probe confirms document is always present.
A15 preload synchronous-apply branch 1 test fails Delete branch and test The probe proves the branch is unreachable, so the test was covering dead code. Module collapsed to a single observer-only function; its "class already present" guard went too, for the same reason.
A18 resolveNativeWindowTheme in window.ts nothing fails Keep Behaviour-identical de-duplication of an inline ternary, not an addition.
A9 settings-store.ts extraction nothing fails Keep Removes ~15 duplicated lines of Conf interop rather than adding a mechanism.
A10 theme-settings.ts try/catch not unit-testable (needs electron) Keep A corrupt settings file must degrade to system, which is exactly the pre-change behaviour, rather than stop the window opening.

Test deletions

Four cases removed, net −198/+84 lines.

  1. route-suspense.test.tsx — "holds the indicator back…" — deleted. It asserted delay-300 and fill-mode-both were present in a className: a class-token snapshot that breaks if the delay is retuned and catches no regression. A3 proved it worse than useless — it missed fade-in, the token whose removal actually disables the feature. The fragile set is now named in the LoadingPlaceholder comment.
  2. Two className assertions (bg-background, not.toContain('min-h-[100dvh]')) — deleted for the same reason; the structural assertions around them already catch the null-fallback regression.
  3. initial-window-theme-argument.test.mjs — "reads the theme alongside the other bootstrap arguments" — folded into the round-trip case, which now carries the sibling argument. It could not fail independently.
  4. window-theme.test.mjs — "opens on committed" and "falls back to the OS appearance" merged into one; the isNativeWindowThemeSource table trimmed from eight rows to three representative ones, since the rest could not fail independently.
  5. onboarding-theme-lifecycle.test.tsx — dropped the second setStartupThemeSource assertion; both exit paths reach the same theme-provider effect, so it re-covered the first.

No new tests were needed: the two ablations that surfaced holes (A3, A11) resolved to a code comment and a documented Electron-runtime limitation rather than to tests that would assert implementation details.

Temporary artefacts

Swept the branch diff. The only console.* calls are two console.warn error logs in theme-settings.ts matching the neighbouring auto-launch-settings.ts style. No /tmp references, debug scripts, instrumentation, commented-out experiments or temporary stories. The Electron probe used for the measurements above lived in /tmp and has been deleted.

pnpm check and pnpm format are clean.

zxch3n and others added 3 commits September 3, 2026 11:53
@zxch3n
chore: merge main into fix/login-white-flash
b9ab2ac 
One conflict, in `apps/electron/package.json`: both sides appended suites to
the single `test` script line. Resolved as the union — main's three Sparkle
suites stay where they were added, and this branch's two theme-bootstrap
suites go back behind `system-language-argument.test.mjs`, the argument
handling they sit next to. Both sides' suites now run (83 passing).

Nothing else overlapped. Main did not touch window creation, the native theme,
or preload, and the onboarding theme lifecycle this branch tests is unchanged,
so the electron `AGENTS.md` merged cleanly with both sides' invariants intact.
@zxch3n @claude
fix(electron): survive an unreadable settings file instead of failing…
5085205 
… to launch

Review found the two try/catch blocks in `theme-settings.ts` guarding the wrong
step. They cover `get` and `set`, but `conf` reads and VALIDATES the config file
inside its constructor, and every store here is constructed at module scope — so
a malformed file threw during import, outside both guards, and the main process
did not start. The read guard's comment claimed to prevent exactly that.

Verified the premise before defending against it, on the installed conf 15.1.0:
`clearInvalidConfig` defaults to `false` and the constructor rethrows. Against a
real file, `{"startupThemeSource": "da` throws `SyntaxError` and
`{"startupThemeSource": "vesper"}` throws `Config schema violation:`. Both from
`new Conf(...)`, not from a later `get`.

The guard goes in `createMainSettingsStore`, introduced by this PR, so both
stores get it; `auto-launch-settings.ts` keeps its behavior on every path that
previously worked and only stops being able to abort startup. The fallback is an
in-memory store seeded with the schema defaults — this launch behaves as though
nothing had been persisted, which for the theme is the pre-persistence `system`
path. Deliberately not `clearInvalidConfig: true`: that would silently empty a
file the user might want back, and it does not cover an unreadable path anyway.

The `get`/`set` guards stay, now describing what they actually cover: `conf`
re-reads on every `get`, so a file damaged after construction still throws.

Split the electron-free half into `settings-store-core.ts`, matching
`image-export-core.ts`, so the test drives real `conf` against real corrupt
files on disk rather than a stubbed failure. Ablating the fallback fails both
corruption cases and leaves the healthy-file case passing.

Also recorded the login prewarm's cost where it is paid: on web and mobile it is
a real request /login did not use to make, including for a visitor who never
signs in. Not gated on a sign-in click on purpose — a social login navigates
away immediately, so a fetch started then is usually discarded.

Model: claude-opus-5[1m]

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@zxch3n @claude
test: cut assertions that cannot catch a regression
7c58240 
The test diff was 318 of the PR's 793 added lines, and a good part of it was
paperwork. Kept one thing per mechanism, dropped the rest.

- `route-suspense`: dropped the scope-forwarding case (it asserted a prop
  reaches a prop; the pane-vs-viewport reasoning it carried lives in the
  component comment and the Storybook story) and, from the remaining case,
  the half that watched the resolved chunk replace the fallback, which was
  testing React's Suspense rather than this change. A never-resolving `lazy`
  removes the promise plumbing with it. 99 -> 46.
- `theme-provider`: kept "a preview must not persist, a commit must", dropped
  the parallel `setNativeTheme` recording, which the onboarding test already
  covers. 60 -> 42.
- `settings-store-core`: dropped the valid-file round trip, which tested `conf`
  and not the fallback; the two corruption cases share one test. 58 -> 36.
- `initial-window-theme-argument`: the serialize/read round trip was a
  tautology through one shared constant. Folded its one useful assertion into
  the validation case. 29 -> 24.
- `window-theme`: merged three cases over one pure function into two.

Also tightened the two longest comment blocks. Net 793 -> 681 added lines, of
which tests are 215.

Model: claude-opus-5[1m]

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

scope: components scope: desktop

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@zxch3n