From 890c49169bc7d3aad4bc0a4d07db3113d68e0f49 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Wed, 4 Nov 2026 11:26:02 +0100 Subject: [PATCH] fix: save the open message as a draft before Compose, Reply all or Forward opens another --- .../mail-app-composer-replace.test.tsx | 52 ++++++++++++++ components/mail/mail-app.tsx | 67 ++++++++++--------- 2 files changed, 88 insertions(+), 31 deletions(-) diff --git a/components/mail/__tests__/mail-app-composer-replace.test.tsx b/components/mail/__tests__/mail-app-composer-replace.test.tsx index f56066c13..1d0283969 100644 --- a/components/mail/__tests__/mail-app-composer-replace.test.tsx +++ b/components/mail/__tests__/mail-app-composer-replace.test.tsx @@ -172,3 +172,55 @@ describe('Compose while a reply is docked (compose-p2-1)', () => { }); } }); + +describe('another composer while one is open (compose-p2-2)', () => { + it('closes the reply through its own close (save, "Draft saved") before Compose opens', async () => { + stub.close = 'saves'; + render(); + fireEvent.click(await screen.findByText('stub reply')); + await screen.findByTestId('stub-composer'); + + fireEvent.click(screen.getByText('stub compose')); + await waitFor(() => expect(composerMode()).toBe('compose')); + expect(stub.closes).toEqual(['reply']); + }); + + for (const [action, mode] of [['stub reply all', 'replyAll'], ['stub forward', 'forward']] as const) { + it(`closes the reply through its own close before ${action} opens`, async () => { + stub.close = 'saves'; + render(); + fireEvent.click(await screen.findByText('stub reply')); + await screen.findByTestId('stub-composer'); + + fireEvent.click(screen.getByText(action)); + await waitFor(() => expect(composerMode()).toBe(mode)); + expect(stub.closes).toEqual(['reply']); + }); + } + + it('closes a new message before another new message opens', async () => { + stub.close = 'saves'; + render(); + fireEvent.click(screen.getByText('stub compose')); + await screen.findByTestId('stub-composer'); + const mounted = stub.mounts.length; + + fireEvent.click(screen.getByText('stub compose')); + await waitFor(() => expect(stub.mounts.length).toBe(mounted + 1)); + expect(stub.closes).toEqual(['compose']); + }); + + it('keeps the open composer when its save fails', async () => { + stub.close = 'fails'; + render(); + fireEvent.click(await screen.findByText('stub reply')); + await screen.findByTestId('stub-composer'); + const mounted = stub.mounts.length; + + fireEvent.click(screen.getByText('stub compose')); + await act(async () => { await new Promise((r) => setTimeout(r, 20)); }); + expect(stub.closes).toEqual(['reply']); + expect(stub.mounts.length).toBe(mounted); + expect(composerMode()).toBe('reply'); + }); +}); diff --git a/components/mail/mail-app.tsx b/components/mail/mail-app.tsx index 1ea598b03..fb2db8863 100644 --- a/components/mail/mail-app.tsx +++ b/components/mail/mail-app.tsx @@ -319,6 +319,15 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { // already uses the same seam), so entry points that replace the session can // ask before discarding unsaved text. const composerRequestCloseRef = useRef<((afterClose?: () => void) => void) | null>(null); + // One composer at a time (compose-p2-2). Whatever opens a composer closes + // the one on screen first, the way its X does: it saves and says "Draft + // saved" with Discard, then the new one opens. When that save fails the old + // composer stays with its question, and this never settles: nothing opens. + const closeOpenComposer = useCallback(() => new Promise((resolve) => { + const requestClose = composerRequestCloseRef.current; + if (showComposerRef.current && requestClose) requestClose(resolve); + else resolve(); + }), []); const { dialogProps: confirmDialogProps, confirm: confirmDialog } = useConfirmDialog(); const { dialogProps: promptDialogProps, prompt: promptDialog } = usePromptDialog(); const { showAppsModal, inlineApp, loadedApps, handleManageApps, handleInlineApp, closeInlineApp, closeAppsModal } = useSidebarApps(); @@ -1176,12 +1185,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { } }, onCompose: () => { - void withComposeGate('compose', {}, () => { - startFreshComposerSession(); - setComposerMode('compose'); - setShowComposer(true); - if (isMobile) setActiveView('viewer'); - }); + void handleCompose(); }, onFocusSearch: () => { if (isScheduledView) return; @@ -2824,11 +2828,24 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { setPendingDraft(null); }, []); + // A new message, from every Compose and New message there is. + const handleCompose = async (onOpen?: () => void) => { + await closeOpenComposer(); + await withComposeGate('compose', {}, () => { + startFreshComposerSession(); + setComposerMode('compose'); + setShowComposer(true); + if (isMobile) setActiveView('viewer'); + onOpen?.(); + }); + }; + // A minimised composer's edits live only in pendingDraft. Before an // automatic sign-out it is opened again and saves them itself. useMinimisedComposerFlush(!showComposer && pendingDraft?.unsaved === true, () => setShowComposer(true)); const handleReply = async (draftText?: string) => { + await closeOpenComposer(); if (selectedEmail) { const formerSelected = { ...selectedEmail }; const enrichedEmail = await emailHooks.onBeforeComposeOpenToReply.transform(selectedEmail); @@ -2867,6 +2884,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { : selectedEmail; let draft = draftCandidate; if (!draft) return; + await closeOpenComposer(); // The login and account the draft lives in (resolveDraftSource). The // composer removes the old version there when it saves, sends or @@ -3121,6 +3139,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { }, [cancelUndoSend, clearPendingUndoSend, client, fetchScheduledEmails, isScheduledView, pendingUndoSend?.submissionId, sendDelaySeconds, t, tComposer, tViewer]); const handleReplyAll = async (draftText?: string) => { + await closeOpenComposer(); if (selectedEmail) { const formerSelected = { ...selectedEmail }; const enrichedEmail = await emailHooks.onBeforeComposeOpenToReplyAll.transform(selectedEmail); @@ -3210,6 +3229,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { }, [isEmbedded]); const handleForward = async () => { + await closeOpenComposer(); if (selectedEmail) { const formerSelected = { ...selectedEmail }; const enrichedEmail = await emailHooks.onBeforeComposeOpenToForward.transform(selectedEmail); @@ -3256,6 +3276,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { // unselected row. See the list context-menu wiring below. const handleForwardAsAttachment = async (email: Email | null = selectedEmail) => { if (!email) return; + await closeOpenComposer(); email = await emailHooks.onBeforeComposeOpenToForwardAsAttachment.transform(email); // Same filename options "Export as .eml" uses (see emailFilenameOptions // in email-viewer.tsx), so the two actions produce consistent filenames @@ -3324,6 +3345,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { const handleBatchForwardAsAttachment = async () => { const selected = resolveSelectedEmails(useEmailStore.getState()); if (selected.length < 2) return; // menu only offers this for >1; guards future wiring + await closeOpenComposer(); const { emailDownloadTemplate, @@ -4817,14 +4839,8 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { onImportEmail: handleImportEmailFromContextMenu, onRefreshMailboxes: handleRefreshMailboxes, onCompose: () => { - void withComposeGate('compose', {}, () => { - startFreshComposerSession(); - setComposerMode('compose'); - setShowComposer(true); - if (isMobile) { - setSidebarOpen(false); - setActiveView('viewer'); - } + void handleCompose(() => { + if (isMobile) setSidebarOpen(false); }); }, onSidebarClose: () => setSidebarOpen(false), @@ -5155,6 +5171,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { // Handle reply from conversation view const handleConversationReply = async (email: Email) => { + await closeOpenComposer(); email = await emailHooks.onBeforeComposeOpenToReply.transform(email); selectEmail(email); await prepareComposerQuoteHeader(email, 'reply'); @@ -5167,6 +5184,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { }; const handleConversationReplyAll = async (email: Email) => { + await closeOpenComposer(); email = await emailHooks.onBeforeComposeOpenToReplyAll.transform(email); selectEmail(email); await prepareComposerQuoteHeader(email, 'replyAll'); @@ -5179,6 +5197,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { }; const handleConversationForward = async (email: Email) => { + await closeOpenComposer(); email = await emailHooks.onBeforeComposeOpenToForward.transform(email); selectEmail(email); await prepareComposerQuoteHeader(email, 'forward'); @@ -5888,12 +5907,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { }} onClearSearch={() => void handleClearSearch()} onCompose={() => { - void withComposeGate('compose', {}, () => { - startFreshComposerSession(); - setComposerMode('compose'); - setShowComposer(true); - if (isMobile) setActiveView('viewer'); - }); + void handleCompose(); }} onMarkAsRead={async (email, read) => { await markReadWithUndo(email, read).catch(toastUpdateFailure); @@ -5936,12 +5950,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { button no longer sits on the list's rows. */} {!mvShell && (isMobile || isEmbedded) &&