fix(apollo-react): improve stage return-edge snapping - #1030
fix(apollo-react): improve stage return-edge snapping#1030jevinkosasih wants to merge 5 commits into
Conversation
|
Apollo Coded App preview deployments are ready.
|
Dependency License Review
License distribution
Excluded packages
|
📊 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 |
github-actions
Bot
added
size:M
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (1)
packages/apollo-react/src/canvas/components/StageNode/StageNodeHandles.tsx:24
selectIsConnectingnow standardizes “connecting” detection onstate.connection.inProgress(avoids the stuckconnectionClickStartHandlecase noted in NodeUtils), but there are still components (e.g. TriggerNode) using!!state.connectionClickStartHandle. That leaves the stuck-handles behavior in place for those nodes and makes connecting UI behavior inconsistent across node types. Consider switching remaining call sites toselectIsConnecting(and updating any related mocks/tests) so all nodes follow the same, non-sticky connecting signal.
const isConnecting = useStore(selectIsConnecting);
const connectedHandleIds = useConnectedHandles(id);
const hasConnections = connectedHandleIds.size > 0;
| geometry: StageEdgeGeometry; | ||
| } | ||
|
|
||
| function stageEdgeGeometryEquality(previous: StageEdgeGeometry, next: StageEdgeGeometry): boolean { |
There was a problem hiding this comment.
Removed because this equality helper only supported the useStore selector that rebuilt fixed upper-right and upper-left node geometry. StageEdge now consumes the live sourceX, sourceY, targetX, and targetY coordinates React Flow calculates from the registered handles, then memoizes that geometry object for StageEdgeInnerMemo. With no local store selector remaining, the custom equality function is unused dead code and React Flow remains responsible for updating the edge when a handle or node moves.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (1)
packages/apollo-react/src/canvas/styles/reactflow-reset.css:55
- The z-index layering comment above the hovered-node rule is now inaccurate: it claims this hover bump clears the connection-line z-index (1001), but the new rule sets the connection line to 1003, which will render above hovered nodes (1002). Please update the comment to reflect the new layering so future changes don’t rely on outdated assumptions.
/* Keep the temporary drag-to-connect edge visible as it crosses stage nodes. */
svg.react-flow__connectionline {
z-index: 1003 !important;
}
| const geometry = useStore((state: ReactFlowState): StageEdgeGeometry => { | ||
| const sourceNode = state.nodeLookup.get(rest.source); | ||
| const targetNode = state.nodeLookup.get(rest.target); | ||
|
|
||
| return { | ||
| sourceX: sourceNode ? sourceNode.position.x + (sourceNode.measured?.width ?? 0) : sourceX, | ||
| sourceY: sourceNode ? sourceNode.position.y + 32 : sourceY, | ||
| targetX: targetNode?.position.x ?? targetX, | ||
| targetY: targetNode ? targetNode.position.y + 32 : targetY, | ||
| }; | ||
| }, stageEdgeGeometryEquality); |
There was a problem hiding this comment.
Removed because this selector discarded the handle coordinates supplied in EdgeProps and rebuilt every stage edge as upper-right header to upper-left header: source x plus width and y plus 32, then target x and y plus 32. That made bottom-to-bottom return edges render from the header even though React Flow had registered the correct bottom-handle coordinates. React Flow already updates sourceX, sourceY, targetX, and targetY when handles or nodes move, so StageEdge now memoizes those supplied values and no longer needs a direct nodeLookup subscription.
Storybook visual diffBaseline is the deployed main Storybook, so changes merged to main after this branch was last updated can also appear here. Logs Updated (PT): Aug 08, 2026, 11:40:05 PM |
| const sourceX = fromNode ? fromNode.position.x + (fromNode.measured?.width ?? 0) : fromX; | ||
| const sourceY = fromNode?.position.y ? fromNode.position.y + 32 : fromY; | ||
|
|
||
| const targetX = toNode ? toNode.position.x : toX; | ||
| const targetY = toNode ? toNode.position.y + 32 : toY; |
There was a problem hiding this comment.
Removed because these calculations forced the connection preview from the source stage upper-right header to the target stage upper-left header whenever node objects were available, regardless of which handles were active. For a bottom-to-bottom return edge, React Flow already provides the exact registered bottom-handle coordinates through fromX, fromY, toX, and toY; overriding them here made the ghost edge originate and land at the header. Passing those coordinates directly preserves the selected handles, the live cursor or snapped target position, and also avoids the position.y equals zero truthiness issue.
|
PO.Frontend integration note: Case Management uses Apollo StageNode and its handles, but it does not use Apollo StageEdge or StageConnectionEdge. Persisted case-management edges are rendered by PO.Frontend through CaseManagementTransitionEdge and CaseManagementEdge, while the drag preview uses PreviewConnectAndReconnectEdge and PreviewEdgePath. That is why production edges could render correctly even though Apollo own edge components always reconstructed upper-right to upper-left header geometry. For the original bottom-handle locking issue, the production-relevant fix is in ButtonHandle and StageNodeHandles: visible handles no longer force pointer events over xyflow connection-state handling, so the overlapping bottom source cannot intercept the pointer from the bottom target; the target also gets a centered 24 by 24 hit area. The connection-line z-index applies to PO custom preview as well. The StageEdge and StageConnectionEdge coordinate changes correct Apollo Storybook and direct consumers, but PO currently bypasses those renderers. Once this change is released and PO.Frontend bumps to the published Apollo version containing it, the bottom target should lock correctly without the PO inline handle workaround. The consumer workaround should be removed when validating the bump so the Apollo behavior is tested directly. |
Summary
Connection compatibility remains consumer-owned through isValidConnection; this change does not restrict bottom handles to specific target handles.
Testing