feat(diagrams): bring the hand-drawn diagrams onto the same palette - #12647
Open
jd wants to merge 2 commits into
Open
feat(diagrams): bring the hand-drawn diagrams onto the same palette#12647jd wants to merge 2 commits into
jd wants to merge 2 commits into
Conversation
Member
Author
|
This pull request is part of a Mergify stack:
|
Contributor
Merge Protections🔴 2 of 6 protections blocking · waiting on 👀 reviews
🔴 👀 Review RequirementsWaiting for
This rule is failing.
🔴 🔎 ReviewsWaiting for
This rule is failing.
Show 4 satisfied protections🟢 🤖 Continuous Integration
🟢 Enforce conventional commitMake sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
🟢 📕 PR description
🟢 🚦 Auto-queueWhen all merge protections are satisfied, this pull request will be queued automatically. |
There was a problem hiding this comment.
🔵 Needs a closer look
It changes shared diagram rendering and styling across many pages, so a final human review with visual verification is advisable.
Pull request overview
This PR unifies all diagram rendering (Graphviz fences, <GitGraph>, and <StackMapping>) under a single role-based palette and shared SVG post-processing, so diagrams stay consistent across themes and pages.
Changes:
- Introduces
finishDiagramSvg()as a shared Graphviz SVG post-processor and a single source of truth for diagram roles. - Refactors
<GitGraph>and the remark Graphviz pipeline to emit role classes (instead of hard-coded colors) and rely on.dgstyling. - Updates hand-drawn SVG diagrams (
<StackMapping>, GitGraph linear mode) and docs call sites to use roles (commitRole,role) instead of colors.
File summaries
| File | Description |
|---|---|
| src/util/diagramSvg.ts | Adds shared SVG post-processing and centralized DiagramRole vocabulary. |
| src/styles/theme.css | Removes the legacy --theme-diagram-edge token in favor of role accents. |
| src/styles/index.css | Extends .dg painting rules to cover hand-placed shapes and sets sizing for fixed-size diagrams. |
| src/content/docs/stacks/updating.mdx | Updates <GitGraph> usage from commitColor to commitRole. |
| src/content/docs/stacks/concepts.mdx | Updates <GitGraph> usage from commitColor to commitRole. |
| src/content/docs/stacks/compare/graphite.mdx | Migrates node objects from color to role. |
| src/content/docs/stacks/compare/gh-stack.mdx | Migrates node objects from color to role. |
| src/content/docs/stacks.mdx | Migrates PR/node coloring from color/commitColor to role-based props. |
| src/content/docs/merge-queue/stacks.mdx | Updates <GitGraph> usage from commitColor to commitRole. |
| src/content/docs/merge-queue/merge-strategies.mdx | Migrates node objects from color to role. |
| src/components/StacksLocalModel.astro | Updates <GitGraph> usage from commitColor to commitRole. |
| src/components/StackMapping.astro | Refactors hand-placed SVG to use .dg roles and content-sized columns (removes hex literals/styles). |
| src/components/GitGraph.astro | Refactors both linear and Graphviz modes to emit role classes and use shared SVG post-processing. |
| plugins/remark-graphviz.ts | Switches to shared SVG post-processing and keeps legacy color-to-role mapping via roleFor. |
Review details
Suppressed comments (1)
plugins/remark-graphviz.ts:140
- To keep the
finishDiagramSvg(..., { roleFor })contract type-safe,legacyRoleForshould returnDiagramRole | undefined(matching the roles actually used inLEGACY_ROLES). This prevents accidentally returning an unsupported role string that.dgcan't style.
function legacyRoleFor(kind: DiagramKind, { fill, stroke }: ShapePaint): string | undefined {
if (kind === 'edge') return stroke ? LEGACY_ROLES[stroke.toLowerCase()] : undefined;
const table = kind === 'cluster' ? LEGACY_CLUSTER_ROLES : LEGACY_ROLES;
const color = !fill || fill === 'none' ? (kind === 'cluster' ? stroke : undefined) : fill;
return color ? table[color.toLowerCase()] : undefined;
- Files reviewed: 14/14 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The diagrams were not ugly because Graphviz is ugly. They were ugly because 63 hand-typed colors across 18 pages had no palette to be consistent with, and their dark mode was a hardcoded list of six strings the plugin string-matched. Four dialects had grown — queue-green, emoji-pastel, nineties-pastel and near-white-blueprint — and `enterprise/architecture` rendered as a stack of white slabs on the dark page, because the plugin deliberately skips a cluster that has a fill and every fill on that page is near-white. Graphviz supports a `class` attribute on graphs, nodes, edges and clusters and copies it verbatim into the SVG. So the plugin stops handling color entirely. It injects shape and spacing defaults, drops the opaque canvas, tags each element with a *role*, strips the inline paint, and one stylesheet resolves surface, border and label at paint time. Dark mode then arrives through the same `:root.theme-dark` block as every other surface on the site — no second render, no flash, no string matching — and the built SVGs now contain no color at all. The 19 fences are not touched. A table in the plugin maps the colors the docs were drawn with onto roles, so the whole corpus is recolored with zero MDX edits and this reverts in one commit. The table maps by hue family, so elements drawn alike still read alike; it is lossy the other way — PostgreSQL and Redis both land on `datastore` — and that is paid back one page at a time as each fence names its own roles. It is a shim with a known end: when no fence spells a color, nothing reaches it. Three things change that are worth knowing before reviewing screenshots: - A graph-level caption moves above the figure. Graphviz's default is below; `labelloc="t"` is injected, so every fence with a `label=` that does not set `labelloc` itself now captions on top. - Diagram text is painted in Inter rather than the browser's default sans. Graphviz still measures in Helvetica, so widths drift by about 3%; the injected node margins absorb it. - Borders lift toward white in dark mode. The product accents themselves are untouched, as DESIGN.md requires — what changes is a value derived from them, because a #347d39 outline on the dark page surface is too dim to read. Two roles deliberately share an accent (`queued` and `mergify` are both Merge Queue teal): the same color under two names, because "waiting in the queue" and "a service we run" are the same idea on two kinds of diagram, and a role name that lies is worse than a duplicated accent. `--color-green-700` is not a new color. It is the green nine diagrams already had hardcoded, promoted so it has a name. Verified by rendering all 35 diagrams on the site in both themes and comparing them against the same page before the change. Change-Id: I4100df168b1a98d79cd2f0eca15697cb2a9db1cb
`<GitGraph>` was a second Graphviz pipeline running beside the `dot` fences, with its own `COLORS` map, its own `EDGE_COLOR`, and its own string-replace for dark mode. `<StackMapping>` was a third, with five more hex literals. Three surfaces, three private palettes, all drifting apart — which is how a page like merge-queue/stacks ends up rendering a themed diagram and a baked one side by side. All three now name roles. `src/util/diagramSvg.ts` holds the post-processing both Graphviz surfaces share — drop the canvas, strip the inline paint, mark a borderless shape as a caption — so there is one place to change and no second list of hex strings to fall out of date. The hand-placed SVG in GitGraph's linear mode and in StackMapping emits the same `node` / `edge` groups and the same role classes, so `.dg` paints all four kinds of diagram from one stylesheet. `--theme-diagram-edge` is deleted: it was the old surfaces' single diagram token and `--dg-a-chrome` now says the same thing. This also fixes the bug that made the case for it. GitGraph marks every node `style=filled`, which reaches a `shape=plaintext` node too, so the `main` branch label sat on an opaque pale box — barely noticeable in light mode and glaring in dark, on seven pages. It is a caption, so it now carries `plain` and the box is gone. Two things changed while I was in there: - `<StackMapping>` sizes its columns to their content. They were fixed at 160px and the generated branch names did not fit; the overflow was invisible while the label was white on a solid fill, and would have been plainly visible once the label became dark text on a tint. - Graph-mode commit graphs are typeset at the same sizes as the fences (13/10 rather than Graphviz's default 14 and a hand-set 9), so the two kinds of diagram on one page no longer disagree about how big a label is. A call site names a role rather than a color: `color: "green"` becomes `role: "queued"`, `commitColor="green"` becomes `commitRole="queued"`. The prop is typed as `DiagramRole`, so a color name no longer compiles. Change-Id: I6bdb5b9fbd6574a37172fa067e81b0103282725f
jd
force-pushed
the
devs/jd/jd/ship-diagram-token-system/bring-hand-drawn-diagrams-onto-same-palette--6bdb5b9f
branch
from
September 2, 2026 00:57
204561e to
e2b246d
Compare
Member
Author
Revision history
|
Member
Author
|
Re-pushed to address the review: 204561e → e2b246d (compare).
|
Base automatically changed from
devs/jd/jd/ship-diagram-token-system/resolve-diagram-color-tokens-paint-time--4100df16
to
main
September 3, 2026 07:47
Contributor
|
@jd this pull request is now in conflict 😩 |
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.
<GitGraph>was a second Graphviz pipeline running beside thedotfences,with its own
COLORSmap, its ownEDGE_COLOR, and its own string-replace fordark mode.
<StackMapping>was a third, with five more hex literals. Threesurfaces, three private palettes, all drifting apart — which is how a page like
merge-queue/stacks ends up rendering a themed diagram and a baked one side by
side.
All three now name roles.
src/util/diagramSvg.tsholds the post-processingboth Graphviz surfaces share — drop the canvas, strip the inline paint, mark a
borderless shape as a caption — so there is one place to change and no second
list of hex strings to fall out of date. The hand-placed SVG in GitGraph's
linear mode and in StackMapping emits the same
node/edgegroups and thesame role classes, so
.dgpaints all four kinds of diagram from onestylesheet.
--theme-diagram-edgeis deleted: it was the old surfaces' singlediagram token and
--dg-a-chromenow says the same thing.This also fixes the bug that made the case for it. GitGraph marks every node
style=filled, which reaches ashape=plaintextnode too, so themainbranch label sat on an opaque pale box — barely noticeable in light mode and
glaring in dark, on seven pages. It is a caption, so it now carries
plainandthe box is gone.
Two things changed while I was in there:
<StackMapping>sizes its columns to their content. They were fixed at160px and the generated branch names did not fit; the overflow was invisible
while the label was white on a solid fill, and would have been plainly
visible once the label became dark text on a tint.
rather than Graphviz's default 14 and a hand-set 9), so the two kinds of
diagram on one page no longer disagree about how big a label is.
A call site names a role rather than a color:
color: "green"becomesrole: "queued",commitColor="green"becomescommitRole="queued". Theprop is typed as
DiagramRole, so a color name no longer compiles.