fix(email-viewer, markdown): handle unreadable emails without manipulating their content

[Kiarokh]

fix: #4065

Summary

Two atomic fixes that together solve the readability problem described in #4065 without ever modifying email content:

1. <limel-markdown> — drops color and background-color from the inline-style allowlist. The component goes back to being a clean, theme-neutral renderer; feed snippets stay readable in any theme. The "preserve sender emphasis colors in the feed" use case from limepkg-email#590 will be solved at the product level — a "See the original email" promoted action on the feed item (in limeb-feed).

2. <limel-email-viewer> — sets a theme-following default background on the email body (using --contrast-300 / --contrast-1500 tokens that auto-invert with the app theme), and adds a small "Hard to read?" toggle that flips the surface to the opposite tone for emails whose hardcoded colors collide with the default. A limel-tooltip on the button explains the why. The email content itself is never modified — only the surface behind it changes.

See #4065 for the full reasoning, including the approaches we considered and rejected (auto-detection of color-scheme hints, contrast-aware DOM walking from #4060, etc.).

Test plan

• In Lime CRM light mode, open the activity feed and verify email snippets render in the theme's text color (no longer invisible-on-light).
• In Lime CRM dark mode, open the activity feed and verify the same — text inherits the theme color.
• Open an .eml file via <limel-file-viewer> while the app is in light mode. Verify the email body has a light off-white surface.
• Repeat in dark mode. Verify the email body has a dark off-black surface.
• Open an email composed in dark mode (white text) while the app is in light mode. Verify the white text is hard to see, then click "Hard to read?" — the surface should flip to dark and the white text becomes readable.
• Open an email composed in light mode (black text) while the app is in dark mode. Verify the same in reverse.
• Hover/focus the toggle button and verify the tooltip appears with the longer helper text.
• Switch between two emails and verify the toggle resets to default for the new email.
• Verify the toggle button is sticky at the top of the body when scrolling a long email.
• Verify the toggle button doesn't block clicks on email content underneath it (pointer-events: none on the wrapper).

Browsers tested

Windows:

• Chrome
• Edge
• Firefox

macOS:

• Chrome
• Firefox
• Safari

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added email surface background toggle to switch between light and dark backgrounds for improved readability.

• Style

  • Removed inline color and background-color styling support from markdown content for consistency.

• Localization

  • Added translations for the new email surface toggle across 8 languages: Danish, German, English, Finnish, French, Dutch, Norwegian, and Swedish.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR adds a light/dark surface-background toggle to the email viewer for readability when sender-defined text colors clash with the app theme, while removing inline color preservation from markdown sanitization to keep feed snippets readable in all themes.

Changes

Email Readability Feature

Layer / File(s)	Summary
Markdown Sanitization Policy src/components/markdown/allowed-css-properties.ts, src/components/markdown/examples/markdown-html.tsx, src/components/markdown/markdown-parser.spec.ts, src/components/markdown/sanitize-style.spec.ts	Removes 'color' and 'background-color' from the allowed inline CSS property list. Markdown-rendered content (feeds, comments) no longer preserves sender-defined text colors; test cases updated to expect font-weight and font-style only, with color properties stripped.
Email Viewer State & Methods src/components/email-viewer/email-viewer.tsx	Adds surfaceBackgroundState and toggleButtonId instance fields. Resets surface state when a new email opens. Adds getBodyClassNames() to compute dynamic CSS classes, renderSurfaceToggle() to conditionally show a dark/light toggle button with tooltip, and toggleSurfaceBackground() handler to invert the state. Both HTML and plain-text body rendering now use computed class names.
Email Viewer Styling & Layout src/components/email-viewer/email-viewer.scss	Adds isolation: isolate to host element stacking context. Adjusts date pill z-index. Reworks .body with overflow-y: auto, light background (color-scheme: light), and .toggled-surface-background variant with dark background (color-scheme: dark). Introduces .toggle-dark-light sticky bar with proper pointer-event wiring, button focus/hover styles, and icon sizing.
Translations src/translations/{en,da,de,fi,fr,nl,no,sv}.ts	Adds three translation keys per language: file-viewer.email.surface.toggle (label), file-viewer.email.surface.toggle.tooltip.label, and file-viewer.email.surface.toggle.tooltip.helper (multi-line explanation of contrast switching).
Test Mocking src/components/file-viewer/file-viewer.spec.tsx	Updates .eml fetch mock to conditionally detect the target email URL and return its payload via arrayBuffer(), while handling other incidental fetches with benign empty responses.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• Lundalogik/lime-elements#3783: Modifies the same email viewer code paths (email-viewer.scss, email-viewer.tsx, translations, file-viewer tests).
• Lundalogik/lime-elements#3909: Overlaps on email-viewer.scss changes to the date pill positioning and z-index adjustments.
• Lundalogik/lime-elements#3910: Related fetch mocking changes for .eml test cases in file-viewer.spec.tsx.

