fix(apollo-react): clear node faces so routed edges exit perpendicular [MST-11337]#834
fix(apollo-react): clear node faces so routed edges exit perpendicular [MST-11337]#834KodudulaAshishUiPath wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Dependency License Review
License distribution
Excluded packages
|
There was a problem hiding this comment.
✅ Ready to approve
The change is well-scoped behind an autoRouted flag, and the new geometry behavior is covered by targeted regression tests.
Note: this review does not count toward required approvals for merging.
Pull request overview
This PR improves auto-routed (graph router/ELK) Canvas edge rendering for multi-handle nodes by ensuring routed edges exit/enter node faces with a consistent perpendicular “stub” offset, eliminating immediate kinks at handles while keeping manually placed waypoints untouched.
Changes:
- Added
clearNodeFace()to shift the nearest routed riser off node faces toEDGE_CONSTANTS.STUB_OFFSET, applied at both source and target inbuildPathVertices. - Threaded a new
autoRoutedflag fromCanvasEdge→useEdgeGeometry→buildPathVerticesto apply face-clearance only to router-produced waypoints. - Added unit tests covering source/target clearance, vertical faces, behind-face pulls, and manual-waypoint non-shifting.
File summaries
| File | Description |
|---|---|
| packages/apollo-react/src/canvas/components/Edges/shared/hooks/useEdgeGeometry.ts | Adds autoRouted arg and passes it through to buildPathVertices. |
| packages/apollo-react/src/canvas/components/Edges/shared/geometry.ts | Implements node-face clearance logic and applies it conditionally for auto-routed waypoints. |
| packages/apollo-react/src/canvas/components/Edges/shared/geometry.test.ts | Adds regression/unit coverage for the new clearance behavior. |
| packages/apollo-react/src/canvas/components/Edges/CanvasEdge.tsx | Sets autoRouted when rendering router-produced routedWaypoints (no manual waypoints). |
Copilot's findings
- Files reviewed: 4/4 changed files
- Comments generated: 1
Note
Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.
📊 Coverage + size by packagePer-package coverage and bundle size on this PR. New-line coverage = of the source lines this PR adds or changes, the % hit by tests.
"Coverage" is each package's own |
There was a problem hiding this comment.
⚠️ Not ready to approve
clearNodeFace currently determines the “nearest” riser via a global extremum across all waypoints, which can select the wrong segment for non-monotonic routed polylines and fail to clear the actual face-adjacent bend.
Copilot's findings
- Files reviewed: 4/4 changed files
- Comments generated: 2
Note
Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.
KodudulaAshishUiPath
force-pushed
the
fix/MST-11337
branch
from
4492cfe to
6030b09
Compare
KodudulaAshishUiPath
force-pushed
the
fix/MST-11337
branch
from
6030b09 to
7fba88c
Compare
KodudulaAshishUiPath
force-pushed
the
fix/MST-11337
branch
from
7fba88c to
8c28afe
Compare
| /** Manual waypoints — only used in `waypoint` routing. */ | ||
| waypoints: Waypoint[]; | ||
| /** | ||
| * True when `waypoints` are router-produced (not user-placed), so the path | ||
| * builder may shift bends off the node faces. Manual waypoints render as-is. | ||
| */ | ||
| autoRouted?: boolean; |
| /** | ||
| * Push the waypoint riser nearest a node face (`anchor` = inset source/target | ||
| * endpoint) out to `EDGE_CONSTANTS.STUB_OFFSET` so the edge keeps a perpendicular | ||
| * offset. Any riser closer than that is shifted forward, including bends behind | ||
| * the face (gap < 0) — common on multi-handle nodes where the router's port sits | ||
| * behind the rendered handle. Shifting stored waypoints (ids kept) survives | ||
| * consolidation, whereas a derived elbow would not. | ||
| */ |
| * True when `waypoints` are router-produced (not user-placed), so the path | ||
| * builder may shift bends off the node faces. Manual waypoints render as-is. | ||
| */ | ||
| autoRouted?: boolean; |
There was a problem hiding this comment.
do you even need this autoRouted
can you just check the waypoints length downstream?
Summary
Routed (auto-layout) edges on multi-handle nodes rendered with a kink right at the source/target handle — no perpendicular exit offset. This shifts the bend nearest each node face out to a fixed perpendicular
STUB_OFFSETso edges leave and enter cleanly.ELK computes bend points from its port positions, which differ from the rendered React Flow handle positions on multi-handle nodes (Decision true/false, Script main/error). The first/last routed bend lands at or behind the handle, so the edge jogs immediately instead of exiting straight.
Demo
Before:
After:
Files Changed
clearNodeFace(waypoints, anchor, position)ingeometry.ts: shifts the waypoint riser nearest a node face out toSTUB_OFFSET(lands exactlySTUB_OFFSETin front for any gap, including bends behind the face). Applied to both source and target faces inbuildPathVertices.autoRoutedflag, threadedbuildPathVertices→useEdgeGeometry→CanvasEdge, so it only affects router-producedroutedWaypoints; manual waypoints render exactly where the user placed them.consolidateWaypoints, unlike a derived stub elbow.Tests
New
buildPathVertices/clearNodeFacecases: source + target shift, vertical (y-axis) face, behind-face forward-pull, and manual-not-shifted. 29 geometry / 85 Edges tests pass; biome + tsc clean.🤖 Generated with Claude Code