Skip to content

fix(flowchart): keep ELK edge labels centered - #8238

Open
ousamabenyounes wants to merge 2 commits into
mermaid-js:developfrom
ousamabenyounes:bug/7341_elk_subgraph_label_margin
Open

fix(flowchart): keep ELK edge labels centered#8238
ousamabenyounes wants to merge 2 commits into
mermaid-js:developfrom
ousamabenyounes:bug/7341_elk_subgraph_label_margin

Conversation

@ousamabenyounes

@ousamabenyounes ousamabenyounes commented Sep 9, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown

📑 Summary

Keep ELK edge labels centered when flowchart.subGraphTitleMargin is configured. The common renderer now accepts a layout-specific vertical label offset, and ELK explicitly uses zero because its label coordinates already share the routed edge's final coordinate space.

Resolves #7341

📏 Design Decisions

The common painter preserves its existing title-margin offset by default, so Dagre and swimlanes are unchanged. Only ELK opts out. This keeps the fix at the shared rendering boundary without changing ELK's layout data or duplicating label positioning logic.

Test verification (RED → GREEN)

  • RED on unmodified develop: the issue diagram rendered in Chromium with edge center y=88.5, label center y=98, a 9.5px error.
  • GREEN: the same Chromium assertion passes with a maximum 1px tolerance.
  • Mutation check: reverting the ELK offset reproduced the same 9.5px RED failure.
  • Full local suite on Node 24.16.0: lint, build/install, 313 Vitest files, TypeScript package checks, config generation verification, and circular-dependency checks passed. Tests improved from 6,411 baseline passes to 6,412 patched passes with the same 16 skips and 2 todos.
  • Browser replay improved from 3/3 baseline tests to 4/4 patched tests. Changed executable production lines are 100% covered (8/8).
  • docs:verify reaches the same pre-existing missing generated parser AST error on both untouched baseline and patched trees; no failure identity changed.
  • Two independent Codex-high reviews approved with no actionable findings.

📋 Tasks

  • 📖 have read the contribution guidelines.
  • 💻 have added necessary unit/e2e tests.
  • 📓 documentation is not needed for this bug fix.
  • 🦋 added a patch changeset for mermaid.

Review Summary

What's working well

  • 🎉 The fix directly addresses issue #7341.
  • 🎉 The shared renderer supports a layout-specific edgeLabelOffsetY.
  • 🎉 ELK sets the offset to 0, while other layouts retain the existing behavior.
  • 🎉 Unit and end-to-end tests cover the regression.
  • 🎉 A patch changeset is included.

Security

No XSS or injection issues identified.

Things to address

No blocking or important issues found.

Verdict

APPROVE

Severity tally: 0 blocking / 0 important / 0 nit / 0 suggestion / 5 praise

@changeset-bot

changeset-bot Bot commented Sep 9, 2026 •
edited
Loading

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 6f04b4f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
mermaid Patch

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

@netlify

netlify Bot commented Sep 9, 2026 •
edited
Loading

Copy link
Copy Markdown

✅ Deploy Preview for mermaid-js ready!

Name Link
🔨 Latest commit 6f04b4f
🔍 Latest deploy log https://app.netlify.com/projects/mermaid-js/deploys/6aa169e4aa1cbc0009c64efc
😎 Deploy Preview https://deploy-preview-8238--mermaid-js.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@github-actions github-actions Bot added the Type: Bug / Error Something isn't working or is incorrect label Sep 9, 2026
@coderabbitai

coderabbitai Bot commented Sep 9, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 249461bc-50a5-4e1a-8554-3b07f35eb669

📥 Commits

Reviewing files that changed from the base of the PR and between 8f63671 and 6f04b4f.

📒 Files selected for processing (6)
  • docs/config/setup/mermaid/functions/clearLayoutRenderState.md
  • docs/config/setup/mermaid/functions/createCommonLayoutRenderer.md
  • docs/config/setup/mermaid/functions/defaultMeasureLayout.md
  • docs/config/setup/mermaid/functions/paintLayoutData.md
  • docs/config/setup/mermaid/interfaces/CommonLayoutPaintOptions.md
  • docs/config/setup/mermaid/interfaces/CommonLayoutRendererDefinition.md

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