Suggested labels

visual design, accessibility

Suggested reviewers

• adrianschmidt
• jgroth

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main changes: removing color/background-color from markdown allowlist and adding theme-following surface with toggle in email viewer, without modifying email content.
Linked Issues check	✅ Passed	The PR successfully implements all primary objectives from issue #4065: removes color/background-color from markdown allowlist [#4065 Part 1], adds theme-following surface with 'Hard to read?' toggle to email viewer [#4065 Part 2], and never modifies email content.
Out of Scope Changes check	✅ Passed	All changes are directly tied to the PR objectives: markdown CSS removal, email viewer styling/toggle, file-viewer test updates for new SVG fetches, and translation additions for the new toggle UI.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/email-color-readability

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

Documentation has been published to https://lundalogik.github.io/lime-elements/versions/PR-4066/

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/components/markdown/sanitize-style.spec.ts (1)

156-192: 🧹 Nitpick | 🔵 Trivial | 💤 Low value

normalizeBackgroundColor background-color conversion branch is a pipeline no-op — consider simplifying the implementation.

normalizeBackgroundColor converts background: <valid-color> → background-color: <color>, but since background-color is not in the allowedCssProperties list, the converted value is immediately filtered out downstream. The function provides security value by always removing the background property (preventing image injection), but the background-color addition for valid colors has no observable effect on the rendered output.

The tests correctly verify the function contract in isolation. Consider simplifying sanitize-style.ts to unconditionally strip background rather than conditionally re-adding background-color, which would avoid confusion for future maintainers.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/markdown/sanitize-style.spec.ts` around lines 156 - 192,
normalizeBackgroundColor currently converts valid background values to
background-color then relies on downstream filtering, which is a no-op; simplify
by making normalizeBackgroundColor always remove the background property
(prevent image injection) and never add a background-color key. Update the
function (normalizeBackgroundColor) to return a shallow copy of the input
without the background property, preserve other properties unchanged, and
maintain immutability (do not mutate the original object); leave tests as-is or
adjust expectations if they assert background-color presence.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/components/email-viewer/email-viewer.scss`:
- Around line 203-212: The hardcoded color-scheme values in the email viewer
rules mismatch the auto-inverting contrast tokens; update the rules so
color-scheme follows the actual surface tone by using prefers-color-scheme media
queries and mirroring the toggled state: keep the base selector (the block with
padding/overflow/background-color) with color-scheme: light by default and set
.toggled-surface-background to color-scheme: dark, then add `@media`
(prefers-color-scheme: dark) that flips those values (base -> color-scheme:
dark, .toggled-surface-background -> color-scheme: light) so browser-native UI
and system color keywords resolve correctly in both app themes.

---

Outside diff comments:
In `@src/components/markdown/sanitize-style.spec.ts`:
- Around line 156-192: normalizeBackgroundColor currently converts valid
background values to background-color then relies on downstream filtering, which
is a no-op; simplify by making normalizeBackgroundColor always remove the
background property (prevent image injection) and never add a background-color
key. Update the function (normalizeBackgroundColor) to return a shallow copy of
the input without the background property, preserve other properties unchanged,
and maintain immutability (do not mutate the original object); leave tests as-is
or adjust expectations if they assert background-color presence.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 833b6f94-3aeb-44de-9adc-78d6de7eed28

📥 Commits

Reviewing files that changed from the base of the PR and between c3e14c2 and 6d4c834.

📒 Files selected for processing (15)

• src/components/email-viewer/email-viewer.scss
• src/components/email-viewer/email-viewer.tsx
• src/components/file-viewer/file-viewer.spec.tsx
• src/components/markdown/allowed-css-properties.ts
• src/components/markdown/examples/markdown-html.tsx
• src/components/markdown/markdown-parser.spec.ts
• src/components/markdown/sanitize-style.spec.ts
• src/translations/da.ts
• src/translations/de.ts
• src/translations/en.ts
• src/translations/fi.ts
• src/translations/fr.ts
• src/translations/nl.ts
• src/translations/no.ts
• src/translations/sv.ts

💤 Files with no reviewable changes (1)

• src/components/markdown/allowed-css-properties.ts

[FredrikWallstrom]

Hey Kia, jumping in on the design direction here. The email-viewer half of this PR (theme-following surface + "Hard to read?" toggle) feels right to me, and I'd accept it independently of where the markdown discussion lands. What I want to question is part 1 — dropping color and background-color from <limel-markdown>'s sanitizer.

