From 2f72a8012ebe8632eb6a7ac697264ba970e7edde Mon Sep 17 00:00:00 2001 From: IanMStevenson Date: Sat, 19 Sep 2026 02:47:04 +0100 Subject: [PATCH] fix(web): save composer draft on visibilitychange, not only on unmount (#1883) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Links in a session open via window.open(_blank). For an installed standalone-display PWA, navigating to a URL outside the manifest's scope is a platform behavior (Chromium/Edge show it as a minimal overlay with a close affordance), not custom code here — but closing it can mean the underlying page had a real top-level teardown, not just an in-app remount. useComposerDraft only ever persisted the draft in its effect's cleanup — reliable for a React-managed unmount (switching sessions), but browsers don't guarantee that cleanup runs to completion before a genuine top-level navigation completes, so the save could simply never happen. Extracts the existing save logic into persistNow() and calls it both from the unmount cleanup (unchanged) and a new visibilitychange listener (when document.visibilityState becomes 'hidden'), mirroring the pattern already used elsewhere in this codebase (useSSE.ts, useAuth.ts, useSyncingState.ts, usePwaUpdate.ts, useVisibilityReporter.ts) for the same class of problem. Fixes #1882 Co-authored-by: Ian Stevenson Co-authored-by: Claude Sonnet 5 --- web/src/hooks/useComposerDraft.test.ts | 38 ++++++++++++++++++++++++++ web/src/hooks/useComposerDraft.ts | 38 ++++++++++++++++++-------- 2 files changed, 65 insertions(+), 11 deletions(-) diff --git a/web/src/hooks/useComposerDraft.test.ts b/web/src/hooks/useComposerDraft.test.ts index 6abc3a25..f081a046 100644 --- a/web/src/hooks/useComposerDraft.test.ts +++ b/web/src/hooks/useComposerDraft.test.ts @@ -151,6 +151,44 @@ describe('useComposerDraft', () => { expect(mockSaveDraftAttachments).toHaveBeenCalledWith('session-1', []) }) + it('saves draft when the page becomes hidden, without unmounting (hapi#1882)', async () => { + mockGetDraft.mockReturnValue('') + const setText = vi.fn() + + const { rerender } = renderHook( + ({ text }) => useComposerDraft('session-1', text, [], true, setText, vi.fn()), + { initialProps: { text: '' } }, + ) + + await act(async () => flushRAF()) + rerender({ text: 'my draft' }) + + // A real top-level page navigation (e.g. an installed PWA's own + // out-of-scope-link overlay) doesn't reliably run React's unmount + // cleanup before the state is gone — this is the fallback save. + Object.defineProperty(document, 'visibilityState', { value: 'hidden', configurable: true }) + document.dispatchEvent(new Event('visibilitychange')) + + expect(mockSaveDraft).toHaveBeenCalledWith('session-1', 'my draft') + expect(mockSaveDraftAttachments).toHaveBeenCalledWith('session-1', []) + + Object.defineProperty(document, 'visibilityState', { value: 'visible', configurable: true }) + }) + + it('does not save on visibilitychange while the page is becoming visible again', async () => { + mockGetDraft.mockReturnValue('') + const setText = vi.fn() + + renderHook(() => useComposerDraft('session-1', 'my draft', [], true, setText, vi.fn())) + await act(async () => flushRAF()) + mockSaveDraft.mockClear() + + Object.defineProperty(document, 'visibilityState', { value: 'visible', configurable: true }) + document.dispatchEvent(new Event('visibilitychange')) + + expect(mockSaveDraft).not.toHaveBeenCalled() + }) + it('does not save draft on unmount before rAF has fired', () => { mockGetDraft.mockReturnValue('') const setText = vi.fn() diff --git a/web/src/hooks/useComposerDraft.ts b/web/src/hooks/useComposerDraft.ts index 59c0c721..c40c9f85 100644 --- a/web/src/hooks/useComposerDraft.ts +++ b/web/src/hooks/useComposerDraft.ts @@ -23,7 +23,12 @@ export type ComposerDraftHydration = { * * - On mount: restores saved draft via `setText` (deferred by one animation frame) * - On mount: restores saved attachment files through the composer adapter - * - On unmount: saves current text and attachment files as a draft + * - On unmount, and whenever the page is hidden: saves current text and + * attachment files as a draft. The page-hidden save matters separately from + * unmount — an in-app remount (switching sessions) reliably runs React's + * cleanup, but a real top-level page navigation (e.g. an installed PWA's + * own out-of-scope-link overlay closing) does not guarantee cleanup runs + * before the state is gone (hapi#1882). * - The `draftReady` guard prevents saving before the initial restore completes, * avoiding the case where the runtime's empty initial text overwrites a real draft. * @@ -169,16 +174,10 @@ export function useComposerDraft( }) }) - return () => { - disposed = true - cancelAnimationFrame(frame) - // Cross-session resume already moved this draft; do not recreate the - // obsolete source id after the route change unmounts the composer. - if (composerDraftWasHandedOff(sessionId)) { - draftReadyRef.current = false - attachmentsReadyRef.current = false - return - } + // Cross-session resume already moved this draft; do not recreate the + // obsolete source id after the route change unmounts the composer. + const persistNow = () => { + if (composerDraftWasHandedOff(sessionId)) return if (draftReadyRef.current) { saveDraft(sessionId, composerTextRef.current) } @@ -195,6 +194,23 @@ export function useComposerDraft( console.warn('[composer-draft] inactive persistence failed', error) }) } + } + + // A real top-level page navigation (see the doc comment above) doesn't + // guarantee this effect's own cleanup runs before the state is gone — + // save proactively whenever the page is hidden, not only on unmount. + const handleVisibilityChange = () => { + if (document.visibilityState === 'hidden') { + persistNow() + } + } + document.addEventListener('visibilitychange', handleVisibilityChange) + + return () => { + disposed = true + cancelAnimationFrame(frame) + document.removeEventListener('visibilitychange', handleVisibilityChange) + persistNow() draftReadyRef.current = false attachmentsReadyRef.current = false }