feature(dark-mode) added dark mode support - #3068
Open
dazgreer wants to merge 59 commits into
Open
Conversation
- mj-title throws an error if missing or empty - optional Outlook and dark mode support - add space to preview text - comprehensive tidy of HTML to reduce code bloat - updated tests and docs
mjml-core - Updated Prettier to use single quotes inside of CSS - Broken out the accordion CSS into its own style block as it was breaking other CSS and causing non functional components, e.g. carousel in Gmail and updated test mjml-section - fixed issues caused by removing background-size and background-repeat as default attributes whereby the default values were used to determine VML settings. Created automated test - removed multiple declarations of the background color and concatenated two divs that were split because of this - updated table to use role=“none”
…d-tidy - Merged cssnano-preset-lite improvements and normalizeMinifyCssOption helper from fix/replace-html-minifier - Adopted Mocha-based *.test.js pattern for mjml-core tests (replacing old *-test.js runner) - Preserved accordion-style and dark-mode skeleton tests from feature branch in skeleton.test.js
cheerio is a webpack external for the browser bundle, so calling load() inside mergeHeadStyleBlocks() crashed the smoke test with 'Cannot read properties of undefined (reading load)'. Replace the cheerio DOM walk with a plain character scanner that tokenises <head> content into plain-style / whitespace / other segments and merges consecutive eligible <style> blocks inline. The merged output is identical; the import of load from cheerio is retained for the mj-html-attributes feature at the call-site that is already correctly guarded by an isEmpty() check. Also fixes no-continue lint errors by using an 'advanced' flag instead of continue statements in the tokenizer loop.
…t tests - extracted mergeHeadStyleBlocks into its own helper module and imported - added test file covering 29 unit tests.
- added support-dark-mode switch to include relevant meta and CSS - added dark- prefixed classes to aid with dark mode changes for colors and images - additional support for image changes in various Outlook clients - added validation for new attributes - added automated testing and updated documentation
There was a problem hiding this comment.
Pull request overview
Expands and standardizes MJML dark-mode support across core rendering and multiple body components, including shared CSS rule emission, optional Outlook-specific dark image handling, and new validator warnings for missing root dark-mode opt-in and missing/empty mj-title.
Changes:
- Added shared dark-mode infrastructure in
mjml-core(rule registration, singleprefers-color-scheme: darkemission path, head/style block merging, Outlook utilities). - Implemented/normalized
dark-*attributes across many components (text/table/spacer/social/navbar/group/divider/body/accordion/wrapper, plus docs and smoke/integration tests). - Added validator rules + tests for missing/empty
mj-titleand usingdark-*attrs withoutsupport-dark-mode="true"on<mjml>.
Reviewed changes
Copilot reviewed 89 out of 89 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/mjml/test/wrapper-dark-smoke.test.js | Wrapper dark-mode smoke coverage (bg color/image + coalescing). |
| packages/mjml/test/validator-title.test.js | Tests for new mj-title validation warnings. |
| packages/mjml/test/utils.js | Safer extractStyle helper for tests. |
| packages/mjml/test/text-dark-color.test.js | mj-text dark color/container bg tests + rule grouping assertions. |
| packages/mjml/test/tableWidth.test.js | Adjusts width assertions (handles auto => omitted). |
| packages/mjml/test/table-dark-color-border-container-background-color.test.js | mj-table dark color/border/container bg tests. |
| packages/mjml/test/spacer-dark-container-background-color.test.js | mj-spacer dark container bg tests. |
| packages/mjml/test/social-dark-src-head-style.test.js | Outlook dark-src head-style behavior tests for mj-social-element. |
| packages/mjml/test/social-dark-color-background-color.test.js | mj-social / mj-social-element dark color/bg inheritance tests. |
| packages/mjml/test/section-dark-background-url.test.js | mj-section dark background-url tests. |
| packages/mjml/test/section-background-url-no-background-size.test.js | Regression test: section bg-url without bg-size + beautify quoting. |
| packages/mjml/test/navbar-dark-colors.test.js | mj-navbar/link dark color and class placement tests. |
| packages/mjml/test/image-dark-src-head-style.test.js | Outlook dark-src head-style emission tests for mj-image. |
| packages/mjml/test/image-dark-border-container-background-color.test.js | mj-image dark border/container bg behavior tests. |
| packages/mjml/test/hero-dark-background-url.test.js | mj-hero dark background-url tests. |
| packages/mjml/test/hero-dark-background-color.test.js | mj-hero dark bg + dark inner bg tests and class placement. |
| packages/mjml/test/group-dark-background-color.test.js | mj-group dark background-color tests. |
| packages/mjml/test/divider-dark-border-container-color.test.js | mj-divider dark border/container bg tests. |
| packages/mjml/test/carousel-dark-src.test.js | mj-carousel dark sources for images/thumbnails/icons tests. |
| packages/mjml/test/carousel-dark-colors.test.js | mj-carousel dark container/bg + thumbnail border override tests. |
| packages/mjml/test/button-dark-color-background-border-container-background-color.test.js | mj-button dark color/bg/border/container bg tests. |
| packages/mjml/test/body-dark-background-color.test.js | mj-body dark background + coalesced rule emission tests. |
| packages/mjml/test/accordion-dark-colors.test.js | mj-accordion* dark color/background/border/icon tests. |
| packages/mjml-wrapper/src/index.js | Refactors MSO wrappers using shared msoConditionalTag. |
| packages/mjml-wrapper/README.md | Documents wrapper dark-* attributes. |
| packages/mjml-validator/src/rules/validTag.js | Allows mjml as a validator-permitted tag. |
| packages/mjml-validator/src/rules/requireSupportDarkModeForDarkSrc.js | New validator rule warning when dark-* used without root opt-in. |
| packages/mjml-validator/src/rules/requiredTitle.js | New validator rule warning for missing/empty mj-title. |
| packages/mjml-validator/src/MJMLRulesCollection.js | Registers new validator rules. |
| packages/mjml-validator/src/index.js | Enables validation traversal for <mjml> (no skip list). |
| packages/mjml-text/src/index.js | Adds dark-color / dark-container-background-color support + shared head style emission. |
| packages/mjml-text/README.md | Documents mj-text dark-mode attributes and note. |
| packages/mjml-table/src/index.js | Adds dark-* support; tweaks width/style emission. |
| packages/mjml-table/README.md | Documents mj-table dark-mode attributes and note. |
| packages/mjml-spacer/src/index.js | Adds dark-container-background-color with shared dark-mode CSS emission. |
| packages/mjml-spacer/README.md | Documents mj-spacer dark-mode attribute and note. |
| packages/mjml-social/src/Social.js | Adds dark container/color support; refactors MSO conditionals usage. |
| packages/mjml-social/README.md | Documents dark-mode attributes and Outlook dark-image support options. |
| packages/mjml-section/README.md | Documents section dark-mode attributes and note. |
| packages/mjml-parser-xml/test/preprocessors.test.js | Updates parser test fixtures to include required mj-title. |
| packages/mjml-navbar/src/NavbarLink.js | Adds dark-color support + shared dark-mode head style emission. |
| packages/mjml-navbar/README.md | Documents navbar/link dark-mode attributes and note. |
| packages/mjml-image/README.md | Documents image dark-* attrs + Outlook dark image option and note. |
| packages/mjml-hero/README.md | Documents hero dark-* attributes and note. |
| packages/mjml-head-title/README.md | Documents new validator behavior for missing/empty mj-title. |
| packages/mjml-head-preview/src/index.js | Adds fill-space preview padding behavior. |
| packages/mjml-head-preview/README.md | Documents mj-preview new attributes. |
| packages/mjml-head-attributes/README.md | Fixes typo (“within”). |
| packages/mjml-group/src/index.js | Adds dark-background-color support + Outlook conditional refactor. |
| packages/mjml-group/README.md | Documents group dark background attribute and note. |
| packages/mjml-divider/src/index.js | Adds dark-* support and refactors divider rendering/outlook handling. |
| packages/mjml-divider/README.md | Documents divider dark-mode attributes and note. |
| packages/mjml-core/tests/skeleton.test.js | Extends skeleton tests for accordion style block + dark-mode meta tags. |
| packages/mjml-core/src/index.js | Tracks new global dark-mode state; head-style merging; output formatting changes. |
| packages/mjml-core/src/helpers/styles.js | Separates accordion head CSS into its own <style type="text/css">. |
| packages/mjml-core/src/helpers/skeleton.js | Adds opt-in dark-mode meta/CSS; restructures head markup and namespaces. |
| packages/mjml-core/src/helpers/outlookDarkMode.js | New Outlook dark-mode image/background rule registry + head emission. |
| packages/mjml-core/src/helpers/mergeOutlookConditionnals.js | Improves conditional merging logic to avoid negation edge cases. |
| packages/mjml-core/src/helpers/mergeHeadStyleBlocks.js | New helper to coalesce consecutive plain <style> blocks in <head>. |
| packages/mjml-core/src/helpers/mediaQueries.js | Adjusts media query <style> generation and optional OWA desktop forcing. |
| packages/mjml-core/src/helpers/fonts.js | Changes font import emission to use conditional-tag wrapper. |
| packages/mjml-core/src/helpers/conditionalTag.js | Updates MSO conditional formats + adds global Outlook-classic enable/disable flag. |
| packages/mjml-core/src/helpers/colorSchemeDarkMode.js | New shared dark-mode rule registry + single head <style> emission. |
| packages/mjml-core/src/createComponent.js | Avoids emitting empty class/style attributes; adds boolean attribute handling. |
| doc/components_1.md | Updates docs example + documents new <mjml> root options. |
| packages/mjml-column/README.md | Documents column dark-mode attributes and note. |
| packages/mjml-carousel/README.md | Documents carousel dark-mode attributes and Outlook image support note. |
| packages/mjml-button/README.md | Documents button dark-mode attributes and note. |
| packages/mjml-body/src/index.js | Adds dark-background-color support and shared dark-mode head style emission. |
| packages/mjml-body/README.md | Documents body dark-background-color and note. |
| packages/mjml-accordion/src/AccordionTitle.js | Adds dark bg/color + dark icon URL support; shared rule grouping. |
| packages/mjml-accordion/src/AccordionText.js | Adds dark bg/color + inherited border dark-mode support. |
| packages/mjml-accordion/src/AccordionElement.js | Adds dark background/border + dark icon attribute plumbing. |
| packages/mjml-accordion/src/Accordion.js | Adds accordion dark container/border support + shared head style emission. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ibutes - background-color applied to body tag in mj-body - background-color applied to parent table tag instead of both td tags in mj-accordion-title
dazgreer
changed the title
- now scans mj-wrapper when deciding whether to include the xmlns:v namespace. - now changes the support-outlook-classic="false" attribute to a string before comparing, correctly evaluating to false. Added automated test
…upport # Conflicts: # packages/mjml-body/src/index.js
totocap
reviewed
Apr 10, 2026
| | lang | string | adds a `lang` attribute in the `html` and `body > div` tags | `und` | | ||
| | support-dark-mode | boolean | setting to `true` will add `<meta>` tags and `:root` CSS to enable support | `false` | | ||
| | support-outlook-classic | boolean | setting to `false` will remove all Outlook specific code when compiled to HTML | `true` | | ||
| | owa | string | if set to `desktop`, this will force the desktop version for older (self-hosted) versions of Outlook.com that don't support media queries (cf. [this issue](https://github.com/mjmlio/mjml/issues/2241)) | `none` | |
Contributor
There was a problem hiding this comment.
Instead of string, should the type be desktop | none? Do we have something like that in the doc already?
Collaborator
Author
There was a problem hiding this comment.
Is this for the owa? That's not a part of these updates, it was already present in the previous version
| .moz-text-html input.mj-accordion-checkbox + * .mj-accordion-content { overflow: hidden; display: block; } | ||
| .moz-text-html input.mj-accordion-checkbox + * .mj-accordion-ico { display: none; } | ||
|
|
||
| /* prettier-ignore */ |
Contributor
There was a problem hiding this comment.
Needed? The comment will stay in the HTML, no?
Collaborator
Author
There was a problem hiding this comment.
This is part of the previous HTML changes PR but anyway, it is necessary otherwise Prettier breaks the Accordion in Gmail as it chokes on the line below.
All prettier comments are removed when compiled though here: https://github.com/mjmlio/mjml/pull/3059/changes#diff-17b00ecc0c06136aa56a592b0e9154976c812427d95ca6ce9c0fc3c4d2e05305R931
| border: this.getAttribute('border'), | ||
| 'border-bottom': 'none', | ||
| 'font-family': this.getAttribute('font-family'), | ||
| ...(this.getAttribute('border') !== 'none' && this.getAttribute('border') !== '0' && this.getAttribute('border') !== '0px' && { 'border-bottom': '0' }), |
Contributor
There was a problem hiding this comment.
Suggested change
| ...(this.getAttribute('border') !== 'none' && this.getAttribute('border') !== '0' && this.getAttribute('border') !== '0px' && { 'border-bottom': '0' }), | |
| ...(!['none', '0', '0px'].includes(this.getAttribute('border')) && { 'border-bottom': '0' }), |
| width: '100%', | ||
| 'border-bottom': this.getAttribute('border'), | ||
| 'background-color': this.getAttribute('background-color'), | ||
| ...(this.getAttribute('border') !== 'none' && this.getAttribute('border') !== '0' && this.getAttribute('border') !== '0px' && { 'border-bottom': this.getAttribute('border') }), |
| 'xml:lang': lang, | ||
| })}> | ||
| ${buildPreview(preview)} | ||
| <div${this.htmlAttributes({ |
Contributor
There was a problem hiding this comment.
I think we are missing spaces in a lot of places between the tag element and the call to ${this.htmlAttributes({..., no?
Collaborator
Author
There was a problem hiding this comment.
Double spaces were compiling in the code
| constructor(initialDatas = {}) { | ||
| super(initialDatas) | ||
| this.carouselId = genRandomHexString(16) | ||
| this.carouselId = genRandomHexString(6) |
Contributor
There was a problem hiding this comment.
Why do we moved from 16 to 6?
Collaborator
Author
There was a problem hiding this comment.
This is also from the previous HTML PR. It's to save bloat. Each carousel writes out the ID 41 times so this saves 0.41kb per carousel. 6 was chosen as still fairly conservative (1 in 16 million) whilst making a significant saving in code
- mj-title throws an error if missing or empty - optional Outlook and dark mode support - add space to preview text - comprehensive tidy of HTML to reduce code bloat - updated tests and docs
mjml-core - Updated Prettier to use single quotes inside of CSS - Broken out the accordion CSS into its own style block as it was breaking other CSS and causing non functional components, e.g. carousel in Gmail and updated test mjml-section - fixed issues caused by removing background-size and background-repeat as default attributes whereby the default values were used to determine VML settings. Created automated test - removed multiple declarations of the background color and concatenated two divs that were split because of this - updated table to use role=“none”
cheerio is a webpack external for the browser bundle, so calling load() inside mergeHeadStyleBlocks() crashed the smoke test with 'Cannot read properties of undefined (reading load)'. Replace the cheerio DOM walk with a plain character scanner that tokenises <head> content into plain-style / whitespace / other segments and merges consecutive eligible <style> blocks inline. The merged output is identical; the import of load from cheerio is retained for the mj-html-attributes feature at the call-site that is already correctly guarded by an isEmpty() check. Also fixes no-continue lint errors by using an 'advanced' flag instead of continue statements in the tokenizer loop.
…t tests - extracted mergeHeadStyleBlocks into its own helper module and imported - added test file covering 29 unit tests.
…ibutes - background-color applied to body tag in mj-body - background-color applied to parent table tag instead of both td tags in mj-accordion-title
- now scans mj-wrapper when deciding whether to include the xmlns:v namespace. - now changes the support-outlook-classic="false" attribute to a string before comparing, correctly evaluating to false. Added automated test
- Updated extraction boundaries from tbody to the raw div itself in in mj-raw - Removed fallback fonts from mj-text - Updated indentation outpu Co-authored-by: Copilot <copilot@github.com>
- Refactored expressions for simplicity - Updated printWidth value for Prettier - Removed mj-title validation and testing
…r following rebase The beautify tests were written against the old Prettier formatter. Now that mjml-core uses js-beautify, two fixtures needed updating: - "keeps raw html comment spacing": js-beautify does not insert a space after HTML comments, so expectedBeautified now matches actual output. Added skipFragmentDiffCheck: true since the fragment is unchanged by beautify (the document-level check still asserts overall reformatting). - "wraps long raw html start tags": deleted. js-beautify uses wrap_line_length: 0 by default and does not split long attribute lists across lines. The same input is already covered by the passing standalone it() "keeps long raw html start tags and attributes intact while beautifying". Also deleted a duplicate standalone it() for the comment-spacing case that searched for <tbody> as its start token. MJML never emits that tag inside an mj-raw block, so the test always failed with "Missing start token". Co-authored-by: Copilot <copilot@github.com>
Now that beautification uses js-beautify instead of prettier, prettier-ignore comments serve no purpose and should not appear in rendered output. - Removed the post-render strip of /* prettier-ignore */ and <!-- prettier-ignore --> from mjml-core - Removed /* prettier-ignore */ comments in mj-accordion and mj-carousel
…apple message formatting - Updated column-align default and inheritance from text-align as the colum-align default was overriding the user specified text-align value - removed <meta name="x-apple-disable-message-reformatting"> from skeleton as it's legacy code (iOS9 and bellow) Co-authored-by: Copilot <copilot@github.com>
dazgreer
changed the base branch from
fix/replace-html-minifier
to
feature/html-updates-and-tidy
May 22, 2026 08:27
…r following rebase The beautify tests were written against the old Prettier formatter. Now that mjml-core uses js-beautify, two fixtures needed updating: - "keeps raw html comment spacing": js-beautify does not insert a space after HTML comments, so expectedBeautified now matches actual output. Added skipFragmentDiffCheck: true since the fragment is unchanged by beautify (the document-level check still asserts overall reformatting). - "wraps long raw html start tags": deleted. js-beautify uses wrap_line_length: 0 by default and does not split long attribute lists across lines. The same input is already covered by the passing standalone it() "keeps long raw html start tags and attributes intact while beautifying". Also deleted a duplicate standalone it() for the comment-spacing case that searched for <tbody> as its start token. MJML never emits that tag inside an mj-raw block, so the test always failed with "Missing start token". Co-authored-by: Copilot <copilot@github.com>
Now that beautification uses js-beautify instead of prettier, prettier-ignore comments serve no purpose and should not appear in rendered output. - Removed the post-render strip of /* prettier-ignore */ and <!-- prettier-ignore --> from mjml-core - Removed /* prettier-ignore */ comments in mj-accordion and mj-carousel
…apple message formatting - Updated column-align default and inheritance from text-align as the colum-align default was overriding the user specified text-align value - removed <meta name="x-apple-disable-message-reformatting"> from skeleton as it's legacy code (iOS9 and bellow) Co-authored-by: Copilot <copilot@github.com>
dazgreer
force-pushed
the
feature/html-updates-and-tidy
branch
from
May 27, 2026 09:20
9ac8c4a to
09954e2
Compare
…ed meta for CSS - reintroduced mso-padding-alt for mj-button with multiline="true" to work the same as the current MJML - added function to remove 1px padding to compensate for the added border - removed <meta name="format-detection"> as it was ineffective. Replaced with CSS - Added regression tests for border comensation fix
- shorthand cover declared in background attribute was not working in Yahoo. Updated to remove and add in its own explicit background-size attribute
- updated docs due to changed column-align default in mj-section - restored default font-size to mj-image so ALT text shows when images are missing - updated mj-preview default characters based onsupport testing - restored defult background-size for mj-section - fixed issue where adding background-url to mj-section causes left padding issues in child mj-column and mj-button and added regression tests
…ormed-head regression test - added explicit bounds checks before positional indexing in head scanner - removed advanced state flag and simplify scanner control flow (lint-safe if/else) - preserved existing merge behavior for plain/attributed style blocks and comments - added regression test for malformed short token in head (<head><!</head>) to ensure no throw and unchanged output - synced compiled helper output with source changes
- restored shorthand 'cover' to fix issue with display in Outlook classic - restored background-color when background-url is set to fix display issue in Gmail/Yahoo on iOS
- requiredTitle was added and later deleted in branch feature/html-updates-and-tidy. The merge into this branch did not carry though the deletion - removed validtion rules and regression tests
- Restore dark-mode prefixed attributes (dark-background-color, dark-border-color, etc.) - Restore support-dark-mode-image attributes where applicable - Add NOTE about dark-mode attributes requiring support-dark-mode="true" on <mjml> tag - Affected components: accordion, button, carousel, column, divider, group, hero, image, navbar, section, social, spacer, table, text, wrapper These attributes were accidentally removed during the merge of feature/html-updates into feature/dark-mode-support but are essential for dark-mode functionality documentation.
dazgreer
force-pushed
the
feature/html-updates-and-tidy
branch
from
July 7, 2026 15:00
351710b to
1b4dd1d
Compare
- changed max-height and max-width to height and width which prevents the spacing characters bleeding into the email body in some email clients
…tons - only emit VML button class and button-specific mso margin rules for left-aligned mj-button instances - added section class vml only when a section emits left-aligned button VML rules - kept non-left-aligned button sections free of unnecessary VML hooks - removed mj-outlook-group-fix class emission from column output and align section gutter expectations - add regression coverage for section/button Outlook behavior and template syntax sanitization under minify with mixed multiline/property/value tokens - clean up unused import in mj-hero
Merge 5.4.0 changes including gutter feature, VML button scoping, and other improvements while preserving dark mode functionality. Resolved conflicts by combining: - Dark mode head style and class management - Gutter attribute support - VML button helpers scoped to left-aligned buttons only - Section/column context with both dark mode and button style tracking Tests: all section-button-outlook-margin, section-gutter, and template-syntax-sanitization tests passing
…ed meta for CSS - reintroduced mso-padding-alt for mj-button with multiline="true" to work the same as the current MJML - added function to remove 1px padding to compensate for the added border - removed <meta name="format-detection"> as it was ineffective. Replaced with CSS - Added regression tests for border comensation fix
Merges feature/html-updates-and-tidy improvements into feature/dark-mode-support: - Cherry-Picked Commits: - f35a14f - fix(mj-preview): updated CSS to remove space in email body - 1b4dd1d - fix(mj-section): scope Outlook VML button helpers to left-aligned buttons - Manually updates - 846bcc1 - fix(eslint): remove unused widthParser import from mjml-hero - bcc63dd - fix(mj-section): restored background-color with background-url and fix undefined background-size - 435ff0c - fix(mj-button/format-detection): reintroduced mso-padding-alt / swapped meta for CSS Plus additional changes to mjml-core, mjml-head-preview, and mjml-image that are currently staged. This commit completes the merge of feature/html-updates-and-tidy into feature/dark-mode-support with all PR #3059 changes applied and tested.
…e-support Merge PR #3059 HTML optimizations into dark-mode-support branch. Resolves merge conflicts: - mjml-carousel: Accept refactored CSS structure + fix hover selector scoping - Changed `.mj-carousel-thumbnail:hover` to `.mj-carousel-${ID}-thumbnail:hover` - Preserves dark-mode hooks and regression test compliance - All other conflicted files: Resolved by keeping feature/dark-mode-support versions - README conflicts: Resolved across 12+ component packages All 283 tests passing.
dazgreer
force-pushed
the
feature/dark-mode-support
branch
from
July 9, 2026 14:42
ef6b42f to
a56245f
Compare
…med dark attributes to match - Introduced `modifierEngine.js` in mjml-core: a generic, reusable engine for CSS modifier rules - `colorSchemeDarkMode.js` refactored to delegate to modifierEngine - Renamed all dark mode attributes across components from the `dark-X` prefix convention to an `X--dark` suffix convention aligning with the double-dash modifier pattern. - removed `icon-wrapped-url--dark` and `icon-unwrapped-url--dark` from `mj-accordion-title` allowedAttributes - mjml-parser-xml test values extended - All tests and READMEs updated to reflect new attribute naming - Validator rule updated for new attribute names
…ode use-cases - Replaced wide/narrow modifier examples with dark mode (prefers-color-scheme: dark) examples throughout the test suite so the test descriptions and assertions accurately reflect the module's intended colour-scheme dark-mode purpose.
| ${this.renderIconImage( | ||
| this.getAttribute('icon-wrapped-url'), | ||
| this.getAttribute('icon-wrapped-alt'), | ||
| this.getAttribute('icon-wrapped-url--dark'), |
Collaborator
Author
There was a problem hiding this comment.
This is incorrect as neither icon-wrapped-alt or icon-unwrapped-alt are default attributes for mj-accordion-title.
So, I've updated AccordionElement.js so that resolved dark icon URLs are now passed via getChildContext() instead of the generic attribute cascade.
AccordionTitle.js now reads these values from this.context rather than this.getAttribute()
Updated docs to remove the incorrectly documented attributes
Moved the existing regression testing to set the dark icon attributes on accordionAttrs
Added a test confirming mj-accordion-element can still override the inherited value.
Added a test confirming attributes set directly on mj-accordion-title are now correctly ignored and flagged as illegal by the validator.
Comment on lines
+192
to
+195
| @media (prefers-color-scheme: dark) { | ||
| .${backgroundImage} { background-image: ${backgroundImageCssValue} !important; } | ||
| } | ||
| `) |
Collaborator
Author
There was a problem hiding this comment.
I've updated to remove registerDarkBackgroundImageClass which reused the unrelated outlookDarkModeImageCount counter and emitted its own per-instance media query.
background-url--dark is now registered via the shared registerDarkModeRule, joining the same darkModeRules list as background-color/border rules.
componentHeadStyle no longer builds a manual @media block for the image; it just triggers emitDarkModeHeadStyle, which emits once and coalesces all registered rules which fixies mj-wrapper too since it extends MjSection.
I've also updated 3 assertions in section-dark-background-url.test.js and 1 in wrapper-dark-smoke.test.js to expect the shared mj-dark-N class and a single coalesced @media block.
And I've added a new test asserting a section's background-url--dark rule coalesces into the same block as a different section's background-color--dark rule.
Comment on lines
+91
to
+94
| @media (prefers-color-scheme: dark) { | ||
| .${backgroundImageClass} { background-image: ${backgroundImageCssValue} !important; } | ||
| } | ||
| `) |
Collaborator
Author
There was a problem hiding this comment.
I've removed registerDarkBackgroundImageClass and its separate @media block emission. background-url--dark is now registered via registerDarkModeRule, joining the shared darkModeRules list alongside background-color--dark/inner-background-color--dark, and componentHeadStyle just triggers the shared emitDarkModeHeadStyle.
Updated stale assertions in hero-dark-background-url.test.js and hero-dark-background-color.test.js that expected the old mj-dark-image- prefix.
Added regression tests to validate
Comment on lines
+266
to
+270
| @media (prefers-color-scheme: dark) { | ||
| ${selectedThumbnailSelectors} { | ||
| border-color: ${darkSelectedBorderColor} !important; | ||
| } | ||
| } |
Collaborator
Author
There was a problem hiding this comment.
I've extended the shared modifier engine to support an explicit selector field on registered rules, so complex compound selectors can be concatenated alongside class-based rules in the same grouped block. Also added registerDarkModeSelectorRule as the dark-mode-specific wrapper for this.
tb-selected-border-color--dark and tb-hover-border-color--dark are now registered via registerDarkModeSelectorRule inside getDarkClasses() instead of building standalone @media blocks inside componentHeadStyle.
Added regression tests in carousel-dark-colors.test.js asserting exactly one @media (prefers-color-scheme: dark) block when both selected/hover attributes are set, and when combined with an unrelated container-background-color--dark rule.
|
|
||
| componentHeadStyle = () => { | ||
| const globalData = this.context && this.context.globalData | ||
| const darkClasses = this.getDarkClasses() |
Collaborator
Author
There was a problem hiding this comment.
I've updated so componentHeadStyle no longer calls this.getDarkClasses() (which lazily registers a new tb-border-color--dark rule on first call). Instead it reads this.darkClasses || {} directly. Only the thumbnail-rendering instance ever populates this.darkClasses (via renderThumbnail()), so the radio/fallback/main-image instances no longer register duplicate, unused classes/rules, and no longer trigger extra growing-content media blocks.
Added a regression assertion in carousel-dark-colors.test.js confirming exactly one .mj-dark-N { border-color: ... } rule and one @media block are emitted for tb-border-color--dark, even though four mj-carousel-image instances are created per image.
…nto feature/dark-mode-support # Conflicts: # packages/mjml-carousel/src/Carousel.js
Dark-mode centralization fixes: - In`mjml-carousel`, passed `globalData` to `getOutlookDarkModeMediaQuery()` in `CarouselImage` so social background overrides aren't dropped depending on emission order - In `mjml-text`, restored the unitless alternative in the `line-height` type declaration so values like `line-height="1.5"` are accepted again - In `mjml-accordion`, routed `icon-wrapped-url--dark`/`icon-unwrapped-url--dark` through `AccordionElement`'s child context instead of `AccordionTitle`'s own attributes, so illegal overrides set directly on `mj-accordion-title` no longer take effect. Also removed these attributes in the docs - In `mjml-section`/`mjml-hero`, registered `background-url--dark` through the shared `registerDarkModeRule` path instead of emitting a standalone `@media` block per instance, so multiple sections/heroes/wrappers are concatenated into a single grouped block - In `mjml-core`, extended the modifier engine (`registerModifierRule/registerModifierRuleGroup/formatGroupedRules`) to support explicit CSS selectors alongside generated class names - In`mjml-carousel`, registered `tb-selected-border-color--dark` and `tb-hover-border-color--dark` via the new selector-aware registration, and do so during the render phase (`getDarkClasses`) rather than `componentHeadStyle`, so the rules exist before any component's shared emitter runs - In `mjml-carousel`, stopped `CarouselImage`'s `componentHeadStyle` from calling `getDarkClasses()` (which re-registers `tb-border-color--dark` on every radio/thumbnail/fallback/main instance). Also reads `this.darkClasses` directly so only the thumbnail-rendering instance owns the rule Also included an unrelated doc typo fix across several `README` files (`support-dark-image` -> `support-dark-mode-image`). Added/updated regression tests covering the above.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Overhauls MJML dark-mode support across body components giving the user the option to support and attribute tools to make simple changes to colours and images in clients that support it.
What Changed
New shared dark-mode infrastructure in core
Component dark-mode support expanded and normalised
Implemented/extended dark attributes and rendering behaviour for:
Addiitioal ‘Outlook’ support for images in various (not all) Outlook clients
Validator rule for dark attribute usage
Added dark-mode test coverage across components, including:
Docs updates