Repository navigation
feat(diagram): make the diagram font configurable - #1624
Draft
dimitri-yatsenko wants to merge 1 commit into
Draft
dimitri-yatsenko wants to merge 1 commit into
dimitri-yatsenko wants to merge 1 commit into
Conversation
Graphviz diagrams set Helvetica on every node and schema label, in two hardcoded places, so a caller had no way to ask for another typeface. A new setting, display.diagram_fontname (environment variable DJ_DIAGRAM_FONTNAME), names the face for both. It defaults to Helvetica, so diagrams are unchanged for anyone who does not set it. An empty name is refused, since Graphviz would fall back to Times. The issue proposed a key in the diagram themes; this is a display setting beside diagram_direction and diagram_theme instead, so the themes still change colors only. Graphviz sizes node boxes with the face it finds on the machine doing the rendering, and a face that machine lacks changes the text metrics and so the layout. The setting is for callers who control that environment. For a published figure, restyling the emitted SVG with a CSS font stack remains the safer route. Mermaid output is unchanged. Mermaid has a font option of its own, but make_mermaid() leaves the typeface to the Mermaid host, as it already leaves dark mode, so neither display.diagram_fontname nor display.diagram_theme applies to it. Its docstring listed the Helvetica font among the Graphviz features with no Mermaid equivalent; it now says the typeface is left to the host. The tests check, on MySQL and PostgreSQL, that every fontname in the dot source is Helvetica by default, and that with the setting each is the configured face, in the same number of places, the SVG names that face, and the Mermaid output does not change. Both fail without the change. The docs' guidance that diagram themes do not change fonts stays true; it should also name this setting, with the layout caveat. Closes #1539.
dimitri-yatsenko
requested review from
MilagrosMarin,
lum-agilyti and
ttngu207
October 17, 2026 23:45
MilagrosMarin
approved these changes
Oct 21, 2026
MilagrosMarin
left a comment
Contributor
There was a problem hiding this comment.
Thanks @dimitri-yatsenko, clean and well-scoped. Keeping the font out of the themes makes sense, and the layout caveat in the docstring is helpful.
Verified locally:
✅ Both hardcoded Helvetica sites (nodes + schema cluster label) now read display.diagram_fontname; no other fontname is set and edges carry no text labels, so nothing falls back to Times
✅ New fontname unit tests pass
Two small nits, neither blocking:
min_length=1rejects""but accepts whitespace:dj.config.display.diagram_fontname = " "validates, and Graphviz would fall back to Times.str_strip_whitespace(or a strip in a validator) would close that.- The
test_diagram_fontname_from_envdocstring says the env var "outranks a config file value", but the test doesn't load a config file. Either add that case or trim the docstring.
On the docs side, I'll add display.diagram_fontname (with the layout caveat) when the diagram config docs are updated.
dimitri-yatsenko
marked this pull request as draft
October 21, 2026 16:46
This branch has not been deployed
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.
Graphviz diagrams set Helvetica on every node and schema label, in two hardcoded places, so a caller had no way to ask for another typeface. A new setting, display.diagram_fontname (environment variable DJ_DIAGRAM_FONTNAME), names the face for both. It defaults to Helvetica, so diagrams are unchanged for anyone who does not set it. An empty name is refused, since Graphviz would fall back to Times.
The issue proposed a key in the diagram themes; this is a display setting beside diagram_direction and diagram_theme instead, so the themes still change colors only. Graphviz sizes node boxes with the face it finds on the machine doing the rendering, and a face that machine lacks changes the text metrics and so the layout. The setting is for callers who control that environment. For a published figure, restyling the emitted SVG with a CSS font stack remains the safer route.
Mermaid output is unchanged. Mermaid has a font option of its own, but make_mermaid() leaves the typeface to the Mermaid host, as it already leaves dark mode, so neither display.diagram_fontname nor display.diagram_theme applies to it. Its docstring listed the Helvetica font among the Graphviz features with no Mermaid equivalent; it now says the typeface is left to the host.
The tests check, on MySQL and PostgreSQL, that every fontname in the dot source is Helvetica by default, and that with the setting each is the configured face, in the same number of places, the SVG names that face, and the Mermaid output does not change. Both fail without the change.
The docs' guidance that diagram themes do not change fonts stays true; it should also name this setting, with the layout caveat.
Closes #1539.