fix(editor): Implement node position checks and overlap detection for canvas operations (#22536)

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Csaba Tuncsik
2025-12-17 17:01:24 +01:00
committed by GitHub
co-authored by Claude Opus 4.5
parent 906e0c01ad
commit 09566a0dd4
4 changed files with 516 additions and 14 deletions
@@ -82,6 +82,7 @@ import {
generateOffsets,
getNodesGroupSize,
PUSH_NODES_OFFSET,
doRectsOverlap,
} from '@/app/utils/nodeViewUtils';
import type { Connection } from '@vue-flow/core';
import type {
@@ -1196,8 +1197,33 @@ export function useCanvasOperations() {
// Compute the y offset for the new node based on the number of main outputs of the source node
// and shift the downstream nodes accordingly
shiftDownstreamNodesPosition(lastInteractedWithNode.name, PUSH_NODES_OFFSET, {
// Calculate actual node size for the new node being inserted
// Configurable nodes (like AI Agent) have non-main inputs and are wider
// Use nodeTypeDescription.inputs directly since the node isn't in the workflow yet
// If inputs is a string (expression), it's a dynamic node like AI Agent which is configurable
let isNewNodeConfigurable = false;
if (typeof nodeTypeDescription.inputs === 'string') {
// Dynamic inputs (expression string) indicates AI/configurable nodes
isNewNodeConfigurable = true;
} else if (Array.isArray(nodeTypeDescription.inputs)) {
const nodeInputTypes = NodeHelpers.getConnectionTypes(nodeTypeDescription.inputs);
isNewNodeConfigurable =
nodeInputTypes.filter((input) => input !== NodeConnectionTypes.Main).length > 0;
}
const newNodeSize: [number, number] = isNewNodeConfigurable
? CONFIGURABLE_NODE_SIZE
: DEFAULT_NODE_SIZE;
// Calculate shift margin: base offset plus extra width for configurable nodes
// For standard nodes: PUSH_NODES_OFFSET (208)
// For configurable nodes: PUSH_NODES_OFFSET + (configurable width - default width)
const extraWidth = isNewNodeConfigurable
? CONFIGURABLE_NODE_SIZE[0] - DEFAULT_NODE_SIZE[0]
: 0;
const shiftMargin = PUSH_NODES_OFFSET + extraWidth;
shiftDownstreamNodesPosition(lastInteractedWithNode.name, shiftMargin, {
trackHistory: true,
nodeSize: newNodeSize,
});
}
@@ -1279,6 +1305,14 @@ export function useCanvasOperations() {
lastInteractedWithNode.position[0] + pushOffset,
lastInteractedWithNode.position[1] + yOffset,
];
// When inserting via edge plus button, adjust Y to fit within overlapping sticky notes
if (lastInteractedWithNodeConnection) {
const adjustedY = getYPositionForStickyOverlap(position, nodeSize);
if (adjustedY !== null) {
position = [position[0], adjustedY];
}
}
}
}
}
@@ -1324,25 +1358,255 @@ export function useCanvasOperations() {
}
/**
* Moves all downstream nodes of a node
* Gets the bounding rectangle of a node including its size
*/
function getNodeRect(node: INodeUi): { x: number; y: number; width: number; height: number } {
if (node.type === STICKY_NODE_TYPE) {
return {
x: node.position[0],
y: node.position[1],
width: (node.parameters.width as number) || DEFAULT_NODE_SIZE[0],
height: (node.parameters.height as number) || DEFAULT_NODE_SIZE[1],
};
}
return {
x: node.position[0],
y: node.position[1],
width: DEFAULT_NODE_SIZE[0],
height: DEFAULT_NODE_SIZE[1],
};
}
/**
* Check if there's enough space for a new node at the insertion position.
* Uses a margin to account for nodes that are touching (edge-to-edge)
* which should still be considered as blocking the insertion.
*/
function hasSpaceForInsertion(
insertPosition: XYPosition,
nodeSize: [number, number],
nodesToCheck: INodeUi[],
): boolean {
// Add a small margin to detect nodes that are touching (edge-to-edge)
const margin = GRID_SIZE;
const insertRect = {
x: insertPosition[0],
y: insertPosition[1],
width: nodeSize[0] + margin,
height: nodeSize[1],
};
for (const node of nodesToCheck) {
if (node.type === STICKY_NODE_TYPE) continue;
const nodeRect = getNodeRect(node);
if (doRectsOverlap(insertRect, nodeRect)) {
return false;
}
}
return true;
}
/**
* Gets all nodes that should be shifted when inserting a node.
* Algorithm:
* 1. Find nodes that overlap with or are to the right of insertion area (similar Y)
* 2. Add nodes connected to them (downstream)
* 3. Filter to only include nodes that need to move
*/
function getNodesToShift(
insertPosition: XYPosition,
sourceNodeName: string,
): { nodesToMove: INodeUi[]; stickiesToStretch: INodeUi[] } {
const allNodes = Object.values(workflowsStore.nodesByName);
const insertX = insertPosition[0];
const insertY = insertPosition[1];
const yTolerance = DEFAULT_NODE_SIZE[1] * 2; // Nodes within ~2 node heights are considered "similar Y"
// Step 1: Find initial candidates - nodes that overlap with or are to the right of insertion
// A node overlaps if its right edge extends into the insertion area
const initialCandidates = allNodes.filter((node) => {
if (node.type === STICKY_NODE_TYPE) return false;
if (node.name === sourceNodeName) return false;
const isSimilarY = Math.abs(node.position[1] - insertY) <= yTolerance;
// Node overlaps or is to the right if its right edge is past the insertion X
// or its left edge is at/past the insertion X
const nodeRightEdge = node.position[0] + DEFAULT_NODE_SIZE[0];
const overlapsOrIsToTheRight = nodeRightEdge > insertX || node.position[0] >= insertX;
return isSimilarY && overlapsOrIsToTheRight;
});
// Step 2: Add all downstream connected nodes from initial candidates
const candidateNames = new Set(initialCandidates.map((n) => n.name));
for (const candidate of initialCandidates) {
const downstream = workflowHelpers.getConnectedNodes(
'downstream',
editableWorkflowObject.value,
candidate.name,
);
downstream.forEach((name) => candidateNames.add(name));
}
// Step 3: Get all candidate regular nodes (they overlap with or are downstream of insertion)
const regularNodesToMove = allNodes.filter((node) => {
if (node.type === STICKY_NODE_TYPE) return false;
return candidateNames.has(node.name);
});
// Step 4: Find sticky notes that need moving or stretching
const stickiesToStretch: INodeUi[] = [];
const stickiesToMove: INodeUi[] = [];
const stickyNodes = allNodes.filter((node) => node.type === STICKY_NODE_TYPE);
// Calculate the vertical area that will be affected (insertion point + nodes being moved)
const affectedMinY = Math.min(insertY, ...regularNodesToMove.map((n) => n.position[1]));
const affectedMaxY = Math.max(
insertY + DEFAULT_NODE_SIZE[1],
...regularNodesToMove.map((n) => n.position[1] + DEFAULT_NODE_SIZE[1]),
);
for (const sticky of stickyNodes) {
const stickyRect = getNodeRect(sticky);
const stickyLeftEdge = sticky.position[0];
const stickyRightEdge = stickyLeftEdge + stickyRect.width;
const stickyTop = sticky.position[1];
const stickyBottom = stickyTop + stickyRect.height;
const overlapsVertically = !(stickyBottom <= affectedMinY || stickyTop >= affectedMaxY);
// Sticky should be moved if its left edge is at or past the insertion position
if (stickyLeftEdge >= insertX && overlapsVertically) {
stickiesToMove.push(sticky);
}
// Sticky should be stretched if:
// 1. Its left edge is before the insertion position
// 2. Its right edge extends into or past the insertion position
// 3. It overlaps vertically with the affected area
else if (stickyLeftEdge < insertX && stickyRightEdge >= insertX && overlapsVertically) {
stickiesToStretch.push(sticky);
}
}
// Combine regular nodes and stickies to move
const nodesToMove = [...regularNodesToMove, ...stickiesToMove];
return { nodesToMove, stickiesToStretch };
}
/**
* Checks if the insertion position overlaps with a sticky note and returns
* an adjusted Y position that fits within the sticky's content area.
* Only adjusts Y if there's enough space for insertion (no downstream nodes blocking).
* Returns null if no adjustment is needed.
*/
function getYPositionForStickyOverlap(
insertPosition: XYPosition,
nodeSize: [number, number],
): number | null {
const allNodes = Object.values(workflowsStore.nodesByName);
const stickyNodes = allNodes.filter((node) => node.type === STICKY_NODE_TYPE);
const regularNodes = allNodes.filter((node) => node.type !== STICKY_NODE_TYPE);
// Add a small margin to detect nodes that are touching (edge-to-edge)
const margin = GRID_SIZE;
const insertRect = {
x: insertPosition[0],
y: insertPosition[1],
width: nodeSize[0] + margin,
height: nodeSize[1],
};
// First check if there are any regular nodes blocking the insertion position
// If so, downstream nodes will be shifted and we should keep the original Y
for (const node of regularNodes) {
const nodeRect = getNodeRect(node);
if (doRectsOverlap(insertRect, nodeRect)) {
// There's a regular node blocking - don't adjust Y, let shift logic handle it
return null;
}
}
// No regular nodes blocking - check if we need to adjust Y for sticky overlap
for (const sticky of stickyNodes) {
const stickyRect = getNodeRect(sticky);
// Check if insertion position overlaps with this sticky
if (doRectsOverlap(insertRect, stickyRect)) {
// Sticky header takes up approximately 40px (title area)
const STICKY_HEADER_HEIGHT = 40;
const stickyContentTop = sticky.position[1] + STICKY_HEADER_HEIGHT;
const stickyContentBottom = sticky.position[1] + stickyRect.height - GRID_SIZE;
// Calculate the ideal Y position centered in the sticky's content area
const contentHeight = stickyContentBottom - stickyContentTop;
const centeredY = stickyContentTop + (contentHeight - nodeSize[1]) / 2;
// Ensure the position is grid-aligned
return Math.round(centeredY / GRID_SIZE) * GRID_SIZE;
}
}
return null;
}
/**
* Stretches a sticky note by increasing its width
*/
function stretchStickyNote(
sticky: INodeUi,
stretchAmount: number,
{ trackHistory = false }: { trackHistory?: boolean },
) {
const currentWidth = (sticky.parameters.width as number) || DEFAULT_NODE_SIZE[0];
const newWidth = currentWidth + stretchAmount;
const newParameters = {
...sticky.parameters,
width: newWidth,
};
replaceNodeParameters(sticky.id, sticky.parameters as INodeParameters, newParameters, {
trackHistory,
trackBulk: false,
});
}
/**
* Moves downstream nodes when inserting a node between existing nodes.
* Only moves nodes if there isn't enough space for the new node.
* Sticky notes that overlap the insertion area are stretched instead of moved.
*/
function shiftDownstreamNodesPosition(
sourceNodeName: string,
margin: number,
{ trackHistory = false }: { trackHistory?: boolean },
{
trackHistory = false,
nodeSize = DEFAULT_NODE_SIZE,
}: { trackHistory?: boolean; nodeSize?: [number, number] },
) {
const sourceNode = workflowsStore.nodesByName[sourceNodeName];
const checkNodes = workflowHelpers.getConnectedNodes(
'downstream',
editableWorkflowObject.value,
sourceNodeName,
);
for (const nodeName of checkNodes) {
const node = workflowsStore.nodesByName[nodeName];
if (!node || !sourceNode || node.position[0] < sourceNode.position[0]) {
continue;
}
if (!sourceNode) return;
// Calculate insertion position (to the right of source node)
// Use PUSH_NODES_OFFSET to match the actual position where nodes are placed
const insertPosition: XYPosition = [
sourceNode.position[0] + PUSH_NODES_OFFSET,
sourceNode.position[1],
];
// Get all nodes except source and stickies
const nodesToCheck = Object.values(workflowsStore.nodesByName).filter(
(n) => n.name !== sourceNodeName && n.type !== STICKY_NODE_TYPE,
);
// Check if there's enough space - if so, don't move anything
if (hasSpaceForInsertion(insertPosition, nodeSize, nodesToCheck)) {
return;
}
// Get nodes to shift and stickies to stretch
const { nodesToMove, stickiesToStretch } = getNodesToShift(insertPosition, sourceNodeName);
// Move regular nodes to the right
for (const node of nodesToMove) {
updateNodePosition(
node.id,
{
@@ -1352,6 +1616,11 @@ export function useCanvasOperations() {
{ trackHistory },
);
}
// Stretch sticky notes instead of moving them
for (const sticky of stickiesToStretch) {
stretchStickyNote(sticky, margin, { trackHistory });
}
}
/**
@@ -12,6 +12,8 @@ import {
snapPositionToGrid,
calculateNodeSize,
GRID_SIZE,
doRectsOverlap,
canUsePosition,
} from './nodeViewUtils';
import type { INode, INodeTypeDescription, INodeExecutionData, Workflow } from 'n8n-workflow';
import type { INodeUi, XYPosition } from '@/Interface';
@@ -575,3 +577,61 @@ function createTestGraphNode(data: Partial<GraphNode> = {}): GraphNode {
...data,
};
}
describe('doRectsOverlap', () => {
it('should return true when rectangles overlap', () => {
const rect1 = { x: 0, y: 0, width: 100, height: 100 };
const rect2 = { x: 50, y: 50, width: 100, height: 100 };
expect(doRectsOverlap(rect1, rect2)).toBe(true);
});
it('should return false when rectangles do not overlap horizontally', () => {
const rect1 = { x: 0, y: 0, width: 100, height: 100 };
const rect2 = { x: 150, y: 0, width: 100, height: 100 };
expect(doRectsOverlap(rect1, rect2)).toBe(false);
});
it('should return false when rectangles do not overlap vertically', () => {
const rect1 = { x: 0, y: 0, width: 100, height: 100 };
const rect2 = { x: 0, y: 150, width: 100, height: 100 };
expect(doRectsOverlap(rect1, rect2)).toBe(false);
});
it('should return false when rectangles touch but do not overlap', () => {
const rect1 = { x: 0, y: 0, width: 100, height: 100 };
const rect2 = { x: 100, y: 0, width: 100, height: 100 };
expect(doRectsOverlap(rect1, rect2)).toBe(false);
});
it('should return true when one rectangle contains another', () => {
const rect1 = { x: 0, y: 0, width: 200, height: 200 };
const rect2 = { x: 50, y: 50, width: 50, height: 50 };
expect(doRectsOverlap(rect1, rect2)).toBe(true);
});
});
describe('canUsePosition', () => {
it('should return true when positions are far apart', () => {
const pos1: XYPosition = [0, 0];
const pos2: XYPosition = [200, 200];
expect(canUsePosition(pos1, pos2)).toBe(true);
});
it('should return false when positions overlap', () => {
const pos1: XYPosition = [0, 0];
const pos2: XYPosition = [50, 50];
expect(canUsePosition(pos1, pos2)).toBe(false);
});
it('should return true when positions are separated horizontally', () => {
const pos1: XYPosition = [0, 0];
const pos2: XYPosition = [DEFAULT_NODE_SIZE[0] + 1, 0];
expect(canUsePosition(pos1, pos2)).toBe(true);
});
it('should return true when positions are separated vertically', () => {
const pos1: XYPosition = [0, 0];
const pos2: XYPosition = [0, DEFAULT_NODE_SIZE[1] + 1];
expect(canUsePosition(pos1, pos2)).toBe(true);
});
});
@@ -125,7 +125,7 @@ export const getNodesGroupSize = (nodes: INodeUi[]): [number, number] => {
/**
* Checks if the given position is available for a new node
*/
const canUsePosition = (
export const canUsePosition = (
position1: XYPosition,
position2: XYPosition,
size: [number, number] = DEFAULT_NODE_SIZE,
@@ -139,6 +139,21 @@ const canUsePosition = (
return true;
};
/**
* Check if two rectangles overlap
*/
export const doRectsOverlap = (
rect1: { x: number; y: number; width: number; height: number },
rect2: { x: number; y: number; width: number; height: number },
): boolean => {
return !(
rect1.x + rect1.width <= rect2.x ||
rect2.x + rect2.width <= rect1.x ||
rect1.y + rect1.height <= rect2.y ||
rect2.y + rect2.height <= rect1.y
);
};
/**
* Returns the closest number divisible by the given number
*/
@@ -4,6 +4,8 @@ import {
CODE_NODE_NAME,
HTTP_REQUEST_NODE_NAME,
CODE_NODE_DISPLAY_NAME,
EDIT_FIELDS_SET_NODE_NAME,
AGENT_NODE_NAME,
} from '../../../../../config/constants';
import { test, expect } from '../../../../../fixtures/base';
@@ -190,4 +192,160 @@ test.describe('Canvas Actions', () => {
await expect(n8n.canvas.selectedNodes()).toHaveCount(2);
});
test.describe('Node insertion positioning', () => {
test('should not shift downstream nodes when there is enough space', async ({ n8n }) => {
// Create trigger -> code with large gap between them
await n8n.canvas.addNode(MANUAL_TRIGGER_NODE_NAME);
await n8n.canvas.nodeByName(MANUAL_TRIGGER_NODE_DISPLAY_NAME).click();
await n8n.canvas.addNode(CODE_NODE_NAME, { action: 'Code in JavaScript', closeNDV: true });
await n8n.canvas.clickZoomToFitButton();
// Move code node far to the right to create space
await n8n.canvas.dragNodeToRelativePosition(CODE_NODE_DISPLAY_NAME, 400, 0);
// Get code node position before insertion
const codePositionBefore = await n8n.canvas.getNodePosition(CODE_NODE_DISPLAY_NAME);
// Insert node between trigger and code
await n8n.canvas.addNodeBetweenNodes(
MANUAL_TRIGGER_NODE_DISPLAY_NAME,
CODE_NODE_DISPLAY_NAME,
HTTP_REQUEST_NODE_NAME,
);
// Get code node position after insertion
const codePositionAfter = await n8n.canvas.getNodePosition(CODE_NODE_DISPLAY_NAME);
// Code node should not have moved since there was enough space
expect(codePositionAfter.x).toBe(codePositionBefore.x);
expect(codePositionAfter.y).toBe(codePositionBefore.y);
});
test('should shift downstream nodes when there is not enough space', async ({ n8n }) => {
// Create trigger -> code (close together)
await n8n.canvas.addNode(MANUAL_TRIGGER_NODE_NAME);
await n8n.canvas.nodeByName(MANUAL_TRIGGER_NODE_DISPLAY_NAME).click();
await n8n.canvas.addNode(CODE_NODE_NAME, { action: 'Code in JavaScript', closeNDV: true });
await n8n.canvas.clickZoomToFitButton();
// Get code node position before insertion
const codePositionBefore = await n8n.canvas.getNodePosition(CODE_NODE_DISPLAY_NAME);
// Insert node between trigger and code
await n8n.canvas.addNodeBetweenNodes(
MANUAL_TRIGGER_NODE_DISPLAY_NAME,
CODE_NODE_DISPLAY_NAME,
HTTP_REQUEST_NODE_NAME,
);
// Get code node position after insertion
const codePositionAfter = await n8n.canvas.getNodePosition(CODE_NODE_DISPLAY_NAME);
// Code node should have moved to the right
expect(codePositionAfter.x).toBeGreaterThan(codePositionBefore.x);
});
test('should shift connected downstream nodes together', async ({ n8n }) => {
// Create trigger -> code -> edit fields chain
await n8n.canvas.addNode(MANUAL_TRIGGER_NODE_NAME);
await n8n.canvas.nodeByName(MANUAL_TRIGGER_NODE_DISPLAY_NAME).click();
await n8n.canvas.addNode(CODE_NODE_NAME, { action: 'Code in JavaScript', closeNDV: true });
await n8n.canvas.nodeByName(CODE_NODE_DISPLAY_NAME).click();
await n8n.canvas.addNode(EDIT_FIELDS_SET_NODE_NAME, { closeNDV: true });
await n8n.canvas.clickZoomToFitButton();
// Get positions before insertion
const codePositionBefore = await n8n.canvas.getNodePosition(CODE_NODE_DISPLAY_NAME);
const editFieldsPositionBefore = await n8n.canvas.getNodePosition('Edit Fields');
// Insert node between trigger and code
await n8n.canvas.addNodeBetweenNodes(
MANUAL_TRIGGER_NODE_DISPLAY_NAME,
CODE_NODE_DISPLAY_NAME,
HTTP_REQUEST_NODE_NAME,
);
// Get positions after insertion
const codePositionAfter = await n8n.canvas.getNodePosition(CODE_NODE_DISPLAY_NAME);
const editFieldsPositionAfter = await n8n.canvas.getNodePosition('Edit Fields');
// Both downstream nodes should have moved by approximately the same amount
const codeDeltaX = codePositionAfter.x - codePositionBefore.x;
const editFieldsDeltaX = editFieldsPositionAfter.x - editFieldsPositionBefore.x;
expect(codeDeltaX).toBeGreaterThan(0);
expect(editFieldsDeltaX).toBeGreaterThan(0);
// They should move by similar amounts (within tolerance for rounding)
expect(Math.abs(codeDeltaX - editFieldsDeltaX)).toBeLessThan(20);
});
test('should shift downstream nodes correctly when inserting configurable nodes like AI Agent', async ({
n8n,
}) => {
// Create trigger -> code (close together)
await n8n.canvas.addNode(MANUAL_TRIGGER_NODE_NAME);
await n8n.canvas.nodeByName(MANUAL_TRIGGER_NODE_DISPLAY_NAME).click();
await n8n.canvas.addNode(CODE_NODE_NAME, { action: 'Code in JavaScript', closeNDV: true });
await n8n.canvas.clickZoomToFitButton();
// Get code node position before insertion
const codePositionBefore = await n8n.canvas.getNodePosition(CODE_NODE_DISPLAY_NAME);
// Insert AI Agent (a configurable/wider node) between trigger and code
await n8n.canvas.addNodeBetweenNodes(
MANUAL_TRIGGER_NODE_DISPLAY_NAME,
CODE_NODE_DISPLAY_NAME,
AGENT_NODE_NAME,
);
// Get positions after insertion
const codePositionAfter = await n8n.canvas.getNodePosition(CODE_NODE_DISPLAY_NAME);
const agentPosition = await n8n.canvas.getNodePosition(AGENT_NODE_NAME);
// Code node should have moved to the right
expect(codePositionAfter.x).toBeGreaterThan(codePositionBefore.x);
// AI Agent node should be positioned between trigger and code (not overlapping)
// AI Agent is 256px wide, so its right edge should be left of code node
// Agent position is top-left, so right edge is agentPosition.x + 256
const agentRightEdge = agentPosition.x + 256;
expect(agentRightEdge).toBeLessThan(codePositionAfter.x);
});
test('should not shift downstream nodes when there is enough space for configurable nodes', async ({
n8n,
}) => {
// Create trigger -> code with large gap between them
await n8n.canvas.addNode(MANUAL_TRIGGER_NODE_NAME);
await n8n.canvas.nodeByName(MANUAL_TRIGGER_NODE_DISPLAY_NAME).click();
await n8n.canvas.addNode(CODE_NODE_NAME, { action: 'Code in JavaScript', closeNDV: true });
await n8n.canvas.clickZoomToFitButton();
// Move code node far to the right to create space for AI Agent (which is 256px wide)
await n8n.canvas.dragNodeToRelativePosition(CODE_NODE_DISPLAY_NAME, 500, 0);
// Get code node position before insertion
const codePositionBefore = await n8n.canvas.getNodePosition(CODE_NODE_DISPLAY_NAME);
// Insert AI Agent between trigger and code
await n8n.canvas.addNodeBetweenNodes(
MANUAL_TRIGGER_NODE_DISPLAY_NAME,
CODE_NODE_DISPLAY_NAME,
AGENT_NODE_NAME,
);
// Get code node position after insertion
const codePositionAfter = await n8n.canvas.getNodePosition(CODE_NODE_DISPLAY_NAME);
// Code node should not have moved since there was enough space
expect(codePositionAfter.x).toBe(codePositionBefore.x);
expect(codePositionAfter.y).toBe(codePositionBefore.y);
});
});
});