Skip to content

feat(diagrams): resolve every diagram color from tokens, at paint time - #12646

Merged
mergify[bot] merged 1 commit into
mainfrom
devs/jd/jd/ship-diagram-token-system/resolve-diagram-color-tokens-paint-time--4100df16
Sep 3, 2026
Merged

feat(diagrams): resolve every diagram color from tokens, at paint time#12646
mergify[bot] merged 1 commit into
mainfrom
devs/jd/jd/ship-diagram-token-system/resolve-diagram-color-tokens-paint-time--4100df16

Conversation

@jd

@jd jd commented Sep 1, 2026

Copy link
Copy Markdown
Member

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.

Copilot AI lite review requested due to automatic review settings September 1, 2026 13:17
@mergify
mergify Bot had a problem deploying to Mergify Merge Protections September 1, 2026 13:17 Failure
@jd

jd commented Sep 1, 2026

Copy link
Copy Markdown
Member Author

This pull request is part of a Mergify stack:

# Pull Request Link
1 feat(diagrams): resolve every diagram color from tokens, at paint time #12646 👈
2 feat(diagrams): bring the hand-drawn diagrams onto the same palette #12647
3 feat(diagrams): let each diagram name its own roles, and lock the door #12648
4 docs(design): write the diagram system into DESIGN.md #12649

@mergify

mergify Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Merge Protections

🟢 All 6 merge protections satisfied — ready to merge.

Show 6 satisfied protections

🟢 🤖 Continuous Integration

  • all of:
    • check-success = build
    • check-success = lint
    • check-success = test
    • any of:
      • check-success = test-broken-links
      • label = ignore-broken-links
    • any of:
      • check-success=Cloudflare Pages
      • -head-repo-full-name~=^Mergifyio/

🟢 👀 Review Requirements

  • any of:
    • #approved-reviews-by >= 2
    • author = dependabot[bot]
    • author = renovate[bot]
    • all of:
      • author = mergify-ci-bot
      • -head ~= ^docs-agent/

🟢 Enforce conventional commit

Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/

  • title ~= ^(fix|feat|internal|docs|style|refactor|perf|test|build|ci|chore|revert|ui)(?:\(.+\))?!?:

🟢 🔎 Reviews

  • #changes-requested-reviews-by = 0
  • #review-requested = 0
  • #review-threads-unresolved = 0

🟢 📕 PR description

  • body ~= (?ms:.{48,})

🟢 🚦 Auto-queue

When all merge protections are satisfied, this pull request will be queued automatically.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Needs a closer look

It introduces broad styling and rendering behavior changes across all diagrams that require careful visual verification in both themes in addition to unit tests.

Pull request overview

This PR refactors Graphviz diagram rendering so diagrams emit semantic role classes (no inline paint) and are fully styled at paint time via CSS tokens, enabling consistent theming (including dark mode) without per-diagram hardcoded colors.

Changes:

  • Introduces diagram role accent tokens in theme.css (plus a new docs-only --color-green-700 primitive) and derives diagram surfaces/borders/labels via color-mix() at paint time.
  • Replaces legacy svg.graph styling with a .dg stylesheet that paints nodes/edges/clusters/captions based on roles.
  • Reworks remark-graphviz to inject layout defaults, strip inline paint, map legacy colors to roles, and adds a Vitest suite to lock down the new behavior.
File summaries
File Description
src/styles/tokens.css Adds a docs-only primitive token --color-green-700 used by the diagram role palette.
src/styles/theme.css Defines diagram role accent variables and theme-dependent mixing parameters (light/dark).
src/styles/index.css Implements .dg paint-time styling rules for Graphviz SVG output, including role mapping and caption/plain-node handling.
plugins/remark-graphviz.ts Updates the Graphviz remark plugin to inject defaults, remove inline paint, assign role classes, and set the .dg class.
plugins/remark-graphviz.test.ts Adds unit tests covering role mapping, paint stripping, canvas removal, overrides, and failure behavior.
DESIGN.md Documents the additional non-product token and clarifies token policy wording.
Review details
  • Files reviewed: 6/6 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread plugins/remark-graphviz.ts Outdated
@mergify
mergify Bot requested a review from a team September 1, 2026 13:21
@jd
jd marked this pull request as ready for review September 1, 2026 15:33
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
@jd
jd force-pushed the devs/jd/jd/ship-diagram-token-system/resolve-diagram-color-tokens-paint-time--4100df16 branch from 65e6056 to 3b720e0 Compare September 2, 2026 00:57
@jd

jd commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

Revision history

# Type Changes Reason Date
1 initial 65e6056 2026-09-02 00:57 UTC
2 content 65e6056 → 3b720e0 review (#12646): retype the mdast node instead of asserting 'html' into Code['type'] 2026-09-02 00:57 UTC

@mergify
mergify Bot deployed to Mergify Merge Protections September 2, 2026 00:57 Active
@jd

jd commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

Re-pushed to address the review: 65e60563b720e0 (compare).

  • The fence node is retyped to mdast.Html rather than asserting 'html' into mdast.Code['type'].

pnpm check, pnpm test and pnpm build green on this commit on its own.

@mergify
mergify Bot requested a review from a team September 2, 2026 13:32
@mergify

mergify Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Merge Queue Status

This pull request spent 3 minutes 40 seconds in the queue, including 2 minutes 35 seconds running CI.

Required conditions to merge

@mergify mergify Bot added the queued label Sep 3, 2026
@mergify
mergify Bot merged commit 01e4e73 into main Sep 3, 2026
10 checks passed
@mergify
mergify Bot deleted the devs/jd/jd/ship-diagram-token-system/resolve-diagram-color-tokens-paint-time--4100df16 branch September 3, 2026 07:47
@mergify mergify Bot removed the queued label Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants