From 9c31de2e80fbb7c52df5e2841294f332378d395d Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Thu, 24 Sep 2026 18:50:22 +0200 Subject: [PATCH] fix: keep the viewer on the opened message when new mail arrives The viewer's pager followed the store's live list. New mail or a refresh shifted the rows under the fixed scroll position, so the screen switched by itself to another message while Star, Delete and Archive kept acting on the one that had been opened. The pages are now taken once when the viewer opens, in `viewerPages`, so the page on screen and the message the toolbar acts on stay the same. Delete, archive, move and spam leave the viewer as before, so no page is left showing a message that is gone. --- src/lib/__tests__/viewer-pages.test.ts | 63 ++++++++++++++++++++++++++ src/lib/viewer-pages.ts | 49 ++++++++++++++++++++ src/screens/EmailThreadScreen.tsx | 44 ++++++------------ 3 files changed, 125 insertions(+), 31 deletions(-) create mode 100644 src/lib/__tests__/viewer-pages.test.ts create mode 100644 src/lib/viewer-pages.ts diff --git a/src/lib/__tests__/viewer-pages.test.ts b/src/lib/__tests__/viewer-pages.test.ts new file mode 100644 index 00000000..25b91cfe --- /dev/null +++ b/src/lib/__tests__/viewer-pages.test.ts @@ -0,0 +1,63 @@ +import { describe, it, expect } from 'vitest'; +import { viewerPages } from '../viewer-pages'; +import type { Email } from '../../api/types'; + +function row(id: string, threadId = id): Email { + return { id, threadId, subject: id } as Email; +} + +const LIST = [row('n1'), row('q2', 'tq'), row('q1', 'tq'), row('o3')]; + +describe('viewerPages', () => { + it('collapses the open folder to one page per thread, the opened message standing in', () => { + const pages = viewerPages({ + emailId: 'q1', threadId: 'tq', list: LIST, listIsMessageAccount: true, threading: true, + }); + expect(pages.map((e) => e.id)).toEqual(['n1', 'q1', 'o3']); + }); + + it('pages over every row when threading is off', () => { + const pages = viewerPages({ + emailId: 'q1', threadId: 'tq', list: LIST, listIsMessageAccount: true, threading: false, + }); + expect(pages.map((e) => e.id)).toEqual(['n1', 'q2', 'q1', 'o3']); + }); + + it('does not follow the list once taken, so new mail cannot shift the page (B5)', () => { + const list = [...LIST]; + const pages = viewerPages({ + emailId: 'q2', threadId: 'tq', list, listIsMessageAccount: true, threading: false, + }); + list.unshift(row('new1'), row('new2')); + expect(pages.map((e) => e.id)).toEqual(['n1', 'q2', 'q1', 'o3']); + expect(pages.findIndex((e) => e.id === 'q2')).toBe(1); + }); + + it('pages over the ids another list handed over, using the store row when there is one', () => { + const pages = viewerPages({ + emailId: 'u1', threadId: 't-u1', emailIds: ['u1', 'o3'], + list: LIST, listIsMessageAccount: true, threading: true, + }); + expect(pages).toEqual([{ id: 'u1', threadId: 't-u1' }, LIST[3]]); + }); + + it('is a single page for a message the list does not hold', () => { + const pages = viewerPages({ + emailId: 'm9', threadId: 't9', list: LIST, listIsMessageAccount: true, threading: true, + }); + expect(pages).toEqual([{ id: 'm9', threadId: 't9' }]); + }); + + it('never takes another account\'s rows, even under the same id (B3)', () => { + const opened = viewerPages({ + emailId: 'q1', threadId: 'tq', list: LIST, listIsMessageAccount: false, threading: true, + }); + expect(opened).toEqual([{ id: 'q1', threadId: 'tq' }]); + + const handed = viewerPages({ + emailId: 'o3', threadId: 't-o3', emailIds: ['o3'], + list: LIST, listIsMessageAccount: false, threading: true, + }); + expect(handed).toEqual([{ id: 'o3', threadId: 't-o3' }]); + }); +}); diff --git a/src/lib/viewer-pages.ts b/src/lib/viewer-pages.ts new file mode 100644 index 00000000..ec7a9272 --- /dev/null +++ b/src/lib/viewer-pages.ts @@ -0,0 +1,49 @@ +import type { Email } from '../api/types'; + +export interface ViewerPagesInput { + /** The message the viewer opens on, and its thread. */ + emailId: string; + threadId: string; + /** Ids handed over by a list other than the open folder (unified inbox, contact activity). */ + emailIds?: string[]; + /** The store's loaded list: the open folder's rows. */ + list: Email[]; + /** Whether that list is the message's account: ids are only unique per account. */ + listIsMessageAccount: boolean; + threading: boolean; +} + +/** + * The pages the viewer swipes through, worked out once when it opens. Following + * the live list let new mail shift the page under the user while Star, Delete + * and Archive still acted on the message they had opened (B5). + * + * A message opened from the open folder pages over that folder, collapsed to + * one page per thread when threading is on (the opened message standing in for + * its thread); one opened from another list pages over the ids that list handed + * over; anything else (notification, deep link, a group message not in the + * folder page) is a single page, so the pager never shows `pages[0]` while the + * toolbar acts on the tapped message. + */ +export function viewerPages(input: ViewerPagesInput): Email[] { + const { emailId, threadId, emailIds, threading } = input; + // Another account's rows say nothing about this message, whatever their ids. + const list = input.listIsMessageAccount ? input.list : []; + if (emailIds && emailIds.length > 0) { + const byId = new Map(list.map((e) => [e.id, e])); + return emailIds.map((id) => byId.get(id) ?? ({ id, threadId } as Email)); + } + const opened = list.find((e) => e.id === emailId); + if (!opened) return [{ id: emailId, threadId } as Email]; + if (!threading) return [...list]; + const openedKey = opened.threadId || opened.id; + const seen = new Set(); + const out: Email[] = []; + for (const e of list) { + const key = e.threadId || e.id; + if (seen.has(key)) continue; + seen.add(key); + out.push(key === openedKey ? opened : e); + } + return out; +} diff --git a/src/screens/EmailThreadScreen.tsx b/src/screens/EmailThreadScreen.tsx index 51f0941c..ccf641de 100644 --- a/src/screens/EmailThreadScreen.tsx +++ b/src/screens/EmailThreadScreen.tsx @@ -35,6 +35,7 @@ import { findTrashMailbox, mailboxAccountId, mailboxesOfAccount, mailboxOfEmail import { pickEmailBody, plainTextBody } from '../lib/email-body'; import { singleLine } from '../lib/single-line'; import { buildForwardAsAttachmentPayload } from '../lib/forward-as-attachment'; +import { viewerPages } from '../lib/viewer-pages'; import type { Email, EmailAddress, Identity } from '../api/types'; import type { RootStackParamList } from '../navigation/types'; @@ -62,7 +63,6 @@ export default function EmailThreadScreen({ route, navigation }: Props) { const archiveEmailAction = useEmailStore((s) => s.archiveEmail); const mailboxes = useEmailStore((s) => s.mailboxes); const currentMailboxId = useEmailStore((s) => s.currentMailboxId); - const storeEmails = useEmailStore((s) => s.emails); const disableThreading = useSettingsStore((s) => s.disableThreading); const identities = useSettingsStore((s) => s.identities); const fetchIdentities = useSettingsStore((s) => s.fetchIdentities); @@ -83,36 +83,18 @@ export default function EmailThreadScreen({ route, navigation }: Props) { [mailboxes, currentMailboxId], ); - // The list the pager pages over. A message opened from the active folder - // pages over that folder (collapsed to one page per thread when threading - // is on, the opened message standing in for its thread); one opened from - // another list (unified inbox, contact activity) pages over the ids that - // list handed us, and a message that is in neither (e.g. a group-inbox - // message not in the folder page) gets a one-element list — otherwise the - // pager would render `emails[0]` while the toolbar acted on the tapped one. - const emails = React.useMemo(() => { - const { emailIds, emailId, threadId } = route.params; - const list = listAccountId === ownerAccountId ? storeEmails : []; - if (emailIds && emailIds.length > 0) { - const byId = new Map(list.map((e) => [e.id, e])); - return emailIds.map((id) => byId.get(id) ?? ({ id, threadId } as Email)); - } - const opened = list.find((e) => e.id === emailId); - if (opened) { - if (disableThreading) return list; - const openedKey = opened.threadId || opened.id; - const seen = new Set(); - const out: Email[] = []; - for (const e of list) { - const key = e.threadId || e.id; - if (seen.has(key)) continue; - seen.add(key); - out.push(key === openedKey ? opened : e); - } - return out; - } - return [{ id: emailId, threadId } as Email]; - }, [storeEmails, route.params, listAccountId, ownerAccountId, disableThreading]); + // The pages the pager swipes through (see viewerPages), taken once when the + // viewer opens: new mail or a refresh must not shift the page under the + // user while the toolbar acts on `activeEmailId` (B5). Delete, archive, + // move and spam leave the screen, so no page outlives its message here. + const [emails] = React.useState(() => viewerPages({ + emailId: route.params.emailId, + threadId: route.params.threadId, + emailIds: route.params.emailIds, + list: useEmailStore.getState().emails, + listIsMessageAccount: listAccountId === ownerAccountId, + threading: !disableThreading, + })); const currentMailboxRole = React.useMemo( () => (currentMailboxId ? mailboxes.find((m) => m.id === currentMailboxId)?.role ?? null : null),