From f627bfc1237d6e8bbc728147022f624c3834d648 Mon Sep 17 00:00:00 2001 From: jcoffey <51408202+jcoffey-dev@users.noreply.github.com> Date: Sun, 20 Sep 2026 14:53:45 -0700 Subject: [PATCH] The toolbar above an open message acts on that message (#414) (#417) With conversation view off, marking a message unread from the list -- the hover button, the right-click menu -- marked that message. Opening it and pressing Mark as unread in the toolbar above it marked every message in its thread, and so did Move to, Report spam and Delete. The setting already reaches all the way into the reading pane: the list draws one row per message, and `visibleMessages` narrows the pane to the one opened. The toolbar was half converted. Its labels were right -- Mark as unread against Mark as read, the star, the labels shown -- all of those read `messages`, which is the narrowed set. Only `rowIds`, the one thing actually handed to the action, still read `thread.emailIds`. So the button said one message and did the whole conversation. `rowIds` is now the same question `visibleMessages` answers for the pane, asked of the same ids, with the same fallback: an id that names nothing in the thread -- a link from somebody with conversation view on, a stale `m` in the URL -- shows the conversation, so the toolbar takes the conversation. Conversation view on is unchanged: nothing is singled out, so the whole thread comes back as before. No new strings. --- web/src/views/mail/ThreadView.tsx | 18 ++- .../__tests__/thread-toolbar-scope.test.tsx | 128 ++++++++++++++++++ 2 files changed, 145 insertions(+), 1 deletion(-) create mode 100644 web/src/views/mail/__tests__/thread-toolbar-scope.test.tsx diff --git a/web/src/views/mail/ThreadView.tsx b/web/src/views/mail/ThreadView.tsx index a9dfc65..44783a8 100644 --- a/web/src/views/mail/ThreadView.tsx +++ b/web/src/views/mail/ThreadView.tsx @@ -224,7 +224,23 @@ export function ThreadView({ threadId, mailboxId, onBack, actions, onNavigate, h }, [messages, reply]); const subject = messages[0]?.subject || emails[thread?.emailIds[0] ?? ""]?.subject || "(no subject)"; - const rowIds = thread ? thread.emailIds.filter((id) => emails[id]) : []; + /* + * What the toolbar acts on: the messages the pane is showing, not the thread + * they belong to. + * + * With conversation view off, opening a message opens that message -- the + * list shows it alone, the pane renders it alone, and the buttons above it + * said so, because `anyUnread` and the rest already read `messages`. Only the + * ids handed to the action still named the whole thread, so Mark as unread, + * Move to, Report spam and Delete quietly took every message in it (#414). + * + * Same fallback as the pane's: an id naming nothing in this thread means the + * whole conversation, so the buttons keep matching what is on screen. + */ + const rowIds = useMemo(() => { + const loaded = thread ? thread.emailIds.filter((id) => emails[id]).map((id) => ({ id })) : []; + return visibleMessages(loaded, messageId).map((m) => m.id); + }, [thread, emails, messageId]); const anyUnread = messages.some((e) => !e.keywords.$seen); const anyStarred = messages.some((e) => e.keywords.$flagged); const inJunk = Boolean(mailboxId && mailboxes[mailboxId]?.role === "junk"); diff --git a/web/src/views/mail/__tests__/thread-toolbar-scope.test.tsx b/web/src/views/mail/__tests__/thread-toolbar-scope.test.tsx new file mode 100644 index 0000000..24cf3c0 --- /dev/null +++ b/web/src/views/mail/__tests__/thread-toolbar-scope.test.tsx @@ -0,0 +1,128 @@ +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { ThreadView } from "../ThreadView"; +import { useMail } from "@/store/mail"; +import type { ListActions } from "../MessageList"; +import type { Email, Id } from "@/jmap/types"; + +(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +/* jsdom has neither of these, and the opening scroll uses both. */ +Element.prototype.scrollIntoView = () => {}; +globalThis.ResizeObserver ??= class { observe() {} unobserve() {} disconnect() {} } as unknown as typeof ResizeObserver; + +/* jsdom has no matchMedia, and the toolbar asks whether this is a phone. */ +window.matchMedia = ((q: string) => ({ matches: false, media: q, addEventListener() {}, removeEventListener() {} })) as unknown as typeof window.matchMedia; + +/* + * Reported from the inbox with conversation view off: marking a message unread + * from the list -- hover button, right-click menu -- touched that message, but + * the same action from the toolbar above the *opened* message marked every + * message in its thread. Move to, Report spam and Delete did it too (#414). + * + * The toolbar's labels were already right: "Mark as unread" read the messages + * on screen. Only the ids it handed the action named the whole thread. + */ + +const msg = (id: Id, subject: string): Email => + ({ + id, threadId: "t1", subject, mailboxIds: { inbox: true }, keywords: { $seen: true }, + from: [{ name: "Ann", email: "ann@example.com" }], to: [{ name: "Me", email: "me@example.org" }], + receivedAt: "2026-09-20T10:00:00Z", size: 10, blobId: "b1", preview: "hi", + htmlBody: [], textBody: [{ partId: "1", type: "text/plain" }], + bodyValues: { "1": { value: "hi", isEncodingProblem: false, isTruncated: false } }, + attachments: [], + }) as unknown as Email; + +const FIRST = msg("m1", "The question"); +const SECOND = msg("m2", "Re: The question"); + +function stubStore() { + useMail.setState({ + accountId: "a1", + threads: { t1: { id: "t1", emailIds: ["m1", "m2"] } } as never, + emails: { m1: FIRST, m2: SECOND } as never, + fullIds: { m1: true, m2: true } as never, + loadingThreads: {} as never, + mailboxes: { inbox: { id: "inbox", name: "Inbox", role: "inbox" } } as never, + loadThread: (async () => undefined) as never, + setOpenThread: (() => undefined) as never, + markRead: (async () => undefined) as never, + roleId: (() => null) as never, + }); +} + +describe("what the toolbar above an opened message acts on", () => { + let host: HTMLDivElement; + let root: Root; + let actions: ListActions; + + const show = async (messageId: Id | null) => { + await act(async () => { + root.render( + undefined} onNavigate={() => undefined} hasPrev={false} hasNext={false} + />, + ); + }); + }; + + /** The toolbar buttons carry their shortcut in the title, as the tooltips show. */ + const press = async (title: string) => { + const btn = [...host.querySelectorAll("button")].find((b) => b.title === title); + expect(btn, `no toolbar button titled ${title}`).toBeTruthy(); + await act(async () => btn!.click()); + }; + + beforeEach(() => { + stubStore(); + actions = { + archive: vi.fn(async () => undefined), trash: vi.fn(async () => undefined), + spam: vi.fn(async () => undefined), read: vi.fn(async () => undefined), + star: vi.fn(async () => undefined), move: vi.fn(async () => undefined), + label: vi.fn(async () => undefined), + } as unknown as ListActions; + host = document.createElement("div"); + document.body.appendChild(host); + root = createRoot(host); + }); + + afterEach(async () => { + await act(async () => root.unmount()); + host.remove(); + }); + + it("marks only the message that is open, not its thread", async () => { + await show("m1"); + await press("Mark as unread"); + expect(actions.read).toHaveBeenCalledWith(false, ["m1"]); + }); + + it("moves, reports and deletes only that message too", async () => { + await show("m1"); + await press("Move to (v)"); + await press("Report spam (!)"); + await press("Delete (#)"); + expect(actions.move).toHaveBeenCalledWith(["m1"]); + expect(actions.spam).toHaveBeenCalledWith(["m1"]); + expect(actions.trash).toHaveBeenCalledWith(["m1"]); + }); + + it("takes the whole thread when the pane is showing the whole thread", async () => { + // Conversation view on: no message singled out, and the toolbar is the + // conversation's toolbar. That is the behaviour this must not disturb. + await show(null); + await press("Mark as unread"); + expect(actions.read).toHaveBeenCalledWith(false, ["m1", "m2"]); + }); + + it("falls back to the thread when the open id names nothing in it", async () => { + // A link from somebody with conversation view on, or a stale `m` in the + // URL. The pane shows the conversation, so the toolbar acts on it. + await show("gone"); + await press("Mark as unread"); + expect(actions.read).toHaveBeenCalledWith(false, ["m1", "m2"]); + }); +});