Skip to content

🐛 Fix needflow nodes rendering as black when no type color is set#1702

Merged
ubmarco merged 2 commits into
masterfrom
fix/needflow-dark-nodes-1664
May 19, 2026
Merged

🐛 Fix needflow nodes rendering as black when no type color is set#1702
ubmarco merged 2 commits into
masterfrom
fix/needflow-dark-nodes-1664

Conversation

@ubmarco
Copy link
Copy Markdown
Member

@ubmarco ubmarco commented May 17, 2026

Summary

Fixes #1664.

When a needs_types entry omitted the color field, generate_need fell back to #000000. That value was then forwarded to PlantUML as the node background colour and to Graphviz as fillcolor with style=filled, producing very dark, unreadable nodes — especially under browser forced dark mode.

The default is now an empty string. Each diagram emitter skips the colour suffix / style=filled when no colour is configured, so the diagram engine's own default node colour is used.

Why this only surfaced recently

The #000000 fallback has been in the code since 1.0, but it was unreachable until 3.0.0 (Aug 2024). Before PR #1185 (commit eee2eb40), the code used bracket access — ntype["color"] or "#000000" — so a needs_types entry missing color raised KeyError at build time and the field was de facto required. PR #1185 made color an optional NotRequired[str] and switched to ntype.get("color") or "#000000", which silently substitutes the black fallback for users who omit the field.

Sphinx / PlantUML / Graphviz versions are not factors — the bug is purely on the Sphinx-Needs side and reproduces with every renderer version. Forced dark mode doesn't cause it; it just removes the surrounding light page that previously made the broken render mildly visible, which is why bug reports started arriving.

Changes

  • sphinx_needs/api/need.py: default type_color to "" instead of "#000000".
  • sphinx_needs/directives/needflow/_plantuml.py: only emit the #color suffix when a colour is set.
  • sphinx_needs/directives/needflow/_graphviz.py (subgraph path): only set style=filled when there is a fill colour. The plain-node path already had this guard.
  • sphinx_needs/directives/needuml.py: same conditional #color emission as needflow.
  • sphinx_needs/directives/needgantt.py: skip the is colored in … line when no colour is set.
  • sphinx_needs/config.py: update the NeedType.color docstring.
  • docs/changelog.rst: bug-fix entry under a new "Unreleased" section.

Test plan

  • New parameterised regression test tests/test_github_issues.py::test_doc_github_1664 covering both plantuml and graphviz engines with a needs_types entry that has no color field.
  • Verified the new test fails on the pre-fix code and passes after the fix.
  • Full test suite: 912 passed, 5 skipped (pre-existing benchmark errors due to a missing sphinx_ai_index extension are unrelated).

When a `needs_types` entry omitted the `color` field, the API fell back to
`#000000`. That value was then forwarded to PlantUML as a node background
color and to Graphviz as `fillcolor` with `style=filled`, producing very
dark, unreadable nodes — especially under browser dark mode (issue #1664).

The default is now an empty string. Each diagram emitter skips the color
suffix / `style=filled` when no color is configured, so the diagram engine's
own default node color is used.
@codecov
Copy link
Copy Markdown

codecov Bot commented May 17, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.31%. Comparing base (4e10030) to head (6bbc940).
⚠️ Report is 278 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1702      +/-   ##
==========================================
+ Coverage   86.87%   89.31%   +2.43%     
==========================================
  Files          56       72      +16     
  Lines        6532    10328    +3796     
==========================================
+ Hits         5675     9224    +3549     
- Misses        857     1104     +247     
Flag Coverage Δ
pytests 89.31% <100.00%> (+2.43%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ubmarco ubmarco requested a review from chrisjsewell May 17, 2026 21:35
Copy link
Copy Markdown
Member

@chrisjsewell chrisjsewell left a comment

Choose a reason for hiding this comment

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

I have scrutinised the code, but the change makes sense in principle.
Perhaps this is somewhat of a breaking change (changing the default) and this should be made clear

Per review feedback on #1702: the switch from a hard-coded `#000000`
fallback to "no color emitted" changes the rendered output for users
with `needs_types` entries that omit the `color` key. Call this out
explicitly in the changelog and document how to restore the previous
appearance.
@ubmarco
Copy link
Copy Markdown
Member Author

ubmarco commented May 18, 2026

I added a note to the changelog entry to point out the possibly backwards incompatible change.

@ubmarco ubmarco merged commit 1e60746 into master May 19, 2026
25 checks passed
@ubmarco ubmarco deleted the fix/needflow-dark-nodes-1664 branch May 19, 2026 08:44
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.

needflow nodes extremely dark under dark mode (Sphinx 9.1.0, Sphinx‑Needs 7.0.0)

2 participants