Skip to content

fix(elk): scope rounded edge curve to ELK layout only#7454

Merged
ashishjain0512 merged 1 commit intomasterfrom
bug/7213_elk-edges-rounded-scoped-master
Mar 5, 2026
Merged

fix(elk): scope rounded edge curve to ELK layout only#7454
ashishjain0512 merged 1 commit intomasterfrom
bug/7213_elk-edges-rounded-scoped-master

Conversation

@knsv-bot
Copy link
Collaborator

@knsv-bot knsv-bot commented Mar 5, 2026

Summary

Scopes the fix from #7425 (issue #7213) so that the rounded curve is only applied when the ELK layout algorithm is used, rather than changing the global default for all flowcharts.

  • Reverts the global flowchart.curve default from 'rounded' back to 'basis'
  • Sets edge.curve = 'rounded' on each ELK edge right before insertEdge, ensuring ELK's orthogonal routes render with right-angle segments and rounded corners
  • Non-ELK layouts are completely unaffected

Resolves #7213

Test plan

  • Unit tests pass (973 flowchart + edge tests)
  • Config type verification passes
  • Build passes (no TS errors)
  • Visual verification — ELK edges route orthogonally with rounded corners
  • Cypress e2e snapshots
  • CI passes (lint, unit, e2e, type-check)

🤖 Generated with Claude Code

Reverts the global flowchart curve default from 'rounded' back to 'basis'
and instead sets edge.curve = 'rounded' on each ELK edge right before
insertEdge. This ensures ELK's orthogonal routes render with right-angle
segments and rounded corners, without changing the default for non-ELK
layouts.

Fixes #7213

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@changeset-bot
Copy link

changeset-bot bot commented Mar 5, 2026

⚠️ No Changeset found

Latest commit: 4c968e2

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions github-actions bot added the Type: Bug / Error Something isn't working or is incorrect label Mar 5, 2026
@knsv knsv marked this pull request as ready for review March 5, 2026 15:43
@codecov
Copy link

codecov bot commented Mar 5, 2026

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 3.53%. Comparing base (547e6d5) to head (4c968e2).
⚠️ Report is 448 commits behind head on master.

Files with missing lines Patch % Lines
packages/mermaid-layout-elk/src/render.ts 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #7454      +/-   ##
=========================================
- Coverage    3.59%   3.53%   -0.06%     
=========================================
  Files         475     493      +18     
  Lines       47191   49010    +1819     
  Branches      735     769      +34     
=========================================
+ Hits         1696    1734      +38     
- Misses      45495   47276    +1781     
Flag Coverage Δ
unit 3.53% <0.00%> (-0.06%) ⬇️

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

Files with missing lines Coverage Δ
packages/mermaid-layout-elk/src/render.ts 0.12% <0.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.

Copy link
Collaborator

@ashishjain0512 ashishjain0512 left a comment

Choose a reason for hiding this comment

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

[sisyphus-bot]

Clean, focused fix — this is exactly the right scope for the ELK edge curve issue.

Note: PR targets master. This appears to be an intentional hotfix by the maintainer. Also note that PR #7452 covers this same fix bundled with other changes — worth clarifying which PR supersedes the other.

What's working well

🎉 [praise] Excellent scoping — the fix is isolated to just 3 files with no collateral changes. Reverting the global default to basis in config.schema.yaml:2160 and setting edge.curve = 'rounded' only in the ELK render path (render.ts:1073) is the cleanest possible approach.

🎉 [praise] Good explanatory comment on the edge.curve = 'rounded' line — "ELK produces orthogonal edge routes" explains the why, not just the what.

🎉 [praise] The changeset correctly scopes to both mermaid and @mermaid-js/mermaid-layout-elk packages with patch bumps. The description clearly communicates what changed and what's unaffected.

Things to address

🟡 [important] The PR checklist shows unchecked items for Cypress e2e snapshots and CI. Since this changes the rendering behavior of ELK edges, visual regression verification is important — especially to confirm that the original issue (#7213, curved ELK edges) is resolved and that non-ELK flowcharts are visually unchanged. It would be great to confirm CI passes cleanly before merge.

💡 [suggestion] Consider adding a Cypress e2e test case using the reproduction diagram from issue #7213 (the flowchart with subgraphs G1/G2 and repeated N11 --- N22 edges). This would prevent regression of this specific fix and serve as documentation that ELK edge routing is intentionally orthogonal.

Security

No XSS or injection issues. This PR only modifies a config default value and sets a string property (edge.curve) on an internal edge object. No user input handling, DOM manipulation, or sanitization changes.

Self-check

  • At least one 🎉 [praise] item exists (3)
  • No duplicate comments
  • Severity tally: 0 🔴 blocking / 1 🟡 important / 0 🟢 nit / 1 💡 suggestion / 3 🎉 praise
  • Verdict: COMMENT (1 🟡 — e2e verification pending)
  • Not a draft PR
  • No inline comments used
  • Tone check: constructive, appreciative

@argos-ci
Copy link

argos-ci bot commented Mar 5, 2026

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

Build Status Details Updated (UTC)
default (Inspect) 👍 Changes approved 401 changed Mar 5, 2026, 3:55 PM

@knsv
Copy link
Collaborator

knsv commented Mar 5, 2026

The e2e test is there already

image

@ashishjain0512 ashishjain0512 merged commit 0d5e35a into master Mar 5, 2026
17 of 18 checks passed
@mermaid-bot
Copy link

mermaid-bot bot commented Mar 5, 2026

@knsv-bot, Thank you for the contribution!
You are now eligible for a year of Premium account on MermaidChart.
Sign up with your GitHub account to activate.

@ashishjain0512 ashishjain0512 deleted the bug/7213_elk-edges-rounded-scoped-master branch March 5, 2026 21:21
sidharthv96 added a commit that referenced this pull request Mar 6, 2026
* 'master' of https://github.com/mermaid-js/mermaid:
  fix(gantt): restore readable outside-text for done tasks in dark mode (#7456)
  fix(elk): scope rounded edge curve to ELK layout only (#7454)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants