From c9628de72b327d901f79f4f58fddde7780369bb2 Mon Sep 17 00:00:00 2001 From: Alex Grozav Date: Fri, 11 Oct 2024 17:03:58 +0300 Subject: [PATCH] feat(editor): Improve insertion algorithm for nodes with multiple main outputs (no-changelog) (#11213) --- .../__tests__/useCanvasOperations.spec.ts | 18 ++-- .../src/composables/useCanvasOperations.ts | 92 ++++++++----------- .../src/utils/__tests__/nodeViewUtils.spec.ts | 34 ++++++- packages/editor-ui/src/utils/nodeViewUtils.ts | 31 +++++++ packages/editor-ui/src/views/NodeView.vue | 13 ++- 5 files changed, 121 insertions(+), 67 deletions(-) diff --git a/packages/editor-ui/src/composables/__tests__/useCanvasOperations.spec.ts b/packages/editor-ui/src/composables/__tests__/useCanvasOperations.spec.ts index e4ece8768f..a6129a7bfa 100644 --- a/packages/editor-ui/src/composables/__tests__/useCanvasOperations.spec.ts +++ b/packages/editor-ui/src/composables/__tests__/useCanvasOperations.spec.ts @@ -285,10 +285,16 @@ describe('useCanvasOperations', () => { it('should place the node at the last cancelled connection position', () => { const uiStore = mockedStore(useUIStore); + const workflowsStore = mockedStore(useWorkflowsStore); + const nodeTypesStore = mockedStore(useNodeTypesStore); const node = createTestNode({ id: '0' }); const nodeTypeDescription = mockNodeTypeDescription(); vi.spyOn(uiStore, 'lastInteractedWithNode', 'get').mockReturnValue(node); + nodeTypesStore.getNodeType = vi.fn().mockReturnValue(nodeTypeDescription); + workflowsStore.getCurrentWorkflow.mockReturnValue( + createTestWorkflowObject(workflowsStore.workflow), + ); uiStore.lastInteractedWithNodeHandle = 'inputs/main/0'; uiStore.lastCancelledConnectionPosition = [200, 200]; @@ -307,6 +313,7 @@ describe('useCanvasOperations', () => { const node = createTestNode({ id: '0' }); const nodeTypeDescription = mockNodeTypeDescription(); + const workflowObject = createTestWorkflowObject(workflowsStore.workflow); uiStore.lastInteractedWithNode = createTestNode({ position: [100, 100], @@ -314,9 +321,8 @@ describe('useCanvasOperations', () => { typeVersion: 1, }); nodeTypesStore.getNodeType = vi.fn().mockReturnValue(nodeTypeDescription); - workflowsStore.getCurrentWorkflow.mockReturnValue( - createTestWorkflowObject(workflowsStore.workflow), - ); + workflowsStore.getCurrentWorkflow.mockReturnValue(workflowObject); + workflowObject.getNode = vi.fn().mockReturnValue(node); const { resolveNodePosition } = useCanvasOperations({ router }); const position = resolveNodePosition({ ...node, position: undefined }, nodeTypeDescription); @@ -331,6 +337,7 @@ describe('useCanvasOperations', () => { const node = createTestNode({ id: '0' }); const nodeTypeDescription = mockNodeTypeDescription(); + const workflowObject = createTestWorkflowObject(workflowsStore.workflow); uiStore.lastInteractedWithNode = createTestNode({ position: [100, 100], @@ -338,9 +345,8 @@ describe('useCanvasOperations', () => { typeVersion: 1, }); nodeTypesStore.getNodeType = vi.fn().mockReturnValue(nodeTypeDescription); - workflowsStore.getCurrentWorkflow.mockReturnValue( - createTestWorkflowObject(workflowsStore.workflow), - ); + workflowsStore.getCurrentWorkflow.mockReturnValue(workflowObject); + workflowObject.getNode = vi.fn().mockReturnValue(node); vi.spyOn(NodeHelpers, 'getNodeOutputs').mockReturnValueOnce([ { type: NodeConnectionType.AiTool }, diff --git a/packages/editor-ui/src/composables/useCanvasOperations.ts b/packages/editor-ui/src/composables/useCanvasOperations.ts index 3cae57885b..af8ef1de88 100644 --- a/packages/editor-ui/src/composables/useCanvasOperations.ts +++ b/packages/editor-ui/src/composables/useCanvasOperations.ts @@ -68,7 +68,7 @@ import { CONFIGURABLE_NODE_SIZE, CONFIGURATION_NODE_SIZE, DEFAULT_NODE_SIZE, - GRID_SIZE, + generateOffsets, PUSH_NODES_OFFSET, } from '@/utils/nodeViewUtils'; import { isValidNodeConnectionType } from '@/utils/typeGuards'; @@ -926,6 +926,9 @@ export function useCanvasOperations({ router }: { router: ReturnType input !== NodeConnectionType.Main); + + const lastInteractedWithNodeOutputs = NodeHelpers.getNodeOutputs( + editableWorkflowObject.value, + lastInteractedWithNodeObject, + lastInteractedWithNodeTypeDescription, + ); + const lastInteractedWithNodeOutputTypes = NodeHelpers.getConnectionTypes( + lastInteractedWithNodeOutputs, + ); + + const lastInteractedWithNodeMainOutputs = lastInteractedWithNodeOutputTypes.filter( + (output) => output === NodeConnectionType.Main, + ); + let yOffset = 0; if (lastInteractedWithNodeConnection) { // When clicking the plus button of a node edge / connection @@ -954,35 +983,16 @@ export function useCanvasOperations({ router }: { router: ReturnType output === NodeConnectionType.Main, + if (lastInteractedWithNodeMainOutputs.length > 1) { + const yOffsetValues = generateOffsets( + lastInteractedWithNodeMainOutputs.length, + NodeViewUtils.NODE_SIZE, + NodeViewUtils.GRID_SIZE, ); - if (lastInteractedWithNodeMainOutputs.length > 1) { - const yOffsetValues = - yOffsetValuesByOutputCount[lastInteractedWithNodeMainOutputs.length - 2]; - yOffset = yOffsetValues[connectionIndex]; - } + yOffset = yOffsetValues[connectionIndex]; } let outputs: Array = []; @@ -999,31 +1009,16 @@ export function useCanvasOperations({ router }: { router: ReturnType 0 && - outputTypes.every((outputName) => outputName !== NodeConnectionType.Main) && - lastInteractedWithNodeObject + outputTypes.every((outputName) => outputName !== NodeConnectionType.Main) ) { // When the added node has only non-main outputs (configuration nodes) // We want to place the new node directly below the last interacted with node. - const lastInteractedWithNodeInputs = NodeHelpers.getNodeInputs( - editableWorkflowObject.value, - lastInteractedWithNodeObject, - lastInteractedWithNodeTypeDescription, - ); - const lastInteractedWithNodeInputTypes = NodeHelpers.getConnectionTypes( - lastInteractedWithNodeInputs, - ); - const lastInteractedWithNodeScopedInputTypes = ( - lastInteractedWithNodeInputTypes || [] - ).filter((input) => input !== NodeConnectionType.Main); const scopedConnectionIndex = lastInteractedWithNodeScopedInputTypes.findIndex( (inputType) => outputs[0] === inputType, ); @@ -1044,15 +1039,6 @@ export function useCanvasOperations({ router }: { router: ReturnType input !== NodeConnectionType.Main) diff --git a/packages/editor-ui/src/utils/__tests__/nodeViewUtils.spec.ts b/packages/editor-ui/src/utils/__tests__/nodeViewUtils.spec.ts index b886df06cb..2ad737f198 100644 --- a/packages/editor-ui/src/utils/__tests__/nodeViewUtils.spec.ts +++ b/packages/editor-ui/src/utils/__tests__/nodeViewUtils.spec.ts @@ -1,4 +1,4 @@ -import { getGenericHints } from '../nodeViewUtils'; +import { generateOffsets, getGenericHints } from '../nodeViewUtils'; import type { INode, INodeTypeDescription, INodeExecutionData, Workflow } from 'n8n-workflow'; import type { INodeUi } from '@/Interface'; import { NodeHelpers } from 'n8n-workflow'; @@ -137,3 +137,35 @@ describe('getGenericHints', () => { ]); }); }); + +describe('generateOffsets', () => { + it('should return correct offsets for 0 nodes', () => { + const result = generateOffsets(0, 100, 20); + expect(result).toEqual([]); + }); + + it('should return correct offsets for 1 node', () => { + const result = generateOffsets(1, 100, 20); + expect(result).toEqual([0]); + }); + + it('should return correct offsets for 2 nodes', () => { + const result = generateOffsets(2, 100, 20); + expect(result).toEqual([-100, 100]); + }); + + it('should return correct offsets for 3 nodes', () => { + const result = generateOffsets(3, 100, 20); + expect(result).toEqual([-120, 0, 120]); + }); + + it('should return correct offsets for 4 nodes', () => { + const result = generateOffsets(4, 100, 20); + expect(result).toEqual([-220, -100, 100, 220]); + }); + + it('should return correct offsets for large node count', () => { + const result = generateOffsets(10, 100, 20); + expect(result).toEqual([-580, -460, -340, -220, -100, 100, 220, 340, 460, 580]); + }); +}); diff --git a/packages/editor-ui/src/utils/nodeViewUtils.ts b/packages/editor-ui/src/utils/nodeViewUtils.ts index 21be904303..a4495cf5dc 100644 --- a/packages/editor-ui/src/utils/nodeViewUtils.ts +++ b/packages/editor-ui/src/utils/nodeViewUtils.ts @@ -1289,3 +1289,34 @@ export function getGenericHints({ return nodeHints; } + +/** + * Generate vertical insertion offsets for the given node count + * + * 2 nodes -> [-nodeSize, nodeSize], + * 3 nodes -> [-nodeSize - 2 * gridSize, 0, nodeSize + 2 * gridSize], + * 4 nodes -> [-2 * nodeSize - 2 * gridSize, -nodeSize, nodeSize, 2 * nodeSize + 2 * gridSize] + * 5 nodes -> [-2 * nodeSize - 2 * gridSize, -nodeSize, 0, nodeSize, 2 * nodeSize + 2 * gridSize] + */ +export function generateOffsets(nodeCount: number, nodeSize: number, gridSize: number) { + const offsets = []; + const half = Math.floor(nodeCount / 2); + const isOdd = nodeCount % 2 === 1; + + if (nodeCount === 0) { + return []; + } + + for (let i = -half; i <= half; i++) { + if (i === 0) { + if (isOdd) { + offsets.push(0); + } + } else { + const offset = i * nodeSize + Math.sign(i) * (Math.abs(i) - (isOdd ? 0 : 1)) * gridSize; + offsets.push(offset); + } + } + + return offsets; +} diff --git a/packages/editor-ui/src/views/NodeView.vue b/packages/editor-ui/src/views/NodeView.vue index b932d9842a..3686333125 100644 --- a/packages/editor-ui/src/views/NodeView.vue +++ b/packages/editor-ui/src/views/NodeView.vue @@ -161,6 +161,7 @@ import { getConnectorPaintStyleData, OVERLAY_ENDPOINT_ARROW_ID, getEndpointScope, + generateOffsets, } from '@/utils/nodeViewUtils'; import { useViewStacks } from '@/components/Node/NodeCreator/composables/useViewStacks'; import { useExternalHooks } from '@/composables/useExternalHooks'; @@ -2275,12 +2276,6 @@ export default defineComponent({ ); if (sourceNodeType) { - const offsets = [ - [-100, 100], - [-140, 0, 140], - [-240, -100, 100, 240], - ]; - const sourceNodeOutputs = NodeHelpers.getNodeOutputs( workflow, lastSelectedNode, @@ -2293,7 +2288,11 @@ export default defineComponent({ ); if (sourceNodeOutputMainOutputs.length > 1) { - const offset = offsets[sourceNodeOutputMainOutputs.length - 2]; + const offset = generateOffsets( + sourceNodeOutputMainOutputs.length, + NodeViewUtils.NODE_SIZE, + NodeViewUtils.GRID_SIZE, + ); const sourceOutputIndex = lastSelectedConnection.__meta ? lastSelectedConnection.__meta.sourceOutputIndex : 0;