fix: align branch label background with text for multi-line labels in LR GitGraph#7393
Conversation
… LR GitGraph The background rectangle for branch labels was misaligned with the text when branch names contained multi-line text (e.g. "Feature A\n(ongoing)") in LR layout. The old transform used `pos - bbox.height / 2` which caused the rect to shift further up as text height increased, while the text position remained centered at `pos - 1`. Fixed by using a constant y-offset (`pos - 11`) in the transform, derived from the constraint that the rect center must equal the text center regardless of bbox.height. Closes mermaid-js#7362
🦋 Changeset detectedLatest commit: c541af4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for mermaid-js ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
@mermaid-js/examples
mermaid
@mermaid-js/layout-elk
@mermaid-js/layout-tidy-tree
@mermaid-js/mermaid-zenuml
@mermaid-js/parser
@mermaid-js/tiny
commit: |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #7393 +/- ##
=======================================
Coverage 3.55% 3.55%
=======================================
Files 490 490
Lines 48785 48785
Branches 765 765
=======================================
Hits 1734 1734
Misses 47051 47051
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
|
Hi team, gentle ping on this PR — it fixes the branch label background misalignment for multi-line labels in LR GitGraph (#7362). The 44 changed Argos snapshots are expected since the fix affects the label positioning formula. Happy to walk through the changes if helpful! |
|
Gentle ping — is there anything else needed from my side for this PR? Happy to make any adjustments if needed. |
|
Hey @veeceey Thanks for the reminder. Will review! |
knsv
left a comment
There was a problem hiding this comment.
[sisyphus-bot]
Thanks for the thorough analysis and clean fix, @veeceey — the mathematical derivation in the PR description is excellent and makes the change easy to verify.
What's working well
🎉 [praise] — Outstanding root cause analysis. Walking through the geometry to show exactly how the old pos - bbox.height / 2 double-counted the height (since the rect's own y attribute already incorporates -bbox.height/2 + 8) makes this a joy to review. The algebraic derivation of the constant 11 is the kind of documentation reviewers dream about.
🎉 [praise] — This fix also quietly improves single-line label centering (from ~3px off to exactly centered), which is a nice bonus. The fact that a single constant replaces a height-dependent expression and works for all line counts confirms the math is right.
Minor notes
🟢 [nit] — The constant 11 is derived from the rect's y offset (8) and half the extra padding (4/2 = 2), giving a center offset of 10, hence pos - 11 to target pos - 1. If the rect attributes (y = -bbox.height/2 + 8, height = bbox.height + 4) ever change, this would need updating too. A brief inline comment like // derived: rect center offset is always +10, text center is pos-1 could help future contributors — but this is entirely optional given the codebase's existing style with magic numbers throughout the renderer.
💡 [suggestion] — The Cypress test covers the fix well. If you wanted to be extra thorough, a test with three-line labels (e.g., "Feature A\n(ongoing)\nv2") would further demonstrate that the fix is truly height-independent. Not needed for merge — just a thought.
Verdict
Clean, minimal, mathematically sound fix with good test coverage and a proper changeset. Approving — let's get this across the finish line. 🚀
|
@veeceey, Thank you for the contribution! |
Fixes #7362
When using GitGraph with LR layout and branch names containing multi-line text (e.g.
"Feature A\n(ongoing)"), the background rectangle for branch labels was vertically misaligned with the actual text.Root cause
In
drawBranches(), the background rect's vertical transform for LR layout usedpos - bbox.height / 2, which is height-dependent. The label text, however, is positioned at a fixed center point ofpos - 1regardless of height. Asbbox.heightincreases with multi-line text, the background rect shifts further up while the text stays centered, creating a visible offset.Single-line (~16px height): rect center at
pos + 2, text center atpos - 1-- 3px offTwo lines (~32px height): rect center at
pos - 6, text center atpos - 1-- 5px off, opposite directionFix
Replaced the height-dependent transform
pos - bbox.height / 2with a constant offsetpos - 11, derived from the constraint that the rect center must equal the text center:This keeps the background centered on the text regardless of how many lines the branch name spans.
Testing
gitGraphRenderer.tspassgitGraph.spec.tspass