Walkthrough

The change adds configurable vertical offsets for rendered edge labels. The ELK renderer uses zero offset because ELK returns final label coordinates. Unit and end-to-end tests verify centered labels when subgraph title margins are configured.

Changes

ELK edge label alignment

Layer / File(s) Summary
Common edge label offset handling
packages/mermaid/src/rendering-util/layout-algorithms/common/index.ts, packages/mermaid/src/rendering-util/layout-algorithms/common/index.spec.ts, docs/config/setup/mermaid/...		 The common layout options accept edgeLabelOffsetY. Edge label positioning uses the configured value when present. Unit coverage verifies zero offset handling. Generated documentation records the new option and updated source locations.
ELK integration and regression coverage
packages/mermaid/src/rendering-util/layout-algorithms/elk/render.ts, e2e/rendering/flowchart/flowchart.spec.js, .changeset/elk-edge-label-margin.md		 The ELK renderer sets edgeLabelOffsetY to 0. End-to-end coverage checks label and edge-path vertical centers. A patch changeset records the fix.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 6f04b

ELK flowchart edge labels now remain centered when subgraph title margins are configured, while existing layout behavior retains its default positioning. The change is covered by targeted unit and end-to-end tests and is ready to merge.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 4 files. (6 skipped: 6 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the primary change: keeping ELK flowchart edge labels centered.
Description check ✅ Passed The description includes the summary, linked issue, design decisions, test evidence, task completion, and changeset information. It is complete for this bug fix.
Linked Issues check ✅ Passed The implementation directly addresses issue #7341 by preventing ELK edge labels from shifting when subGraphTitleMargin is configured. Unit and end-to-end tests cover the fix.
Out of Scope Changes check ✅ Passed The changeset, renderer update, tests, and generated documentation updates all support the linked ELK label-centering fix. No unrelated code changes are identified.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 4 files. (6 skipped: 6 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@pkg-pr-new

pkg-pr-new Bot commented Sep 9, 2026 •
edited
Loading

Copy link
Copy Markdown

Open in StackBlitz

@mermaid-js/examples

npm i https://pkg.pr.new/@mermaid-js/examples@8238

mermaid

npm i https://pkg.pr.new/mermaid@8238

@mermaid-js/layout-elk

npm i https://pkg.pr.new/@mermaid-js/layout-elk@8238

@mermaid-js/layout-tidy-tree

npm i https://pkg.pr.new/@mermaid-js/layout-tidy-tree@8238

@mermaid-js/mermaid-zenuml

npm i https://pkg.pr.new/@mermaid-js/mermaid-zenuml@8238

@mermaid-js/parser

npm i https://pkg.pr.new/@mermaid-js/parser@8238

@mermaid-js/tiny

npm i https://pkg.pr.new/@mermaid-js/tiny@8238

commit: 6f04b4f

@codecov

codecov Bot commented Sep 9, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.13%. Comparing base (fe0e237) to head (6f04b4f).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #8238   +/-   ##
========================================
  Coverage    80.13%   80.13%           
========================================
  Files          624      624           
  Lines        85311    85316    +5     
  Branches     16183    16184    +1     
========================================
+ Hits         68366    68371    +5     
  Misses       15909    15909           
  Partials      1036     1036
Flag Coverage Δ
e2e 73.90% <100.00%> (+<0.01%) ⬆️
unit 77.90% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...c/rendering-util/layout-algorithms/common/index.ts 88.70% <100.00%> (+0.18%) ⬆️
...src/rendering-util/layout-algorithms/elk/render.ts 94.28% <100.00%> (+<0.01%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@argos-ci

argos-ci Bot commented Sep 9, 2026 •
edited
Loading

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Argos notifications ↗︎

Build Status Details Updated (UTC)
default (Inspect) ⚠️ Changes detected (Review) 3 changed Sep 9, 2026, 2:38 PM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Type: Bug / Error Something isn't working or is incorrect

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Flowchart subGraphTitleMargin moves link label down when using layout elk

1 participant

@ousamabenyounes