diff --git a/components/mail/__tests__/mail-app-conversation-pane.test.tsx b/components/mail/__tests__/mail-app-conversation-pane.test.tsx index e26eeca3e..bb762955c 100644 --- a/components/mail/__tests__/mail-app-conversation-pane.test.tsx +++ b/components/mail/__tests__/mail-app-conversation-pane.test.tsx @@ -21,13 +21,19 @@ vi.mock('next/dynamic', () => ({ default: (load: () => Promise; }; } })); -type ViewerProps = { mvConversation?: { id: string }[]; conversationPane?: boolean }; +type ViewerProps = { email?: { id: string } | null; mvConversation?: { id: string }[]; mvConversationPending?: boolean; conversationPane?: boolean }; +// What the reader was drawn with, render by render: message, conversation, waiting. +const readerDraws = vi.hoisted(() => [] as string[]); vi.mock('@/components/email/email-viewer', () => ({ - EmailViewer: ({ mvConversation, conversationPane }: ViewerProps) => ( -
- {mvConversation ? mvConversation.map((e) => e.id).join(',') : 'no conversation'} -
- ), + EmailViewer: ({ email, mvConversation, mvConversationPending, conversationPane }: ViewerProps) => { + const conversation = mvConversation ? mvConversation.map((e) => e.id).join(',') : 'no conversation'; + readerDraws.push(`${email?.id ?? '-'} ${conversation}${mvConversationPending ? ' waiting' : ''}`); + return ( +
+ {conversation} +
+ ); + }, })); vi.mock('@/components/email/email-composer', () => ({ EmailComposer: () => null })); vi.mock('@/components/layout/sidebar', () => ({ Sidebar: () => null })); @@ -124,6 +130,7 @@ beforeEach(() => { fetchEmailContent, } as never); opened = []; + readerDraws.length = 0; unsubscribe = useEmailStore.subscribe((state, prev) => { if (state.selectedEmail !== prev.selectedEmail) opened.push(state.selectedEmail?.id ?? null); }); @@ -155,6 +162,35 @@ describe('The reading pane in the default layout', () => { await waitFor(() => expect(reader).toHaveTextContent('m0,m1')); }); + it('opens a message with its conversation at once, in one draw', async () => { + useEmailStore.setState({ selectedEmail: inbox()[1], listRows: null } as never); + render(); + const reader = await screen.findByTestId('reader', {}, { timeout: 5000 }); + await waitFor(() => expect(reader).toHaveTextContent('no conversation')); + readerDraws.length = 0; + await press('k'); + await waitFor(() => expect(selectedId()).toBe('m1')); + await waitFor(() => expect(reader).toHaveTextContent('m0,m1')); + await act(async () => { await new Promise((resolve) => setTimeout(resolve, 50)); }); + // The open brought the conversation with the message; fetching it again + // after the open drew the whole mail view once more. + expect(readerDraws.filter((draw) => draw.startsWith('m1 '))).toEqual(['m1 m0,m1']); + }); + + it('shows a linked message with its conversation, not alone first', async () => { + useEmailStore.setState({ selectedEmail: null } as never); + fetchEmailContent.mockImplementation(async (_client: unknown, id: string, _owner?: string, options?: { select?: boolean }) => { + const email = { ...inbox().find((e) => e.id === id)!, bodyValues: {} }; + if (options?.select !== false) useEmailStore.setState({ selectedEmail: email } as never); + return email; + }); + render(); + const reader = await screen.findByTestId('reader', {}, { timeout: 5000 }); + await waitFor(() => expect(reader).toHaveTextContent('m0,m1'), { timeout: 5000 }); + await act(async () => { await new Promise((resolve) => setTimeout(resolve, 50)); }); + expect(readerDraws.filter((draw) => draw.startsWith('m1 ') && draw !== 'm1 m0,m1')).toEqual([]); + }); + it('shows the message alone with "Show emails as individual messages"', async () => { useSettingsStore.setState({ disableThreading: true } as never); render(); diff --git a/components/mail/__tests__/mail-app-source-routing.test.tsx b/components/mail/__tests__/mail-app-source-routing.test.tsx index add5d3d12..fdb670d02 100644 --- a/components/mail/__tests__/mail-app-source-routing.test.tsx +++ b/components/mail/__tests__/mail-app-source-routing.test.tsx @@ -272,7 +272,8 @@ describe('shared message deep link from the personal Inbox (#1491)', () => { }); const mounted = render(); try { - await waitFor(() => expect(fetchContent).toHaveBeenCalledWith(client, 'eaaaaab', 'ce')); + // Fetched without opening: it opens with its conversation, in the reading pane. + await waitFor(() => expect(fetchContent).toHaveBeenCalledWith(client, 'eaaaaab', 'ce', { select: false })); await waitFor(() => expect(useEmailStore.getState().selectedMailbox).toBe('ce:a')); expect(fetchEmails).toHaveBeenCalledWith(client, 'ce:a', { background: true }); expect(useEmailStore.getState().selectedEmail?.subject).toBe('GRP one'); diff --git a/components/mail/mail-app.tsx b/components/mail/mail-app.tsx index f1192cee7..a2b1548fc 100644 --- a/components/mail/mail-app.tsx +++ b/components/mail/mail-app.tsx @@ -396,6 +396,13 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { threadId: string; emailId: string; startedAt: number; promise: Promise; } | null>(null); const mvConversationOpenSeqRef = useRef(0); + // The open that put a message on screen together with its conversation (or + // with none, a message alone in its thread). The effect below then has + // nothing to fetch, and resetting and setting it again would draw the whole + // mail view twice more for each message opened. + const mvConversationSettledRef = useRef<{ + emailId: string; threadId: string; client: EmailBlobContext['client']; accountId?: string; + } | null>(null); // compose-p2-6: a reply to the open message saved as a draft. The reader // shows it under the message, and Reply opens it again. The conversation // fetch notes the thread's drafts; a scan reads them fresh after a composer @@ -2299,11 +2306,30 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { } setLoadingEmail(true); try { - const email = await fetchEmailContent(activeClient, link.id, link.ownerAccountId); + // With the conversation in the reader, the message waits for it, as on + // a click: shown alone first, it would move down once it came. + const withConversation = readerConversationView && !cursorOnly; + const email = withConversation + ? await fetchEmailContent(activeClient, link.id, link.ownerAccountId, { select: false }) + : await fetchEmailContent(activeClient, link.id, link.ownerAccountId); if (!email) { toast.error(t('deep_link.message_not_found')); return; } + if (withConversation) { + const source = resolveEmailBlobContext(email, activeClient); + const conversation = await Promise.race([ + loadMvConversation(email, activeClient), + new Promise((resolve) => setTimeout(() => resolve(undefined), MV_CONVERSATION_WAIT_MS)), + ]); + if (conversation !== undefined) { + setMvConversationResult(conversation ? { emails: conversation, source, threadId: email.threadId } : null); + mvConversationSettledRef.current = { + emailId: email.id, threadId: email.threadId, client: source.client, accountId: source.accountId, + }; + } + useEmailStore.setState({ selectedEmail: email }); + } await showInOwningFolder(activeClient, email); if (link.fullscreen && !isMobile) { // The owning-folder branch above may have skipped selecting (the @@ -4990,11 +5016,19 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { useEffect(() => { ++mvConversationOpenSeqRef.current; + const settled = mvConversationSettledRef.current; + mvConversationSettledRef.current = null; if (!mvConversationThreadId || !client || !mvConversationRef) { setMvConversationResult(null); setMvConversationLoading(null); return; } + if (settled && settled.emailId === mvConversationRef.id && settled.threadId === mvConversationThreadId + && settled.client === mvBlobSource.client && settled.accountId === mvBlobSource.accountId) { + // The open already has the conversation on screen with the message. + setMvConversationLoading(null); + return () => { ++mvConversationOpenSeqRef.current; }; + } let cancelled = false; setMvConversationResult(null); setMvConversationLoading(mvConversationThreadId); @@ -5442,9 +5476,14 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { new Promise((resolve) => setTimeout(() => resolve(undefined), MV_CONVERSATION_WAIT_MS)), ]); if (openSeq !== emailOpenSeqRef.current) return 'opened'; - if (conversation !== undefined) setMvConversationResult(conversation ? { - emails: conversation, source, threadId: fullEmail.threadId, - } : null); + if (conversation !== undefined) { + setMvConversationResult(conversation ? { + emails: conversation, source, threadId: fullEmail.threadId, + } : null); + mvConversationSettledRef.current = { + emailId: fullEmail.id, threadId: fullEmail.threadId, client: source.client, accountId: source.accountId, + }; + } } // One write: the message, and the end of its loading state. finishOpenEmail(fullEmail); diff --git a/stores/email-store.ts b/stores/email-store.ts index 429d1dcdc..69720d6a4 100644 --- a/stores/email-store.ts +++ b/stores/email-store.ts @@ -467,8 +467,11 @@ interface EmailStore { // times; later calls are no-ops while a prior one is in flight. prefetchInitialData: (client: IJMAPClient) => Promise; loadMoreEmails: (client: IJMAPClient) => Promise; - /** `ownerAccountId`: the JMAP account a link names for the message (#1351). */ - fetchEmailContent: (client: IJMAPClient, emailId: string, ownerAccountId?: string) => Promise; + /** + * `ownerAccountId`: the JMAP account a link names for the message (#1351). + * `select: false`: hand the message back without opening it. + */ + fetchEmailContent: (client: IJMAPClient, emailId: string, ownerAccountId?: string, options?: { select?: boolean }) => Promise; fetchQuota: (client: IJMAPClient) => Promise; /** * `options.outbox`: a send from the composer or quick reply, which offline @@ -4349,7 +4352,7 @@ export const useEmailStore = create((set, get) => ({ } }, - fetchEmailContent: async (client, emailId, ownerAccountId) => { + fetchEmailContent: async (client, emailId, ownerAccountId, options) => { try { // Route to the owning account. In aggregate views (All Mail, unified, // cross-account) the selected mailbox is virtual, so derive the client + @@ -4382,7 +4385,7 @@ export const useEmailStore = create((set, get) => ({ annotatedEmail.accountId = listEmail?.accountId; annotatedEmail.accountLabel = listEmail?.accountLabel; annotatedEmail.sourceFolder = listEmail?.sourceFolder; - set({ selectedEmail: annotatedEmail }); + if (options?.select !== false) set({ selectedEmail: annotatedEmail }); return annotatedEmail; } return email;