diff --git a/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.test.ts b/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.test.ts index 3a1fc24327b..72a72f4c109 100644 --- a/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.test.ts +++ b/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.test.ts @@ -117,7 +117,7 @@ import { AGENT_NODE_SIZE, DEFAULT_NODE_SIZE, GRID_SIZE, - PUSH_NODES_OFFSET, + HORIZONTAL_NODE_STEP, } from '@/app/utils/nodeViewUtils'; vi.mock('n8n-workflow', async (importOriginal) => { @@ -550,7 +550,8 @@ describe('useCanvasOperations', () => { const { resolveNodePosition } = useCanvasOperations(); const position = resolveNodePosition({ ...node, position: undefined }, nodeTypeDescription); - expect(position).toEqual([320, 112]); + // 112 + HORIZONTAL_NODE_STEP (224) = 336, matching the auto-layout step + expect(position).toEqual([336, 112]); }); it('should place the node clear of the agent card when added after a message an agent node', () => { @@ -579,9 +580,11 @@ describe('useCanvasOperations', () => { const { resolveNodePosition } = useCanvasOperations(); const position = resolveNodePosition({ ...node, position: undefined }, nodeTypeDescription); + // The new node keeps a constant NODE_X_SPACING gap past the agent card's + // right edge: HORIZONTAL_NODE_STEP + (agent width - default width). expect(position).toEqual([ lastInteracted.position[0] + - PUSH_NODES_OFFSET + + HORIZONTAL_NODE_STEP + (AGENT_NODE_SIZE[0] - DEFAULT_NODE_SIZE[0]), lastInteracted.position[1], ]); @@ -643,7 +646,7 @@ describe('useCanvasOperations', () => { const { resolveNodePosition } = useCanvasOperations(); const position = resolveNodePosition({ ...node, position: undefined }, nodeTypeDescription); - expect(position).toEqual([448, 96]); + expect(position).toEqual([464, 96]); }); it('should place the node at the last clicked position if no other position is set', () => { @@ -1524,7 +1527,7 @@ describe('useCanvasOperations', () => { name: nodes[1].name, type: nodeTypeName, typeVersion: 1, - position: [32 + PUSH_NODES_OFFSET + 2 * GRID_SIZE, 32 + GRID_SIZE], + position: [32 + HORIZONTAL_NODE_STEP + 2 * GRID_SIZE, 32 + GRID_SIZE], parameters: {}, }); }); diff --git a/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.ts b/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.ts index 2c3a4dd688d..b95388cf223 100644 --- a/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.ts +++ b/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.ts @@ -97,6 +97,8 @@ import { generateOffsets, getNodesGroupSize, PUSH_NODES_OFFSET, + HORIZONTAL_NODE_STEP, + NODE_X_SPACING, doRectsOverlap, } from '@/app/utils/nodeViewUtils'; import { isAgentNodeV2 } from '@/features/agents/utils/agentNode'; @@ -190,6 +192,33 @@ type AddNodeOptions = AddNodesBaseOptions & { actionName?: string; }; +/** + * Rendered-width estimate for the multi-node sequential placement step, so a + * bulk-added node doesn't overlap a wide neighbor. The agent card + * (AGENT_NODE_SIZE) and default widths are exact; configurable nodes render at + * a dynamic width (see calculateNodeSize) that no constant matches, so + * CONFIGURABLE_NODE_SIZE is an overlap-safe estimate only — exact configurable + * spacing needs the measured width and is tracked as a follow-up. + */ +function getPlacementNodeWidth(node: INodeUi, nodeTypeDescription: INodeTypeDescription): number { + if (isAgentNodeV2(node)) { + return AGENT_NODE_SIZE[0]; + } + + // A dynamic-inputs expression (e.g. AI Agent) or any non-main input means the + // node renders at the wider configurable size. + const { inputs } = nodeTypeDescription; + const hasNonMainInput = + Array.isArray(inputs) && + NodeHelpers.getConnectionTypes(inputs).some((input) => input !== NodeConnectionTypes.Main); + + if (typeof inputs === 'string' || hasNonMainInput) { + return CONFIGURABLE_NODE_SIZE[0]; + } + + return DEFAULT_NODE_SIZE[0]; +} + export function useCanvasOperations() { const rootStore = useRootStore(); const workflowsStore = useWorkflowsStore(); @@ -989,9 +1018,13 @@ export function useCanvasOperations() { continue; } - // When we're adding multiple nodes, increment the X position for the next one + // When we're adding multiple nodes, place the next one a constant + // NODE_X_SPACING gap past this node's right edge — using its actual width + // so the gap stays 128 even next to a wide agent/configurable node. insertPosition = [ - lastAddedNode.position[0] + DEFAULT_NODE_SIZE[0] * 2 + GRID_SIZE, + lastAddedNode.position[0] + + getPlacementNodeWidth(lastAddedNode, nodeTypeDescription) + + NODE_X_SPACING, lastAddedNode.position[1], ]; } @@ -1524,13 +1557,13 @@ export function useCanvasOperations() { 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) + // Calculate shift margin: base horizontal step plus extra width for configurable nodes + // For standard nodes: HORIZONTAL_NODE_STEP (224, matches auto-layout) + // For configurable nodes: HORIZONTAL_NODE_STEP + (configurable width - default width) const extraWidth = isNewNodeConfigurable ? CONFIGURABLE_NODE_SIZE[0] - DEFAULT_NODE_SIZE[0] : 0; - const shiftMargin = PUSH_NODES_OFFSET + extraWidth; + const shiftMargin = HORIZONTAL_NODE_STEP + extraWidth; shiftDownstreamNodesPosition(lastInteractedWithNode.value.name, shiftMargin, { trackHistory: true, @@ -1601,7 +1634,7 @@ export function useCanvasOperations() { // When the node has only main outputs, mixed outputs, or no outputs at all // We want to place the new node directly to the right of the last interacted with node. - let pushOffset = PUSH_NODES_OFFSET; + let pushOffset = HORIZONTAL_NODE_STEP; if (isAgentNodeV2(lastInteractedWithNodeObject)) { // The agent card is wider than a default node, so offset by its width // to keep the standard gap to its right edge @@ -1924,10 +1957,10 @@ export function useCanvasOperations() { if (associatedWithMovedNode) { // Sticky has nodes that will move - check if new node will be close enough to the sticky const newNodeRightEdge = insertX + nodeSize[0]; - // If the new node's right edge is within 2/3 of PUSH_NODES_OFFSET from the sticky's left edge, + // If the new node's right edge is within 2/3 of the horizontal step from the sticky's left edge, // stretch the sticky to include the new node const isNewNodeCloseToSticky = - newNodeRightEdge > stickyLeftEdge + (2 * PUSH_NODES_OFFSET) / 3; + newNodeRightEdge > stickyLeftEdge + (2 * HORIZONTAL_NODE_STEP) / 3; if (isNewNodeCloseToSticky) { // New node is close enough to sticky - move AND stretch @@ -2014,9 +2047,9 @@ export function useCanvasOperations() { 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, + // Use HORIZONTAL_NODE_STEP to match the actual position where nodes are placed, // including the wider agent card offset applied in resolveNodePosition - let insertOffset = PUSH_NODES_OFFSET; + let insertOffset = HORIZONTAL_NODE_STEP; if (isAgentNodeV2(sourceNode)) { insertOffset += AGENT_NODE_SIZE[0] - DEFAULT_NODE_SIZE[0]; } diff --git a/packages/frontend/editor-ui/src/app/utils/nodeViewUtils.test.ts b/packages/frontend/editor-ui/src/app/utils/nodeViewUtils.test.ts index f5704dabe7b..354cd6be881 100644 --- a/packages/frontend/editor-ui/src/app/utils/nodeViewUtils.test.ts +++ b/packages/frontend/editor-ui/src/app/utils/nodeViewUtils.test.ts @@ -13,6 +13,8 @@ import { snapPositionToGrid, calculateNodeSize, GRID_SIZE, + NODE_X_SPACING, + HORIZONTAL_NODE_STEP, doRectsOverlap, canUsePosition, } from './nodeViewUtils'; @@ -681,3 +683,18 @@ describe('getNodeViewTab', () => { expect(getNodeViewTab(route)).toBeNull(); }); }); + +describe('horizontal spacing constants', () => { + it('should keep the placement step in lockstep with the auto-layout node step', () => { + // The plus button / connection-drop places a node HORIZONTAL_NODE_STEP to the + // right of its source; the cleanup auto-layout leaves NODE_X_SPACING between + // adjacent node edges. They must agree so a freshly placed node lands exactly + // where cleanup would put it (see CAT-2395). + expect(HORIZONTAL_NODE_STEP).toBe(DEFAULT_NODE_SIZE[0] + NODE_X_SPACING); + }); + + it('should resolve to the canonical 8-dot step (224px on a 16px grid)', () => { + expect(NODE_X_SPACING).toBe(GRID_SIZE * 8); + expect(HORIZONTAL_NODE_STEP).toBe(224); + }); +}); diff --git a/packages/frontend/editor-ui/src/app/utils/nodeViewUtils.ts b/packages/frontend/editor-ui/src/app/utils/nodeViewUtils.ts index c07e79a3b91..0807a1973d6 100644 --- a/packages/frontend/editor-ui/src/app/utils/nodeViewUtils.ts +++ b/packages/frontend/editor-ui/src/app/utils/nodeViewUtils.ts @@ -50,6 +50,13 @@ export const DEFAULT_START_POSITION_X = GRID_SIZE * 11; export const DEFAULT_START_POSITION_Y = GRID_SIZE * 15; export const HEADER_HEIGHT = 65; export const PUSH_NODES_OFFSET = DEFAULT_NODE_SIZE[0] * 2 + GRID_SIZE; +// Horizontal gap the auto-layout leaves between adjacent nodes (dagre `ranksep`). +// Shared so manual placement and cleanup stay in lockstep. +export const NODE_X_SPACING = GRID_SIZE * 8; +// Center-to-center horizontal step when placing a node directly after another +// (plus button, connection drop, mid-flow insert). Must equal a node width plus +// NODE_X_SPACING so a freshly placed node lands exactly where cleanup would put it. +export const HORIZONTAL_NODE_STEP = DEFAULT_NODE_SIZE[0] + NODE_X_SPACING; export const DEFAULT_VIEWPORT_BOUNDARIES: ViewportBoundaries = { xMin: -Infinity, yMin: -Infinity, diff --git a/packages/frontend/editor-ui/src/features/workflows/canvas/composables/useCanvasLayout.ts b/packages/frontend/editor-ui/src/features/workflows/canvas/composables/useCanvasLayout.ts index 6374d6fb0e1..25a50699b71 100644 --- a/packages/frontend/editor-ui/src/features/workflows/canvas/composables/useCanvasLayout.ts +++ b/packages/frontend/editor-ui/src/features/workflows/canvas/composables/useCanvasLayout.ts @@ -15,6 +15,7 @@ import { AGENT_NODE_SIZE, DEFAULT_NODE_SIZE, GRID_SIZE, + NODE_X_SPACING, snapPositionToGridByCenter, } from '@/app/utils/nodeViewUtils'; import { @@ -55,7 +56,6 @@ export type CanvasLayoutEvent = { export type CanvasNodeDictionary = Record>; -const NODE_X_SPACING = GRID_SIZE * 8; const NODE_Y_SPACING = GRID_SIZE * 6; const SUBGRAPH_SPACING = GRID_SIZE * 8; const AI_X_SPACING = GRID_SIZE * 3; diff --git a/packages/testing/playwright/tests/e2e/regression/CAT-2395-node-spacing.spec.ts b/packages/testing/playwright/tests/e2e/regression/CAT-2395-node-spacing.spec.ts new file mode 100644 index 00000000000..780b7d89106 --- /dev/null +++ b/packages/testing/playwright/tests/e2e/regression/CAT-2395-node-spacing.spec.ts @@ -0,0 +1,47 @@ +import { HTTP_REQUEST_NODE_NAME, EDIT_FIELDS_SET_NODE_NAME } from '../../../config/constants'; +import { test, expect } from '../../../fixtures/base'; + +// Canonical horizontal gap the auto-layout (dagre `ranksep` = GRID_SIZE * 8) +// leaves between adjacent nodes. The plus-button placement must match it so a +// freshly built workflow doesn't shift when the user runs Tidy-up. +const NODE_X_SPACING = 128; +// Small tolerance for node border widths in the rendered bounding box. +const GAP_TOLERANCE = 4; + +// On-canvas node name differs from the node-creator search term (`Edit Fields (Set)`). +const DST_CANVAS_NAME = 'Edit Fields'; + +test.describe( + 'CAT-2395: consistent node spacing from the plus button', + { annotation: [{ type: 'owner', description: 'Catalysts' }] }, + () => { + test('keeps the canonical gap when adding a node off a default node, unchanged by Tidy-up', async ({ + n8n, + }) => { + await n8n.start.fromBlankCanvas(); + await n8n.canvas.addNode(HTTP_REQUEST_NODE_NAME, { closeNDV: true }); + await n8n.canvas.addNode(EDIT_FIELDS_SET_NODE_NAME, { + closeNDV: true, + fromNode: HTTP_REQUEST_NODE_NAME, + }); + + // boundingBox() is screen px; normalize by zoom (pan cancels in an + // edge-to-edge difference) so we compare logical units. + const logicalGap = async () => { + const src = await n8n.canvas.nodeByName(HTTP_REQUEST_NODE_NAME).boundingBox(); + const dst = await n8n.canvas.nodeByName(DST_CANVAS_NAME).boundingBox(); + expect(src).not.toBeNull(); + expect(dst).not.toBeNull(); + const zoom = await n8n.canvas.getCanvasZoomLevel(); + return (dst!.x - (src!.x + src!.width)) / zoom; + }; + + // Pre-fix the plus button left 208 (7 dots); it must be the canonical 224/128. + expect(Math.abs((await logicalGap()) - NODE_X_SPACING)).toBeLessThanOrEqual(GAP_TOLERANCE); + + // User-visible promise: Tidy-up must not re-space an already correctly-placed node. + await n8n.canvas.clickTidyUpButton(); + expect(Math.abs((await logicalGap()) - NODE_X_SPACING)).toBeLessThanOrEqual(GAP_TOLERANCE); + }); + }, +);