Mermaid diagram lightbox: dark-mode contrast, sizing, and click-to-zoom

[julianken]

Diagrams

sequenceDiagram
    actor Reader
    participant Trigger as Inline trigger button
    participant Dialog as <dialog> lightbox
    participant Panzoom
    participant SVG as Mermaid SVG

    Note over Reader,SVG: First render — capDiagramWidth() pins every diagram to scale 0.625
    Reader->>Trigger: Click figure
    Trigger->>Dialog: showModal()
    Note over Dialog: Backdrop fades in (250ms ease-out)
    Note over Dialog: Panel runs glitch-reveal (~400ms RGB-split)
    Dialog->>Panzoom: panzoom(svg) on next rAF
    Panzoom->>SVG: fitToScreen() with 56px inset
    Reader->>SVG: Drag / wheel / pinch / +-fit buttons
    Reader->>Dialog: Esc / × / backdrop click
    Note over Dialog: Panel runs glitch-conceal (~150ms ease-in)
    Note over Dialog: Backdrop fades out (600ms ease-in-out)
    Dialog->>Trigger: focus restored after 600ms
    Note over Panzoom: Disposed lazily on next openDialog (preserves transform during exit)

Loading

Summary

• Mermaid diagrams in posts had three structural problems: dark-mode contrast was poor (gray text on hardcoded pale rect bands), text size differed wildly between diagrams of different intrinsic widths, and there was no way to read fine detail without leaving the page. This PR fixes all three.
• Adds a click-to-zoom lightbox built on the native <dialog> + panzoom (~3kb) — drag to pan, wheel/pinch to zoom, fit-to-screen control. Initial open fits the diagram with a 56px inset so it doesn't crowd the panel chrome.
• Animates entry/exit with the site's existing glitch language (RGB-split via <filter id="rgb-split">) and re-uses the bg-surface / border-border / text-accent tokens so the lightbox feels native rather than grafted-on.

Screenshots

Desktop 1440×900

Inline	Lightbox (fit-on-open)	Lightbox (zoomed)

Mobile 390×844

Inline	Lightbox

Test plan

• npm run lint — 0 errors (19 pre-existing warnings unrelated)
• npx tsc --noEmit — green
• npx vitest run tests/unit/components/MermaidDiagram — 4/4 pass
• npm run build — not run locally; CI will verify
• New unit tests added — existing 4 still cover the component; new behavior (capDiagramWidth scale, panzoom lifecycle, glitch class swap) is covered by the manual MCP drive below
• New Playwright e2e — not added in this PR (existing mermaid.spec.ts smoke covers render path)
• Playwright/Chrome-DevTools MCP smoke — drove the feature at desktop (1440×900) and mobile (390×844). list_console_messages returns no errors/warnings.

Notable engineering details (for the reviewer)

1. Consistent text size across diagrams — capDiagramWidth() post-processes Mermaid's emitted SVG to pin every diagram's render scale to 0.625 (text appears at the same effective size whether the viewBox is 599px or 1230px wide). uncapDiagramWidth() strips that cap for the lightbox copy.
2. Panzoom transform preservation through close — the original close path called setClosing(true), which re-rendered the panel. React's reconciler then re-set the stage's innerHTML (because reading domElement.innerHTML returns serialized markup that differs from the input string), wiping panzoom's style.transform mid-animation and producing a "diagram flashes zoomed in" frame. Replaced with closingRef + panelRef so the class swap happens via direct DOM mutation, no React re-render.
3. Panzoom dispose deferred to next open — disposing during close removed the SVG transform mid-fade. Now openDialog disposes any leftover instance before initializing a new one, so the SVG keeps its fit transform throughout the exit.
4. Asymmetric backdrop fade — 250ms ease-out on open (responsive), 600ms ease-in-out on close (less jarring). The close timeout matches the longer of the two so the backdrop has time to finish fading before display:none.
5. .env.example now documents that GCS_BUCKET / GCS_HMAC_* are required in dev for images to render — the s3Storage plugin is gated on GCS_BUCKET and falls back to (gitignored) public/media/ when unset.

Plan reference

Out of plan — surfaced while reviewing post drafts; user asked for diagram polish, click-to-zoom, then iterated on aesthetic / animation match with the site's terminal-violet theme.

---

🤖 Generated with Claude Code

[julianken-bot]

Verdict: APPROVE — feature works as described and the engineering choices are mostly principled, but two interaction-state bugs and a test-coverage gap warrant fixing before merge. None block merge on their own; flagging as IMPORTANT for the interaction bugs because they are easily user-triggerable.

What I verified (this turn)

• git rev-parse HEAD vs pr-115-review → both 0c078f7 (PR head, fresh)
• npx tsc --noEmit → exit 0 (no type errors)
• npx vitest run tests/unit/components/MermaidDiagram → Test Files 1 passed (1), Tests 4 passed (4) (the same 4 that predate this PR — no new coverage was added)
• npx eslint src/components/MermaidDiagram.tsx → 0 errors, 0 warnings on the changed component
• Read node_modules/panzoom/index.d.ts (panzoom 9.4.4) — dispose(), moveTo(), zoomAbs(), smoothZoom(clientX, clientY, scaleMultiplier) all match the call sites; no API drift
• Inspected node_modules/mermaid/dist/.../chunk-5YUVU3PZ.mjs configureSvgSize — confirmed mermaid 11.14 emits style="max-width: <N>px;" exactly when useMaxWidth: true, so the capDiagramWidth regex matches current output

Findings (3 — see inline comments)

• [IMPORTANT] MermaidDiagram.tsx:145 — close timeout never cancelled; rapid close-then-reopen closes the just-reopened dialog ~600ms later
• [IMPORTANT] MermaidDiagram.tsx:83 — theme change while dialog is open re-renders the stage, wiping the panzoomed SVG and stranding pzRef on a detached node
• [SUGGESTION] MermaidDiagram.tsx:20 — 236 added lines and zero new tests; the two pure SVG post-processors (capDiagramWidth / uncapDiagramWidth) are trivially unit-testable and would lock the regex against future mermaid upgrades

On the things you flagged for second-pass scrutiny

• closeDialog DOM-mutation diagnosis (your concern #1): I think the comment misattributes the fix. The "diagram flashes zoomed in" frame is prevented by the dispose-on-next-open pattern, not by closingRef. closingRef is just a re-entrancy guard against double-close. If you re-set setClosing(true), the re-render would re-evaluate dangerouslySetInnerHTML={{__html: uncapDiagramWidth(svg)}} — and since svg hasn't changed, React's reconciler should skip the innerHTML re-set (it dedupes by string, not object identity, in current versions). A cleaner React-idiomatic alternative: useMemo the uncapped string, then useState for closing is safe. Not blocking, but your stated rationale is shakier than the code's actual behavior. If you keep the current pattern, consider correcting the comment to say "re-render risks re-applying innerHTML in some React internals scenarios; we sidestep with classList directly" rather than asserting the wipe is guaranteed.
• dangerouslySetInnerHTML XSS (#2): With securityLevel: 'strict' mermaid runs DOMPurify on the output (verified — the bundle imports DOMPurify and applies it via Bt.sanitize(...) on strict/antiscript/sandbox). Source string comes from your CMS, not user input, in this codebase. Rationale is solid for now; would tighten by adding a runtime assertion that securityLevel is 'strict' if you ever generalize.
• Asymmetric 250/600ms (#3): Reasonable on its own — the 600ms close timeout is the source of finding F1 below; if you cancel it on re-open, the asymmetry stays principled.
• Dispose-on-next-open (#4): The trade-off works during normal use, but leaks the panzoom listeners on component unmount with the dialog never re-opened (e.g., navigating away from a post page mid-session). One-line useEffect cleanup would close it.
• a11y (#5): Native <dialog> + showModal() handles focus trap, role, modal semantics — those are fine. The remaining a11y limitation isn't from this PR: the trigger button's aria-label="Expand diagram" doesn't tell SR users which diagram. Pre-existing scope; not flagging under R7.
• capDiagramWidth regex (#7): Robust against current Mermaid 11.14 output, but only because you set useMaxWidth: true on sequence and flowchart. Other diagram types (class, gantt, etc.) won't have the style="max-width:..." attribute and the function silently no-ops. Add a global default or expand the per-type config to keep the size cap working across diagram kinds.
• Dispose-vs-preserve trade-off (#8): Yes, you can do both — capture pz.getTransform() before dispose, then re-apply via applyTransform (panzoom's controller interface) on next open. But that's a refactor, not a bug; current pattern is fine as long as F1 (timeout cancel) is fixed.

Bottom line

Approve with two real fixes recommended before merge (timeout-cancel and theme-change guard). The lightbox itself is well-shaped — single dialog, native <dialog> semantics, correct panzoom lifecycle for the common path, principled animation timing.

---

Reviewer: @julianken-bot (opus) — fresh-context subagent dispatched via the reviewing-as-julianken-bot skill. Same-tier risk noted: implementer likely also opus. Verdict above is binding regardless of GitHub's review label (this PR is in detached-node, where bot collaborator status was not verified this turn, so posted as event: COMMENT).

[julianken-bot]

[IMPORTANT] — close timeout is not cancellable; rapid close→reopen closes the just-reopened dialog

Issue: closeDialog schedules dialogRef.current?.close() 600ms later but stores no timeout id. If the user clicks the trigger again within those 600ms (a very plausible "oh I closed too fast" interaction, or a stray double-click on the X that lands on the trigger underneath after the dialog closes), openDialog runs showModal() and then the previous timeout fires and calls .close() on the just-reopened dialog. Panzoom is now initialized against a closed dialog and the user sees the lightbox blink shut.

Scenario: User opens lightbox, clicks X, immediately clicks the trigger again. ~500–600ms later the dialog closes itself. Confusing and reproducible.

Fix: Store the timeout id in a ref and cancel on openDialog:

const closeTimerRef = useRef<number | null>(null)
// in closeDialog:
closeTimerRef.current = window.setTimeout(() => { ... ; closeTimerRef.current = null }, 600)
// in openDialog (top):
if (closeTimerRef.current !== null) {
  window.clearTimeout(closeTimerRef.current)
  closeTimerRef.current = null
}
closingRef.current = false  // also reset the re-entrancy guard

Bonus: cancel in a useEffect cleanup on unmount so a dialog left mid-close doesn't fire .close() against an unmounted component.

[julianken-bot]

[IMPORTANT] — theme change while lightbox is open wipes the SVG and strands panzoom on a detached node

Issue: This useEffect runs whenever resolvedTheme changes and calls setSvg(capDiagramWidth(rendered)). That re-render reaches the dialog's <div ref={stageRef} dangerouslySetInnerHTML={{__html: uncapDiagramWidth(svg)}}> — React replaces the stage's child SVG with the freshly-themed one, but pzRef.current still points at the previous (now detached) SVG node. All gestures stop working; visually the diagram also re-fits at theme defaults, losing the user's zoom.

Scenario: User opens the lightbox in light mode, OS auto-switches to dark at sunset (or user hits the theme toggle), or resolvedTheme === 'system' flips. The lightbox is still open. Diagram silently becomes uninteractive until they close + reopen.

Fix: Either (a) skip the re-render path while the dialog is open and defer it until close, or (b) when the SVG changes while the dialog is open, dispose the old panzoom and re-init against the new SVG inside a useEffect keyed on [svg, dialogIsOpen]. Option (b) is more correct because users who change theme expect the diagram to update. Pseudocode for (b):

useEffect(() => {
  if (!dialogRef.current?.open) return
  pzRef.current?.dispose()
  pzRef.current = null
  requestAnimationFrame(() => { /* same init as openDialog */ })
}, [svg])

[julianken-bot]

[SUGGESTION] — pure SVG post-processors are trivially testable; locking them in protects against mermaid upgrades

Issue: capDiagramWidth and uncapDiagramWidth (lines 20–32) are pure string→string functions whose correctness depends on the exact shape of mermaid's emitted markup (style="max-width: <N>px", viewBox numeric format). The 4 existing tests cover mermaid.initialize / mermaid.render lifecycle but nothing about these. A future mermaid bump that changes width emission (e.g., width="…" attribute, or style="max-width:none") would silently no-op the cap and every diagram would render at full intrinsic size — which is exactly the regression this PR is designed to prevent.

Scenario: pnpm up mermaid ships in 4 months. Output becomes style="max-width:none; width: 1230px;". CI passes (existing tests don't touch these functions). Production diagrams render huge.

Fix: Export the two functions and add ~6 unit tests:

• capDiagramWidth with a representative <svg viewBox="0 0 1230 512" style="max-width:1230px"> → asserts the resulting max-width:769px (1230 × 0.625 rounded)
• capDiagramWidth with no viewBox match → returns input unchanged (graceful degradation)
• capDiagramWidth with floating-point viewBox dims (mermaid sometimes emits these)
• uncapDiagramWidth strips max-width: <N>px; while preserving the rest of the style attribute
• A guard test: real mermaid 11.14 sample fixture (committed once) round-trips through cap → uncap and ends up with no max-width

This is the highest-leverage test addition for the surface area changed in this PR. Lifecycle behavior (panzoom init, focus restore, glitch class swap) is harder to unit-test and the Playwright smoke + manual MCP runs are reasonable substitutes there.

[julianken-bot]

Verdict: APPROVE

Verification ledger

• Read both new commits (c1ff820, 5d52e51) and the full current MermaidDiagram.tsx end-to-end.
• Ran npx vitest run tests/unit/components/MermaidDiagram against 5d52e51 — 5/5 pass (including the new F1 race test).
• Ran npx tsc --noEmit — clean.
• HEAD verified: 5d52e51e28ce8ee6c01a4e8ecd4eff6cbf6018aa matches the dispatcher’s expected SHA.
• Same-tier risk: NO. Implementer co-author is Sonnet 4.6; reviewer is Opus.

What I checked, scoped to commits since 4fc860b

F1 (close-timer race) — addressed. closeTimerRef stores the setTimeout id (line 172), openDialog cancels it on a rapid reopen (lines 118-128), and an unmount cleanup effect clears any pending timer (lines 104-111). The new unit test (lines 143-193) uses vi.useFakeTimers() after async setup, simulates open → close → reopen, advances 700ms past the 600ms deadline, and asserts dialog.close was not called. This is the right assertion shape — it tests the observable race outcome rather than internal state.

F2 (theme-change-while-open) — addressed. New useEffect keyed on [svg] (lines 232-249) disposes the stale pzRef and re-initializes against the freshly mounted SVG when the dialog is open. The if (!dialogRef.current?.open) return guard correctly no-ops on the initial mount where svg transitions null → string while the dialog is closed. I traced the double-init concern from the dispatcher’s prompt: in steady state (open without theme change), openDialog initializes panzoom inside its own RAF callback and the F2 effect does not re-fire because svg did not change — single init. In the theme-flip-while-open path, the F2 effect disposes-then-reinits exactly once. No double-init in either case.

F3 (post-processor unit tests) — still open per the dispatcher’s note; intentionally deferred. Pure SVG transforms are testable and worth locking in against future mermaid upgrades, but I would not block merge on this. Filing a follow-up issue is the right move.

Findings

1 SUGGESTION inline; no IMPORTANT, no BLOCKER.

Bottom line

The two new commits land what they claim, the F1 test actually exercises the race, and I did not find a new defect introduced by either fix. Approving. Queue when ready.

— @julianken-bot (opus, fresh-context, anti-slop rubric)

[julianken-bot]

SUGGESTION (low priority): These three lines are effectively dead code in the recovery path, because setGlitchKey((k) => k + 1) two lines later forces React to unmount and remount the keyed <div ref={panelRef}>. By the time the next paint happens, panelRef.current is a freshly mounted node whose className already starts with glitch-reveal (the JSX default), and the manual classList.remove/add calls on the now-detached previous node have no effect.

The recovery actually works — is-closing removal on dialogRef.current (which is not keyed and survives the remount) plus the keyed remount of the panel together restore the open visual state. But the comment above ("swap the panel back to its enter animation so the re-opened dialog doent land mid-exit") implies the manual class manipulation is load-bearing, when only the is-closing removal on the dialog actually is. A future reader debugging this could delete the dead lines and break nothing, or copy this pattern elsewhere expecting it to work without a keyed remount.

Either drop the two panelRef classList lines and tighten the comment to focus on is-closing, or leave the lines and re-word the comment to acknowledge they are belt-and-suspenders against a remount-skipped path. Not blocking.

[julianken-bot]

Verdict: APPROVE

Verification ledger

• HEAD verified: 94ea08fd2bc5eb524bb22afa7a6c824ed9ac9c25 matches both origin/feat/mermaid-lightbox and the dispatcher's claim. Origin has not moved.
• Re-read both new commits (f61f899, 94ea08f) and the full current MermaidDiagram.tsx end-to-end.
• npx vitest run tests/unit/components/MermaidDiagram tests/unit/components/mermaid-svg-helpers → 2 files, 12 tests, all pass (5 component + 7 helper).
• npx tsc --noEmit → clean.
• npx eslint src/components/MermaidDiagram.tsx tests/unit/components/mermaid-svg-helpers.test.ts → 0 errors / 0 warnings on changed files.
• Same-tier risk: NO. Implementer co-author is Sonnet 4.6; reviewer is opus.
• Bot collaborator status on julianken/detached-node: write → posting as event: APPROVE.

Scoped to commits since 5d52e51

f61f899 (drop dead classList ops in close-timer recovery) — verified. The two removed panelRef.current?.classList.* calls operated on a node React was about to discard via setGlitchKey keyed remount; refs do not update synchronously, so even pre-removal the ops mutated a soon-detached node. Reopen path is now: cancel timer → remove is-closing from dialog (cancels backdrop fade) → setGlitchKey mounts a fresh <div key={k+1}> with className="glitch-reveal …" from JSX → entry animation runs on the new node. Clean.

94ea08f (export + 7 unit tests for cap/uncap helpers) — verified the F1-race test still passes alongside the 7 new ones. Fixture is realistic (viewBox with negative minX/minY, width="100%", scoped id, embedded <style>, style="max-width: 1230px" on root <svg>) and the round-trip guard correctly anchors the cap output at 769px (1230 × 0.625 = 768.75 → round up). Negative offsets in the viewBox attribute are implicitly exercised by the fixture, which is non-trivial regex correctness. One coverage gap below.

Findings

1 SUGGESTION inline; no IMPORTANT, no BLOCKER.

Bottom line

Both new commits land what they claim. No new defects introduced. The two open items from the prior round are addressed cleanly. APPROVE.

— @julianken-bot (opus, fresh-context, anti-slop rubric)

[julianken-bot]

SUGGESTION — coverage gap on the max-width regex no-op.

capDiagramWidth has two regex matches that can fail silently:

1. viewBox="…" match — covered by the "no matching viewBox" test (line 38).
2. style="…; max-width: <N>px" replacement — not covered.

If a future mermaid version emits style="max-width: none" (non-numeric), drops the inline style attribute, or moves max-width to a presentation attribute, .replace() returns the input string unchanged and the size cap silently disappears — exactly the regression class the round-trip guard is meant to detect.

Suggested test:

it('returns input unchanged when style has no numeric max-width to replace', () => {
  const input = '<svg viewBox="0 0 1230 512" style="max-width: none;">x</svg>'
  expect(capDiagramWidth(input)).toBe(input)
})

Two more polish notes (not blocking, take or leave):

• Consider extracting the helpers into a sibling mermaid-svg.ts so the test file doesn't pull 'use client', mermaid, and panzoom through the component import just to exercise four lines of regex.
• The .tsx extension on the new .ts test file is fine; the pre-existing MermaidDiagram.test.tsx is the convention outlier per CLAUDE.md — not in scope here.

[julianken]

@Mergifyio queue

[mergify]

Merge Queue Status

• ✅ Entered queue — 2026-04-25 02:15 UTC · Rule: default
• ✅ Checks passed · on draft merge queue: embarking main (d819a0b) and #115 together #117
• ✅ Merged — 2026-04-25 02:18 UTC · at 94ea08fd2bc5eb524bb22afa7a6c824ed9ac9c25 · squash

This pull request spent 2 minutes 26 seconds in the queue, including 2 minutes 11 seconds running CI.

Required conditions to merge

• #approved-reviews-by >= 1 [🛡 GitHub branch protection]
  • Mermaid diagram lightbox: dark-mode contrast, sizing, and click-to-zoom #115
• #changes-requested-reviews-by = 0 [🛡 GitHub branch protection]
  • Mermaid diagram lightbox: dark-mode contrast, sizing, and click-to-zoom #115
• branch-protection-review-decision = APPROVED [🛡 GitHub branch protection]
  • Mermaid diagram lightbox: dark-mode contrast, sizing, and click-to-zoom #115
• any of [🛡 GitHub branch protection]:
  • check-success = ESLint
  • check-neutral = ESLint
  • check-skipped = ESLint
• any of [🛡 GitHub branch protection]:
  • check-success = TypeScript
  • check-neutral = TypeScript
  • check-skipped = TypeScript
• any of [🛡 GitHub branch protection]:
  • check-success = Vitest
  • check-neutral = Vitest
  • check-skipped = Vitest
• any of [🛡 GitHub branch protection]:
  • check-success = Next.js Build
  • check-neutral = Next.js Build
  • check-skipped = Next.js Build
• any of [🛡 GitHub branch protection]:
  • check-success = Analyze Bundle
  • check-neutral = Analyze Bundle
  • check-skipped = Analyze Bundle
• any of [🛡 GitHub branch protection]:
  • check-success = CodeQL Analysis
  • check-neutral = CodeQL Analysis
  • check-skipped = CodeQL Analysis