fix(gantt): readable done-task text in dark theme#7432
Conversation
In the dark theme, taskTextDarkColor was set to darkTextColor — a light color (~hsl 86% lightness) intended for the dark background. When applied to done-task text over doneTaskBkgColor (lightgrey), this produced unreadable light-on-light text. Derive taskTextDarkColor via invert(doneTaskBkgColor) so it stays contrast-aware within the theme pipeline. If a user overrides doneTaskBkgColor through themeVariables, text color adjusts automatically. Resolves #5979 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 4cd3fff The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for mermaid-js ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
@mermaid-js/examples
mermaid
@mermaid-js/layout-elk
@mermaid-js/layout-tidy-tree
@mermaid-js/mermaid-zenuml
@mermaid-js/parser
@mermaid-js/tiny
commit: |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #7432 +/- ##
==========================================
- Coverage 3.55% 3.54% -0.02%
==========================================
Files 489 490 +1
Lines 48774 48938 +164
Branches 765 766 +1
==========================================
Hits 1734 1734
- Misses 47040 47204 +164
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
ashishjain0512
left a comment
There was a problem hiding this comment.
[sisyphus-bot]
Review — PR #7432: fix(gantt): readable done-task text in dark theme
Thanks @knsv — excellent root-cause analysis and a clean fix for a long-standing readability issue. The PR description is thorough and the TDD approach is spot on.
What's working well
🎉 [praise] The root-cause analysis in the PR description is exemplary — tracing taskTextDarkColor → darkTextColor → lighten(invert('#323D47'), 10) to show why the color ends up light is exactly the level of detail that makes theme bugs debuggable later.
🎉 [praise] Using invert(this.doneTaskBkgColor) instead of a hardcoded color keeps the fix within the theme pipeline — if a user overrides doneTaskBkgColor via themeVariables, the text color adapts automatically. This is the right pattern.
🎉 [praise] Both a unit test and an e2e visual regression test are included. The e2e test covers multiple gantt task states (done, active, future, crit+done) which gives good regression coverage.
Things to address
🟡 [important] Pie chart regression via taskTextDarkColor fallback — theme-dark.js:233,237
taskTextDarkColor is used as the fallback for pieTitleTextColor and pieLegendTextColor:
this.pieTitleTextColor = this.pieTitleTextColor || this.taskTextDarkColor; // line 233
this.pieLegendTextColor = this.pieLegendTextColor || this.taskTextDarkColor; // line 237Before this fix, taskTextDarkColor was a light color (darkTextColor ≈ lightened #CDC2B8), which reads well against the dark theme background (#333). After the fix, it becomes invert('lightgrey') ≈ #2C2C2C, which is dark-on-dark against the diagram background — likely unreadable pie chart titles and legends.
A straightforward fix would be to set these explicitly before the fallback takes effect, e.g. adding to updateColors() before the pie section:
this.pieTitleTextColor = this.pieTitleTextColor || this.mainContrastColor;
this.pieLegendTextColor = this.pieLegendTextColor || this.mainContrastColor;mainContrastColor is 'lightgrey' in dark theme, which reads well on #333. This decouples pie text from the gantt-specific variable. It would be great to add a pie chart dark-theme e2e test alongside the gantt one to verify.
🟢 [nit] Unit test assertion strength — theme-dark.spec.js:20
The test asserts expect(color).not.toBe(theme.darkTextColor) which is a negative assertion — it confirms the bug is gone but doesn't verify the fix is correct. A positive assertion checking the computed value is a recognizably dark color (e.g., checking the hex value or using a simple lightness heuristic) would catch future regressions more precisely.
Summary: The fix direction is correct and well-reasoned. The one item to address is the downstream impact on pie chart colors in dark theme — once that's handled, this should be good to go. Let's get this across the finish line! 🚀
…heme Address PR review: pieTitleTextColor and pieLegendTextColor previously fell back to taskTextDarkColor, which is now a dark color (#2c2c2c). This would produce unreadable dark-on-dark text for pie charts. Decouple by falling back to mainContrastColor (lightgrey) instead, which reads well on the dark background (#333). Also strengthen unit test with positive assertion on computed hex value and add pie chart dark-mode e2e test for regression coverage. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ashishjain0512
left a comment
There was a problem hiding this comment.
[sisyphus-bot]
Re-review — PR #7432 (Round 2)
Both items from round 1 are fully addressed in commit 4cd3fff:
🎉 [praise] The pie chart decoupling is clean — pieTitleTextColor and pieLegendTextColor now fall back to mainContrastColor ('lightgrey') instead of taskTextDarkColor, breaking the cross-diagram coupling that would have caused dark-on-dark text. Exactly the right fix.
🎉 [praise] The test coverage is now thorough — positive assertion on the computed hex value ('#2c2c2c'), separate pie chart unit tests confirming 'lightgrey', and a new dark-mode pie e2e snapshot test. This covers both the fix and the regression guard.
Previous items — resolved:
🟡 Pie chart regression— Fixed by decoupling tomainContrastColor🟢 Unit test assertion strength— Now uses positivetoBe('#2c2c2c')assertion
No remaining issues. LGTM — nice work @knsv! 🚀
|
A regression issue was found while testing: Please note that sometimes when the label does not fit inside the rectangle, the text is displayed next to the graphical bar. After the change in this PR, #7432, it renders with black text in dark mode which is unreadable. |
…k mode
When a Gantt done-task label is too wide to fit inside the bar, the renderer
places the text outside (next to the bar) with class `taskTextOutsideLeft` or
`taskTextOutsideRight` plus `doneText{N}`. The `.doneText{N}` rule uses
`taskTextDarkColor !important`, which PR #7432 changed to a dark color
(inverted from lightgrey) for contrast inside the bar. That `!important` also
overrides the lighter `taskTextOutsideColor` that `.taskTextOutside{N}` sets,
making outside-label text black/dark on the dark diagram background.
Fix: add combined-class selectors (`.doneText{N}.taskTextOutsideLeft/Right`)
after the `.doneText{N}` block, using `taskTextOutsideColor !important`. The
combined selectors have higher specificity and appear later in the cascade, so
they win over the single-class rule. Same fix applied to `.doneCritText{N}`.
Fixes regression from PR #7432 (fix for #5979).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…#7456) * fix(gantt): restore readable outside-text color for done tasks in dark mode When a Gantt done-task label is too wide to fit inside the bar, the renderer places the text outside (next to the bar) with class `taskTextOutsideLeft` or `taskTextOutsideRight` plus `doneText{N}`. The `.doneText{N}` rule uses `taskTextDarkColor !important`, which PR #7432 changed to a dark color (inverted from lightgrey) for contrast inside the bar. That `!important` also overrides the lighter `taskTextOutsideColor` that `.taskTextOutside{N}` sets, making outside-label text black/dark on the dark diagram background. Fix: add combined-class selectors (`.doneText{N}.taskTextOutsideLeft/Right`) after the `.doneText{N}` block, using `taskTextOutsideColor !important`. The combined selectors have higher specificity and appear later in the cascade, so they win over the single-class rule. Same fix applied to `.doneCritText{N}`. Fixes regression from PR #7432 (fix for #5979). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test(gantt): add e2e snapshot for done-task outside-text in dark mode Covers the regression from #7433 where done-task labels displayed outside the bar (text too wide for the bar width) rendered in unreadable dark color in dark theme. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test(gantt): add separate inside/outside e2e snapshots for dark mode done tasks Split the dark mode done-task coverage into two explicit tests: - inside-text: label fits within the bar (long bar, short label) - outside-text: label overflows the bar (short bar, long label) — regression case for #7433 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: knsv-bot <knsv-bot@macos.shared> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

Summary
Resolves #5979
taskTextDarkColorwas derived fromdarkTextColor— a light color (~86% lightness) meant for use against the dark diagram background. When applied to done-task text overdoneTaskBkgColor(lightgrey), this produced unreadable light-on-light text.taskTextDarkColortoinvert(this.doneTaskBkgColor)so the text color is derived from the background it sits on, staying contrast-aware within the theme pipeline. If a user overridesdoneTaskBkgColorviathemeVariables, the text color adjusts automatically.Classification
theme-dark.jsVerification
pnpm vitest run packages/mermaid/src/themes/)#2c2c2conlightgrey— high contrast confirmed🤖 Generated with Claude Code