Skip to content

Fix dark mode secondary text colour broken by postcss-color-mod-function v4#1040

Merged
jonhickman merged 3 commits into
TryGhost:mainfrom
jonhickman:doitdoug-dev
Mar 25, 2026
Merged

Fix dark mode secondary text colour broken by postcss-color-mod-function v4#1040
jonhickman merged 3 commits into
TryGhost:mainfrom
jonhickman:doitdoug-dev

Conversation

@jonhickman
Copy link
Copy Markdown
Contributor

@jonhickman jonhickman commented Mar 23, 2026

What broke and why

Two reports arrived around 2026-03-12 describing the same issue: tag names, post meta (date, read time, comment count) had become black and illegible in dark mode. Both customers used Casper and had dark colour scheme set and had made no changes to their theme.

The root cause is a breaking change in postcss-color-mod-function between v3 and v4.

The dark mode rule in screen.css uses a PostCSS-only color-mod() function (not supported natively in any browser):

html.dark-mode :is(.post-card-tags, .post-card-meta, .article-tag a, .byline-meta-content, .pagination .page-number) {
    color: color-mod(var(--color-secondary-text) l(-22%));
}

Under v3.0.3 this compiled correctly to #5f5f5f. Under v4.1.1 (updated in #932, January 2026) the same expression compiles to #000 — black on a near-black background, essentially invisible.

The rebuilt assets/built/ files containing the broken output first shipped in #1031 ("Update theme asset builds") as part of v5.10.0 on 2026-03-09, which is consistent with when customers started seeing the issue.

The identical rule exists in the @media (prefers-color-scheme: dark) / auto-color block and has the same problem.

The fix

Replace color-mod(var(--color-secondary-text) l(-22%)) with var(--color-secondary-text) directly in both the html.dark-mode block and the auto-color media query variant.

--color-secondary-text resolves to #979797, which gives ~4.6:1 contrast against --color-darkmode (#151719) — passing WCAG AA — and sits correctly below the #fff used for primary text like titles and featured labels, preserving the visual hierarchy.

This also removes the dependency on color-mod() for this rule entirely, which is the right direction given the function was dropped from the CSS Color spec in favour of color-mix().

Before

CleanShot 2026-03-23 at 19 44 41@2x

After

CleanShot 2026-03-23 at 19 43 12@2x

Also included

