From fc39e3ca16231c176957e2504d55df6b416874fe Mon Sep 17 00:00:00 2001 From: Alex Grozav Date: Thu, 7 Nov 2024 16:20:48 +0200 Subject: [PATCH] fix(editor): Add stickies to node insert position conflict check allowlist (#11624) --- packages/editor-ui/src/constants.ts | 2 + .../editor-ui/src/utils/nodeViewUtils.test.ts | 60 ++++++++++++++++++- packages/editor-ui/src/utils/nodeViewUtils.ts | 6 ++ 3 files changed, 66 insertions(+), 2 deletions(-) diff --git a/packages/editor-ui/src/constants.ts b/packages/editor-ui/src/constants.ts index 5b9254b2ff..832004e58e 100644 --- a/packages/editor-ui/src/constants.ts +++ b/packages/editor-ui/src/constants.ts @@ -221,6 +221,8 @@ export const NODES_USING_CODE_NODE_EDITOR = [ AI_TRANSFORM_NODE_TYPE, ]; +export const NODE_POSITION_CONFLICT_ALLOWLIST = [STICKY_NODE_TYPE]; + export const PIN_DATA_NODE_TYPES_DENYLIST = [SPLIT_IN_BATCHES_NODE_TYPE, STICKY_NODE_TYPE]; export const OPEN_URL_PANEL_TRIGGER_NODE_TYPES = [ diff --git a/packages/editor-ui/src/utils/nodeViewUtils.test.ts b/packages/editor-ui/src/utils/nodeViewUtils.test.ts index 09c8a46366..9e01b3449c 100644 --- a/packages/editor-ui/src/utils/nodeViewUtils.test.ts +++ b/packages/editor-ui/src/utils/nodeViewUtils.test.ts @@ -1,10 +1,12 @@ -import { generateOffsets, getGenericHints } from './nodeViewUtils'; +import { generateOffsets, getGenericHints, getNewNodePosition } from './nodeViewUtils'; import type { INode, INodeTypeDescription, INodeExecutionData, Workflow } from 'n8n-workflow'; -import type { INodeUi } from '@/Interface'; +import type { INodeUi, XYPosition } from '@/Interface'; import { NodeHelpers } from 'n8n-workflow'; import { describe, it, expect, beforeEach } from 'vitest'; import { mock, type MockProxy } from 'vitest-mock-extended'; +import { SET_NODE_TYPE, STICKY_NODE_TYPE } from '@/constants'; +import { createTestNode } from '@/__tests__/mocks'; describe('getGenericHints', () => { let mockWorkflowNode: MockProxy; @@ -169,3 +171,57 @@ describe('generateOffsets', () => { expect(result).toEqual([-580, -460, -340, -220, -100, 100, 220, 340, 460, 580]); }); }); + +describe('getNewNodePosition', () => { + it('should return the new position when there are no conflicts', () => { + const nodes: INodeUi[] = []; + const newPosition: XYPosition = [100, 100]; + const result = getNewNodePosition(nodes, newPosition); + expect(result).toEqual([100, 100]); + }); + + it('should adjust the position to the closest grid size', () => { + const nodes: INodeUi[] = []; + const newPosition: XYPosition = [105, 115]; + const result = getNewNodePosition(nodes, newPosition); + expect(result).toEqual([120, 120]); + }); + + it('should move the position to avoid conflicts', () => { + const nodes: INodeUi[] = [ + createTestNode({ id: '1', position: [100, 100], type: SET_NODE_TYPE }), + ]; + const newPosition: XYPosition = [100, 100]; + const result = getNewNodePosition(nodes, newPosition); + expect(result).toEqual([180, 180]); + }); + + it('should skip nodes in the conflict allowlist', () => { + const nodes: INodeUi[] = [ + createTestNode({ id: '1', position: [100, 100], type: STICKY_NODE_TYPE }), + ]; + const newPosition: XYPosition = [100, 100]; + const result = getNewNodePosition(nodes, newPosition); + expect(result).toEqual([100, 100]); + }); + + it('should use the provided move position to resolve conflicts', () => { + const nodes: INodeUi[] = [ + createTestNode({ id: '1', position: [100, 100], type: SET_NODE_TYPE }), + ]; + const newPosition: XYPosition = [100, 100]; + const movePosition: XYPosition = [50, 50]; + const result = getNewNodePosition(nodes, newPosition, movePosition); + expect(result).toEqual([200, 200]); + }); + + it('should handle multiple conflicts correctly', () => { + const nodes: INodeUi[] = [ + createTestNode({ id: '1', position: [100, 100], type: SET_NODE_TYPE }), + createTestNode({ id: '2', position: [140, 140], type: SET_NODE_TYPE }), + ]; + const newPosition: XYPosition = [100, 100]; + const result = getNewNodePosition(nodes, newPosition); + expect(result).toEqual([220, 220]); + }); +}); diff --git a/packages/editor-ui/src/utils/nodeViewUtils.ts b/packages/editor-ui/src/utils/nodeViewUtils.ts index a4495cf5dc..a382180598 100644 --- a/packages/editor-ui/src/utils/nodeViewUtils.ts +++ b/packages/editor-ui/src/utils/nodeViewUtils.ts @@ -2,6 +2,7 @@ import { isNumber, isValidNodeConnectionType } from '@/utils/typeGuards'; import { LIST_LIKE_NODE_OPERATIONS, NODE_OUTPUT_DEFAULT_KEY, + NODE_POSITION_CONFLICT_ALLOWLIST, SET_NODE_TYPE, SPLIT_IN_BATCHES_NODE_TYPE, STICKY_NODE_TYPE, @@ -582,6 +583,11 @@ export const getNewNodePosition = ( conflictFound = false; for (i = 0; i < nodes.length; i++) { node = nodes[i]; + + if (NODE_POSITION_CONFLICT_ALLOWLIST.includes(node.type)) { + continue; + } + if (!canUsePosition(node.position, targetPosition)) { conflictFound = true; break;