Merge pull request #88 from LINUXexpert-org/open-thread-at-first-unread

Open a conversation on its first unread message

server/src/mock/index.ts

@@ -137,6 +137,25 @@ addEmail({ from: ["Demo User", USER], to: "[email protected]", subject: "Draft: id
 addEmail({ from: ["Spammy", "[email protected]"], subject: "You have WON!!!", daysAgo: 2, mailbox: "junk", unread: true });
 addEmail({ from: ["Finance Team", "[email protected]"], subject: "Invoice 2201 approved", daysAgo: 1, mailbox: "work-inv", unread: true });
 addEmail({ from: ["Finance Team", "[email protected]"], subject: "Invoice 2202 pending", daysAgo: 2, mailbox: "work-inv", unread: true });
+// A thread whose unread message is not the last one: someone's server queued
+// their reply for hours, so it landed after messages that answer it and sits in
+// the middle of the conversation. Opening this thread at the newest message
+// left that reply above the fold until the mark-read timer swept it (#87).
+{
+  const subj = "Compiler timings for the release";
+  const t = addEmail({ from: ["Grace Hopper", "[email protected]"], subject: subj, daysAgo: 6, mailbox: "inbox", html: true });
+  const tid = t.threadId as string;
+  const reply = (o: { from: [string, string]; daysAgo: number; mailbox: string; to?: string; unread?: boolean; html?: boolean }) =>
+    addEmail({ ...o, subject: `Re: ${subj}`, threadId: tid, inReplyTo: `${t.id}@mock` });
+  reply({ from: ["Alan Turing", "[email protected]"], daysAgo: 5.5, mailbox: "inbox", unread: true });
+  // Long enough after the unread one that the thread scrolls: opening at the
+  // bottom put four messages between the reader and the mail they had not read.
+  reply({ from: ["Demo User", USER], to: "[email protected]", daysAgo: 5, mailbox: "sent", html: true });
+  reply({ from: ["Grace Hopper", "[email protected]"], daysAgo: 4.5, mailbox: "inbox" });
+  reply({ from: ["Margaret Hamilton", "[email protected]"], daysAgo: 4, mailbox: "inbox", html: true });
+  reply({ from: ["Demo User", USER], to: "[email protected]", daysAgo: 3.5, mailbox: "sent" });
+  reply({ from: ["Grace Hopper", "[email protected]"], daysAgo: 3, mailbox: "inbox", html: true });
+}
 // Invitation email
 {
   const ics = `BEGIN:VCALENDAR\r\nVERSION:2.0\r\nPRODID:-//mock//EN\r\nMETHOD:REQUEST\r\nBEGIN:VEVENT\r\nUID:inv-1@mock\r\nDTSTAMP:20260820T100000Z\r\nDTSTART:20260825T140000Z\r\nDTEND:20260825T150000Z\r\nSUMMARY:Project kickoff\r\nORGANIZER;CN=Ada Lovelace:mailto:[email protected]\r\nATTENDEE;CN=Demo User;RSVP=TRUE;PARTSTAT=NEEDS-ACTION:mailto:${USER}\r\nLOCATION:Room 4B\r\nEND:VEVENT\r\nEND:VCALENDAR\r\n`;

web/src/lib/__tests__/threadScroll.test.ts

@@ -0,0 +1,48 @@
+import { describe, expect, it } from "vitest";
+import { threadScrollTarget } from "@/lib/threadScroll";
+
+/**
+ * Issue #87: a conversation opened on its newest message, so unread mail sat
+ * above the fold with nothing to announce it but a marker you had to scroll up
+ * to see — and the auto-mark-read timer marked it read while you were still
+ * looking at the bottom of the thread.
+ *
+ * The case that makes "second to last" the wrong answer is out-of-order
+ * delivery: a message sent hours ago but queued on the sender's server arrives
+ * last and sorts early. Messages here are in the order the pane renders them,
+ * oldest first, which is receivedAt order.
+ */
+
+const thread = (n: number) => Array.from({ length: n }, (_, i) => ({ id: `m${i + 1}` }));
+const unread = (...ids: string[]) => new Set(ids);
+
+describe("where a conversation opens", () => {
+  it("opens on the oldest unread message", () => {
+    expect(threadScrollTarget(thread(5), unread("m3", "m4"))).toBe("m3");
+  });
+
+  it("opens on an unread message that arrived late and sorted early", () => {
+    // The one the issue is about: m2 was delivered after m5, so opening at the
+    // bottom hides it three messages up.
+    expect(threadScrollTarget(thread(5), unread("m2"))).toBe("m2");
+  });
+
+  it("opens on the newest message when the thread is all read", () => {
+    expect(threadScrollTarget(thread(5), unread())).toBe("m5");
+  });
+});
+
+describe("when it leaves the pane where it is", () => {
+  it("stays at the top when the first message is the unread one", () => {
+    // Scrolling to it would push the subject off the top for nothing.
+    expect(threadScrollTarget(thread(4), unread("m1", "m3"))).toBeNull();
+  });
+
+  it("does not scroll a single message", () => {
+    expect(threadScrollTarget(thread(1), unread("m1"))).toBeNull();
+  });
+
+  it("does not scroll an empty thread", () => {
+    expect(threadScrollTarget([], unread())).toBeNull();
+  });
+});

web/src/lib/threadScroll.ts

@@ -0,0 +1,35 @@
+/**
+ * Where a conversation opens.
+ *
+ * It used to open on the newest message, which is wrong whenever anything in
+ * the thread is unread: the unread mail sits above the fold, and the only clue
+ * it exists is the marker on a message you have to scroll up to find. The
+ * auto-mark-read timer then sweeps the whole thread, so scrolling up late is
+ * scrolling up to mail that is already marked read (#87).
+ *
+ * Order is receivedAt, not arrival, so the first unread is not the second-to-
+ * last message or any other position you can guess at. A thread where one
+ * participant's server queued a message for hours delivers it late and sorts it
+ * early -- exactly the case where opening at the bottom hides the most.
+ *
+ * Two answers are "don't move":
+ *
+ *   - a single message, which is already the whole pane
+ *   - the first unread being the first message, where the top of the pane
+ *     shows it anyway, together with the subject
+ *
+ * `unread` is the set captured when the thread was opened rather than live
+ * `$seen` state, for the same reason expansion uses it: the mark-read timer
+ * must not change the shape of what you are looking at (#69).
+ */
+export function threadScrollTarget<T extends { id: string }>(
+  messages: readonly T[],
+  unread: ReadonlySet<string>,
+): string | null {
+  if (messages.length < 2) return null;
+  const firstUnread = messages.findIndex((m) => unread.has(m.id));
+  if (firstUnread === 0) return null;
+  if (firstUnread > 0) return messages[firstUnread]!.id;
+  // Nothing unread: the newest message, which is what you came for.
+  return messages[messages.length - 1]!.id;
+}

web/src/views/mail/ThreadView.tsx

@@ -10,6 +10,7 @@ import { MenuItem, MenuSep, Popover, useMenu } from "@/ui/popover";
 import { Spinner } from "@/ui/misc";
 import { client } from "@/jmap/client";
 import { LabelPicker } from "./LabelPicker";
+import { threadScrollTarget } from "@/lib/threadScroll";
 
 interface Props {
   threadId: Id;
@@ -115,11 +116,13 @@ export function ThreadView({ threadId, mailboxId, onBack, actions, onNavigate, h
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [messages.map((m) => m.id + (m.keywords.$seen ? "1" : "0")).join(","), settings.markReadDelay]);
 
-  // Scroll last expanded into view on load
+  // Open on the first unread message rather than the newest one (#87).
   useEffect(() => {
     if (!messages.length || !scrollRef.current) return;
-    const el = scrollRef.current.querySelector<HTMLElement>(`[data-msg-id="${CSS.escape(lastId ?? "")}"]`);
-    if (el && messages.length > 1) el.scrollIntoView({ block: "start" });
+    const target = threadScrollTarget(messages, wasUnread);
+    if (!target) return;
+    const el = scrollRef.current.querySelector<HTMLElement>(`[data-msg-id="${CSS.escape(target)}"]`);
+    el?.scrollIntoView({ block: "start" });
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [threadId, messages.length > 0]);