The framing of part 1 as "removing email-shaped responsibility from a generic component" is what I want to question. Inline color in HTML isn't an email-specific feature — it's a basic markdown/HTML feature. GitHub renders it. CommonMark with raw HTML enabled renders it. Any markdown surface that allows <span style="..."> will respect colors. <limel-markdown> is a public component in @limetech/lime-elements, used outside Lime CRM by partners and other internal products. Removing colors from the inline-style allowlist isn't undoing a special case — it's removing a generic feature that any consumer might be relying on. Status messages with colored severity, formatted docs, inline emphasis, anything that uses <span style="color: ..."> in their markdown will silently change when we ship this. That's a public API change that goes beyond fixing the email-readability issue, and we have no good visibility into who outside this repo depends on it.

The other part of the framing I'd question is treating color use as a deliberate, flagged thing — "see my responses in red." That's one shape of color use, but plenty of it isn't deliberate at all. People highlight a name in blue, color a date red, paste an action item in green, use a colored heading from a template — without thinking of it as "color emphasis." Senders don't consciously decide "I'm using color here," they just write that way, and recipients read it that way. That's what #590 was actually about, even if the phrasing made it sound niche — it's not really one feature request, it's a pattern in how people compose emails. When we strip color universally, we lose all of that. For most messages the loss is invisible. For some it's load-bearing in a way that only becomes obvious after it's gone. The "View original email" promoted action is a fine fallback when someone consciously wants to see the rendered original, but it doesn't help in the case where someone is just glancing through their feed and would naturally read the colored phrase inline if it were there.

Separate point — the writeup undersells how slimmed-down the contrast approach got. The cost it lists (DOM walk, contrast calc, MutationObserver, media-query listener, per-element bookkeeping for restoration) isn't what's in the merged code. Those got pruned earlier in the discussion. The live version is roughly: post-render, querySelectorAll('[style*="color"]'), compute contrast, strip declarations under 3:1. No observers, no listeners, no restoration data — theme change requires a page reload, and that's fine. That's a meaningfully smaller picture than the writeup paints, and the trade-off looks different when weighed against the actual size.

What I find compelling about the contrast approach is that it intervenes only in cases where leaving the content alone would already produce something unreadable. For the organic color use we've both seen — colored names, dates, highlights, headings — those mostly meet 3:1 against a normal CRM surface, so they pass through untouched. The walk kicks in only for the genuinely-broken combinations: a sender composing in dark mode whose color: white lands on our light feed, or Outlook's auto-injected near-black landing on our dark feed. Those are cases where doing nothing is also destroying the user's reading — silently, with no recourse short of clicking through to the viewer.

So I'd reframe the part 1 trade-off: it's not "preserve sender colors at the cost of architectural purity vs. drop them and keep the renderer clean." It's "drop a generic markdown feature from a public component (breaking external consumers as collateral) and lose the organic-color reading that #590 was actually about, vs. keep the public API as-is and add ~50 lines of CRM-specific logic in lime-crm-building-blocks to handle the cases where ignoring contrast would already be unreadable."

[Kiarokh]

Thanks @FredrikWallstrom

Regarding dropping or keeping colors in markdown. You're right that we have consumers that might be using it as is. I'm not sure how serious these use cases are, but perhaps that's a bad idea to suddenly remove the functionality. So I'm not insisting on doing so. But if I was gonna redesign this component again, I'd probably decide to exclude colors via sanitizer again 😄 It feels, for a markdown renderer component which is going to appear on Web Application UIs (as a natural part of the UI itself), that's an overkill. Imagine if Slack, Teams, Github, etc… all rendered text with color (with any user-chosen color) everywhere. That would open the Pandora's box, specially with dark mode / light mode support of most app UIs these days (as it has already done in our case). I think it would have been better to keep that door shut from the beginning.

But if we drop this, and instead go with the contrast solution that you proposed, then the other commit in the current PR will also become meaningless. We should rather use your solution in both components in that case. That'll give a much better out of the box UX at the end.

I'm not sure how much does it cost when it comes to performance. Probably nothing noticable if it is only one instance doing it. But should we consider to add it under a prop? In the web client, we could have lots of limel-markdown in the DOM simultaneously. For example, in the Table or Kanban view, each row or item has markdown inbuilt. They are also used in other parts of the UI. Imagine to have the feed in the sidepanel opend in the table view. So on a page, we could potentially have hundreds of them. I strongly feel this contrast checks are unnecessary in majority of cases, and the only place they make sense is in the feed and almost-ONLY for emails scenarios. So if we go that way, I'd like to have that under a prop for limel-markdown