Skip to content

Commit 2f72a80

Browse files
IanMStevensonIan Stevensonclaude
authored
fix(web): save composer draft on visibilitychange, not only on unmount (#1883)
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 <is@lockhouse.systems> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 773af2c commit 2f72a80

2 files changed

Lines changed: 65 additions & 11 deletions

File tree

‎web/src/hooks/useComposerDraft.test.ts‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,44 @@ describe('useComposerDraft', () => {
151151
expect(mockSaveDraftAttachments).toHaveBeenCalledWith('session-1', [])
152152
})
153153

154+
it('saves draft when the page becomes hidden, without unmounting (hapi#1882)', async () => {
155+
mockGetDraft.mockReturnValue('')
156+
const setText = vi.fn()
157+
158+
const { rerender } = renderHook(
159+
({ text }) => useComposerDraft('session-1', text, [], true, setText, vi.fn()),
160+
{ initialProps: { text: '' } },
161+
)
162+
163+
await act(async () => flushRAF())
164+
rerender({ text: 'my draft' })
165+
166+
// A real top-level page navigation (e.g. an installed PWA's own
167+
// out-of-scope-link overlay) doesn't reliably run React's unmount
168+
// cleanup before the state is gone — this is the fallback save.
169+
Object.defineProperty(document, 'visibilityState', { value: 'hidden', configurable: true })
170+
document.dispatchEvent(new Event('visibilitychange'))
171+
172+
expect(mockSaveDraft).toHaveBeenCalledWith('session-1', 'my draft')
173+
expect(mockSaveDraftAttachments).toHaveBeenCalledWith('session-1', [])
174+
175+
Object.defineProperty(document, 'visibilityState', { value: 'visible', configurable: true })
176+
})
177+
178+
it('does not save on visibilitychange while the page is becoming visible again', async () => {
179+
mockGetDraft.mockReturnValue('')
180+
const setText = vi.fn()
181+
182+
renderHook(() => useComposerDraft('session-1', 'my draft', [], true, setText, vi.fn()))
183+
await act(async () => flushRAF())
184+
mockSaveDraft.mockClear()
185+
186+
Object.defineProperty(document, 'visibilityState', { value: 'visible', configurable: true })
187+
document.dispatchEvent(new Event('visibilitychange'))
188+
189+
expect(mockSaveDraft).not.toHaveBeenCalled()
190+
})
191+
154192
it('does not save draft on unmount before rAF has fired', () => {
155193
mockGetDraft.mockReturnValue('')
156194
const setText = vi.fn()

‎web/src/hooks/useComposerDraft.ts‎

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,12 @@ export type ComposerDraftHydration = {
2323
*
2424
* - On mount: restores saved draft via `setText` (deferred by one animation frame)
2525
* - On mount: restores saved attachment files through the composer adapter
26-
* - On unmount: saves current text and attachment files as a draft
26+
* - On unmount, and whenever the page is hidden: saves current text and
27+
* attachment files as a draft. The page-hidden save matters separately from
28+
* unmount — an in-app remount (switching sessions) reliably runs React's
29+
* cleanup, but a real top-level page navigation (e.g. an installed PWA's
30+
* own out-of-scope-link overlay closing) does not guarantee cleanup runs
31+
* before the state is gone (hapi#1882).
2732
* - The `draftReady` guard prevents saving before the initial restore completes,
2833
* avoiding the case where the runtime's empty initial text overwrites a real draft.
2934
*
@@ -169,16 +174,10 @@ export function useComposerDraft(
169174
})
170175
})
171176

172-
return () => {
173-
disposed = true
174-
cancelAnimationFrame(frame)
175-
// Cross-session resume already moved this draft; do not recreate the
176-
// obsolete source id after the route change unmounts the composer.
177-
if (composerDraftWasHandedOff(sessionId)) {
178-
draftReadyRef.current = false
179-
attachmentsReadyRef.current = false
180-
return
181-
}
177+
// Cross-session resume already moved this draft; do not recreate the
178+
// obsolete source id after the route change unmounts the composer.
179+
const persistNow = () => {
180+
if (composerDraftWasHandedOff(sessionId)) return
182181
if (draftReadyRef.current) {
183182
saveDraft(sessionId, composerTextRef.current)
184183
}
@@ -195,6 +194,23 @@ export function useComposerDraft(
195194
console.warn('[composer-draft] inactive persistence failed', error)
196195
})
197196
}
197+
}
198+
199+
// A real top-level page navigation (see the doc comment above) doesn't
200+
// guarantee this effect's own cleanup runs before the state is gone —
201+
// save proactively whenever the page is hidden, not only on unmount.
202+
const handleVisibilityChange = () => {
203+
if (document.visibilityState === 'hidden') {
204+
persistNow()
205+
}
206+
}
207+
document.addEventListener('visibilitychange', handleVisibilityChange)
208+
209+
return () => {
210+
disposed = true
211+
cancelAnimationFrame(frame)
212+
document.removeEventListener('visibilitychange', handleVisibilityChange)
213+
persistNow()
198214
draftReadyRef.current = false
199215
attachmentsReadyRef.current = false
200216
}

0 commit comments

Comments
 (0)