fix: keep an opened mail in place in unread-first lists until another is opened

This commit is contained in:
Linus Rath
2026-09-29 20:28:39 +02:00
parent 3bccca715c
commit 324fb48312
8 changed files with 210 additions and 33 deletions
+5 -2
View File
@@ -143,12 +143,15 @@ export function EmailList({
// Search results and cross-account views are always chronological.
const fetchedListOrder = useEmailStore((state) => state.listOrder);
const crossView = useEmailStore((state) => state.crossView);
// The row opened last stays where it was clicked while that order would
// move it (e.g. read in "unread first").
const listHold = useEmailStore((state) => state.listHold);
const threadGroups = useMemo(() => {
const listOrder = searchQuery || crossView || !isFilterEmpty(searchFilters) ? [] : fetchedListOrder;
const groups = groupEmailsByThread(emails, disableThreading || isScheduledView, threadEmailCounts);
return sortThreadGroups(groups, listOrder);
}, [emails, disableThreading, isScheduledView, threadEmailCounts, fetchedListOrder, searchQuery, crossView, searchFilters]);
return sortThreadGroups(groups, listOrder, listHold?.keywords);
}, [emails, disableThreading, isScheduledView, threadEmailCounts, fetchedListOrder, searchQuery, crossView, searchFilters, listHold]);
const { contextMenu, openContextMenu, closeContextMenu, menuRef } = useContextMenu<Email>();
/**
+2
View File
@@ -3525,6 +3525,8 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
const handleOpenConversation = async (thread: ThreadGroup) => {
if (!client) return;
// The view marks the thread read; keep its row where it was tapped.
useEmailStore.getState().holdListRow(thread.threadKey);
setConversationThread(thread);
setIsLoadingConversation(true);
setActiveView("viewer");
+38
View File
@@ -9,6 +9,7 @@ import {
levelKeyword,
orderForMailbox,
orderKeywords,
placeHeldRows,
presetLevels,
sanitizeSortLevels,
MAX_SORT_LEVELS,
@@ -207,6 +208,43 @@ describe('compareEmails', () => {
]));
expect(sorted.map(e => e.id)).toEqual(['unread-new', 'starred-old', 'read-newest']);
});
it('sorts a held mail by its held keywords', () => {
const justRead = makeEmail({ id: 'just-read', receivedAt: '2026-01-04T00:00:00Z', keywords: { $seen: true } });
const held = new Map([['just-read', {}]]);
const sorted = [unreadOld, readNewest, justRead, unreadNew].sort(compareEmails(UNREAD_FIRST, { held }));
expect(sorted.map(e => e.id)).toEqual(['just-read', 'unread-new', 'unread-old', 'read-newest']);
});
});
describe('placeHeldRows', () => {
const mail = (id: string, day: number, read: boolean, extra: Record<string, boolean> = {}) =>
makeEmail({ id, receivedAt: `2026-01-0${day}T00:00:00Z`, keywords: read ? { $seen: true, ...extra } : { ...extra } });
it('moves a held mail back to where it sorted when it was opened', () => {
// Server order after reading u2: the unread, then the read by date.
const list = [mail('u3', 3, false), mail('u1', 1, false), mail('r4', 4, true), mail('u2', 2, true)];
const placed = placeHeldRows(list, UNREAD_FIRST, new Map([['u2', {}]]));
expect(placed.map(e => e.id)).toEqual(['u3', 'u2', 'u1', 'r4']);
});
it('leaves the other rows in their order', () => {
const list = [mail('u3', 3, false), mail('u1', 1, false), mail('r4', 4, true), mail('r2', 2, true)];
const placed = placeHeldRows(list, UNREAD_FIRST, new Map([['r4', { $seen: true }]]));
expect(placed.map(e => e.id)).toEqual(['u3', 'u1', 'r4', 'r2']);
});
it('keeps pinned mail above a held one', () => {
const list = [mail('p', 1, true, { $pinned: true }), mail('u1', 1, false), mail('r9', 9, true)];
const placed = placeHeldRows(list, UNREAD_FIRST, new Map([['r9', {}]]));
expect(placed.map(e => e.id)).toEqual(['p', 'r9', 'u1']);
});
it('does nothing for an order without keyword levels', () => {
const list = [mail('a', 3, true), mail('b', 2, false)];
expect(placeHeldRows(list, [], new Map([['a', {}]]))).toBe(list);
expect(placeHeldRows(list, UNREAD_FIRST, undefined)).toBe(list);
});
});
describe('orderForMailbox', () => {
+14
View File
@@ -177,6 +177,20 @@ describe('sortThreadGroups', () => {
expect(sorted.map(g => g.threadId)).toEqual(['mixed', 'read-newer']);
});
it('keeps a thread read since it was opened where it was', () => {
const opened = makeGroup('opened', '2024-02-01T00:00:00Z');
opened.emails = [makeEmail({ id: 'o', receivedAt: '2024-02-01T00:00:00Z', keywords: { $seen: true } })];
const unread = makeGroup('unread', '2024-01-01T00:00:00Z');
unread.emails = [makeEmail({ id: 'u', receivedAt: '2024-01-01T00:00:00Z', keywords: {} })];
const readNewer = makeGroup('read-newer', '2024-06-01T00:00:00Z');
const order = [{ criterion: 'unread' as const, direction: 'desc' as const }];
expect(sortThreadGroups([readNewer, unread, opened], order).map(g => g.threadId))
.toEqual(['unread', 'read-newer', 'opened']);
expect(sortThreadGroups([readNewer, unread, opened], order, new Map([['o', {}]])).map(g => g.threadId))
.toEqual(['opened', 'unread', 'read-newer']);
});
it('keeps pinned threads on top of a configured order', () => {
const pinnedRead = makeGroup('pinned-read', '2024-01-01T00:00:00Z', true);
const unread = makeGroup('unread', '2024-06-01T00:00:00Z');
+37 -7
View File
@@ -220,11 +220,11 @@ function time(value: string | undefined): number {
return Number.isNaN(t) ? 0 : t;
}
function levelValue(level: SortLevel, email: Email): number | string {
function levelValue(level: SortLevel, email: Email, keywords: Email['keywords'] | undefined): number | string {
switch (level.criterion) {
case 'unread': return email.keywords?.$seen ? 0 : 1;
case 'starred': return email.keywords?.$flagged ? 1 : 0;
case 'tag': return level.tagId && email.keywords?.[`$label:${level.tagId}`] ? 1 : 0;
case 'unread': return keywords?.$seen ? 0 : 1;
case 'starred': return keywords?.$flagged ? 1 : 0;
case 'tag': return level.tagId && keywords?.[`$label:${level.tagId}`] ? 1 : 0;
case 'receivedAt': return time(email.receivedAt);
case 'sentAt': return time(email.sentAt || email.receivedAt);
case 'size': return email.size ?? 0;
@@ -240,25 +240,37 @@ function compareValues(a: number | string, b: number | string): number {
return a - b;
}
/**
* Keywords by email id that a list sorts some rows by instead of their current
* ones: the conversation opened last keeps the place it had when it was opened,
* so reading it in an "unread first" list does not move it (see
* EmailStore.listHold).
*/
export type HeldKeywords = ReadonlyMap<string, Email['keywords']>;
/**
* Client-side mirror of `buildEmailSort` for the same level list. Used as the
* within-page safety net (some servers ignore the sort on unfiltered queries),
* for merging per-account pages in the unified view, for the demo client, and
* to keep thread grouping from undoing the server order.
* to keep thread grouping from undoing the server order. `held` rows sort by
* their held keywords.
*/
export function compareEmails(
levels: SortLevel[],
opts: { pinnedFirst?: boolean } = {},
opts: { pinnedFirst?: boolean; held?: HeldKeywords } = {},
): (a: Email, b: Email) => number {
const hasDateLevel = levels.some(l => l.criterion === 'receivedAt');
const held = opts.held;
return (a, b) => {
if (opts.pinnedFirst) {
const pa = a.keywords?.['$pinned'] ? 1 : 0;
const pb = b.keywords?.['$pinned'] ? 1 : 0;
if (pa !== pb) return pb - pa;
}
const ka = held?.get(a.id) ?? a.keywords;
const kb = held?.get(b.id) ?? b.keywords;
for (const level of levels) {
const diff = compareValues(levelValue(level, a), levelValue(level, b));
const diff = compareValues(levelValue(level, a, ka), levelValue(level, b, kb));
if (diff !== 0) return level.direction === 'asc' ? diff : -diff;
}
if (!hasDateLevel) {
@@ -269,6 +281,24 @@ export function compareEmails(
};
}
/**
* Moves the held rows of a list in `levels` order to where they sort by their
* held keywords; every other row keeps its place. The list shows them there,
* and next/previous and the pick after a delete walk this array.
*/
export function placeHeldRows(emails: Email[], levels: SortLevel[], held: HeldKeywords | undefined): Email[] {
if (!held || held.size === 0 || orderKeywords(levels).length === 0) return emails;
if (!emails.some((e) => held.has(e.id))) return emails;
const compare = compareEmails(levels, { pinnedFirst: true, held });
const out = emails.filter((e) => !held.has(e.id));
for (const row of emails) {
if (!held.has(row.id)) continue;
const at = out.findIndex((e) => compare(row, e) < 0);
out.splice(at === -1 ? out.length : at, 0, row);
}
return out;
}
/**
* The order to use for a given folder: the configured levels when they apply
* to every folder, or to the Inbox when scoped to it; chronological otherwise.
+5 -4
View File
@@ -1,5 +1,5 @@
import type { Email, ThreadGroup } from "./jmap/types";
import { compareEmails, type SortLevel } from "./message-list-order";
import { compareEmails, type HeldKeywords, type SortLevel } from "./message-list-order";
/**
* Client-side identity of a thread.
@@ -108,10 +108,11 @@ export function groupEmailsByThread(
* the server-side pinned-first sort of the email list. Below that, each thread
* takes the position of whichever of its emails sorts first under `order`
* (RFC 8621 §4.4.3 thread collapsing semantics) - with the default order that
* is the latest email's receivedAt date, newest first.
* is the latest email's receivedAt date, newest first. `held` emails sort by
* the keywords they had when their conversation was opened.
*/
export function sortThreadGroups(groups: ThreadGroup[], order: SortLevel[] = []): ThreadGroup[] {
const compare = compareEmails(order);
export function sortThreadGroups(groups: ThreadGroup[], order: SortLevel[] = [], held?: HeldKeywords): ThreadGroup[] {
const compare = compareEmails(order, { held });
const representative = new Map<ThreadGroup, Email>();
for (const group of groups) {
representative.set(
@@ -55,10 +55,11 @@ function fakeServer(rows: Email[]) {
return { client, setSeen };
}
/** The mail at the top of the list as the user sees it. */
/** The first unread mail in the list as the user sees it. */
function topMail(): Email {
const { emails, listOrder } = useEmailStore.getState();
return [...emails].sort(compareEmails(listOrder))[0];
const { emails, listOrder, listHold } = useEmailStore.getState();
const shown = [...emails].sort(compareEmails(listOrder, { held: listHold?.keywords }));
return shown.find(e => !e.keywords.$seen) ?? shown[0];
}
async function readTopMail(client: IJMAPClient): Promise<Email> {
@@ -96,13 +97,13 @@ describe('keyword-ordered folder list refill (#718)', () => {
for (let i = 0; i < 5; i++) opened.push((await readTopMail(client)).id);
expect(opened).toEqual(['u1', 'u2', 'u3', 'u4', 'u5']);
// All read now: the head of the server order is newest first, plus the
// open mail, which sorts further down.
expect(ids()).toEqual(['u1', 'r1', 'u2', 'u5']);
// All read now: the head of the server order is newest first, below the
// open mail, which stays where it was clicked.
expect(ids()).toEqual(['u5', 'u1', 'r1', 'u2']);
expect(useEmailStore.getState().retainedInViewIds).toEqual(new Set(['u5']));
});
it('keeps the open mail listed after it moved past the loaded rows, and drops it once another is opened', async () => {
it('keeps the open mail where it was clicked, and drops it once another is opened', async () => {
const { client } = fakeServer([
mail('r1', 22, true), mail('r2', 21, true),
mail('u1', 20, false), mail('u2', 18, false), mail('u3', 17, false), mail('u4', 15, false),
@@ -110,10 +111,47 @@ describe('keyword-ordered folder list refill (#718)', () => {
await useEmailStore.getState().fetchEmails(client);
await readTopMail(client);
expect(ids()).toEqual(['u2', 'u3', 'u4', 'u1']);
expect(ids()).toEqual(['u1', 'u2', 'u3', 'u4']);
await readTopMail(client);
expect(ids()).toEqual(['u3', 'u4', 'r1', 'u2']);
expect(ids()).toEqual(['u2', 'u3', 'u4', 'r1']);
});
it('keeps the read mail in place after it is closed, until another is opened', async () => {
const { client } = fakeServer([
mail('u1', 20, false), mail('u2', 18, false), mail('u3', 17, false), mail('u4', 15, false), mail('r1', 22, true),
]);
await useEmailStore.getState().fetchEmails(client);
await readTopMail(client);
useEmailStore.getState().selectEmail(null);
await useEmailStore.getState().refreshCurrentMailbox(client);
expect(ids()).toEqual(['u1', 'u2', 'u3', 'u4']);
useEmailStore.getState().selectEmail(useEmailStore.getState().emails.find(e => e.id === 'u3')!);
await useEmailStore.getState().refreshCurrentMailbox(client);
expect(ids()).toEqual(['u2', 'u3', 'u4']);
});
it('holds a thread when it is expanded, before it is marked read', async () => {
const { client } = fakeServer([mail('u1', 20, false), mail('u2', 18, false), mail('u3', 17, false), mail('u4', 15, false)]);
await useEmailStore.getState().fetchEmails(client);
useEmailStore.getState().toggleThreadExpansion('t-u1');
expect(useEmailStore.getState().listHold?.keywords).toEqual(new Map([['u1', {}]]));
});
it('forgets the held mail when the folder is loaded again', async () => {
const { client } = fakeServer([mail('u1', 20, false), mail('u2', 18, false), mail('u3', 17, false), mail('u4', 15, false)]);
await useEmailStore.getState().fetchEmails(client);
await readTopMail(client);
expect(useEmailStore.getState().listHold?.rowKey).toBe('t-u1');
await useEmailStore.getState().fetchEmails(client);
expect(useEmailStore.getState().listHold).toBeNull();
expect(ids()).toEqual(['u2', 'u3', 'u4']);
});
it('does not skip a mail when loading more after the open mail moved', async () => {
@@ -123,13 +161,13 @@ describe('keyword-ordered folder list refill (#718)', () => {
]);
await useEmailStore.getState().fetchEmails(client);
await readTopMail(client);
expect(ids()).toEqual(['u2', 'u3', 'u4', 'u1']);
expect(ids()).toEqual(['u1', 'u2', 'u3', 'u4']);
await useEmailStore.getState().loadMoreEmails(client);
// Server order is now u2 u3 u4 u5 u1 r1 r2: the page starts at u5.
expect(client.getEmails.mock.lastCall?.[3]).toBe(3);
expect(ids()).toEqual(['u2', 'u3', 'u4', 'u1', 'u5', 'r1']);
expect(ids()).toEqual(['u1', 'u2', 'u3', 'u4', 'u5', 'r1']);
// u1 came back in order with that page.
expect(useEmailStore.getState().retainedInViewIds.size).toBe(0);
});
+60 -9
View File
@@ -4,7 +4,7 @@ import type { UnifiedMailboxRole, CrossView } from "@/lib/jmap/types";
import type { IJMAPClient } from "@/lib/jmap/client-interface";
import { useSettingsStore, getMessageListOrderFor } from "@/stores/settings-store";
import { useCalendarStore } from "@/stores/calendar-store";
import { orderKeywords, type SortLevel } from "@/lib/message-list-order";
import { orderKeywords, placeHeldRows, type HeldKeywords, type SortLevel } from "@/lib/message-list-order";
import { SearchFilters, DEFAULT_SEARCH_FILTERS, buildJMAPFilter, isFilterEmpty } from "@/lib/jmap/search-utils";
import { emailHooks } from "@/lib/plugin-hooks";
import { resolveThreadRoute } from "@/lib/thread-routing";
@@ -75,15 +75,20 @@ interface EmailStore {
// Emails the user just read (unread view) or unstarred (starred view) that
// should stay visible in that self-filtering cross view until it is re-opened,
// instead of vanishing on the next push refresh. In a folder sorted on a
// keyword (#718): the open conversation's rows, kept although a read/star/tag
// change moved them past the loaded part of the server order. Cleared on
// navigation.
// keyword (#718): the open conversation's rows and the held ones (listHold),
// kept although a read/star/tag change moved them past the loaded part of
// the server order. Cleared on navigation.
retainedInViewIds: Set<string>;
hasMoreEmails: boolean; // Track if more emails are available to load
totalEmails: number; // Total number of emails in the current mailbox/query
// The configured message-list order the current folder view was fetched
// with (#718); empty for chronological. Thread grouping mirrors it.
listOrder: SortLevel[];
// The list row opened last and the keywords its messages had then. Its
// messages sort by those, so reading one in an "unread first" list leaves
// the row where it was clicked; opening another row releases it. Cleared
// on navigation.
listHold: ListHold | null;
isPushConnected: boolean; // Track if push notifications are connected
lastPushUpdate: number | null; // Timestamp of last push update
// Delta sync (RFC 8620 §5.2). The Email collection state the plain
@@ -194,6 +199,8 @@ interface EmailStore {
*/
fetchAccountMailboxes: (client: IJMAPClient, accountId: string) => Promise<void>;
selectEmail: (email: Email | null) => void;
/** Holds a list row in place (see listHold); `rowKey` as from listRowKey. */
holdListRow: (rowKey: string) => void;
selectMailbox: (mailboxId: string) => void;
setLoading: (loading: boolean) => void;
setLoadingEmail: (loading: boolean) => void;
@@ -491,6 +498,20 @@ function coalesceRefresh(client: IJMAPClient, key: string, run: () => Promise<vo
return entry.promise;
}
/** See EmailStore.listHold. */
export interface ListHold {
rowKey: string;
keywords: HeldKeywords;
}
/**
* The list row an email is shown in: its thread, or the email itself when the
* list is not threaded - the same key EmailList groups by.
*/
export function listRowKey(email: Email, isScheduledView: boolean): string {
return useSettingsStore.getState().disableThreading || isScheduledView ? email.id : threadKeyFor(email);
}
/** Delta-sync baseline of the plain folder list (see EmailStore.emailListSync). */
export interface EmailListSync {
/** Email collection state the list was last synced at. */
@@ -1469,6 +1490,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
hasMoreEmails: false,
totalEmails: 0,
listOrder: [],
listHold: null,
isPushConnected: false,
lastPushUpdate: null,
emailListSync: null,
@@ -1589,6 +1611,15 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
emailHooks.onEmailOpen.emitSync(email);
}
},
holdListRow: (rowKey) => {
const { listHold, emails, isScheduledView } = get();
if (listHold?.rowKey === rowKey) return;
const keywords = new Map<string, Email['keywords']>();
for (const email of emails) {
if (listRowKey(email, isScheduledView) === rowKey) keywords.set(email.id, email.keywords);
}
set({ listHold: { rowKey, keywords } });
},
selectKeyword: (keyword) => set(state => ({
selectedKeyword: keyword,
isLoadingMore: false,
@@ -1806,8 +1837,8 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
// an already-visited account doesn't flash a spinner over the visible mail.
const background = opts?.background ?? false;
// Loading a real mailbox is a fresh navigation (leaving any cross view), so
// drop the unread/starred retain set.
set(background ? { error: null, retainedInViewIds: new Set() } : { isLoading: true, error: null, retainedInViewIds: new Set() }); // Keep previous emails visible during transition
// drop the unread/starred retain set and the held row.
set(background ? { error: null, retainedInViewIds: new Set(), listHold: null } : { isLoading: true, error: null, retainedInViewIds: new Set(), listHold: null }); // Keep previous emails visible during transition
try {
const targetMailboxId = mailboxId || get().selectedMailbox;
if (targetMailboxId === VIRTUAL_SCHEDULED_MAILBOX_ID) {
@@ -4276,15 +4307,17 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
// load-more brings them back where they now belong. Not the open
// conversation's, though: every action on an email looks it up in this
// list, so "mark as unread" on the mail just read must still find it.
// They stay until something else is opened, and load-more does not
// count them.
// Nor the held row, which stays where it was clicked. They stay until
// something else is opened, and load-more does not count them.
const held = get().listHold?.keywords;
let retainedInViewIds: Set<string> | undefined;
if (keepOpenThread) {
const open = get().selectedEmail;
const openThread = open ? threadKeyFor(open) : null;
retainedInViewIds = new Set();
for (const email of currentEmails) {
if (mergedIds.has(email.id) || threadKeyFor(email) !== openThread) continue;
if (mergedIds.has(email.id)) continue;
if (threadKeyFor(email) !== openThread && !held?.has(email.id)) continue;
merged.push(email);
mergedIds.add(email.id);
retainedInViewIds.add(email.id);
@@ -4303,6 +4336,10 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
retainedAddedCount = merged.length - beforeLen;
}
// The fresh order has the held row where its new read/star/tag state
// sorts; put it back where the list shows it (the order EmailList uses).
merged = placeHeldRows(merged, searchQuery || crossView || hasFilters ? [] : get().listOrder, held);
// Check if anything actually changed to avoid unnecessary re-renders
const hasChanged =
currentEmails.length !== merged.length ||
@@ -4424,6 +4461,8 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
newExpandedThreadIds.delete(threadKey);
} else {
newExpandedThreadIds.add(threadKey);
// Expanding marks the thread read; hold it before that lands.
get().holdListRow(threadKey);
}
set({ expandedThreadIds: newExpandedThreadIds });
@@ -4936,6 +4975,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
crossView: null,
selectedKeyword: null,
retainedInViewIds: new Set(),
listHold: null,
viewToken: token,
});
try {
@@ -5020,6 +5060,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
crossView: view,
selectedKeyword: null,
retainedInViewIds: new Set(),
listHold: null,
viewToken: token,
});
try {
@@ -5066,6 +5107,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
crossView: null,
unifiedErrors: new Map(),
retainedInViewIds: new Set(),
listHold: null,
viewToken: state.viewToken + 1,
}));
},
@@ -5499,6 +5541,15 @@ useEmailStore.subscribe((state, prev) => {
useEmailStore.setState(projected);
});
// Opening a message holds its list row (see listHold), however it was opened:
// a click, next/previous, or the pick after a delete. This runs before the
// message is marked read, so the hold records it unread.
useEmailStore.subscribe((state, prev) => {
const email = state.selectedEmail;
if (!email || email.id === prev.selectedEmail?.id) return;
state.holdListRow(listRowKey(email, state.isScheduledView));
});
// ---------------------------------------------------------------------------
// Boot snapshot: persist the mailbox list and the first page of the current
// list so a returning visit paints rows the moment auth resolves, instead of