fix(decrypt,addons): dark-mode email body + mobile decrypt overhaul#212
Conversation
- EmailView: render decrypted email in a forced light-themed iframe so emails that hardcode dark text on no background stay legible when the host page is in dark mode (#159). - Marketing page: store the returning-visitor flag in a cookie scoped to the registrable domain so it carries across postguard.eu subdomains instead of being trapped per-origin in localStorage (#208).
Previously the iframe forced a light surface unconditionally. That fixed the original dark-mode bug for HTML emails (which set their own dark text on no background), but it also overrode the site theme for plain- text emails — leaving them stuck in light mode forever. - HTML emails: don't touch the styling. Email designs are self-contained. - Plain-text emails: wrap the text in <pre> and inject body styles that follow the host theme (background, color, color-scheme), reactively switching when the user toggles the theme via a MutationObserver on the <html> class.
…nner On mobile the "install Thunderbird/Outlook extension" banner is irrelevant, and the dashed "drop the postguard.encrypted file here" zone implies a drag-and-drop interaction that isn't available on touch. - Hide the extension banner under 768px. - Restyle the upload label as a solid primary button on mobile and swap its text to "Upload \"postguard.encrypted\"" so it reads as a tappable action rather than a drop target. The underlying <input type="file"> already opens the native picker on tap. - Add the new fallback.upload i18n key in EN and NL.
On a phone the side-by-side panels squeezed both lists and the email view into ~40vh slices. Switch to a phone-style two-step flow instead. - Inbox view: full-screen panel with search/cog at the top, email list filling the middle, and the upload button pinned to the bottom (via flex order — desktop layout is unchanged). - Reader view: clicking an email or hitting Decrypt swaps to a full-screen right panel with a "Back to inbox" button up top. - "Back" resets currSelected and hashMode so the effect collapses currRight back to Nothing, restoring the list view. - Add fallback.back i18n key in EN and NL.
…ents - decrypt mobile: anchor .fallback-page to the viewport height and add min-height:0 + explicit flex bases to the panel/list chain so the email list can shrink instead of growing to fit content and pushing the upload button below the fold. - ListView: replace the 3px left-border selected accent with a solid primary-coloured background and white text — clearer affordance and drops the "AI-template" stripe look. - Addons callout: drop the left-border stripe in favour of a full bordered, slightly muted amber card.
rubenhensen
changed the title
|
/dobby review |
|
Wiggles ears excitedly! This agent will review the PR right away — checking out the code, reading the diff, and reporting back with findings! |
![dobby-coder[bot]](https://avatars.githubusercontent.com/in/3365457?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review
Reviewed branch fix/website-misc-fixes locally. Verified:
npm ci --legacy-peer-deps && npm run build && npm run lint && npm run check— all green.- CI on the PR is green too.
The PR does what the body claims and the diffs are tight. A few notes, ordered by significance.
Nice work
- XSS hardening of plain-text emails. Previously the iframe got
${parsed.html ?? parsed.text ?? ''}—parsed.textwas interpolated as HTML, so<characters in plain text rendered as broken tags. The newescapeHtml(parsed.text)fixes that. Thesandbox=""attribute was already neutralising script execution, but this is the correct change either way. - Theme observer.
MutationObserveron<html>.classis the right hook — matches how the theme toggle works elsewhere — and the cleanup on unmount is wired up correctly. - Mobile flex chain. The combination of
height: calc(100vh - 52px)on.fallback-pageplusmin-height: 0+flex: 1 1 0down the panel/list chain is the textbook fix for "flex child grows past its container". Looks right. backToList()ordering. ResettingcurrSelectedandhashModelets the existing reactiveruncollapsecurrRighttoNothingon its own; the explicit assignment is belt-and-braces and harmless.- i18n parity. Both
en.jsonandnl.jsongetfallback.uploadandfallback.back. No drift.
Worth a look
-
registrableDomain()doesn't know about the Public Suffix List. Forpostguard.eu/postguard.nlthe naive "last two parts" works. But on any multi-part public suffix (*.co.uk,*.github.io,*.vercel.app,*.pages.dev) the result equals the public suffix itself, and browsers will rejectDomain=.co.ukoutright — so no cookie gets set and the returning-visitor flag silently breaks on preview deploys. Probably fine for production today, but worth either a code comment acknowledging it or (longer term) a PSL-aware helper. Not blocking. -
hasVisited()prefix match is slightly loose.startsWith('pg_visited=')would also match a hypotheticalpg_visited_v2=. Nothing in the repo today shares that prefix, so it's purely a hygiene point — splitting on=and comparing the name exactly would be tidier. -
--pg-font-weight-semibold(pre-existing, but you're adding a new caller). That custom property isn't defined insrc/lib/global.scss, sofont-weight: var(--pg-font-weight-semibold)falls back tonormal(400). The new.mobile-backbutton atsrc/routes/(app)/decrypt/+page.svelte:531is now the fifth caller of this undefined token. Not introduced by this PR, but adding--pg-font-weight-semibold: 600;toglobal.scssis a one-line, repo-wide win and fits the spirit of "visual tells cleanup". -
Plain-text email font. The new
bodyStylesetsfont-family: system-ui, -apple-system, sans-serifand the<pre>inherits it. Plain-text email bodies (ASCII tables, code blocks, formatted signatures) typically rely on monospace alignment. Most real mail clients rendertext/plainin a monospace face. Worth consideringfont-family: ui-monospace, SFMono-Regular, Menlo, monospaceon the<pre>— purely a design call, not a correctness issue. -
HTML email srcdoc.
${parsed.html}is still concatenated directly into the wrapper. Givensandbox="", this is fine — scripts can't run and malformed markup is contained to the iframe. Just flagging it stays an implicit trust point: anything that loosens the sandbox later would re-open this.
Test plan coverage
The manual checklist in the PR body covers the right things. The one scenario I'd add: tap an email → "Back to inbox" → tap a different email — make sure the second email actually loads (i.e. currSelected = -1 between selections doesn't introduce a stale parsed flash). Looking at the reactive run, it should be fine, but worth eyeballing once.
Verdict
LGTM with the small notes above. None of them are blockers; (1) and (3) are the most worth picking up, and both are tiny.
Cookies cannot be shared across different registrable domains (e.g. postguard.eu ↔ postguard.nl), so the cookie-scoped flag only solved the subdomain case — which already worked well enough via the existing localStorage approach for our deployed surfaces. Cross-TLD sharing would require either consolidating to a single canonical domain or a third-party tracker iframe, neither of which is in scope. Revert to localStorage and close #208.
rubenhensen
changed the title
There was a problem hiding this comment.
Re-reviewed after dea6567. The cookie migration is fully gone — src/routes/(marketing)/+page.{svelte,js} are back on localStorage.getItem('pg_visited') / setItem('pg_visited', 'true'), no registrableDomain / hasVisited helpers remain, and the marketing routes are no longer touched by this PR at all.
That moots points (1) and (2) from the prior review (PSL handling, prefix match on pg_visited=). The remaining diff is exactly what the description says:
- EmailView.svelte —
escapeHtml(parsed.text)+ theme-aware<pre>wrapper +MutationObserveron<html>class, HTML emails left untouched. - decrypt/+page.svelte — mobile inbox/reader two-step flow,
backToList()resetscurrSelected/hashMode, anchoredcalc(100vh - 52px)+min-height: 0+flex: 1 1 0chain. - ListView.svelte — selected row is now a full primary-coloured background.
- addons/+page.svelte — Outlook callout converted to a fully bordered amber card.
- en.json / nl.json —
fallback.upload+fallback.backadded in parity.
CI green on dea6567 (Svelte Check, Lint, Build amd64/arm64, Conventional Commit). Prior local run of npm ci --legacy-peer-deps && npm run build && npm run lint && npm run check was green on the pre-revert tip and the revert only deletes code; nothing to re-verify.
The smaller non-blocking notes from before still apply if anyone wants to pick them up later:
--pg-font-weight-semiboldis undefined inglobal.scss(new caller in.mobile-back); one-line fix.<pre>for plain-text emails usessystem-ui; monospace would match other mail clients but is a design call.- HTML
srcdockeeps an implicit trust point that the iframe stayssandbox="".
LGTM, approving.
Two notes from the dobby review. - Define --pg-font-weight-semibold in global.scss. It was referenced in five places (Header, marketing root, marketing layout, blog post page, and the new .mobile-back button) but never declared, silently falling back to 400. Mapping it to 600 matches the existing --medium token. - Switch the plain-text email <pre> to a monospace face. text/plain bodies routinely rely on column alignment (ASCII tables, quoted replies with leading '>', signatures), which broke under system-ui. Drop the font-family from <body> so it doesn't shadow the <pre>.
…edium Both pointed at 600, so there were two tokens for one value. Drop the just-added --pg-font-weight-semibold and migrate the existing five callers (Header, marketing root + layout, blog post page, decrypt .mobile-back) to --pg-font-weight-medium.
1 participant
Summary
A bundle of small site fixes on
/decryptand/addons.color-scheme) and react live to the theme switcher via aMutationObserveron<html>'s class. HTML emails are rendered untouched so their own design isn't overridden — they carry their own self-contained styling./decryptoverhaul. The desktop-only "Install the Outlook/Thunderbird extension" banner and the "drop the postguard.encrypted attachment here" drag affordance both made no sense on phones. The mobile flow now mirrors a real inbox:order, so the desktop layout is unchanged).currSelectedandhashModeso the existing effect collapsescurrRighttoNothingand restores the list..fallback-pagetocalc(100vh - 52px)and addedmin-height: 0/ explicitflex: 1 1 0along the panel/list chain so the email list can shrink instead of growing to its content and pushing the upload button below the fold.rgba(255,255,255,0.85)), instead of the 3px primary stripe./addonsis now a fully bordered amber card with deeper amber body text instead of the 4px left stripe.Changes
src/lib/components/fallback/EmailView.svelte— theme-aware iframe rendering, dark-class observer, HTML escaping for plain-text bodies.src/routes/(app)/decrypt/+page.svelte— mobile layout, single-screen list↔reader flow, "Back to inbox" button, anchored heights, mobile upload button styling.src/lib/components/fallback/ListView.svelte— selected-item full-background style.src/routes/(marketing)/addons/+page.svelte— Outlook callout box restyle.src/lib/locales/{en,nl}.json—fallback.uploadandfallback.backstrings.Test plan
/decryptin dark mode with a plain-text email: text legible, recolours live when the theme switcher toggles./decryptin dark mode with an HTML email whose body hardcodes black on white: email keeps its own design untouched.<768px)/decrypt: extension banner gone; upload button pinned at the bottom; tapping an email opens the reader full-screen with a "Back to inbox" button; tapping back returns to the list./addons→ Outlook tab: warning callout uses a full amber border, no left-border stripe.Closes #159