A fix for the gulp zip task, which was broken by the same batch of dependency updates (#964). gulp-zip v6 switched to a named export; the require() call needed .default appended to restore the task.

…ction v4

The color-mod(var(--color-secondary-text) l(-22%)) value compiled to
#000 after postcss-color-mod-function was bumped from v3 to v4, making
tags, post meta, byline and pagination text invisible on dark backgrounds.

Replace with var(--color-secondary-text) directly. #979797 gives ~4.6:1
contrast on the dark mode background and sits correctly below the #fff
primary text in the visual hierarchy. Same fix applied to the auto-color
media query variant.
gulp-zip v6 switched to a named export. Update require() to use .default
to restore the zip build task.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 23, 2026

Walkthrough

This pull request updates styling for dark-mode and auto-color theme CSS, regenerates a minified CSS build artifact, and modifies a JavaScript import statement. The CSS color changes replace color-mod() function calls with direct color variable references for post card tags, article tag links, byline meta content, and pagination page numbers in both dark-mode and auto-color theme blocks. The gulpfile.js is updated to import gulp-zip via its .default export instead of the module object directly.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: fixing a dark mode secondary text color issue caused by postcss-color-mod-function v4, which is the primary reason for the CSS file changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed The PR description comprehensively explains the root cause (postcss-color-mod-function v4 breaking change), the impact (illegible secondary text in dark mode), customer reports, the fix applied to both dark-mode and auto-color blocks, and an additional gulp-zip fix.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

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

@jonhickman jonhickman marked this pull request as draft March 23, 2026 19:41
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@gulpfile.js`:
- Line 10: The gulpfile currently uses require('gulp-zip').default which fails
because gulp-zip v6 is ESM-only; replace the CommonJS require by either
converting the gulpfile to ESM and importing zip via an ES import (e.g., import
zip from 'gulp-zip') or, if keeping CommonJS, load the package with a dynamic
import inside an async init function and assign the default export to the
existing zip identifier (replace the require call that creates the zip
constant). Ensure all tasks that use zip wait for that async initialization.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 479dcbf9-ef6d-4f1a-b8ad-34b64a05da9d

📥 Commits

Reviewing files that changed from the base of the PR and between 6e22fbd and 9a71b49.

⛔ Files ignored due to path filters (1)
  • assets/built/screen.css.map is excluded by !**/*.map
📒 Files selected for processing (3)
  • assets/built/screen.css
  • assets/css/screen.css
  • gulpfile.js

Comment thread gulpfile.js Outdated
const livereload = require('gulp-livereload');
const postcss = require('gulp-postcss');
const zip = require('gulp-zip');
const zip = require('gulp-zip').default;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

For gulp-zip v6.x, what is the documented CommonJS import pattern, and is const zip = require('gulp-zip').default; the correct usage?

💡 Result:

For gulp-zip v6.x, there is no documented CommonJS import pattern. The package is pure ESM-only (as of v6.0.0), with "type": "module" in package.json and exports: "./index.js", and the index.js uses ES module syntax (import/export). Official documentation and README only show ES module usage: import zip from 'gulp-zip';. Standard CommonJS require('gulp-zip') will fail with ERR_REQUIRE_ESM. const zip = require('gulp-zip').default; is not correct and will not work, as there is no CJS build/export; accessing .default on an ESM error object won't provide the function. To use in CommonJS (e.g., traditional gulpfile.js), use dynamic import: const {default: zip} = await import('gulp-zip');. Note this requires Node.js >=18 (per engines) and makes the task async.

Citations:


gulp-zip v6 import pattern is incorrect and will fail at runtime.

gulp-zip v6 is pure ESM-only (no CommonJS build or .default export). Using require('gulp-zip').default will throw ERR_REQUIRE_ESM because the package cannot be required via CommonJS. To fix this, either:

  • Use dynamic import in an async context: const {default: zip} = await import('gulp-zip'); (requires Node.js ≥18)
  • Or migrate the entire gulpfile.js to ES modules
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gulpfile.js` at line 10, The gulpfile currently uses
require('gulp-zip').default which fails because gulp-zip v6 is ESM-only; replace
the CommonJS require by either converting the gulpfile to ESM and importing zip
via an ES import (e.g., import zip from 'gulp-zip') or, if keeping CommonJS,
load the package with a dynamic import inside an async init function and assign
the default export to the existing zip identifier (replace the require call that
creates the zip constant). Ensure all tasks that use zip wait for that async
initialization.

@jonhickman jonhickman marked this pull request as ready for review March 23, 2026 19:47
Comment thread gulpfile.js Outdated
const livereload = require('gulp-livereload');
const postcss = require('gulp-postcss');
const zip = require('gulp-zip');
const zip = require('gulp-zip').default;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! Could you revert the change to gulpfile.js? The .default addition to the gulp-zip require isn't needed and we'd like to keep that file as-is. The rest of the changes look good.

@jonhickman jonhickman merged commit 24e244e into TryGhost:main Mar 25, 2026
weylandswart added a commit that referenced this pull request Apr 6, 2026
Closes https://linear.app/ghost/issue/DES-1319/table-headings-render-completely-black

The Tailwind v4 upgrade broke colormod. Similar fix to the one from #1040.
weylandswart added a commit that referenced this pull request Apr 6, 2026
Closes https://linear.app/ghost/issue/DES-1319/table-headings-render-completely-black

The Tailwind v4 upgrade broke colormod. Similar fix to the one from #1040.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants