diff --git a/components/email/__tests__/composer-discard-uploads.test.tsx b/components/email/__tests__/composer-discard-uploads.test.tsx index 36cfc0199..cf1960591 100644 --- a/components/email/__tests__/composer-discard-uploads.test.tsx +++ b/components/email/__tests__/composer-discard-uploads.test.tsx @@ -1,5 +1,5 @@ import React, { StrictMode, useState } from 'react'; -import { act, cleanup, fireEvent, render, screen, within } from '@testing-library/react'; +import { act, cleanup, fireEvent, render, screen } from '@testing-library/react'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { NextIntlClientProvider } from 'next-intl'; import { EmailComposer, type ComposerDraftData } from '@/components/email/email-composer'; @@ -134,9 +134,10 @@ async function openComposer(strict = false) { add: async (...files: File[]) => { fireEvent.change(document.querySelector('input[type="file"]')!, { target: { files } }); await advance(); }, - dialog: (control: Control) => { + // The bin asks nothing (compose-p2-10). + discard: (control: Control) => { fireEvent.click(screen.getByTestId(control === 'toolbar' ? 'composer-discard' : 'composer-close')); - return within(screen.getByRole('alertdialog')); + expect(screen.queryByRole('alertdialog')).toBeNull(); }, reopen: async () => { act(() => reopen()); await advance(); }, }; @@ -167,7 +168,7 @@ describe('explicit Discard with pending attachment uploads', () => { void upload.mock.results[0].value.then(settled, settled); expect([...resources.files.values()].map(f => f.name)).toEqual(['A.txt']); expect(resources.progress.size).toBe(1); - fireEvent.click(c.dialog(control).getByRole('button', { name: /^Discard$/ })); await advance(); + c.discard(control); await advance(); await c.reopen(); await c.add(file('B.txt')); await complete(1); expectNewComposer(); @@ -197,17 +198,6 @@ describe('explicit Discard with pending attachment uploads', () => { expect(resources.files.size).toBe(0); expect(resources.progress.size).toBe(0); expectNewComposer(); expect(c.closed).toHaveBeenCalledOnce(); }); - - it('keeps active and queued uploads when the close/discard dialog is cancelled', async () => { - const uploaded = vi.spyOn(emailHooks.onAfterAttachmentUpload, 'emit'); - const c = await openComposer(strict); await c.add(file('A.txt'), file('C.txt')); - const dialog = c.dialog(control); - expect(UploadXHR.instances[0].abort).not.toHaveBeenCalled(); - fireEvent.click(dialog.getByRole('button', { name: /^Cancel$/ })); await advance(); - await complete(0); expect(UploadXHR.instances[1].body?.name).toBe('C.txt'); await complete(1); - expect(uploaded.mock.calls.map(([attachment]) => attachment.name)).toEqual(['A.txt', 'C.txt']); - expect(c.closed).not.toHaveBeenCalled(); expect(resources.files.size).toBe(0); expect(resources.progress.size).toBe(0); - }); }); it.each(['saveFile', 'getFile', 'deleteFile'] as const)('cancels while asynchronous %s is pending', async method => { @@ -234,7 +224,7 @@ describe('explicit Discard with pending attachment uploads', () => { await c.add(file('A.txt'), file('C.txt')); if (method === 'deleteFile') await complete(0); expect(entered).toHaveBeenCalledOnce(); - fireEvent.click(c.dialog('toolbar').getByRole('button', { name: /^Discard$/ })); await advance(); + c.discard('toolbar'); await advance(); await c.reopen(); await act(async () => { gate.resolve(); }); await advance(); await c.add(file('B.txt')); await complete(UploadXHR.instances.findIndex(xhr => xhr.body?.name === 'B.txt')); diff --git a/components/email/__tests__/composer-discard.test.tsx b/components/email/__tests__/composer-discard.test.tsx index 4df7b6d90..bc90773ee 100644 --- a/components/email/__tests__/composer-discard.test.tsx +++ b/components/email/__tests__/composer-discard.test.tsx @@ -1,4 +1,4 @@ -import { render, screen, fireEvent, within } from '@testing-library/react'; +import { render, screen, fireEvent } from '@testing-library/react'; import { describe, it, expect, vi, beforeEach } from 'vitest'; import React from 'react'; import { EmailComposer } from '../email-composer'; @@ -7,6 +7,8 @@ import { EmailComposer } from '../email-composer'; const editorKeyDown = vi.hoisted(() => vi.fn()); const toastError = vi.hoisted(() => vi.fn()); +const discardWithUndo = vi.hoisted(() => vi.fn()); +vi.mock('@/lib/compose/draft-close-toast', () => ({ discardWithUndo, showDraftSavedToast: vi.fn() })); vi.mock('@/components/email/rich-text-editor', () => ({ RichTextEditor: () => React.createElement('div', { @@ -209,27 +211,56 @@ const REOPENED_DRAFT = { describe('composer Discard button', () => { beforeEach(() => { + discardWithUndo.mockClear(); toastError.mockClear(); updateSetting.mockClear(); editorKeyDown.mockClear(); }); - it('deletes a reopened draft without unsaved changes after confirming', async () => { + // compose-p2-10: no question; "Draft discarded" with Undo, as the toast's Discard. + it('closes at once and deletes a reopened draft once Undo is over', () => { const onDiscardDraft = vi.fn(); const onClose = vi.fn(); render(); fireEvent.click(screen.getByText('discard')); - const dialog = await screen.findByRole('alertdialog'); - expect(within(dialog).getByText('discard_draft_title')).toBeInTheDocument(); - // The user already chose to discard; there is nothing to save. - expect(within(dialog).queryByText('save')).toBeNull(); - - fireEvent.click(within(dialog).getByText('discard')); + expect(screen.queryByRole('alertdialog')).toBeNull(); + expect(onClose).toHaveBeenCalledTimes(1); + expect(discardWithUndo).toHaveBeenCalledTimes(1); + const [commit, labels] = discardWithUndo.mock.calls[0]; + expect(labels).toEqual({ discarded: 'draft_discarded', undo: 'undo' }); + // Not before the Undo time is over. + expect(onDiscardDraft).not.toHaveBeenCalled(); + commit(); expect(onDiscardDraft).toHaveBeenCalledWith('draft-1', { accountId: undefined, localAccountId: undefined }); }); + it('brings the text back on Undo, and deletes nothing', () => { + const onDiscardDraft = vi.fn(); + const onRestoreDiscarded = vi.fn(); + render( {}} />); + + fireEvent.click(screen.getByText('discard')); + const restore = discardWithUndo.mock.calls[0][2] as () => void; + restore(); + + expect(onRestoreDiscarded).toHaveBeenCalledWith(expect.objectContaining({ subject: 'Lunch', draftId: 'draft-1', to: 'bob@example.com' })); + expect(onDiscardDraft).not.toHaveBeenCalled(); + }); + + it('has the main window say it for a composer in its own window', () => { + const onDraftDiscarded = vi.fn(); + const onDiscardDraft = vi.fn(); + render( {}} />); + + fireEvent.click(screen.getByText('discard')); + + expect(onDraftDiscarded).toHaveBeenCalledWith('draft-1', { accountId: undefined, localAccountId: undefined }); + expect(discardWithUndo).not.toHaveBeenCalled(); + expect(onDiscardDraft).not.toHaveBeenCalled(); + }); + it('closes an empty, never saved composer without asking', () => { const onDiscardDraft = vi.fn(); const onClose = vi.fn(); diff --git a/components/email/email-composer.tsx b/components/email/email-composer.tsx index 6561bd670..baa7e79ad 100644 --- a/components/email/email-composer.tsx +++ b/components/email/email-composer.tsx @@ -72,7 +72,7 @@ import { isSingleKeyShortcutOff } from "@/lib/single-key-shortcuts"; import { markReplyQuoteHeader, userTextEnd } from "@/components/email/reply-quote-collapse"; import { attachmentIcon } from "@/components/email/attachment-chips"; import { attachmentsForDisplay } from "@/lib/compose/attachment-order"; -import { showDraftSavedToast } from "@/lib/compose/draft-close-toast"; +import { discardWithUndo, showDraftSavedToast } from "@/lib/compose/draft-close-toast"; import { buildQuotedHtmlBlock, serializeEditorContent } from "@/components/email/quoted-html"; import { containsEmbeddedSignature } from "@/components/email/signature-block"; import { @@ -411,6 +411,11 @@ interface EmailComposerProps { */ onDiscardDraft?: (draftId: string, draft?: { accountId?: string; localAccountId?: string }) => void; onSaveState?: (data: ComposerDraftData) => void; + /** + * Undo after the bin: opens a composer again with what this one held. The + * draft on the server is still there then. + */ + onRestoreDiscarded?: (data: ComposerDraftData) => void; /** * Offers "Open in new window". Called in the click, since a browser only * opens a window for a user action: the host opens it and answers with what @@ -444,6 +449,8 @@ interface EmailComposerProps { * this one is gone before a toast here could be read. */ onDraftKept?: (draftId: string, where: { accountId?: string; localAccountId?: string }) => void; + /** The bin closed the window: another window says "Draft discarded" with Undo, and deletes it after. */ + onDraftDiscarded?: (draftId: string, where: { accountId?: string; localAccountId?: string }) => void; }; /** The subject as typed, for a host that shows it (a Pro compose tab's title). */ onSubjectChange?: (subject: string) => void; @@ -606,6 +613,7 @@ export function EmailComposer({ requestCloseRef, onDiscardDraft, onSaveState, + onRestoreDiscarded, popOut, popOutToMainLabel, ownWindow, @@ -4699,7 +4707,9 @@ export function EmailComposer({ ); - const handleDiscardAndClose = () => { + // `keepDraft`: the bin's discard, which deletes the draft once its Undo is + // over, not here. + const handleDiscardAndClose = ({ keepDraft = false }: { keepDraft?: boolean } = {}) => { cancelSaveAndClose(); sendCancelledRef.current = true; explicitCloseRef.current = true; @@ -4717,7 +4727,7 @@ export function EmailComposer({ // From belongs to: switching From doesn't move the draft until it is // saved again, and the same id names another message on another server. const discardedDraftId = draftIdRef.current; - if (discardedDraftId && onDiscardDraft) { + if (discardedDraftId && onDiscardDraft && !keepDraft) { onDiscardDraft(discardedDraftId, { accountId: draftAccountIdRef.current, localAccountId: draftLocalAccountIdRef.current, @@ -4753,6 +4763,11 @@ export function EmailComposer({ // Discard deletes the draft, also one that has no unsaved changes, such as // a scheduled message reopened with "Cancel and edit". It used to share the // X button's path, which closed such a composer and kept the draft. + // It asks nothing (compose-p2-10): the composer goes at once with "Draft + // discarded ยท Undo", as Discard on the "Draft saved" toast does, and the + // draft is deleted once Undo is no longer offered. Undo brings the composer + // back with its text where the host can; elsewhere the last saved draft + // stays in Drafts. A window of its own has the main window say it. const handleDiscardClick = () => { if (saveCloseAttemptRef.current?.saving) return; cancelSaveAndClose(); @@ -4761,7 +4776,22 @@ export function EmailComposer({ return; } afterCloseRef.current = null; - openCloseDialog('discard'); + const draftId = draftIdRef.current; + const where = { accountId: draftAccountIdRef.current, localAccountId: draftLocalAccountIdRef.current }; + const snapshot = snapshotStateRef.current(); + const discard = onDiscardDraft; + const restore = onRestoreDiscarded; + const windowDiscarded = ownWindowRef.current?.onDraftDiscarded; + handleDiscardAndClose({ keepDraft: true }); + if (windowDiscarded) { + if (draftId) windowDiscarded(draftId, where); + return; + } + discardWithUndo( + () => { if (draftId) discard?.(draftId, where); }, + { discarded: tUx('draft_discarded'), undo: tUx('undo') }, + restore ? () => restore(snapshot) : undefined, + ); }; // X, Escape, another message, a host's tab: one rule, as drafts save @@ -6215,7 +6245,7 @@ export function EmailComposer({ - {closeDialogKind === 'close' && ( diff --git a/components/mail/__tests__/mail-app-composer-replace.test.tsx b/components/mail/__tests__/mail-app-composer-replace.test.tsx index 1d0283969..d3ade0db1 100644 --- a/components/mail/__tests__/mail-app-composer-replace.test.tsx +++ b/components/mail/__tests__/mail-app-composer-replace.test.tsx @@ -21,6 +21,7 @@ const stub = vi.hoisted(() => ({ close: 'none' as 'none' | 'saves' | 'fails', closes: [] as string[], mounts: [] as string[], + restore: null as null | ((data: Record) => void), })); vi.mock('@/components/layout/mv-top-bar', () => ({ MvTopBar: () => null })); @@ -32,8 +33,9 @@ vi.mock('@/components/layout/sidebar', () => ({ vi.mock('@/components/email/email-composer', async () => { const { useEffect, useRef } = await import('react'); return { - EmailComposer: ({ mode, onSaveState, onClose, requestCloseRef }: { + EmailComposer: ({ mode, onSaveState, onClose, requestCloseRef, onRestoreDiscarded }: { mode: string; + onRestoreDiscarded?: (data: Record) => void; onSaveState: (data: Record) => void; onClose: () => void; requestCloseRef?: { current: unknown }; @@ -41,6 +43,7 @@ vi.mock('@/components/email/email-composer', async () => { const closed = useRef(false); useEffect(() => { stub.mounts.push(mode); + stub.restore = onRestoreDiscarded ?? null; // The old host moved the composer between dock and pane for ever // (React error 185 in the browser); stop it here instead of hanging. if (stub.mounts.length > 40) throw new Error('composer remount loop'); @@ -118,6 +121,7 @@ beforeEach(() => { stub.close = 'none'; stub.closes = []; stub.mounts = []; + stub.restore = null; vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new Error('Unexpected network request'))); vi.stubGlobal('ResizeObserver', class { observe() {} unobserve() {} disconnect() {} }); usePluginStore.setState({ initializePlugins: vi.fn().mockResolvedValue(undefined) }); @@ -224,3 +228,18 @@ describe('another composer while one is open (compose-p2-2)', () => { expect(composerMode()).toBe('reply'); }); }); + +// compose-p2-10: Undo after the bin brings the composer back. +describe('Undo after the bin', () => { + it('opens the discarded text again, closing whatever is open first', async () => { + stub.close = 'saves'; + render(); + fireEvent.click(screen.getByText('stub compose')); + await screen.findByTestId('stub-composer'); + const restore = stub.restore!; + + act(() => restore({ to: 'bob@example.org', cc: '', bcc: '', subject: 'Back', body: 'kept', mode: 'forward', draftId: 'd1' })); + await waitFor(() => expect(composerMode()).toBe('forward')); + expect(stub.closes).toEqual(['compose']); + }); +}); diff --git a/components/mail/mail-app.tsx b/components/mail/mail-app.tsx index fb2db8863..e06b393e6 100644 --- a/components/mail/mail-app.tsx +++ b/components/mail/mail-app.tsx @@ -5362,7 +5362,18 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { requestCloseRef={composerRequestCloseRef} onDiscardDraft={(draftId, draft) => { handleDiscardDraft(draftId, draft); - setPendingDraft(null); + // The bin and the toasts' Discard delete once their Undo is over, + // when another composer may be open: its draft stays. + if (composerSessionRef.current === composerSessionId) setPendingDraft(null); + }} + onRestoreDiscarded={(data) => { + void closeOpenComposer().then(() => { + startFreshComposerSession(); + setPendingDraft(data); + setComposerMode(data.mode); + setShowComposer(true); + if (isMobile) setActiveView('viewer'); + }); }} popOut={popoutsOn && !isEmbedded ? composerPopOut : undefined} /> diff --git a/components/popout/__tests__/popout-host.test.tsx b/components/popout/__tests__/popout-host.test.tsx index afcae4fcf..5850bc78f 100644 --- a/components/popout/__tests__/popout-host.test.tsx +++ b/components/popout/__tests__/popout-host.test.tsx @@ -320,6 +320,18 @@ describe('PopoutHost', () => { expect(toasts[0].action?.label).toBe('email_composer.discard'); }); + // compose-p2-10: the window's bin. + it('says a pop-out discarded its draft, with Undo', async () => { + render(); + await settle(); + popout.post({ type: 'draft-kept', requestId: 'k2', notice: { draftId: 'd1', localAccountId: 'alice', discarded: true } }); + await settle(); + const toasts = useToastStore.getState().toasts; + expect(toasts).toHaveLength(1); + expect(toasts[0].title).toBe('compose_ux.draft_discarded'); + expect(toasts[0].action?.label).toBe('compose_ux.undo'); + }); + it('refreshes the list and counts when a pop-out changed mail, once for a burst', async () => { render(); await settle(); diff --git a/components/popout/popout-compose-app.tsx b/components/popout/popout-compose-app.tsx index 5f8f5433b..9e37034d6 100644 --- a/components/popout/popout-compose-app.tsx +++ b/components/popout/popout-compose-app.tsx @@ -487,6 +487,9 @@ function PopoutComposer({ payload, handoffId, onReopen }: { onDraftKept: (draftId: string, where: { accountId?: string; localAccountId?: string }) => { announceDraftKept({ draftId, ...where, localAccountId: where.localAccountId ?? useAuthStore.getState().activeAccountId ?? undefined }); }, + onDraftDiscarded: (draftId: string, where: { accountId?: string; localAccountId?: string }) => { + announceDraftKept({ draftId, ...where, localAccountId: where.localAccountId ?? useAuthStore.getState().activeAccountId ?? undefined, discarded: true }); + }, onDraftSaved: (draftId: string, where: { accountId?: string; localAccountId?: string } = {}) => { const active = useAuthStore.getState().activeAccountId; announceMailChanged(active); diff --git a/components/popout/popout-host.tsx b/components/popout/popout-host.tsx index 1308ca0bf..9969d4dda 100644 --- a/components/popout/popout-host.tsx +++ b/components/popout/popout-host.tsx @@ -8,7 +8,7 @@ import { useAccountStore } from "@/stores/account-store"; import { useAuthStore } from "@/stores/auth-store"; import { useEmailStore } from "@/stores/email-store"; import { toast } from "@/stores/toast-store"; -import { showDraftSavedToast } from "@/lib/compose/draft-close-toast"; +import { discardWithUndo, showDraftSavedToast } from "@/lib/compose/draft-close-toast"; import { newPopoutId, openPopoutChannel, type PopoutChannel, type PopoutMessage, type PopoutSentNotice, type RescuedDraft, type SharedAccountSession } from "@/lib/popout/channel"; import { takeOverMoved } from "@/lib/popout/moved-undo"; import { keepRescuedDraft } from "@/lib/popout/rescued-drafts"; @@ -180,10 +180,10 @@ export function PopoutHost() { break; case "draft-kept": { // Said here, as the pop-out closes with its Escape or X. - const { draftId, accountId, localAccountId } = message.notice; + const { draftId, accountId, localAccountId, discarded } = message.notice; claimOnce(`bulwark-popout-kept:${message.requestId}`, () => { const tr = tRef.current; - showDraftSavedToast(() => { + const discard = () => { const auth = useAuthStore.getState(); const login = localAccountId ? auth.getClientForAccount(localAccountId) : auth.client; if (!login) { @@ -191,12 +191,16 @@ export function PopoutHost() { return; } void login.deleteEmail(draftId, accountId).catch((error: unknown) => console.error("Failed to discard draft:", error)); - }, { + }; + const labels = { saved: tr("email_composer.draft_saved"), discard: tr("email_composer.discard"), discarded: tr("compose_ux.draft_discarded"), undo: tr("compose_ux.undo"), - }); + }; + // The window's bin: Undo leaves the draft in Drafts. + if (discarded) discardWithUndo(discard, labels); + else showDraftSavedToast(discard, labels); }); break; } diff --git a/lib/popout/channel.ts b/lib/popout/channel.ts index b249c5bb4..46eeb0de1 100644 --- a/lib/popout/channel.ts +++ b/lib/popout/channel.ts @@ -127,6 +127,8 @@ export interface DraftKeptNotice { accountId?: string; /** The login it was saved through. */ localAccountId?: string; + /** Thrown away with the bin: "Draft discarded" with Undo, deleted after. */ + discarded?: boolean; } export interface PopoutChannel {