Stop the reading view rearranging itself as you read
Opening a conversation with several unread messages showed them all expanded, each with its unread bar. The moment the auto-mark-read timer fired, every one of them collapsed except the last, and the bars vanished -- so the messages you had just been given were taken away again, and the only record of which ones they were went with them (#69). Both came from the same place: expansion and the bar were derived from `$seen`, live. Marking read on the server changed what the view thought it was looking at. Marking read is not the problem. Opening a thread is the signal that you are reading it, and mbunkus was explicit that turning the setting off is not the answer he wants. What was wrong was letting a change *this view caused* alter its own shape underneath the reader. The thread now remembers which messages were unread when it was opened, and uses that for expansion and for the bar. The set only grows while a thread is open -- a message arriving unread joins it -- and is discarded on the way to another thread. The server still gets marked read on the timer, exactly as before, and the message list still updates. It is accumulated during render rather than in an effect. It is derived purely from the messages already in hand and adding an id twice does nothing, while an effect would repaint a frame later -- which is the flicker this exists to remove. Verified against the mock with markReadDelay at 0, the harshest setting, where the timer fires immediately: six seconds after opening a three-message thread, the server reports all three seen while the view still shows all three expanded with their bars. Before, two of the three would have collapsed in the first instant.
This commit is contained in:
@@ -28,12 +28,14 @@ import { sendReadReceipt } from "@/store/mdn";
|
||||
interface Props {
|
||||
email: Email;
|
||||
expanded: boolean;
|
||||
/** Unread when the conversation was opened, which is what the bar marks. */
|
||||
wasUnread?: boolean;
|
||||
onToggle: () => void;
|
||||
isLast: boolean;
|
||||
actions: ListActions;
|
||||
}
|
||||
|
||||
export const MessageView = memo(function MessageView({ email: e, expanded, onToggle, actions }: Props) {
|
||||
export const MessageView = memo(function MessageView({ email: e, expanded, wasUnread, onToggle, actions }: Props) {
|
||||
const accountId = useMail((s) => s.accountId)!;
|
||||
const settings = useSettings((s) => s.settings);
|
||||
const updateSettings = useSettings((s) => s.update);
|
||||
@@ -134,7 +136,10 @@ export const MessageView = memo(function MessageView({ email: e, expanded, onTog
|
||||
};
|
||||
|
||||
return (
|
||||
<article className={`message ${expanded ? "" : "collapsed"} ${!e.keywords.$seen ? "unread-msg" : ""}`} data-msg-id={e.id} onClick={collapsedClick}>
|
||||
/* `wasUnread` rather than `$seen`: the bar marks what was unread when the
|
||||
conversation was opened, and keeps marking it after the auto-mark-read
|
||||
timer has told the server otherwise. Losing it mid-read was half of #69. */
|
||||
<article className={`message ${expanded ? "" : "collapsed"} ${wasUnread ?? !e.keywords.$seen ? "unread-msg" : ""}`} data-msg-id={e.id} onClick={collapsedClick}>
|
||||
<header className="message-head" onClick={(ev) => { if (expanded && !(ev.target as HTMLElement).closest("button,a,.message-details")) onToggle(); }}>
|
||||
<Avatar who={from ?? null} />
|
||||
<div className="who">
|
||||
|
||||
@@ -65,15 +65,41 @@ export function ThreadView({ threadId, mailboxId, onBack, actions, onNavigate, h
|
||||
return (filtered.length ? filtered : all).sort((a, b) => a.receivedAt.localeCompare(b.receivedAt));
|
||||
}, [thread, emails, fullIds, mailboxId]);
|
||||
|
||||
// Default expansion: unread + last message expanded, others collapsed
|
||||
/*
|
||||
* Which messages were unread when this conversation was opened.
|
||||
*
|
||||
* Expansion and the unread bar used to read `$seen` directly, so the moment
|
||||
* the auto-mark-read timer fired, every message expanded *because* it was
|
||||
* unread collapsed again -- all but the last -- and the only record of which
|
||||
* ones they were disappeared with them (#69). Opening a thread with several
|
||||
* unread messages gave you a few seconds before the view rearranged itself
|
||||
* underneath you.
|
||||
*
|
||||
* Marking read on the server is still right: opening the thread is the signal
|
||||
* that you are reading it. What was wrong was letting that change the shape
|
||||
* of what you are looking at. The set only ever grows while a thread is open
|
||||
* -- a message that arrives unread joins it -- and is discarded on the way to
|
||||
* another thread.
|
||||
*
|
||||
* Accumulated during render rather than in an effect because it is derived
|
||||
* purely from `messages`, and adding an id twice does nothing. An effect
|
||||
* would repaint a frame later, which is the flicker this exists to remove.
|
||||
*/
|
||||
const threadKey = thread?.id ?? null;
|
||||
const unreadAtOpen = useRef<{ key: Id | null; ids: Set<Id> }>({ key: null, ids: new Set() });
|
||||
if (unreadAtOpen.current.key !== threadKey) unreadAtOpen.current = { key: threadKey, ids: new Set() };
|
||||
for (const m of messages) if (!m.keywords.$seen) unreadAtOpen.current.ids.add(m.id);
|
||||
const wasUnread = unreadAtOpen.current.ids;
|
||||
|
||||
// Default expansion: unread when opened + last message expanded, others collapsed
|
||||
const lastId = messages[messages.length - 1]?.id;
|
||||
const isExpanded = useCallback(
|
||||
(e: Email) => {
|
||||
if (e.id in expanded) return expanded[e.id]!;
|
||||
if (allExpanded) return true;
|
||||
return !e.keywords.$seen || e.id === lastId || messages.length === 1;
|
||||
return wasUnread.has(e.id) || e.id === lastId || messages.length === 1;
|
||||
},
|
||||
[expanded, allExpanded, lastId, messages.length],
|
||||
[expanded, allExpanded, lastId, messages.length, wasUnread],
|
||||
);
|
||||
|
||||
// Mark as read after delay
|
||||
@@ -190,6 +216,7 @@ export function ThreadView({ threadId, mailboxId, onBack, actions, onNavigate, h
|
||||
key={e.id}
|
||||
email={e}
|
||||
expanded={isExpanded(e)}
|
||||
wasUnread={wasUnread.has(e.id)}
|
||||
onToggle={() => setExpanded((x) => ({ ...x, [e.id]: !isExpanded(e) }))}
|
||||
isLast={i === messages.length - 1}
|
||||
actions={actions}
|
||||
|
||||
Reference in New Issue
Block a user