feat: add node alignment helper lines with toggle control in flow editor#8279
Conversation
…r nodes during drag and drop operations. This feature includes the ability to toggle helper lines on and off, snap nodes to alignment positions, and visually display horizontal and vertical lines for alignment.
…adability by removing redundant comments explaining basic logic in helper-lines.ts
dosubot
Bot
added
the
size:L
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughA helper lines feature was introduced for the flow editor. This includes state management for enabling helper lines, logic for calculating and snapping node positions, rendering helper lines during node dragging, new React components, and corresponding CSS for visual styling. The control panel now includes a toggle for this feature. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CanvasControls
participant FlowStore
participant PageComponent
participant HelperLines
User->>CanvasControls: Clicks "Helper Lines" toggle
CanvasControls->>FlowStore: setHelperLineEnabled(newState)
FlowStore-->>PageComponent: helperLineEnabled state updated
User->>PageComponent: Drags node
PageComponent->>PageComponent: Compute helper lines & snap position
PageComponent->>HelperLines: Render helper lines (if enabled)
PageComponent->>PageComponent: Update node position (if snapping)
User->>PageComponent: Releases node
PageComponent->>HelperLines: Remove helper lines
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
github-actions
Bot
added
enhancement
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
src/frontend/src/style/applies.css (1)
1266-1284: Good styling implementation with minor redundancy.The helper lines styling is well-implemented:
- Proper overlay positioning with
pointer-events-none- Good visual design with dashed lines, opacity, and drop shadow
- Correct use of CSS custom properties for theming
However, there's redundancy in the
.horizontaland.verticalsubclasses - they both set the samestroke: hsl(var(--primary))which is already defined in the parent.helper-lineclass.Consider removing the redundant stroke declarations:
-.helper-line.horizontal { - stroke: hsl(var(--primary)); -} - -.helper-line.vertical { - stroke: hsl(var(--primary)); -} +.helper-line.horizontal, +.helper-line.vertical { + /* Additional specific styling if needed */ +}src/frontend/src/pages/FlowPage/components/PageComponent/components/helper-lines.tsx (1)
15-36: Add explicitwidth/heightand use consistent units inside the<svg>element
<svg>without an explicitwidth/heightinherits the intrinsic size of its parent – in many layouts this ends up being0×0, so the lines never render.
In addition,x1={0}/y1={0}are numeric pixels, whereasx2="100%"/y2="100%"are percent strings. Mixing the two is valid, but makes the coordinate-system harder to reason about and trips some strict TS/linters.- return ( - <svg className="helper-lines"> + return ( + <svg + className="helper-lines" + width="100%" + height="100%" + /* preserveAspectRatio keeps lines aligned when the canvas is resized */ + preserveAspectRatio="none" + >If you prefer to rely on CSS, at minimum add a comment clarifying that
.helper-lines { width:100%; height:100%; }is required.Make sure the helper lines are still visible after this change in all browsers.
src/frontend/src/pages/FlowPage/components/PageComponent/index.tsx (2)
356-360: State explosion risk – throttlesetHelperLinesduring drag
setHelperLinesis called on everypositionchange while dragging. When the cursor moves quickly this can trigger dozens of React state updates per animation frame, leading to dropped frames on large flows.Add a small throttle (
requestAnimationFrameorlodash.throttle(16ms)) around the updater:- const [helperLines, setHelperLines] = useState<HelperLinesState>({}); + const [helperLines, _setHelperLines] = useState<HelperLinesState>({}); + const setHelperLines = useRef( + _.throttle((value: HelperLinesState) => _setHelperLines(value), 16) + ).current;(or debounce with
{ leading:true, trailing:true }).
664-705: Don’t re-render<HelperLines>when the feature is disabled
{helperLineEnabled && <HelperLines … />}is fine, but the surrounding component still re-renders becausehelperLineschanges in state even whenhelperLineEnabledisfalse. Consider movinghelperLinesinto auseMemothat returns an empty object when the flag is off, or early-return in the state updater – this removes an unnecessary render path.🧰 Tools
🪛 Biome (1.9.4)
[error] 688-689: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
src/frontend/src/pages/FlowPage/components/PageComponent/helpers/helper-lines.ts (1)
133-178: RedundantdraggingNodeBoundscalculation & unused variablesInside both horizontal and vertical snap sections you recompute
draggingNodeBoundsbut never use them afterwards. Remove these dead variables to reduce GC pressure:- const draggingNodeBounds = { - top: …, - };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/frontend/src/components/core/canvasControlsComponent/index.tsx(3 hunks)src/frontend/src/pages/FlowPage/components/PageComponent/components/helper-lines.tsx(1 hunks)src/frontend/src/pages/FlowPage/components/PageComponent/helpers/helper-lines.ts(1 hunks)src/frontend/src/pages/FlowPage/components/PageComponent/index.tsx(7 hunks)src/frontend/src/stores/flowStore.ts(1 hunks)src/frontend/src/style/applies.css(1 hunks)src/frontend/src/types/zustand/flow/index.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/frontend/src/pages/FlowPage/components/PageComponent/components/helper-lines.tsx (1)
src/frontend/src/pages/FlowPage/components/PageComponent/helpers/helper-lines.ts (1)
HelperLinesState(9-12)
src/frontend/src/components/core/canvasControlsComponent/index.tsx (1)
src/frontend/src/utils/utils.ts (1)
cn(36-38)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: autofix
🔇 Additional comments (5)
src/frontend/src/stores/flowStore.ts (1)
1045-1048: LGTM! Helper lines state implementation follows store patterns.The implementation correctly follows the established patterns in the flow store:
- Proper initialization with
falseas a sensible default- Simple setter function with consistent naming convention
- Correctly positioned within the store object
src/frontend/src/types/zustand/flow/index.ts (1)
288-289: LGTM! Type definitions correctly match the store implementation.The type definitions are properly structured:
helperLineEnabled: booleanmatches the store initializationsetHelperLineEnabled: (helperLineEnabled: boolean) => voidmatches the setter implementation- Consistent with other boolean state properties in the interface
src/frontend/src/components/core/canvasControlsComponent/index.tsx (3)
82-85: LGTM! State selectors follow established patterns.The helper lines state selectors are properly implemented:
- Consistent with other state selectors in the component
- Proper use of the flow store
- Good separation of setter and getter
116-118: LGTM! Toggle handler is correctly implemented.The toggle handler follows good practices:
- Simple and focused function
- Proper useCallback with correct dependencies
- Consistent naming convention with other handlers
161-169: LGTM! Toggle button integrates well with existing controls.The helper lines toggle button is well-implemented:
- Consistent with other
CustomControlButtonusage- Good icon choice (
UnfoldHorizontal) for the feature- Proper conditional styling using the
cnutility- Appropriate tooltip text and test ID
| const onNodesChangeWithHelperLines = useCallback( | ||
| (changes: NodeChange<AllNodeType>[]) => { | ||
| // Only process helper lines if the feature is enabled | ||
| if (helperLineEnabled) { | ||
| const dragChange = changes.find( | ||
| (change) => | ||
| change.type === "position" && | ||
| "dragging" in change && | ||
| change.dragging && | ||
| "id" in change, | ||
| ); | ||
|
|
||
| if ( | ||
| dragChange && | ||
| dragChange.type === "position" && | ||
| "position" in dragChange && | ||
| isDragging | ||
| ) { | ||
| const draggingNode = nodes.find((node) => node.id === dragChange.id); | ||
| if (draggingNode && dragChange.position) { | ||
| const updatedNode = { | ||
| ...draggingNode, | ||
| position: dragChange.position, | ||
| }; | ||
|
|
||
| const currentHelperLines = getHelperLines(updatedNode, nodes); | ||
| setHelperLines(currentHelperLines); | ||
|
|
||
| const snapPosition = getSnapPosition(updatedNode, nodes); | ||
|
|
||
| if ( | ||
| snapPosition.x !== dragChange.position.x || | ||
| snapPosition.y !== dragChange.position.y | ||
| ) { | ||
| const updatedChanges = changes.map((change) => { | ||
| if ( | ||
| "id" in change && | ||
| change.id === dragChange.id && | ||
| change.type === "position" && | ||
| "position" in change | ||
| ) { | ||
| return { | ||
| ...change, | ||
| position: snapPosition, | ||
| }; | ||
| } | ||
| return change; | ||
| }); | ||
| onNodesChange(updatedChanges); | ||
| return; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (!isDragging && (helperLines.horizontal || helperLines.vertical)) { | ||
| setHelperLines({}); | ||
| } | ||
| } else { | ||
| // Clear helper lines if feature is disabled | ||
| if (helperLines.horizontal || helperLines.vertical) { | ||
| setHelperLines({}); | ||
| } | ||
| } | ||
|
|
||
| onNodesChange(changes); | ||
| }, | ||
| [onNodesChange, nodes, isDragging, helperLines, helperLineEnabled], | ||
| ); |
There was a problem hiding this comment.
Potential TypeScript runtime error & unnecessary O(N²) loop
-
getHelperLines/getSnapPositionare called twice per event with the same arguments. Cache the result or merge both calls to save ~50 % work per drag event. -
The helper util already iterates over all nodes; wrapping it again inside a
nodes.find+mapadds another O(N). With many nodes the drag can hitch. Consider computing snap & position once, then directly mutatingdragChange.positionbefore passingchangesdownstream ─ this avoids the expensive secondmap. -
When there is no
dragChange,onNodesChangeWithHelperLinesfalls through toonNodesChange(changes)but keepsisDraggingtrue. If the user releases the mouse outside of a node theonNodeDragStopcallback might not fire and the flag is never cleared. Add a fallback:
if (!dragChange) {
- …
+ setIsDragging(false);
+ setHelperLines({});
+ onNodesChange(changes);
+ return;
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/frontend/src/pages/FlowPage/components/PageComponent/index.tsx around
lines 383 to 450, optimize onNodesChangeWithHelperLines by calling
getHelperLines and getSnapPosition only once per event and caching their results
to avoid duplicate computation. Replace the nodes.find and changes.map with
direct mutation of dragChange.position to snapPosition to eliminate the extra
O(N) iteration. Also, add a fallback condition to clear isDragging and
helperLines when no dragChange is found but isDragging remains true, ensuring
the drag state resets properly if the mouse is released outside a node.
| const SNAP_DISTANCE = 5; | ||
|
|
||
| export function getHelperLines( | ||
| draggingNode: Node, | ||
| nodes: Node[], | ||
| nodeWidth = 150, | ||
| nodeHeight = 50, | ||
| ): HelperLinesState { | ||
| const helperLines: HelperLinesState = {}; | ||
|
|
||
| const draggingNodeBounds = { | ||
| left: draggingNode.position.x, | ||
| right: | ||
| draggingNode.position.x + (draggingNode.measured?.width || nodeWidth), | ||
| top: draggingNode.position.y, | ||
| bottom: | ||
| draggingNode.position.y + (draggingNode.measured?.height || nodeHeight), | ||
| centerX: | ||
| draggingNode.position.x + (draggingNode.measured?.width || nodeWidth) / 2, | ||
| centerY: | ||
| draggingNode.position.y + | ||
| (draggingNode.measured?.height || nodeHeight) / 2, | ||
| }; | ||
|
|
||
| const otherNodes = nodes.filter((node) => node.id !== draggingNode.id); | ||
|
|
||
| for (const node of otherNodes) { | ||
| const nodeBounds = { | ||
| left: node.position.x, | ||
| right: node.position.x + (node.measured?.width || nodeWidth), | ||
| top: node.position.y, | ||
| bottom: node.position.y + (node.measured?.height || nodeHeight), | ||
| centerX: node.position.x + (node.measured?.width || nodeWidth) / 2, | ||
| centerY: node.position.y + (node.measured?.height || nodeHeight) / 2, | ||
| }; | ||
|
|
||
| if (Math.abs(draggingNodeBounds.top - nodeBounds.top) < SNAP_DISTANCE) { | ||
| helperLines.horizontal = { | ||
| id: `horizontal-top-${node.id}`, | ||
| position: nodeBounds.top, | ||
| orientation: "horizontal", | ||
| }; | ||
| } | ||
|
|
||
| if ( | ||
| Math.abs(draggingNodeBounds.bottom - nodeBounds.bottom) < SNAP_DISTANCE | ||
| ) { | ||
| helperLines.horizontal = { | ||
| id: `horizontal-bottom-${node.id}`, | ||
| position: nodeBounds.bottom, | ||
| orientation: "horizontal", | ||
| }; | ||
| } | ||
|
|
||
| if ( | ||
| Math.abs(draggingNodeBounds.centerY - nodeBounds.centerY) < SNAP_DISTANCE | ||
| ) { | ||
| helperLines.horizontal = { | ||
| id: `horizontal-center-${node.id}`, | ||
| position: nodeBounds.centerY, | ||
| orientation: "horizontal", | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| for (const node of otherNodes) { | ||
| const nodeBounds = { | ||
| left: node.position.x, | ||
| right: node.position.x + (node.measured?.width || nodeWidth), | ||
| top: node.position.y, | ||
| bottom: node.position.y + (node.measured?.height || nodeHeight), | ||
| centerX: node.position.x + (node.measured?.width || nodeWidth) / 2, | ||
| centerY: node.position.y + (node.measured?.height || nodeHeight) / 2, | ||
| }; | ||
|
|
||
| if (Math.abs(draggingNodeBounds.left - nodeBounds.left) < SNAP_DISTANCE) { | ||
| helperLines.vertical = { | ||
| id: `vertical-left-${node.id}`, | ||
| position: nodeBounds.left, | ||
| orientation: "vertical", | ||
| }; | ||
| } | ||
|
|
||
| if (Math.abs(draggingNodeBounds.right - nodeBounds.right) < SNAP_DISTANCE) { | ||
| helperLines.vertical = { | ||
| id: `vertical-right-${node.id}`, | ||
| position: nodeBounds.right, | ||
| orientation: "vertical", | ||
| }; | ||
| } | ||
|
|
||
| if ( | ||
| Math.abs(draggingNodeBounds.centerX - nodeBounds.centerX) < SNAP_DISTANCE | ||
| ) { | ||
| helperLines.vertical = { | ||
| id: `vertical-center-${node.id}`, | ||
| position: nodeBounds.centerX, | ||
| orientation: "vertical", | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| return helperLines; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Combine the two passes over otherNodes & store the closest snap
The file loops over otherNodes twice – first for horizontal, then for vertical checks – performing identical bounding-box calculations twice. This is an avoidable O(N²) cost on every mouse-move.
Suggested refactor (outline):
for (const node of otherNodes) {
const bounds = …;
const dxTop = Math.abs(draggingBounds.top - bounds.top);
…
if (dxTop < SNAP_DISTANCE && dxTop < bestHorizontalDelta) {
bestHorizontalDelta = dxTop;
helperLines.horizontal = { … };
}
// same loop, but collect vertical as well
}Besides performance, storing the nearest helperLine prevents the later line overwriting the earlier one (current implementation replaces the line if two conditions are met).
🤖 Prompt for AI Agents
In
src/frontend/src/pages/FlowPage/components/PageComponent/helpers/helper-lines.ts
between lines 14 and 116, the code loops twice over otherNodes to calculate
horizontal and vertical helper lines separately, repeating bounding box
calculations and potentially overwriting helperLines. Refactor by combining
these loops into a single loop that calculates node bounds once per node, then
checks all horizontal and vertical snap distances within that loop. Track the
closest snap distance for horizontal and vertical separately, updating
helperLines only if the current node is closer than the previously stored one,
to improve performance and prevent overwriting helper lines.
| import { Node, XYPosition } from "@xyflow/react"; | ||
|
|
||
| export interface HelperLine { | ||
| id: string; | ||
| position: number; | ||
| orientation: "horizontal" | "vertical"; | ||
| } | ||
|
|
||
| export interface HelperLinesState { | ||
| horizontal?: HelperLine; | ||
| vertical?: HelperLine; | ||
| } |
There was a problem hiding this comment.
measured property does not exist on Node – breaks TS compile
@xyflow/react’s Node type exposes width and height, not measured.
All occurrences of node.measured?.width|height will raise:
Property 'measured' does not exist on type 'Node<...>'
Replace every usage with the canonical fields (with fallback):
- node.measured?.width || nodeWidth
+ (node.width ?? nodeWidth)and likewise for height.
This change affects lines 27–36, 42–48, 82–88, 137–175.
- right: draggingNode.position.x + (draggingNode.measured?.width || nodeWidth),
+ right: draggingNode.position.x + (draggingNode.width ?? nodeWidth),(continue for all occurrences)
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
src/frontend/src/pages/FlowPage/components/PageComponent/helpers/helper-lines.ts
at lines 27–36, 42–48, 82–88, and 137–175, the code incorrectly accesses a
non-existent 'measured' property on the 'Node' type from '@xyflow/react',
causing TypeScript compilation errors. Replace all instances of
'node.measured?.width' and 'node.measured?.height' with 'node.width ?? 0' and
'node.height ?? 0' respectively to use the correct properties with a fallback to
zero. Update all these occurrences accordingly to fix the type errors.
| const onToggleHelperLines = useCallback(() => { | ||
| setHelperLineEnabled(!helperLineEnabled); | ||
| }, [setHelperLineEnabled, helperLineEnabled]); | ||
|
|
There was a problem hiding this comment.
On toggle can we change the icon? Right now there is no indication that the button was clicked while the user is still hovering over it.
…he state of helperLineEnabled to provide better user experience
github-actions
Bot
added
enhancement
Empreiteiro
left a comment
Empreiteiro
left a comment
There was a problem hiding this comment.
Dashed lines appear to lend a hand,
Guiding you to build your plan.
Where once was guesswork, now there's flow
Components click and neatly glow.
For those who hate misaligned plight,
This feature brings pure work delight!
Gravando.2025-05-29.191126.mp4
github-actions
Bot
added
enhancement
…er lines and snapping to grid position during drag for better user experience.
github-actions
Bot
added
enhancement
Empreiteiro
requested review from
rodrigosnader
and removed request for
rodrigosnader
github-actions
Bot
added
enhancement
Frontend Unit Test Coverage ReportCoverage Summary
Unit Test Results
|
github-actions
Bot
added
enhancement
3 participants
This pull request introduces a new feature to display helper lines for node alignment in the flow editor, along with associated UI updates and state management. The changes include adding helper line rendering, snapping functionality, and a toggle control to enable or disable the feature.
Helper Line Rendering and Snapping
HelperLinescomponent to render horizontal and vertical alignment lines based on node positions (src/frontend/src/pages/FlowPage/components/PageComponent/components/helper-lines.tsx).getHelperLinesandgetSnapPositionto calculate alignment lines and snap nodes to aligned positions (src/frontend/src/pages/FlowPage/components/PageComponent/helpers/helper-lines.ts).UI and Interaction Updates
CanvasControlscomponent for toggling helper lines, with visual feedback based on the toggle state (src/frontend/src/components/core/canvasControlsComponent/index.tsx). [1] [2] [3]Pagecomponent to handle node dragging events, calculate helper lines, and snap positions dynamically (src/frontend/src/pages/FlowPage/components/PageComponent/index.tsx). [1] [2] [3] [4] [5]State Management
useFlowStorestate management to includehelperLineEnabledand its corresponding setter (src/frontend/src/stores/flowStore.ts).FlowStoreTypedefinition to include the new state properties (src/frontend/src/types/zustand/flow/index.ts).Styling
src/frontend/src/style/applies.css).This pull request adds a new button to the React Flow custom controls that enables alignment lines between components during use.
When enabled, alignment lines will appear as shown in the screenshot below:
Summary by CodeRabbit
New Features
Style