feat(apollo-react): add loop node [MST-8596]#556
Conversation
32bb7bb to
2039ef0
Compare
There was a problem hiding this comment.
Pull request overview
Adds a new Loop container node experience to the canvas, including loop-scoped preview insertion behavior and improved handle/preview positioning semantics.
Changes:
- Introduces
LoopNode(component, types, helpers, preview utilities) plus Storybook fixtures and unit tests. - Extends preview-node creation to support centered placement and loop-aware edge insertion previews (including preview-edge detection).
- Enhances handle behavior via
connectionPosition, and preserves parented preview placement when materializing a real node.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/apollo-react/src/canvas/utils/createPreviewNode.ts | Adds positionMode (drop vs center) and centered placement math for preview nodes. |
| packages/apollo-react/src/canvas/utils/createPreviewNode.test.ts | Adds coverage for default drop semantics vs centered positioning. |
| packages/apollo-react/src/canvas/components/index.ts | Re-exports LoopNode from the canvas components barrel. |
| packages/apollo-react/src/canvas/components/Toolbar/EdgeToolbar/useEdgeToolbarState.ts | Attempts loop-aware preview creation first when inserting along an edge. |
| packages/apollo-react/src/canvas/components/Toolbar/EdgeToolbar/useEdgeToolbarState.test.ts | Adds test ensuring loop preview utilities are preferred when applicable. |
| packages/apollo-react/src/canvas/components/LoopNode/index.ts | Exports LoopNode and its types. |
| packages/apollo-react/src/canvas/components/LoopNode/LoopNode.types.ts | Defines LoopNode public props/handles/header contract. |
| packages/apollo-react/src/canvas/components/LoopNode/LoopNode.tsx | Implements resizable loop container UI, inner/outer handles, and drag accept/reject visuals. |
| packages/apollo-react/src/canvas/components/LoopNode/LoopNode.test.tsx | Unit tests for header rendering, empty-state add, drag state, and resize affordances. |
| packages/apollo-react/src/canvas/components/LoopNode/LoopNode.stories.tsx | Storybook fixtures for empty/connected loops, handle-add previews, and drag validation. |
| packages/apollo-react/src/canvas/components/LoopNode/LoopNode.preview.ts | Adds loop-scoped preview graph creation and apply/remove helpers. |
| packages/apollo-react/src/canvas/components/LoopNode/LoopNode.preview.test.ts | Tests centered previews, edge insertion previews, and apply/remove behavior. |
| packages/apollo-react/src/canvas/components/LoopNode/LoopNode.helpers.ts | Helpers for wall handles, loop-body center, and dragged payload parsing. |
| packages/apollo-react/src/canvas/components/LoopNode/LoopNode.helpers.test.ts | Tests helper behaviors (groups, center calc, payload parsing). |
| packages/apollo-react/src/canvas/components/Edges/SequenceEdge.tsx | Treats edges connected to the preview node as preview edges (not only by preview edge id). |
| packages/apollo-react/src/canvas/components/ButtonHandle/ButtonHandle.tsx | Adds connectionPosition so emitted direction/handle metadata can differ from visual placement. |
| packages/apollo-react/src/canvas/components/ButtonHandle/ButtonHandle.test.tsx | Tests that connectionPosition drives emitted position while layout stays on position. |
| packages/apollo-react/src/canvas/components/ButtonHandle/ButtonHandle.styles.tsx | Supports $visualPosition so layout can use visual placement while Handle uses connection placement. |
| packages/apollo-react/src/canvas/components/AddNodePanel/createAddNodePreview.ts | Adds optional placement options (position + positionMode) for previews created from handles. |
| packages/apollo-react/src/canvas/components/AddNodePanel/AddNodeManager.tsx | Preserves parentId/extent when turning a preview into a real node. |
| packages/apollo-react/src/canvas/components/AddNodePanel/AddNodeManager.test.tsx | Adds test verifying parented preview placement is preserved on materialization. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Dependency License Review
License distribution
Excluded packages
|
2039ef0 to
d48603d
Compare
45a3534 to
a84decf
Compare
a84decf to
3c9a626
Compare
3c9a626 to
91d80d5
Compare
91d80d5 to
d1047d6
Compare
📦 Dev Packages🧹 Dev packages cleaned up after PR close. Last updated: 2026-04-28 22:03:35 PT |
BenGSchulz
left a comment
There was a problem hiding this comment.
I noticed some unexpected behavior when adding nodes within a loop.
The "add between" seems to preserve parenting (i.e. I can't move the node out of the container).
But the "add from handle" does not seem to preserve parenting. Is this expected, like we could technically break out of the loop that way? Or do we want to lock down parenting so any node created off of a node with a parent will also be parented?
Dragging a node within the container causes the full canvas to pan, which was a bit disorienting.
Screen.Recording.2026-04-22.at.1.32.58.PM.mov
The behaviour I was aiming for is -
So the reported issues are valid -
My original thought was to keep the detachment logic in the consuming layer (flow-workbench), since that’s where the full reparenting/edge-repair behaviour belongs. But I agree the current Storybook demo feels broken/confusing without it, so for now I can add a small story-only helper to make the interaction behave as intended in the demo, while keeping the real production logic in flow-workbench. wdyt? |
d1047d6 to
fe55e35
Compare
b1f8013 to
b8727fd
Compare
bf742a3 to
3bac354
Compare
b9d523d to
893886b
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 40 out of 40 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
packages/apollo-react/src/canvas/schema/node-definition/handle.ts:150
boundarywas added tohandleGroupManifestSchema, but instance-levelhandleGroupOverrideSchemastill can’t specify a boundary. That means a node instance can’t override a handle group to beinnerwithout failing schema validation, which makes the new container-handle feature incomplete for handle customizations. Consider addingboundary?: HandleBoundarytohandleGroupOverrideSchema(and any downstream types) so overrides can target inner/outer boundaries too.
export const handleGroupManifestSchema = z.object({
/** Position on the node */
position: handlePositionSchema,
/**
* Optional boundary for container-style nodes.
* `outer` renders on the node shell; `inner` renders on an inner frame/wall.
* Defaults to `outer` when omitted.
*/
boundary: handleBoundarySchema.optional(),
customPositionAndOffsets: handleConfigurationSpecificPositionSchema.optional(),
/** Array of handles at this position */
handles: z.array(handleManifestSchema),
/** Whether the handle group is visible */
visible: z.boolean().optional(),
});
/**
* Instance-level handle group replacement
* Allows individual nodes to completely replace a handle group
*/
export const handleGroupOverrideSchema = z.object({
/** Position identifier (matches manifest group) */
position: handlePositionSchema,
/** Replacement handles for this position */
handles: z.array(handleManifestSchema),
/** Whether the handle group is visible */
visible: z.boolean().optional(),
});
893886b to
99645ab
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 40 out of 40 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
packages/apollo-react/src/canvas/schema/node-definition/handle.ts:150
handleGroupManifestSchemanow supportsboundary: 'outer' | 'inner', buthandleGroupOverrideSchema(used by node instancehandleCustomization) can’t express or disambiguate boundary—especially now that a node can legitimately have multiple groups with the sameposition(outer + inner). This makes instance-level handle customization ambiguous for container nodes. Consider addingboundaryto the override schema (and matching logic by{position, boundary}), or introducing a stable group identifier to target overrides reliably.
export const handleGroupManifestSchema = z.object({
/** Position on the node */
position: handlePositionSchema,
/**
* Optional boundary for container-style nodes.
* `outer` renders on the node shell; `inner` renders on an inner frame/wall.
* Defaults to `outer` when omitted.
*/
boundary: handleBoundarySchema.optional(),
customPositionAndOffsets: handleConfigurationSpecificPositionSchema.optional(),
/** Array of handles at this position */
handles: z.array(handleManifestSchema),
/** Whether the handle group is visible */
visible: z.boolean().optional(),
});
/**
* Instance-level handle group replacement
* Allows individual nodes to completely replace a handle group
*/
export const handleGroupOverrideSchema = z.object({
/** Position identifier (matches manifest group) */
position: handlePositionSchema,
/** Replacement handles for this position */
handles: z.array(handleManifestSchema),
/** Whether the handle group is visible */
visible: z.boolean().optional(),
});
CalinaCristian
left a comment
There was a problem hiding this comment.
Follow up (should log tickets if it makes sense to take them):
- On empty loop, the + is not aligned with the start:
- Initial placement of the loop node should be centered by the edge (edge should go straight).
-
Adding a new node from the initally added node adds it outside (this is blocker for this PR in case it behaves the same way in flow workbench and it's not just a storybook isssue):
https://github.com/user-attachments/assets/74a883dd-4526-489a-b7b0-76c5702440d3 -
Is the break handle optional ? (it should be toggle from the property panel in flow-workbench).
Thanks Cristian
2 - I'm tracking this & fixed in #605 Ticket for the issues 1, 2 |
826aeb9 to
6dabf73
Compare
| export function getHandleActionPortal({ | ||
| nodeId, | ||
| position, | ||
| positionPercent, | ||
| total, | ||
| nodeWidth, | ||
| nodeHeight, | ||
| }: HandleActionPortalOptions): HandleButtonPortal | undefined { | ||
| if (!nodeWidth || !nodeHeight) { | ||
| return undefined; | ||
| } | ||
|
|
||
| const edgeCoverageRatio = HANDLE_EDGE_COVERAGE_RATIO / total; | ||
| const horizontalWidth = nodeWidth * edgeCoverageRatio; | ||
| const verticalHeight = nodeHeight * edgeCoverageRatio; | ||
| const x = nodeWidth * (positionPercent / 100); | ||
| const y = nodeHeight * (positionPercent / 100); | ||
|
|
||
| switch (position) { | ||
| case Position.Top: | ||
| return { | ||
| nodeId, | ||
| left: x, | ||
| top: 0, | ||
| width: horizontalWidth, | ||
| height: HANDLE_CROSS_AXIS_SIZE_PX, | ||
| transform: 'translate(-50%, -50%)', | ||
| }; | ||
| case Position.Bottom: | ||
| return { | ||
| nodeId, | ||
| left: x, | ||
| top: nodeHeight - HANDLE_CROSS_AXIS_SIZE_PX, | ||
| width: horizontalWidth, | ||
| height: HANDLE_CROSS_AXIS_SIZE_PX, | ||
| transform: 'translate(-50%, 50%)', | ||
| }; | ||
| case Position.Left: | ||
| return { | ||
| nodeId, | ||
| left: 0, | ||
| top: y, | ||
| width: HANDLE_CROSS_AXIS_SIZE_PX, | ||
| height: verticalHeight, | ||
| transform: 'translate(-50%, -50%)', | ||
| }; | ||
| case Position.Right: | ||
| return { | ||
| nodeId, | ||
| left: nodeWidth - HANDLE_CROSS_AXIS_SIZE_PX, | ||
| top: y, | ||
| width: HANDLE_CROSS_AXIS_SIZE_PX, | ||
| height: verticalHeight, | ||
| transform: 'translate(50%, -50%)', | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| export function getInwardHandleLayout( | ||
| position: Position, | ||
| handleType: HandleType | ||
| ): InwardHandleLayout { | ||
| const notchOverlap = -INWARD_NOTCH_OVERLAP_PX[handleType]; | ||
| const anchorSize = { | ||
| width: INWARD_HANDLE_ANCHOR_SIZE_PX, | ||
| height: INWARD_HANDLE_ANCHOR_SIZE_PX, | ||
| }; | ||
|
|
||
| switch (position) { | ||
| case Position.Left: | ||
| return { | ||
| rootTransform: 'translate(0, -50%)', | ||
| contentDirectionClassName: 'flex-row', | ||
| notchStyle: { marginLeft: notchOverlap }, | ||
| anchorStyle: { | ||
| ...anchorSize, | ||
| left: `calc(100% - ${INWARD_HANDLE_ANCHOR_RADIUS_PX}px)`, | ||
| top: '50%', | ||
| transform: 'translateY(-50%)', |
There was a problem hiding this comment.
ButtonHandleLayoutUtils introduces new layout/positioning math that affects handle hit areas and portal anchoring, but it is currently untested. Since this logic is easy to regress and there are already tests for other ButtonHandle utilities, consider adding focused unit tests for getHandleActionPortal and getInwardHandleLayout (at least one per Position) to lock in expected geometry.
| const shouldShowHandles = (isConnecting || selected || isHovered) && !dragging; | ||
|
|
||
| const showHandleAddButtons = isDesignMode && !multipleNodesSelected && !isConnecting && !dragging; | ||
| const showResizeControls = selected && !dragging && isDesignMode; | ||
| const showEmptyStateButton = isDesignMode && !hasChildNodes && !!onAddFirstChild; | ||
|
|
There was a problem hiding this comment.
This PR introduces a new LoopNode implementation with a fair amount of new interaction logic (empty-state CTA, hover/selection-driven handle visibility, resize snapping, toolbar portaling, container handle boundaries), but there are no unit/component tests alongside it. Given the existing test coverage for other canvas nodes/components, please add a LoopNode.test.tsx (and/or LoopCanvasNode.test.tsx) covering the core behaviors to prevent regressions.



This PR adds a container-style loop node to
apollo-reactand wires the canvas preview/add-node flows needed to place real child nodes inside the loop body.Core changes
LoopNode/LoopCanvasNodefor manifests withdisplay.shape: 'container', including the loop shell, header, body frame, empty-state add CTA, resize controls, adornments, status styling, and toolbar supportcontainershape and handle groups withboundary: 'outer' | 'inner', allowing loop handles to render on the outer shell or inner body frame while keeping React Flow edge anchors alignedButtonHandle,HandleButton, andNodeViewportOverlayso handle affordances can use separate visual/connection positions, inward-facing container handles, stable hit areas, and portal-rendered buttons/labels above React Flow node layerscreatePreviewGraphon top ofcreatePreviewNode, supporting one or two preview edges, replacing an existing edge while inserting a node, and reparenting preview nodes into containers withparentId/extent: 'parent'createAddNodePreview, connect-end insertion, edge-toolbar insertion, andAddNodeManager) to use preview graphs, preserve container parentage when materializing nodes, restore removed edges on cancellation, and treat any edge touching the preview node as a preview edgeLoopCanvasNodeinHierarchicalCanvasand Storybook helpers while keeping other manifest-driven nodes onBaseNodeuipath.control-flow.foreachmanifest toshape: 'container'with outer input/success handles and innerStart,Continue, andBreakhandlesReviewer focus
LoopNode.tsx/LoopCanvasNode.tsx: container rendering, resize behavior, empty-state first-child preview, inner/outer handle visibility, toolbar/adornment/status behaviorButtonHandle.tsx/HandleButton.tsx/NodeViewportOverlay.tsx: separate visual vs connection anchors and portal layering for handle affordances and toolbarscreatePreviewGraph.tsand callers: multi-edge preview creation, edge replacement/restoration, and reparenting previews into containersHierarchicalCanvas.tsxand Storybook manifest helpers: container manifest detection and preview-edge cleanup during connect flowsCoverage
useAddNodeOnConnectEndanduseEdgeToolbarStateunit tests for the move toshowPreviewGraphCanvas/LoopNodescenario with connected ingress, loop body, and egress nodesRef: https://uipath.atlassian.net/browse/MST-8596