From fdd7651e0551a5aa3cb33c2ec241af776260890a Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Wed, 4 Nov 2026 11:21:28 +0100 Subject: [PATCH] fix: show the whole conversation in a message window and answer the reader's keys there --- .../__tests__/popout-message-app.test.tsx | 7 + components/popout/popout-message-app.tsx | 1 + .../pro-email-window-conversation.test.tsx | 161 ++++++++++++++++++ components/pro/pro-email-tab-body.tsx | 75 ++++++-- components/pro/use-view-conversation.ts | 100 +++++++++++ 5 files changed, 327 insertions(+), 17 deletions(-) create mode 100644 components/pro/__tests__/pro-email-window-conversation.test.tsx create mode 100644 components/pro/use-view-conversation.ts diff --git a/components/popout/__tests__/popout-message-app.test.tsx b/components/popout/__tests__/popout-message-app.test.tsx index 82e6a7d69..9c1e8e48f 100644 --- a/components/popout/__tests__/popout-message-app.test.tsx +++ b/components/popout/__tests__/popout-message-app.test.tsx @@ -94,6 +94,13 @@ describe('a message opened from a bulwark:// link', () => { }); describe('PopoutMessageApp', () => { + // pass-2 mail-p2-5: the window showed one message of a conversation. + it('reads the message in its conversation, as the reading pane does', async () => { + render(); + await settle(); + expect((view as unknown as { conversation?: boolean }).conversation).toBe(true); + }); + it('opens the message of the address, through its login and account', async () => { render(); await settle(); diff --git a/components/popout/popout-message-app.tsx b/components/popout/popout-message-app.tsx index 0c68ce71a..74273e0c7 100644 --- a/components/popout/popout-message-app.tsx +++ b/components/popout/popout-message-app.tsx @@ -195,6 +195,7 @@ function PopoutMessage({ emailId, accountId, ownerAccountId, fromLink }: { email liveState={liveState} onMailChanged={handleMailChanged} noAutomaticActions={fromLink} + conversation className="min-h-0 w-full flex-1" /> diff --git a/components/pro/__tests__/pro-email-window-conversation.test.tsx b/components/pro/__tests__/pro-email-window-conversation.test.tsx new file mode 100644 index 000000000..0a0805061 --- /dev/null +++ b/components/pro/__tests__/pro-email-window-conversation.test.tsx @@ -0,0 +1,161 @@ +import { act, fireEvent, render, waitFor } from '@testing-library/react'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import React from 'react'; + +/** + * A message window (desktop app, installed app) showed only the message it was + * opened with, and r, e, # and ? did nothing there: its shortcuts waited for a + * Pro tab to be in front, and a window has no tabs (pass-2 mail-p2-5). + */ + +const viewerProps = vi.hoisted(() => ({ current: null as Record | null })); + +vi.mock('@/components/email/email-viewer', () => ({ + EmailViewer: (props: Record) => { + viewerProps.current = props; + return null; + }, +})); + +vi.mock('@/components/error', () => ({ + ErrorBoundary: ({ children }: { children: React.ReactNode }) => React.createElement(React.Fragment, null, children), + EmailViewerErrorFallback: () => null, +})); + +vi.mock('next-intl', () => ({ useTranslations: () => (key: string) => key })); + +vi.mock('@/lib/popout/window-role', async (importOriginal) => ({ + ...await importOriginal(), + isPopoutWindow: () => true, +})); +vi.mock('@/components/email/move-to-dialog', () => ({ MoveToDialog: () => null })); +vi.mock('@/stores/toast-store', () => ({ toast: { error: () => {}, success: () => {} } })); +const help = vi.hoisted(() => ({ open: false })); +vi.mock('@/components/keyboard-shortcuts-modal', () => ({ + KeyboardShortcutsModal: ({ isOpen }: { isOpen: boolean }) => { + help.open = isOpen; + return null; + }, +})); + +function selectorStore(state: T) { + const hook = (sel?: (s: T) => unknown) => (typeof sel === 'function' ? sel(state) : state); + hook.getState = () => state; + return hook; +} + +vi.mock('@/stores/auth-store', () => ({ + useAuthStore: selectorStore({ client: null, activeAccountId: 'dso', getClientForAccount: () => undefined }), +})); +vi.mock('@/stores/email-store', () => ({ + resolveEmailBlobContext: (_email: unknown, client: unknown) => ({ client }), + useEmailStore: selectorStore({ + fetchEmailContent: async () => null, + deleteEmail: async () => {}, + markAsRead: async () => {}, + toggleStar: async () => {}, + moveToMailbox: async () => {}, + setEmailKeywordsLocal: () => {}, + mailboxes: [], + emails: [], + selectedEmail: null, + }), +})); +vi.mock('@/stores/identity-store', () => ({ + useIdentityStore: selectorStore({ identities: [{ id: 'b', email: 'dso@example.org', name: 'Dso' }] }), +})); +const settings = vi.hoisted(() => ({ state: { singleKeyShortcuts: true, disableThreading: false } })); +vi.mock('@/stores/settings-store', () => ({ useSettingsStore: selectorStore(settings.state) })); +vi.mock('@/stores/pro-tab-store', () => ({ + useProTabStore: selectorStore({ openComposeTab: () => {}, closeTab: () => {}, updateTabTitle: () => {} }), +})); + +import { ProEmailView } from '../pro-email-tab-body'; + +const OLDER = { + id: 'm1', threadId: 't1', mailboxIds: { inbox: true }, keywords: { $seen: true }, + from: [{ email: 'aylin@example.org', name: 'Aylin' }], subject: 'Question about hiring plan', + receivedAt: '2026-10-20T08:00:00Z', preview: 'First', +}; +const NEWER = { + ...OLDER, id: 'm2', from: [{ email: 'robin@example.org', name: 'Robin' }], subject: 'Re: Question about hiring plan', + receivedAt: '2026-10-21T08:00:00Z', preview: 'Second', keywords: {}, +}; +const BODY = { + bodyValues: { '1': { value: 'Hi' } }, + textBody: [{ partId: '1', type: 'text/plain' }], + htmlBody: [{ partId: '1', type: 'text/plain' }], + attachments: [], +}; + +function makeClient() { + return { + getEmail: vi.fn(async (id: string) => (id === 'm1' ? { ...OLDER, ...BODY } : { ...NEWER, ...BODY })), + getThread: vi.fn(async () => ({ id: 't1', emailIds: ['m2', 'm1'] })), + // Not in date order: the window sorts. + getSomeEmails: vi.fn(async () => [NEWER, OLDER]), + markAsRead: vi.fn(async () => {}), + getUsername: () => 'dso@example.org', + getAccountId: () => 'a', + }; +} + +describe('a message window', () => { + beforeEach(() => { + viewerProps.current = null; + help.open = false; + settings.state.disableThreading = false; + }); + + async function open() { + const client = makeClient(); + const onOpenCompose = vi.fn(async (_data: Record) => true); + render(); + await waitFor(() => expect(viewerProps.current?.email).toBeTruthy()); + return { client, onOpenCompose }; + } + + it('shows the whole conversation, oldest first', async () => { + const { client } = await open(); + await waitFor(() => expect((viewerProps.current?.mvConversation as Array<{ id: string }> | undefined)?.map((e) => e.id)).toEqual(['m1', 'm2'])); + expect(viewerProps.current?.conversationPane).toBe(true); + expect(client.getThread).toHaveBeenCalledWith('t1', 'a'); + }); + + it('keeps one message with "Show emails as individual messages"', async () => { + settings.state.disableThreading = true; + const { client } = await open(); + await act(async () => { await new Promise((r) => setTimeout(r, 5)); }); + expect(viewerProps.current?.mvConversation).toBeUndefined(); + expect(client.getThread).not.toHaveBeenCalled(); + }); + + it('replies to the message of the conversation whose button was pressed', async () => { + const { onOpenCompose } = await open(); + await waitFor(() => expect(viewerProps.current?.mvConversation).toBeTruthy()); + await act(async () => { + await (viewerProps.current!.onReplyConversationEmail as (e: unknown) => Promise)(NEWER); + }); + await waitFor(() => expect(onOpenCompose).toHaveBeenCalled()); + expect(onOpenCompose.mock.calls[0][0]).toMatchObject({ mode: 'reply', sourceEmailId: 'm2' }); + }); + + it('marks an unfolded unread message read on the server', async () => { + const { client } = await open(); + await waitFor(() => expect(viewerProps.current?.mvConversation).toBeTruthy()); + await act(async () => { + (viewerProps.current!.onMarkConversationEmailRead as (e: unknown) => void)(NEWER); + await new Promise((r) => setTimeout(r, 5)); + }); + expect(client.markAsRead).toHaveBeenCalledWith('m2', true, 'a'); + }); + + it('answers r and ? with no Pro tab around it', async () => { + const { onOpenCompose } = await open(); + fireEvent.keyDown(document.body, { key: 'r', code: 'KeyR' }); + await waitFor(() => expect(onOpenCompose).toHaveBeenCalledTimes(1)); + expect(onOpenCompose.mock.calls[0][0]).toMatchObject({ mode: 'reply', sourceEmailId: 'm1' }); + fireEvent.keyDown(document.body, { key: '?', code: 'Slash', shiftKey: true }); + await waitFor(() => expect(help.open).toBe(true)); + }); +}); diff --git a/components/pro/pro-email-tab-body.tsx b/components/pro/pro-email-tab-body.tsx index 5ddae4ffc..72c36b7a1 100644 --- a/components/pro/pro-email-tab-body.tsx +++ b/components/pro/pro-email-tab-body.tsx @@ -41,10 +41,13 @@ import { isFilePreviewable } from '@/lib/file-preview'; import { useDesktopCommands } from '@/lib/desktop/commands'; import { isPopoutWindow } from '@/lib/popout/window-role'; import { offerMoveUndo } from '@/lib/popout/moved-undo'; +import { useViewConversation } from './use-view-conversation'; import dynamic from 'next/dynamic'; // Only needed when the device policy turns downloads off. const FilePreviewModal = dynamic(() => import('@/components/files/file-preview-modal').then((m) => m.FilePreviewModal), { ssr: false }); +// Only a message window opens the shortcuts help from here. +const KeyboardShortcutsModal = dynamic(() => import('@/components/keyboard-shortcuts-modal').then((m) => m.KeyboardShortcutsModal), { ssr: false }); interface ProEmailTabBodyProps { tabId: string; @@ -86,6 +89,11 @@ interface ProEmailViewProps { onMailChanged?: () => void; /** See EmailViewer. */ noAutomaticActions?: boolean; + /** + * Show the message inside its conversation, as the mail page's reading + * pane does (a message window). "Show emails as individual messages" keeps one. + */ + conversation?: boolean; /** * The message left this view: deleted, archived, moved, or reopened in the * composer. The host decides what that means (close the tab / clear the @@ -152,7 +160,7 @@ function buildReplyContext(email: Email, source: EmailWriteSource): ProReplyCont * Hosted by `ProEmailTabBody` (a dedicated email tab) and by the folder tab's * reading pane. */ -export function ProEmailView({ emailId, client: clientOverride, clientAccountId, accountId, onLoaded, onOpenCompose, liveState, onMailChanged, noAutomaticActions, onClose, className }: ProEmailViewProps) { +export function ProEmailView({ emailId, client: clientOverride, clientAccountId, accountId, onLoaded, onOpenCompose, liveState, onMailChanged, noAutomaticActions, conversation, onClose, className }: ProEmailViewProps) { const t = useTranslations(); const tNotifications = useTranslations('notifications'); const tRemove = useTranslations('email_list.confirm_remove'); @@ -295,17 +303,21 @@ export function ProEmailView({ emailId, client: clientOverride, clientAccountId, return mb?.role; }, [email, mailboxes]); - const handleReply = useCallback((draftText?: string) => { - if (!email) return; + const replyToMessage = useCallback((target: Email, draftText?: string) => { openComposeTab({ mode: 'reply', - replyTo: buildReplyContext(email, writeSource), - sourceEmailId: email.id, + replyTo: buildReplyContext(target, writeSource), + sourceEmailId: target.id, sourceEmailSource: writeSource, initialDraftText: draftText, - title: buildReplySubject(email.subject || t('email_composer.new_message'), t('email_composer.prefix.reply')), - }, { source: email, body: draftText ?? '' }); - }, [email, openComposeTab, t, writeSource]); + title: buildReplySubject(target.subject || t('email_composer.new_message'), t('email_composer.prefix.reply')), + }, { source: target, body: draftText ?? '' }); + }, [openComposeTab, t, writeSource]); + + const handleReply = useCallback((draftText?: string) => { + if (!email) return; + replyToMessage(email, draftText); + }, [email, replyToMessage]); // A notification's Reply, once this tab has the message (mail-app.tsx). const pendingTabReply = useNotificationTabReply((s) => s.pending); @@ -315,16 +327,25 @@ export function ProEmailView({ emailId, client: clientOverride, clientAccountId, if (text !== null) handleReply(text); }, [email, pendingTabReply, handleReply]); - const handleReplyAll = useCallback(() => { - if (!email) return; + const replyAllToMessage = useCallback((target: Email) => { openComposeTab({ mode: 'replyAll', - replyTo: buildReplyContext(email, writeSource), - sourceEmailId: email.id, + replyTo: buildReplyContext(target, writeSource), + sourceEmailId: target.id, sourceEmailSource: writeSource, - title: buildReplySubject(email.subject || t('email_composer.new_message'), t('email_composer.prefix.reply')), - }, { source: email }); - }, [email, openComposeTab, t, writeSource]); + title: buildReplySubject(target.subject || t('email_composer.new_message'), t('email_composer.prefix.reply')), + }, { source: target }); + }, [openComposeTab, t, writeSource]); + + const handleReplyAll = useCallback(() => { + if (!email) return; + replyAllToMessage(email); + }, [email, replyAllToMessage]); + + // A message window reads the message in its conversation (mail-p2-5). + const disableThreading = useSettingsStore((s) => s.disableThreading); + const conversationView = !!conversation && !disableThreading; + const viewConversation = useViewConversation({ enabled: conversationView, email, source: blobSource, onChanged: onMailChanged }); const handleForward = useCallback(() => { if (!email) return; @@ -521,6 +542,7 @@ export function ProEmailView({ emailId, client: clientOverride, clientAccountId, // This view's message answers the same keys as a message in the mail tab, // while its tab is the one in front. c comes from the mail page, as on any tab. const isFocusedTab = useIsFocusedProTab(); + const [showShortcuts, setShowShortcuts] = useState(false); const singleKeyShortcuts = useSettingsStore((s) => s.singleKeyShortcuts); const shortcutEmails = useMemo(() => (email ? [email] : []), [email]); const shortcutHandlers = useMemo(() => ({ @@ -532,9 +554,13 @@ export function ProEmailView({ emailId, client: clientOverride, clientAccountId, onDelete: () => void handleDelete(), onMarkAsUnread: email ? () => void handleMarkAsRead(email.id, false) : undefined, onMarkAsRead: email ? () => void handleMarkAsRead(email.id, true) : undefined, - }), [email, handleReply, handleReplyAll, handleForward, handleToggleStar, archiveActionEnabled, handleArchive, handleDelete, handleMarkAsRead]); + // A message window has no mail page behind it to answer ?. + onShowHelp: inOwnWindow ? () => setShowShortcuts(true) : undefined, + }), [email, handleReply, handleReplyAll, handleForward, handleToggleStar, archiveActionEnabled, handleArchive, handleDelete, handleMarkAsRead, inOwnWindow]); + // A Pro tab answers while it is the one in front; a message window is + // always in front of itself (it has no tabs). useKeyboardShortcuts({ - enabled: isFocusedTab && !!email, + enabled: (isFocusedTab || inOwnWindow) && !!email && !showShortcuts, singleKeyShortcuts, emails: shortcutEmails, selectedEmailId: email?.id, @@ -633,6 +659,20 @@ export function ProEmailView({ emailId, client: clientOverride, clientAccountId, void viewConversation.loadBody(e) : undefined} + onMarkConversationEmailRead={conversationView ? (e) => void viewConversation.markRead(e) : undefined} + onReplyConversationEmail={conversationView ? async (e) => { + // The reply quotes the message, so it needs the body. + const full = e.bodyValues ? e : await viewConversation.loadBody(e); + replyToMessage(full ?? e); + } : undefined} + onReplyAllConversationEmail={conversationView ? async (e) => { + const full = e.bodyValues ? e : await viewConversation.loadBody(e); + replyAllToMessage(full ?? e); + } : undefined} blobSource={blobSource} isLoading={isLoading} onReply={handleReply} @@ -659,6 +699,7 @@ export function ProEmailView({ emailId, client: clientOverride, clientAccountId, /> + {inOwnWindow && setShowShortcuts(false)} />} {inOwnWindow && ( Promise; + /** An unfolded message was read. */ + markRead: (target: Email) => Promise; +} + +/** + * The conversation around a message read outside the mail page (a message + * window), fetched the way the mail page's reading pane fetches it: the + * thread's ids, then their list properties, drafts left out except the open + * one, oldest first. A conversation of one message is none. + */ +export function useViewConversation({ enabled, email, source, onChanged }: { + enabled: boolean; + email: Email | null; + source: EmailBlobContext; + onChanged?: () => void; +}): ViewConversation { + const threadId = enabled ? email?.threadId ?? null : null; + const openId = email?.id ?? null; + const [result, setResult] = useState<{ threadId: string; emails: Email[] } | null>(null); + const [pending, setPending] = useState(false); + const seqRef = useRef(0); + const onChangedRef = useRef(onChanged); + onChangedRef.current = onChanged; + + useEffect(() => { + const seq = ++seqRef.current; + const client = source.client; + if (!threadId || !client) { + setResult(null); + setPending(false); + return; + } + setPending(true); + void (async () => { + const thread = await client.getThread(threadId, source.accountId); + const ids = thread?.emailIds ?? []; + if (ids.length < 2) return null; + const emails = await client.getSomeEmails(ids, source.accountId); + return emails + .filter((e) => e.id === openId || !e.keywords?.$draft) + .sort((a, b) => a.receivedAt.localeCompare(b.receivedAt)); + })().catch(() => null).then((emails) => { + if (seq !== seqRef.current) return; + setResult(emails && emails.length > 1 ? { threadId, emails } : null); + setPending(false); + }); + }, [threadId, openId, source.client, source.accountId]); + + const patch = useCallback((id: string, change: (e: Email) => Email) => { + setResult((prev) => (prev ? { ...prev, emails: prev.emails.map((e) => (e.id === id ? change(e) : e)) } : prev)); + }, []); + + const loadBody = useCallback(async (target: Email) => { + const client = source.client; + if (!client) return null; + const seq = seqRef.current; + const full = await client.getEmail(target.id, source.accountId).catch(() => null); + if (!full || seq !== seqRef.current) return null; + patch(target.id, (e) => ({ ...full, keywords: e.keywords })); + return full; + }, [source.client, source.accountId, patch]); + + const markRead = useCallback(async (target: Email) => { + const client = source.client; + if (!client || target.keywords?.$seen) return; + try { + if (emailHooks.onBeforeEmailReadStateChange.size > 0 + && !(await emailHooks.onBeforeEmailReadStateChange.intercept({ emailIds: [target.id], read: true }))) { + return; + } + await client.markAsRead(target.id, true, source.accountId); + void emailHooks.onEmailReadStateChange.emit({ emailIds: [target.id], read: true }); + patch(target.id, (e) => ({ ...e, keywords: { ...e.keywords, $seen: true } })); + onChangedRef.current?.(); + } catch (error) { + console.error("Failed to mark conversation message read:", error); + } + }, [source.client, source.accountId, patch]); + + return { + emails: result && result.threadId === threadId ? result.emails : undefined, + pending: pending && !!threadId, + loadBody, + markRead, + }; +}