mirror of
https://github.com/bulwarkmail/webmail.git
synced 2026-11-05 00:14:18 +00:00
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.
This commit is contained in:
@@ -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'));
|
||||
|
||||
@@ -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(<EmailComposer initialData={REOPENED_DRAFT} onDiscardDraft={onDiscardDraft} onClose={onClose} />);
|
||||
|
||||
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(<EmailComposer initialData={REOPENED_DRAFT} onDiscardDraft={onDiscardDraft} onRestoreDiscarded={onRestoreDiscarded} onClose={() => {}} />);
|
||||
|
||||
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(<EmailComposer initialData={REOPENED_DRAFT} onDiscardDraft={onDiscardDraft} ownWindow={{ onDraftDiscarded }} onClose={() => {}} />);
|
||||
|
||||
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();
|
||||
|
||||
@@ -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({
|
||||
</button>
|
||||
);
|
||||
|
||||
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({
|
||||
<Button variant="outline" onClick={() => dismissCloseDialog()}>
|
||||
{t('cancel')}
|
||||
</Button>
|
||||
<Button variant="destructive" onClick={handleDiscardAndClose}>
|
||||
<Button variant="destructive" onClick={() => handleDiscardAndClose()}>
|
||||
{t('discard')}
|
||||
</Button>
|
||||
{closeDialogKind === 'close' && (
|
||||
|
||||
@@ -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<string, unknown>) => 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<string, unknown>) => void;
|
||||
onSaveState: (data: Record<string, unknown>) => 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(<MailApp />);
|
||||
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']);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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}
|
||||
/>
|
||||
|
||||
@@ -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(<PopoutHost />);
|
||||
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(<PopoutHost />);
|
||||
await settle();
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user