From 801aa51b24ec883ff2faeca84aeeb48f9ff5982d Mon Sep 17 00:00:00 2001
From: Linus Rath <139418639+rathlinus@users.noreply.github.com>
Date: Wed, 4 Nov 2026 12:20:42 +0100
Subject: [PATCH] fix: discard a draft with the bin at once and offer Undo
The bin no longer asks "Discard draft?". The discard tests now expect no
question: the first one checks the late delete and the Undo instead of
the dialog, and the upload test that cancelled the question is gone,
since there is no question to cancel.
---
.../composer-discard-uploads.test.tsx | 22 +++------
.../email/__tests__/composer-discard.test.tsx | 47 +++++++++++++++----
components/email/email-composer.tsx | 40 ++++++++++++++--
.../mail-app-composer-replace.test.tsx | 21 ++++++++-
components/mail/mail-app.tsx | 13 ++++-
.../popout/__tests__/popout-host.test.tsx | 12 +++++
components/popout/popout-compose-app.tsx | 3 ++
components/popout/popout-host.tsx | 14 ++++--
lib/popout/channel.ts | 2 +
9 files changed, 138 insertions(+), 36 deletions(-)
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({
-