feat: make email bodies follow the active theme - #332
Conversation
527ab59 to
a4485ae
Compare
|
Going to pass on this implementation, but I'm keeping the dark mode request (#228) open. Appreciate the work. |
|
@maathimself seeing you reopen this gives me hope for #321 🌭 |
|
Hey @salmonumbrella, I pulled #332 onto current main to give it a proper evaluation and ran it against my real inbox. Two things came up that I wanted to get back to you on. 1. Blank preview on some emails (I have a fix) A few HTML emails rendered as a completely blank preview pane. Tracked it down: the appearance engine only reveals the body on the iframe's Fix that worked for me: reveal as soon as the DOM is parsed ( 2. Design question on the complexity fallback Once emails actually show, the branded ones render stripped of all styling. Digging in, that's the The same email in "original colors" mode renders perfectly, with its full design, since that keeps the sanitized styles forced-light (which is how old MailFlow and Gmail/Apple Mail handle complex mail). So the question is really: should a complexity fallback keep the sender's styles (forced-light) rather than strip them? That'd give up the readability guarantee for rare pathological emails but fix the common case. Didn't want to override your call on it or rewrite the ~24 tests that lock in the strip behavior without checking with you first. What's your read? |
|
good catch on the blank preview, and ya you read it right: gating reveal on on the complexity fallback, i think you're right and i'd make the change. stripping was me collapsing two different claims:
so the plan i'd go with: route |
|
Here's the reveal-on-DOM-parsed fix as a diff (the cross-fork PR route hit a wall pushing the branch, so pasting it here per your "whatever's easiest"). Two files, MessagePane's iframe effect and the appearance harness, mirrored. Verified: frontend lint + build clean, and the email-appearance e2e suite passes on chromium. diff --git a/frontend/e2e/email-appearance/harness.jsx b/frontend/e2e/email-appearance/harness.jsx
index 66905db..e8d541a 100644
--- a/frontend/e2e/email-appearance/harness.jsx
+++ b/frontend/e2e/email-appearance/harness.jsx
@@ -1259,8 +1259,26 @@ function Harness() {
window.mailflowFixtureIframeLifecycle.readyStateShortcuts += 1;
void processFrameDocument(iframe.contentDocument);
}
+ // Mirror MessagePane: reveal on DOM-parsed rather than the full `load` event, so an
+ // email whose remote subresources never resolve (readyState stuck "interactive")
+ // still processes. Scoped to normal fixtures so the timing-scenario tests keep their
+ // exact wiring. processFrameDocument is idempotent (processedDocument guard).
+ let domReadyRafId = 0;
+ if (!scenario) {
+ const revealWhenParsed = () => {
+ const doc = iframe.contentDocument;
+ if (doc && doc.readyState !== 'loading'
+ && emailFrameDocumentMatchesSource(doc, frameSourceToken)) {
+ void processFrameDocument(doc);
+ return;
+ }
+ domReadyRafId = requestAnimationFrame(revealWhenParsed);
+ };
+ revealWhenParsed();
+ }
return () => {
cancelled = true;
+ if (domReadyRafId) cancelAnimationFrame(domReadyRafId);
iframe.removeEventListener('load', onLoaded);
};
}, [appearance.processDraft, appearance.recovery, appearance.rootKey, frameSourceToken, html, renderer, scenario]);
diff --git a/frontend/src/components/MessagePane.jsx b/frontend/src/components/MessagePane.jsx
index 7b8fc7b..3c5c9f7 100644
--- a/frontend/src/components/MessagePane.jsx
+++ b/frontend/src/components/MessagePane.jsx
@@ -590,14 +590,29 @@ export default function MessagePane() {
}
};
+ // The iframe `load` event waits for EVERY subresource. Marketing emails routinely
+ // carry remote images/fonts that never resolve, which leaves the document stuck at
+ // readyState "interactive" and the message permanently hidden. Reveal as soon as the
+ // DOM is parsed instead; onLoaded is idempotent (its processedDocument + source-token
+ // guards), so the later `load` event — if it ever fires — is a harmless no-op, and
+ // image-driven re-measurement still runs.
iframe.addEventListener('load', onLoaded, { once: true });
- if (iframe.contentDocument?.readyState === 'complete') {
- onLoaded();
- }
+ let domReadyRafId = 0;
+ const revealWhenParsed = () => {
+ const doc = iframe.contentDocument;
+ if (doc && doc.readyState !== 'loading'
+ && emailFrameDocumentMatchesSource(doc, iframeSourceToken)) {
+ onLoaded();
+ return;
+ }
+ domReadyRafId = requestAnimationFrame(revealWhenParsed);
+ };
+ revealWhenParsed();
return () => {
cancelled = true;
cancelAnimationFrame(rafId);
+ if (domReadyRafId) cancelAnimationFrame(domReadyRafId);
if (roRef.current) { roRef.current.disconnect(); roRef.current = null; }
iframe.removeEventListener('load', onLoaded);
emailScaleRef.current = 1; |
Summary
Fixes #228.
HTML email bodies now follow MailFlow's effective theme by default, including built-in themes and custom CSS overrides. The renderer lets the browser resolve sender CSS, then performs a bounded, transactional repair of computed colors; if adaptation cannot complete safely, it reveals the existing readable forced-light rendering instead. Users can choose original colors globally or for the current message.
Changes
Testing
npm run lintnpm run lintnpm run buildnpm run test:bundle(appearance chunk 7,182 gzip bytes; eager safety chunk 445 gzip bytes)git diff --checkContributor License Agreement
By submitting this pull request I confirm that:
third-party material and confirmed it is compatible with the CLA).