Stop rebuilding the message body when it is marked read
Marking a thread read redrew the message pane: the mail vanished and came back, white to dark to white on an HTML message that brings its own colours, half a second after the reader started reading it. Worst with auto-mark set to "immediately", where it happens the moment the thread opens (#100). The pane was not re-mounting. The *body* was being thrown away and built again, and the reason is one dependency. `HtmlBody` writes the message into a shadow root in an effect, and that effect had the click handler in its dependency list. The handler is a `useCallback` over `onShowImages`, which the parent passed as an arrow created inline, so it was a new function on every render -- and therefore the effect ran on every render, and every render replaced the rendered message with an identical one. Marking as read is exactly such a render: the store hands back a new email object and the thread re-renders. The listener now lives in its own effect. It is attached to the shadow root rather than to the contents, which survives the rewriting anyway, so a handler that changes identity costs a listener swap and nothing else. `onShowImages` is stable now too, but the split is the fix: it is what makes the body immune to the next handler that changes. This also stops the quoted-text toggle collapsing. `setQuoteOpen(false)` lives in the same effect and had been resetting on every render, so expanding a quote and waiting for the timer put it away again. Measured rather than watched, since a flicker is exactly the thing an eye will agree with you about. Holding a node from inside the shadow root across the transition, on the same three-message thread with the delay at 0: before, 21 childList mutations on the root and the held node detached and replaced; after, no mutations at all and the same node still attached. Clicking a blocked image still reveals remote images, which is what the moved listener is for. Closes #100.
This commit is contained in:
@@ -45,6 +45,10 @@ export const MessageView = memo(function MessageView({ email: e, expanded, wasUn
|
|||||||
const [showHeaders, setShowHeaders] = useState(false);
|
const [showHeaders, setShowHeaders] = useState(false);
|
||||||
const [source, setSource] = useState<string | null>(null);
|
const [source, setSource] = useState<string | null>(null);
|
||||||
const [allowRemote, setAllowRemote] = useState(false);
|
const [allowRemote, setAllowRemote] = useState(false);
|
||||||
|
/* Stable, so the body's click handler keeps its identity between renders.
|
||||||
|
Passing an inline arrow here is what made the handler change on every
|
||||||
|
render in the first place. */
|
||||||
|
const showImages = useCallback(() => setAllowRemote(true), []);
|
||||||
const [filterOpen, setFilterOpen] = useState(false);
|
const [filterOpen, setFilterOpen] = useState(false);
|
||||||
const moreMenu = useMenu();
|
const moreMenu = useMenu();
|
||||||
const addrMenu = useAddressMenu();
|
const addrMenu = useAddressMenu();
|
||||||
@@ -269,7 +273,7 @@ export const MessageView = memo(function MessageView({ email: e, expanded, wasUn
|
|||||||
{icsPart && <InviteCard email={e} part={icsPart} />}
|
{icsPart && <InviteCard email={e} part={icsPart} />}
|
||||||
{vcfParts.map((p) => <VCardCard key={p.blobId ?? p.partId ?? ""} part={p} accountId={accountId} />)}
|
{vcfParts.map((p) => <VCardCard key={p.blobId ?? p.partId ?? ""} part={p} accountId={accountId} />)}
|
||||||
<div className="message-body">
|
<div className="message-body">
|
||||||
{showHtml && rendered ? <HtmlBody html={rendered.html} bodyStyle={rendered.bodyStyle} themed={themed} onShowImages={() => setAllowRemote(true)} /> : <TextBody text={textRaw ?? ""} />}
|
{showHtml && rendered ? <HtmlBody html={rendered.html} bodyStyle={rendered.bodyStyle} themed={themed} onShowImages={showImages} /> : <TextBody text={textRaw ?? ""} />}
|
||||||
</div>
|
</div>
|
||||||
{attachments.length > 0 && <AttachmentList attachments={attachments} accountId={accountId} email={e} />}
|
{attachments.length > 0 && <AttachmentList attachments={attachments} accountId={accountId} email={e} />}
|
||||||
{unsubscribe && (
|
{unsubscribe && (
|
||||||
@@ -405,9 +409,32 @@ function HtmlBody({ html, bodyStyle, themed, onShowImages }: { html: string; bod
|
|||||||
}
|
}
|
||||||
setHasQuote(found);
|
setHasQuote(found);
|
||||||
setQuoteOpen(false);
|
setQuoteOpen(false);
|
||||||
|
/*
|
||||||
|
* `onClick` is deliberately not a dependency of this effect.
|
||||||
|
*
|
||||||
|
* This is the effect that writes the body into the shadow root, so anything
|
||||||
|
* in its dependencies rebuilds the entire message. The click handler used
|
||||||
|
* to be in here, and it changes identity on every render -- it closes over
|
||||||
|
* a prop the parent recreates inline -- so every render of the message
|
||||||
|
* threw the rendered body away and built it again. Marking as read does
|
||||||
|
* exactly that: the store hands back a new email object, the thread
|
||||||
|
* re-renders, and the reader watched the message vanish and come back,
|
||||||
|
* white to dark to white on an unstyled HTML mail, half a second after they
|
||||||
|
* started reading it (#100). The quoted-text toggle reset with it.
|
||||||
|
*
|
||||||
|
* The listener lives in its own effect below. It is attached to the shadow
|
||||||
|
* root rather than to its contents, which survives this rewriting anyway,
|
||||||
|
* so a changing handler now costs a listener swap and nothing else.
|
||||||
|
*/
|
||||||
|
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||||
|
}, [html, bodyStyle, themed]);
|
||||||
|
|
||||||
|
useEffect(() => {
|
||||||
|
const root = hostRef.current?.shadowRoot;
|
||||||
|
if (!root) return;
|
||||||
root.addEventListener("click", onClick);
|
root.addEventListener("click", onClick);
|
||||||
return () => root.removeEventListener("click", onClick);
|
return () => root.removeEventListener("click", onClick);
|
||||||
}, [html, bodyStyle, themed, onClick]);
|
}, [onClick]);
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
const root = hostRef.current?.shadowRoot;
|
const root = hostRef.current?.shadowRoot;
|
||||||
|
|||||||
Reference in New Issue
Block a user