mirror of
https://github.com/n8n-io/n8n.git
synced 2026-09-24 23:22:38 +08:00
fix(editor): Standardize automatic node spacing to match cleanup layout (no-changelog) (#34413)
This commit is contained in:
@@ -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: {},
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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];
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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,
|
||||
|
||||
+1
-1
@@ -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<string, GraphNode<CanvasNodeData>>;
|
||||
|
||||
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;
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
},
|
||||
);
|
||||
Reference in New Issue
Block a user