From 1a2700d8fc66f808a20cf660a9bc8798589e101d Mon Sep 17 00:00:00 2001 From: npeter83 Date: Wed, 1 Jul 2026 14:27:41 +0200 Subject: [PATCH] fix(modal): stop Release Notes modal flashing shut on modal-to-modal handoff useBackToClose eagerly pushed a history entry on mount and called history.back() on unmount. During a modal->modal handoff (e.g. About -> Release Notes) the two ran interleaved in one React commit, so the entering modal's popstate listener mistook the leaving modal's back() for a genuine user Back and closed itself instantly. It also left the history pointer behind the surviving entry, so a later browser Back walked off the app. Replace the eager per-mount push/pop with a single shared popstate handler plus a coalesced microtask that reconciles history depth to the live overlay count once per tick. A handoff's -1/+1 nets to zero, so the new modal simply reuses the old entry -- no churn, no flash, and Back closes the modal in-app. --- frontend/src/lib/history.ts | 102 +++++++++++++++++++++++------------- 1 file changed, 67 insertions(+), 35 deletions(-) diff --git a/frontend/src/lib/history.ts b/frontend/src/lib/history.ts index 270fff2..d921e59 100644 --- a/frontend/src/lib/history.ts +++ b/frontend/src/lib/history.ts @@ -35,11 +35,65 @@ export function useHistorySubview(root: T): { } // --- Overlays / modals (one history entry each, nesting-safe) ------------------------------- -const overlayStack: number[] = []; +// A SINGLE module-level popstate handler owns the whole overlay stack, and history is kept in +// sync LAZILY via a coalesced microtask rather than eagerly per mount/unmount. This matters for +// modal→modal handoffs (e.g. About → Release notes): the leaving modal unmounts and the entering +// one mounts in the SAME React commit, so within one tick the desired overlay-entry count drops +// by one then rises by one — a net zero. Reconciling once, after the tick settles, sees that net +// zero and touches history NOT AT ALL: the new modal simply reuses the old one's entry. +// +// The previous eager design pushed on mount and called history.back() on unmount independently. +// Interleaved during a handoff that produced (a) a flash-and-vanish where the new modal's popstate +// listener mistook the leaving modal's back() for a genuine user Back and closed itself, and +// (b) a mangled history where the pointer ended up BEHIND the surviving entry, so a later browser +// Back walked off the app entirely instead of closing the modal. Coalescing removes both. +type Overlay = { token: number; cb: () => void }; +const overlayStack: Overlay[] = []; let overlayCounter = 0; -// Set while we pop our OWN entry programmatically (button/ESC close) so the other overlays' -// popstate handlers don't mistake it for a user Back and close themselves too. -let suppressPop = false; +// Overlay history entries we've actually pushed. Reconciled toward overlayStack.length. +let historyDepth = 0; +// Programmatic history.back() calls still awaiting their popstate — each must be ignored. +let pendingProgrammatic = 0; +let reconcileScheduled = false; +let listenerInstalled = false; + +// Push/pop history entries until their count matches the live overlay count. Runs once per tick. +function reconcileHistory(): void { + reconcileScheduled = false; + const want = overlayStack.length; + while (historyDepth < want) { + historyDepth++; + window.history.pushState({ ...window.history.state, _ov: ++overlayCounter }, ""); + } + while (historyDepth > want) { + historyDepth--; + pendingProgrammatic++; + window.history.back(); + } +} + +function scheduleReconcile(): void { + if (reconcileScheduled) return; + reconcileScheduled = true; + queueMicrotask(reconcileHistory); +} + +function ensureOverlayListener(): void { + if (listenerInstalled) return; + listenerInstalled = true; + window.addEventListener("popstate", () => { + if (pendingProgrammatic > 0) { + pendingProgrammatic--; // our own back(): consume and ignore + return; + } + // Genuine user Back: the browser already dropped one overlay entry, so close the topmost + // overlay to match. Its unmount cleanup finds itself already off the stack and only re-syncs. + if (historyDepth > 0) historyDepth--; + const top = overlayStack.pop(); + if (top) top.cb(); + scheduleReconcile(); + }); +} /** While the calling component is mounted, Back (or the browser/mouse back button) closes it via * `onClose` instead of navigating away. Mount the component only while the overlay is open. */ @@ -47,38 +101,16 @@ export function useBackToClose(onClose: () => void): void { const cb = useRef(onClose); cb.current = onClose; useEffect(() => { - const token = ++overlayCounter; - overlayStack.push(token); - window.history.pushState({ ...window.history.state, _ov: token }, ""); - const onPop = () => { - if (suppressPop) { - suppressPop = false; - return; - } - if (overlayStack[overlayStack.length - 1] === token) { - overlayStack.pop(); - cb.current(); - } - }; - window.addEventListener("popstate", onPop); + ensureOverlayListener(); + const entry: Overlay = { token: ++overlayCounter, cb: () => cb.current() }; + overlayStack.push(entry); + scheduleReconcile(); return () => { - window.removeEventListener("popstate", onPop); - const idx = overlayStack.lastIndexOf(token); - if (idx !== -1) { - // Closed programmatically (not via Back): drop our own history entry, and tell the - // remaining overlays' handlers to ignore the resulting popstate. A one-shot listener - // clears the flag on that very popstate even when NO overlay remains underneath to - // consume it — otherwise suppressPop leaks `true` and silently swallows the NEXT - // overlay's first Back (reopen-after-button-close → first Back does nothing). - overlayStack.splice(idx, 1); - suppressPop = true; - const clearSuppress = () => { - suppressPop = false; - window.removeEventListener("popstate", clearSuppress); - }; - window.addEventListener("popstate", clearSuppress); - window.history.back(); - } + const idx = overlayStack.indexOf(entry); + // idx === -1 means the shared handler already removed us on a real user Back; either way, + // just re-sync history to the (possibly unchanged) overlay count on the next tick. + if (idx !== -1) overlayStack.splice(idx, 1); + scheduleReconcile(); }; }, []); }