Skip to content

fix(routers.rightAngle): allow zero-value margins and un-clamp minPathMargin - #3435

Merged
zbynekstara merged 3 commits into
clientIO:masterfrom
Geliogabalus:fix-right-angle
Jul 24, 2026
Merged

fix(routers.rightAngle): allow zero-value margins and un-clamp minPathMargin#3435
zbynekstara merged 3 commits into
clientIO:masterfrom
Geliogabalus:fix-right-angle

Conversation

@Geliogabalus

Copy link
Copy Markdown
Contributor

Summary

  • rightAngle router used || for margin, sourceMargin, and targetMargin, so 0 was silently coerced back to the default (20/margin) instead of being honored.
  • minPathMargin was clamped with Math.min(opt.minPathMargin, sourceMargin/targetMargin), so it could only ever narrow the overlap-detection zone, never widen it beyond margin.
  • Both are now resolved with ??, so 0 is respected and minPathMargin can be set larger than margin to intentionally widen the detour-detection zone.
  • Rewrote the right-angle-playground-js example into an interactive demo with live sliders for sourceMargin, targetMargin, and minPathMargin, visualizing each margin as a band around the elements/link.

Test plan

  • Added tests covering margin: 0, sourceMargin: 0, and targetMargin: 0.
  • Added a test demonstrating minPathMargin larger than margin actually changes the route.
  • Renamed the pre-existing (mistyped) minMargin test option to minPathMargin and adjusted element positions so the "with"/"without" assertions in each of those tests actually diverge instead of asserting identical paths.
  • grunt karma:joint passes for test/jointjs/routers.js (66/66); full joint suite shows only 2 pre-existing, unrelated failures (util.breakText ellipsis, port labels label attributes).
  • eslint clean on the modified test file.

🤖 Generated with Claude Code

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.

Pull request overview

This PR adjusts the rightAngle router option handling in @joint/core so zero-valued margins are honored and minPathMargin can widen (not just narrow) the overlap-detection zone. It also updates router tests accordingly and rewrites the right-angle-playground-js example into an interactive, visual demo for tuning these margins.

Changes:

  • Use nullish coalescing (??) for margin, sourceMargin, and targetMargin, and stop clamping minPathMargin to per-side margins.
  • Update/add QUnit coverage for margin: 0, sourceMargin: 0, targetMargin: 0, and minPathMargin > margin, plus rename prior minMargin test usage to minPathMargin.
  • Rework the right-angle-playground-js example into a slider-driven demo that visualizes margin bands.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
packages/joint-core/src/routers/rightAngle.mjs Updates router option defaults so 0 is respected and minPathMargin is no longer clamped.
packages/joint-core/test/jointjs/routers.js Adjusts/extends tests to cover zero-margins and widened minPathMargin, and aligns naming with the public option.
examples/right-angle-playground-js/index.html Adds a toolbar UI for live-tuning margin-related router options.
examples/right-angle-playground-js/src/main.js Implements interactive router option binding and margin visualization on elements/link.
examples/right-angle-playground-js/src/styles.css Adds toolbar/control styling and updates the demo’s look/feel.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread examples/right-angle-playground-js/src/styles.css
Comment thread examples/right-angle-playground-js/src/main.js
Comment thread examples/right-angle-playground-js/index.html Outdated
@zbynekstara zbynekstara changed the title fix(routers): rightAngle - allow zero-value margins and un-clamp minPathMargin fix(routers.rightAngle): allow zero-value margins and un-clamp minPathMargin Jul 24, 2026
@zbynekstara
zbynekstara merged commit 8a633e3 into clientIO:master Jul 24, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants