diff --git a/docs/changelog.rst b/docs/changelog.rst index 42353e6d1..93599c00c 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -4,6 +4,28 @@ Changelog ========= +Unreleased +---------- + +Bug fixes +......... + +- 🐛 Fix ``needflow`` rendering very dark / black nodes when a need type has no + ``color`` set in ``needs_types`` (:issue:`1664`). + Previously a hard-coded ``#000000`` fallback was used as the fill color, which + produced unreadable nodes — especially under browser dark mode. + When no color is configured, no color is emitted and the diagram engine's + default node color is used. + + .. note:: + + This is a minor behavior change for users with ``needs_types`` entries + that omit the ``color`` key: diagrams (``needflow``, ``needuml``, + ``needgantt``) that previously rendered such nodes as solid black will + now render them with the diagram engine's default node color (typically + light). To preserve the old appearance, set ``"color": "#000000"`` + explicitly on the affected ``needs_types`` entry. + .. _`release:8.0.0`: 8.0.0 diff --git a/sphinx_needs/api/need.py b/sphinx_needs/api/need.py index 423952ac6..0aa4e562a 100644 --- a/sphinx_needs/api/need.py +++ b/sphinx_needs/api/need.py @@ -373,7 +373,7 @@ def generate_need( "type": need_type, "type_name": need_type_data["title"], "type_prefix": need_type_data["prefix"], - "type_color": need_type_data.get("color") or "#000000", + "type_color": need_type_data.get("color") or "", "type_style": need_type_data.get("style") or "node", "title": core_values["title"], "status": core_values["status"], diff --git a/sphinx_needs/config.py b/sphinx_needs/config.py index 7f53b5783..6e4fd5fbc 100644 --- a/sphinx_needs/config.py +++ b/sphinx_needs/config.py @@ -325,7 +325,8 @@ class NeedType(TypedDict): prefix: str """The prefix to use for the need ID.""" color: NotRequired[str] - """The default color to use in diagrams (default: "#000000").""" + """The default color to use in diagrams. + If unset or empty, no color is applied and the diagram engine's default is used.""" style: NotRequired[str] """The default node style to use in diagrams (default: "node").""" diff --git a/sphinx_needs/directives/needflow/_graphviz.py b/sphinx_needs/directives/needflow/_graphviz.py index cf6fd5228..71094dc2b 100644 --- a/sphinx_needs/directives/needflow/_graphviz.py +++ b/sphinx_needs/directives/needflow/_graphviz.py @@ -336,8 +336,8 @@ def _render_subgraph( params.append(("shape", "rectangle")) # fill color - params.append(("style", "filled")) if need["type_color"]: + params.append(("style", "filled")) params.append(("fillcolor", _quote(need["type_color"]))) # outline color diff --git a/sphinx_needs/directives/needflow/_plantuml.py b/sphinx_needs/directives/needflow/_plantuml.py index 7d44e277d..490803f2d 100644 --- a/sphinx_needs/directives/needflow/_plantuml.py +++ b/sphinx_needs/directives/needflow/_plantuml.py @@ -76,11 +76,12 @@ def get_need_node_rep_for_plantuml( node_style = need_info["type_style"] if need_info["is_need"] else "rectangle" # node representation for plantuml - need_node_code = '{style} "{node_text}" as {id} [[{link}]] #{color}'.format( + color_suffix = f" #{';'.join(node_colors)}" if node_colors else "" + need_node_code = '{style} "{node_text}" as {id} [[{link}]]{color_suffix}'.format( id=make_entity_name(need_info["id_complete"]), node_text=node_text, link=node_link, - color=";".join(node_colors), + color_suffix=color_suffix, style=node_style, ) return need_node_code diff --git a/sphinx_needs/directives/needgantt.py b/sphinx_needs/directives/needgantt.py index 61a5d3d45..9c76c5e1c 100644 --- a/sphinx_needs/directives/needgantt.py +++ b/sphinx_needs/directives/needgantt.py @@ -289,9 +289,10 @@ def process_needgantt( need["title"], complete ) - el_color_string += "[{}] is colored in {}\n".format( - need["title"], need["type_color"] - ) + if need["type_color"]: + el_color_string += "[{}] is colored in {}\n".format( + need["title"], need["type_color"] + ) puml_node["uml"] += gantt_element diff --git a/sphinx_needs/directives/needuml.py b/sphinx_needs/directives/needuml.py index 0021853fc..098b84d12 100644 --- a/sphinx_needs/directives/needuml.py +++ b/sphinx_needs/directives/needuml.py @@ -447,11 +447,16 @@ def flow(self, need_id: str) -> str: new_env=True, ) - need_uml = '{style} "{node_text}" as {id} [[{link}]] #{color}'.format( + color_suffix = ( + f" #{need_info['type_color'].replace('#', '')}" + if need_info["type_color"] + else "" + ) + need_uml = '{style} "{node_text}" as {id} [[{link}]]{color_suffix}'.format( id=make_entity_name(need_id), node_text=node_text, link=link, - color=need_info["type_color"].replace("#", ""), + color_suffix=color_suffix, style=need_info["type_style"], ) diff --git a/tests/doc_test/doc_github_issue_1664/conf.py b/tests/doc_test/doc_github_issue_1664/conf.py new file mode 100644 index 000000000..5603c0db1 --- /dev/null +++ b/tests/doc_test/doc_github_issue_1664/conf.py @@ -0,0 +1,14 @@ +extensions = ["sphinx_needs", "sphinxcontrib.plantuml"] + +# note, the plantuml executable command is set globally in the test suite +plantuml_output_format = "svg" + +# needs_types entries without an explicit 'color' field. +# This reproduces the situation that caused dark/black nodes in needflow. +needs_types = [ + { + "directive": "spec", + "title": "Specification", + "prefix": "S_", + }, +] diff --git a/tests/doc_test/doc_github_issue_1664/index.rst b/tests/doc_test/doc_github_issue_1664/index.rst new file mode 100644 index 000000000..d1e67611d --- /dev/null +++ b/tests/doc_test/doc_github_issue_1664/index.rst @@ -0,0 +1,11 @@ +Github Issue 1664 test +====================== + +.. spec:: Test + :id: ID_SPC_1001 + :status: open + :tags: test + +.. needflow:: + :tags: test + :debug: diff --git a/tests/test_github_issues.py b/tests/test_github_issues.py index b9244c11f..4ce312c13 100644 --- a/tests/test_github_issues.py +++ b/tests/test_github_issues.py @@ -80,3 +80,45 @@ def test_doc_github_160(test_app): app.build() html = Path(app.outdir, "index.html").read_text() assert 'A-001' in html + + +@pytest.mark.parametrize( + "test_app", + [ + { + "buildername": "html", + "srcdir": "doc_test/doc_github_issue_1664", + "confoverrides": {"needs_flow_engine": "plantuml"}, + }, + { + "buildername": "html", + "srcdir": "doc_test/doc_github_issue_1664", + "confoverrides": { + "needs_flow_engine": "graphviz", + "graphviz_output_format": "svg", + }, + }, + ], + ids=["plantuml", "graphviz"], + indirect=True, +) +def test_doc_github_1664(test_app): + """ + https://github.com/useblocks/sphinx-needs/issues/1664 + + When a ``needs_types`` entry has no ``color`` field, needflow used to fall + back to ``#000000`` (black), producing very dark / unreadable nodes. + After the fix, no color should be emitted so the diagram engine's own + default is used. + """ + app = test_app + app.build() + html = Path(app.outdir, "index.html").read_text() + + # No #000000 fill color should appear in the rendered output when the + # user did not set a color in needs_types. + # For plantuml the rendered source is shown via :debug:; for graphviz the + # fill color would appear in the embedded SVG when the bug is present. + assert "#000000" not in html + for graphviz_file in Path(app.outdir, "_images").glob("graphviz-*.svg"): + assert "#000000" not in graphviz_file.read_text()