mirror of
https://github.com/wu736139669/hapi.git
synced 2026-10-26 22:30:50 +00:00
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>
This commit is contained in:
co-authored by
Ian Stevenson
Claude Sonnet 5
parent
773af2cf65
commit
2f72a8012e
@@ -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()
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user