fix(emails): stop the newsletter's design living in its own mj-head - #3299
Conversation
The whole design sat in one mj-attributes block: the text colours, the section backgrounds, and every padding. Anything that takes the body without the head drops it, and MJML falls back to its own defaults. That is black text on the plum canvas at 1.6:1, no cards, and 20px of padding everywhere, in a layout that still compiles, still sends, and still looks like an email. Reproduced by stripping the block: 19 elements rendered #000000 on #270035. Every colour, size, weight and space is now written on the element that uses it. No mj-attributes, no mj-class, and link colours on each anchor rather than in mj-style, which several clients drop anyway. The head keeps the title, the preview text, the web font, and one media query for the gap between stacked cards, whose entire blast radius is that gap. Deleting the whole head now changes nothing about colour or size. Three guards, because this failure is silent: test_nothing_visual_depends_on_the_head rejects the two constructs. test_every_styled_element_carries_its_own_styling requires each mj-text and mj-button to name its own colour and type, since an element that sets no colour still inherits MJML's black. The build compiles a second time with mj-head deleted and fails on one occurrence of #000000. Only mjml can say whether it still renders; a static guard can only look for known causes. Design fixes found while re-reading the render: The type ramp was squashed. The hero, the card heading and the item headings sat at 30/20/18px, so a section heading barely outranked the paragraph above it. Now 36/24/18 off --text-4xl and --text-2xl. Body width 640 -> 600px. 600 is the width Outlook's renderer is safe at, and at 640 the card's copy ran to about 95 characters a line. A short accent rule opens the hero, echoed by the card's divider, so the email has one structural device rather than none. Measured rather than reasoned about this time: axe-core's color-contrast rule, the same one PageSpeed uses, over the rendered DOM at 760px and 412px, with and without the head. 27 text elements, zero below 4.5:1, worst 5.93:1. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
4bff9af to
597caab
Compare
There was a problem hiding this comment.
Pull request overview
This PR hardens the Anthias newsletter email pipeline by making the MJML template fully self-styling (so it renders correctly even if mj-head is stripped) and by fixing CI packaging/deploy behavior to avoid shipping the wrong artifacts or triggering duplicate Pages deploys.
Changes:
- Refactors
emails/newsletter.mjmlto inline all visual styles on each element (nomj-attributes/mj-class), with accompanying README updates. - Extends
tests/test_email_tokens.pyto validate the new font token usage and to add guards preventing reintroduction of head-dependent styling. - Updates GitHub Actions workflows to (1) bundle the MJML source (not compiled HTML) and (2) prevent duplicate website deployments for the same commit SHA.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tests/test_email_tokens.py |
Expands token scanning/normalization and adds template guards to prevent silent styling regressions. |
emails/README.md |
Updates contributor/operator documentation to reflect the new “MJML source is the artifact” approach and head-independence rules. |
emails/newsletter.mjml |
Moves styling from mj-head to per-element attributes and updates layout/typography accordingly. |
.github/workflows/deploy-website.yaml |
Adds a triage job and gating to avoid duplicate Pages deploys caused by overlapping triggers. |
.github/workflows/build-email.yaml |
Ships newsletter.mjml + README as the artifact, keeps compilation as validation, and adds a headless-compile guard. |
Suppressed comments (2)
tests/test_email_tokens.py:206
_STYLED_TAGcurrently requires at least one attribute (<mj-text\s...>), so a newly-added bare<mj-text>/<mj-button>tag would bypasstest_every_styled_element_carries_its_own_stylingentirely (even though that's the exact case the test is meant to catch).
# Elements that paint text, and the attributes each must set on
# itself for the design to survive without the head.
_STYLED_TAG = re.compile(r'<(mj-text|mj-button)\s[^>]*>', re.DOTALL)
SELF_CONTAINED = ('color', 'font-family', 'font-size', 'line-height')
tests/test_email_tokens.py:253
- The attribute-presence check only looks for
attr="and will treatattr='...'as missing. Using a small regex here makes the guard resilient to quote-style changes in the MJML.
offenders = []
for tag in _STYLED_TAG.finditer(_template()):
missing = [a for a in SELF_CONTAINED if f'{a}="' not in tag[0]]
if missing:
head = ' '.join(tag[0].split())[:58]
offenders.append(f' <{tag[1]}> missing {missing}: {head}')
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3299 +/- ##
=========================================
Coverage ? 90.31%
=========================================
Files ? 85
Lines ? 9942
Branches ? 1098
=========================================
Hits ? 8979
Misses ? 709
Partials ? 254 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (3)
tests/test_email_tokens.py:85
- Like
_STYLED_TAG, this pattern requires at least one attribute (\s[^>]*). If a<mj-text>ever loses its attributes, this test will no longer see it at all (and therefore can’t report missingline-height/ mismatched ramp step). Matching<mj-text>with or without attributes keeps the guard effective.
# Tags that set paragraph type, and so owe a leading as well as a size.
#
# mj-button is deliberately not one: its label is a single line in a box
# whose height comes from inner-padding, so a paragraph leading would
# only change the box. Neither is the code chip, which is inline and has
# to keep the leading of the paragraph around it.
_TYPE_TAG = re.compile(r'<(mj-text)\s[^>]*>', re.DOTALL)
_ATTR_SIZE = re.compile(r'font-size="([\d.]+)px"')
_ATTR_LEADING = re.compile(r'line-height="([\d.]+)"')
.github/workflows/build-email.yaml:116
- The grep used to detect MJML’s fallback text color will also match properties like
background-color:#000000(because it contains the substringcolor:#000000), and it may misscolor: #000000if MJML outputs a space after the colon. That can create false positives/negatives in CI. Use an extended regex that matches thecolor:property specifically with optional whitespace, and avoids*-colorproperties.
leaked=$(grep -c 'color:#000000' /tmp/headless.html || true)
tests/test_email_tokens.py:206
- This regex only matches
<mj-text ...>/<mj-button ...>tags that already have at least one attribute (\s[^>]*). If a future edit introduces a bare<mj-text>or<mj-button>with no attributes (exactly the case this guard is trying to prevent), it won’t be matched and the test will silently skip it. Adjust the pattern to also match tags with zero attributes.
# Elements that paint text, and the attributes each must set on
# itself for the design to survive without the head.
_STYLED_TAG = re.compile(r'<(mj-text|mj-button)\s[^>]*>', re.DOTALL)
SELF_CONTAINED = ('color', 'font-family', 'font-size', 'line-height')
Six statements in the comments were asserted rather than checked. The behaviour they describe was verified by compiling; the explanations were not, and a wrong explanation in a file this heavily commented is worse than no comment, because it is what the next editor will act on. Checked against MJML's documented component defaults and against the compiled output: The divider claim was backwards. It said mj-divider was used BECAUSE a divider compiles to a table and a border would not survive Word. mj-divider emits a <p> with border-top, which is exactly the thing it supposedly avoids. The real reason to use the component is the mso conditional table it emits alongside, carrying the same border-top. Right conclusion, invented reason. "20px of default padding everywhere" was loose. It is 20px 0 on mj-section and 10px 25px on mj-text, and the type falls to 13px on a line-height of 1 in Ubuntu, which is worth naming because it is more of the design going than the padding alone. 600px was justified by Outlook's safe width. It is MJML's own documented default for mj-body, which is checkable. The 480px breakpoint is MJML's default, now stated as verified: compiling without the mj-breakpoint tag still emits min-width:480px. "Several clients drop a <style> block" was vague. The reason link colours are inline is the same as everything else here: an inline style needs no head to have survived. The Outlook black-matte mechanism behind flattening the masthead is not something this repo can test. The defensible statement is that a flattened PNG asks no client to composite alpha at all. The default black is also no longer written as a hex, here or in prose. test_no_literal_reaches_past_the_token_table caught the first draft of this commit doing exactly that, and treating every hex in the file as a colour the email paints is worth more than quoting one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (1)
.github/workflows/build-email.yaml:116
- The default-color leak check is currently brittle and can be both a false negative (MJML may emit
color: #000000with whitespace) and a false positive (background-color:#000000contains the substringcolor:#000000). Consider matching thecolor:property with optional whitespace and ensuring it’s not part of*-color.
bunx mjml@4.15.3 /tmp/headless.mjml -o /tmp/headless.html \
--config.validationLevel=strict
leaked=$(grep -c 'color:#000000' /tmp/headless.html || true)
if [ "${leaked:-0}" -gt 0 ]; then
… guards All three were real, and each was verified by causing the failure it should catch before and after. A bare <mj-text> or <mj-button> was invisible to both guards. The patterns required `\s[^>]*` after the tag name, so a tag with no attributes did not match, and a tag with no attributes is exactly the one inheriting every MJML default. The guard was blind to its own worst case. Now `(?=[\s>])`. mj-class detection was an exact substring, so `mj-class = "x"` walked past the one test meant to stop it. XML tolerates whitespace around the '='; the check now does too. The CI leak grep was wrong in both directions. `color:#000000` is a substring of `background-color:#000000`, so a black-filled section would have been reported as a text failure, and it would have missed `color: #000000` if MJML ever emits the space. Both reproduced. Now an extended regex for the `color:` property with optional whitespace, excluding `*-color`, checked against four crafted cases plus the real regressed output, which it still counts at 19. The font is checked alongside it, because Ubuntu cannot false positive: nothing in the template mentions it, so one occurrence is an element that took MJML's default stack. That is the same failure showing up in the half of the design that is type rather than colour. Both greps also take `|| true`. grep exits 1 when it finds nothing, which is the passing case here; the runner's default `bash -e` has no pipefail so the pipeline survives today, but adding `shell: bash` would turn a clean result into a failed job. Found by running the workflow locally under pipefail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (2)
.github/workflows/build-email.yaml:135
- The headless-compile guard won’t detect
color:#000000if it appears at the start of a line, because the regex requires a preceding character ([^-[:alnum:]]). This can let a regression slip through depending on how MJML formats the output.
black=$(grep -Eio '[^-[:alnum:]]color:[[:space:]]*#000000' \
/tmp/headless.html | wc -l || true)
tests/test_email_tokens.py:260
test_every_styled_element_carries_its_own_stylingchecks for required attributes via simple substrings likecolor=", but MJML/XML allows whitespace around=(and single quotes). This can cause false failures if the template is reformatted without changing meaning.
offenders = []
for tag in _STYLED_TAG.finditer(_template()):
missing = [a for a in SELF_CONTAINED if f'{a}="' not in tag[0]]
if missing:
head = ' '.join(tag[0].split())[:58]
offenders.append(f' <{tag[1]}> missing {missing}: {head}')
assert not offenders, (
…s colour Second review pass, and the attribute check was wrong in the direction that matters. It asked whether `color="` appeared in the tag, and `color="` is a substring of `background-color="`. An mj-button that lost its own colour but kept its fill therefore satisfied the guard built to catch exactly that, and would have rendered MJML's default black label on the yellow fill. Reproduced by deleting the button's color attribute: the check passed. It now fails. The same test could also report a tag as missing something it sets, since XML allows whitespace around the '=' and either quote style, so a reformat with no change of meaning would have failed the build. Both directions are closed by one regex with a negative lookbehind for `-` and word characters, and both were checked. The CI leak regex needed the `^` arm. `[^-[:alnum:]]` has to consume a character, so a declaration starting a line was missed, and how MJML wraps its output is not our decision to depend on. Still counts the real regressed output at 19. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|



Issues Fixed
Follow-up to the newsletter template. Two things: the CI bundle was
shipping the wrong file, and merging it exposed a duplicate Pages deploy.
Description
The template was broken, and this is the fix for it.
The whole design sat in one
mj-attributesblock inmj-head: the textcolours, the section backgrounds, and every padding. Anything that takes
the body without the head drops it, and MJML falls back to its own
defaults. Reproduced by stripping the block: 19 elements render
#000000on#270035, which is 1.6:1, with no cards and 20px ofdefault padding everywhere. It still compiles, still sends, and still
looks like an email, which is exactly what made it ship.
Every colour, size, weight and space is now written on the element that
uses it. No
mj-attributes, nomj-class, and link colours on eachanchor rather than in
mj-style, which several clients drop anyway. Thehead keeps the title, preview text, web font, and one media query for the
gap between stacked cards, whose entire blast radius is that gap.
Deleting the whole head now changes nothing about colour or size.
Three guards, because the failure is silent:
test_nothing_visual_depends_on_the_headrejects anmj-attributesblock or an
mj-classreference.test_every_styled_element_carries_its_own_stylingrequires eachmj-textandmj-buttonto name its owncolor,font-family,font-sizeandline-height. Removing the block is only half of it,since an element that sets no colour still inherits MJML's black.
mj-headdeleted and fails on asingle
#000000. Only mjml can say whether the result still renders; astatic guard can only look for the causes already known.
Each was checked by causing the failure it exists for.
Design fixes found while re-reading the render:
30/20/18px, so a section heading barely outranked the paragraph above
it. Now 36/24/18, off
--text-4xland--text-2xl.and at 640 the card's copy ran to about 95 characters a line.
the email has one structural device rather than none.
On contrast: the previous round checked design tokens on paper, which
is not the same as checking the email. This one runs axe-core's
color-contrastrule, the same rule PageSpeed uses, over the renderedDOM at 760px and 412px, with and without the head: 27 text elements, zero
below 4.5:1, worst pair 5.93:1.
The bundle ships the
.mjml, not the compiled HTML.Mailjet reads MJML directly, so the source is the file to hand over, and
it is the only version anyone can still edit. Nobody is going to change a
headline inside 35 KB of nested tables, which is how the sent mail and the
repo stop being the same email. The bundle is now
newsletter.mjmlplusthe README, and the packaged template is byte-identical to the one in the
repo.
The compile still runs, as a check rather than as a deliverable, so strict
validation and the Gmail clipping limit still gate the workflow.
The masthead PNG is out of the bundle and out of the trigger paths. The
template references it by URL from the site, so the
.mjmlis already thewhole email, and a copy of the image alongside it was only ever an
invitation to re-host something that did not need re-hosting.
On inlining the masthead: it is deliberately not inlined, and the file
now records why. A base64
data:URI would make the templateself-contained offline, but Gmail, Outlook and Yahoo all refuse
data:image URIs, so the masthead would be a broken image at the top of the
newsletter for most of the list. Apple Mail and Thunderbird do render
them, which is exactly enough to make the problem invisible in testing.
The form of embedding that works everywhere is a
cid:inlineattachment, and that is send-side setup in Mailjet rather than something
the template can carry.
One commit no longer produces two competing Pages deploys.
A commit touching both
website/and application code matches thisworkflow's push paths and the schema workflow's, so it fires the deploy
twice for one SHA: once on push, and again about ninety seconds later when
Generate OpenAPI Schema finishes. Both build the same site and both want
the
pagesconcurrency group, so the later cancels the earlier and leavesa cancelled run against master that reads as a broken deploy on a green
commit. The newsletter merge was the first commit to do both; the site
itself deployed fine, from the surviving run.
A triage job now stands the push copy down when a schema run already
exists for the same SHA. The
workflow_runcopy is the one worth keeping:it is the only one whose Fetch OpenAPI schema step can find a schema built
from that commit, where the push copy runs before the schema workflow has
finished and falls back to the previous run's, publishing an API page one
commit behind.
It asks the Actions API rather than inferring from the changed paths. The
schema workflow's filter is a long list of exclusions, and a second copy
of it here would go stale the first time that list was edited, silently
and in whichever direction is worse.
Concurrency is untouched, so
cancel-in-progressstill stops a slow olderrun republishing over a newer one.
One consequence worth stating, and it is commented in the file: a
commit whose schema workflow fails now does not deploy at all, because
the push copy has stood down and the existing gate stops the
workflow_runcopy. That is the intended trade. Shipping a stale APIreference is the failure this chain exists to prevent, and a commit that
broke the schema workflow is getting fixed or reverted anyway.
Verification
newsletter.mjmlandREADME.md, anddiffconfirms the packagedtemplate is byte-identical to the repo's
03a8ae72, the commit that actually produced the cancelled run, returnsproceed=false; the website-only0467a783returnsproceed=trueafterpolling; a
workflow_runevent returnsproceed=trueimmediatelyworkflow_runcopy starts, so the run that stands down finishes beforeanything could cancel it
actionlintclean,ruff checkclean,pytest -m "not integration"unaffected
Checklist
Not applicable: CI and a marketing asset, neither ships to devices.
Not applicable, same reason.
emails/README.mdupdated for the new bundle contents, and bothworkflows carry the reasoning inline.