From 685d66526831f5514a60339822fa28d716daa393 Mon Sep 17 00:00:00 2001
From: Alex Grozav <alex@grozav.com>
Date: Thu, 19 Dec 2024 16:18:51 +0200
Subject: [PATCH 1/4] fix(editor): fix configurable node design

---
 .../canvas/elements/nodes/CanvasNode.test.ts  |  27 +++++
 .../canvas/elements/nodes/CanvasNode.vue      | 114 ++++++++++++------
 2 files changed, 102 insertions(+), 39 deletions(-)

diff --git a/packages/editor-ui/src/components/canvas/elements/nodes/CanvasNode.test.ts b/packages/editor-ui/src/components/canvas/elements/nodes/CanvasNode.test.ts
index 6b72cdf037afa..816310ac7d5d8 100644
--- a/packages/editor-ui/src/components/canvas/elements/nodes/CanvasNode.test.ts
+++ b/packages/editor-ui/src/components/canvas/elements/nodes/CanvasNode.test.ts
@@ -84,6 +84,33 @@ describe('CanvasNode', () => {
 			expect(inputHandles.length).toBe(3);
 			expect(outputHandles.length).toBe(2);
 		});
+
+		it('should insert spacers after required non-main input handle', () => {
+			const { getAllByTestId } = renderComponent({
+				props: {
+					...createCanvasNodeProps({
+						data: {
+							inputs: [
+								{ type: NodeConnectionType.Main, index: 0 },
+								{ type: NodeConnectionType.AiAgent, index: 0, required: true },
+								{ type: NodeConnectionType.AiTool, index: 0 },
+							],
+							outputs: [],
+						},
+					}),
+				},
+				global: {
+					stubs: {
+						Handle: true,
+					},
+				},
+			});
+
+			const inputHandles = getAllByTestId('canvas-node-input-handle');
+
+			expect(inputHandles[1]).toHaveStyle('left: 20%');
+			expect(inputHandles[2]).toHaveStyle('left: 80%');
+		});
 	});
 
 	describe('toolbar', () => {
diff --git a/packages/editor-ui/src/components/canvas/elements/nodes/CanvasNode.vue b/packages/editor-ui/src/components/canvas/elements/nodes/CanvasNode.vue
index dd558bbef5365..01801ff4e632e 100644
--- a/packages/editor-ui/src/components/canvas/elements/nodes/CanvasNode.vue
+++ b/packages/editor-ui/src/components/canvas/elements/nodes/CanvasNode.vue
@@ -65,12 +65,18 @@ const { connectingHandle } = useCanvas();
 const inputs = computed(() => props.data.inputs);
 const outputs = computed(() => props.data.outputs);
 const connections = computed(() => props.data.connections);
-const { mainInputs, nonMainInputs, mainOutputs, nonMainOutputs, isValidConnection } =
-	useNodeConnections({
-		inputs,
-		outputs,
-		connections,
-	});
+const {
+	mainInputs,
+	nonMainInputs,
+	requiredNonMainInputs,
+	mainOutputs,
+	nonMainOutputs,
+	isValidConnection,
+} = useNodeConnections({
+	inputs,
+	outputs,
+	connections,
+});
 
 const isDisabled = computed(() => props.data.disabled);
 
@@ -101,45 +107,45 @@ function emitCanvasNodeEvent(event: CanvasEventBusEvents['nodes:action']) {
  * Inputs
  */
 
+const nonMainInputsWithSpacer = computed(() =>
+	insertSpacersBetweenEndpoints(nonMainInputs.value, requiredNonMainInputs.value.length),
+);
+
 const mappedInputs = computed(() => {
 	return [
-		...mainInputs.value.map(
-			createEndpointMappingFn({
-				mode: CanvasConnectionMode.Input,
-				position: Position.Left,
-				offsetAxis: 'top',
-			}),
-		),
-		...nonMainInputs.value.map(
-			createEndpointMappingFn({
-				mode: CanvasConnectionMode.Input,
-				position: Position.Bottom,
-				offsetAxis: 'left',
-			}),
-		),
+		...mainInputs.value.map(mainInputsMappingFn).filter(Boolean),
+		...nonMainInputsWithSpacer.value.map(nonMainInputsMappingFn).filter(Boolean),
 	];
 });
 
+function insertSpacersBetweenEndpoints(
+	endpoints: CanvasConnectionPort[],
+	requiredEndpointsCount = 0,
+	minEndpointsCount = 4,
+) {
+	const endpointsWithSpacers: Array<CanvasConnectionPort | null> = [...endpoints];
+	const optionalNonMainInputsCount = endpointsWithSpacers.length - requiredEndpointsCount;
+	const spacerCount = minEndpointsCount - requiredEndpointsCount - optionalNonMainInputsCount;
+
+	// Insert `null` in between required non-main inputs and non-required non-main inputs
+	// to separate them visually if there are less than 4 inputs in total
+	if (endpointsWithSpacers.length < minEndpointsCount && requiredEndpointsCount > 0) {
+		for (let i = 0; i < spacerCount; i++) {
+			endpointsWithSpacers.splice(requiredEndpointsCount + i, 0, null);
+		}
+	}
+
+	return endpointsWithSpacers;
+}
+
 /**
  * Outputs
  */
 
 const mappedOutputs = computed(() => {
 	return [
-		...mainOutputs.value.map(
-			createEndpointMappingFn({
-				mode: CanvasConnectionMode.Output,
-				position: Position.Right,
-				offsetAxis: 'top',
-			}),
-		),
-		...nonMainOutputs.value.map(
-			createEndpointMappingFn({
-				mode: CanvasConnectionMode.Output,
-				position: Position.Top,
-				offsetAxis: 'left',
-			}),
-		),
+		...mainOutputs.value.map(mainOutputsMappingFn).filter(Boolean),
+		...nonMainOutputs.value.map(nonMainOutputsMappingFn).filter(Boolean),
 	];
 });
 
@@ -166,10 +172,14 @@ const createEndpointMappingFn =
 		offsetAxis: 'top' | 'left';
 	}) =>
 	(
-		endpoint: CanvasConnectionPort,
+		endpoint: CanvasConnectionPort | null,
 		index: number,
-		endpoints: CanvasConnectionPort[],
-	): CanvasElementPortWithRenderData => {
+		endpoints: Array<CanvasConnectionPort | null>,
+	): CanvasElementPortWithRenderData | undefined => {
+		if (!endpoint) {
+			return;
+		}
+
 		const handleId = createCanvasConnectionHandleString({
 			mode,
 			type: endpoint.type,
@@ -194,6 +204,30 @@ const createEndpointMappingFn =
 		};
 	};
 
+const mainInputsMappingFn = createEndpointMappingFn({
+	mode: CanvasConnectionMode.Input,
+	position: Position.Left,
+	offsetAxis: 'top',
+});
+
+const nonMainInputsMappingFn = createEndpointMappingFn({
+	mode: CanvasConnectionMode.Input,
+	position: Position.Bottom,
+	offsetAxis: 'left',
+});
+
+const mainOutputsMappingFn = createEndpointMappingFn({
+	mode: CanvasConnectionMode.Output,
+	position: Position.Right,
+	offsetAxis: 'top',
+});
+
+const nonMainOutputsMappingFn = createEndpointMappingFn({
+	mode: CanvasConnectionMode.Output,
+	position: Position.Top,
+	offsetAxis: 'left',
+});
+
 /**
  * Events
  */
@@ -293,9 +327,10 @@ onBeforeUnmount(() => {
 	<div :class="classes" data-test-id="canvas-node" :data-node-type="data.type">
 		<template
 			v-for="source in mappedOutputs"
-			:key="`${source.handleId}(${source.index + 1}/${mappedOutputs.length})`"
+			:key="source ? `${source.handleId}(${source.index + 1}/${mappedOutputs.length})` : ''"
 		>
 			<CanvasHandleRenderer
+				v-if="source"
 				v-bind="source"
 				:mode="CanvasConnectionMode.Output"
 				:is-read-only="readOnly"
@@ -309,9 +344,10 @@ onBeforeUnmount(() => {
 
 		<template
 			v-for="target in mappedInputs"
-			:key="`${target.handleId}(${target.index + 1}/${mappedInputs.length})`"
+			:key="target ? `${target.handleId}(${target.index + 1}/${mappedInputs.length})` : ''"
 		>
 			<CanvasHandleRenderer
+				v-if="target"
 				v-bind="target"
 				:mode="CanvasConnectionMode.Input"
 				:is-read-only="readOnly"

From ef050945fbd8e78946ed6218ef33aebfc089fe36 Mon Sep 17 00:00:00 2001
From: Alex Grozav <alex@grozav.com>
Date: Thu, 19 Dec 2024 17:20:38 +0200
Subject: [PATCH 2/4] feat: improve node handle spacing function

---
 .../canvas/elements/nodes/CanvasNode.vue      | 25 +------
 .../editor-ui/src/utils/canvasUtilsV2.test.ts | 74 ++++++++++++++++++-
 packages/editor-ui/src/utils/canvasUtilsV2.ts | 20 +++++
 3 files changed, 96 insertions(+), 23 deletions(-)

diff --git a/packages/editor-ui/src/components/canvas/elements/nodes/CanvasNode.vue b/packages/editor-ui/src/components/canvas/elements/nodes/CanvasNode.vue
index 01801ff4e632e..043591abff00d 100644
--- a/packages/editor-ui/src/components/canvas/elements/nodes/CanvasNode.vue
+++ b/packages/editor-ui/src/components/canvas/elements/nodes/CanvasNode.vue
@@ -28,7 +28,10 @@ import { useContextMenu } from '@/composables/useContextMenu';
 import type { NodeProps, XYPosition } from '@vue-flow/core';
 import { Position } from '@vue-flow/core';
 import { useCanvas } from '@/composables/useCanvas';
-import { createCanvasConnectionHandleString } from '@/utils/canvasUtilsV2';
+import {
+	createCanvasConnectionHandleString,
+	insertSpacersBetweenEndpoints,
+} from '@/utils/canvasUtilsV2';
 import type { EventBus } from 'n8n-design-system';
 import { createEventBus } from 'n8n-design-system';
 import { isEqual } from 'lodash-es';
@@ -118,26 +121,6 @@ const mappedInputs = computed(() => {
 	];
 });
 
-function insertSpacersBetweenEndpoints(
-	endpoints: CanvasConnectionPort[],
-	requiredEndpointsCount = 0,
-	minEndpointsCount = 4,
-) {
-	const endpointsWithSpacers: Array<CanvasConnectionPort | null> = [...endpoints];
-	const optionalNonMainInputsCount = endpointsWithSpacers.length - requiredEndpointsCount;
-	const spacerCount = minEndpointsCount - requiredEndpointsCount - optionalNonMainInputsCount;
-
-	// Insert `null` in between required non-main inputs and non-required non-main inputs
-	// to separate them visually if there are less than 4 inputs in total
-	if (endpointsWithSpacers.length < minEndpointsCount && requiredEndpointsCount > 0) {
-		for (let i = 0; i < spacerCount; i++) {
-			endpointsWithSpacers.splice(requiredEndpointsCount + i, 0, null);
-		}
-	}
-
-	return endpointsWithSpacers;
-}
-
 /**
  * Outputs
  */
diff --git a/packages/editor-ui/src/utils/canvasUtilsV2.test.ts b/packages/editor-ui/src/utils/canvasUtilsV2.test.ts
index 61425172b345f..63c73218088b5 100644
--- a/packages/editor-ui/src/utils/canvasUtilsV2.test.ts
+++ b/packages/editor-ui/src/utils/canvasUtilsV2.test.ts
@@ -1,14 +1,15 @@
 import {
+	checkOverlap,
 	createCanvasConnectionHandleString,
 	createCanvasConnectionId,
+	insertSpacersBetweenEndpoints,
 	mapCanvasConnectionToLegacyConnection,
 	mapLegacyConnectionsToCanvasConnections,
 	mapLegacyEndpointsToCanvasConnectionPort,
 	parseCanvasConnectionHandleString,
-	checkOverlap,
 } from '@/utils/canvasUtilsV2';
+import type { IConnection, IConnections, INodeTypeDescription } from 'n8n-workflow';
 import { NodeConnectionType } from 'n8n-workflow';
-import type { IConnections, INodeTypeDescription, IConnection } from 'n8n-workflow';
 import type { CanvasConnection } from '@/types';
 import { CanvasConnectionMode } from '@/types';
 import type { INodeUi } from '@/Interface';
@@ -976,3 +977,72 @@ describe('checkOverlap', () => {
 		expect(checkOverlap(node1, node2)).toBe(false);
 	});
 });
+
+describe('insertSpacersBetweenEndpoints', () => {
+	it('should insert spacers when there are less than min endpoints count', () => {
+		const endpoints = [{ index: 0, required: true }];
+		const requiredEndpointsCount = endpoints.filter((endpoint) => endpoint.required).length;
+		const result = insertSpacersBetweenEndpoints(endpoints, requiredEndpointsCount, 4);
+		expect(result).toEqual([{ index: 0, required: true }, null, null, null]);
+	});
+
+	it('should not insert spacers when there are at least min endpoints count', () => {
+		const endpoints = [{ index: 0, required: true }, { index: 1 }, { index: 2 }, { index: 3 }];
+		const requiredEndpointsCount = endpoints.filter((endpoint) => endpoint.required).length;
+		const result = insertSpacersBetweenEndpoints(endpoints, requiredEndpointsCount, 4);
+		expect(result).toEqual(endpoints);
+	});
+
+	it('should handle zero required endpoints', () => {
+		const endpoints = [{ index: 0, required: false }];
+		const requiredEndpointsCount = endpoints.filter((endpoint) => endpoint.required).length;
+		const result = insertSpacersBetweenEndpoints(endpoints, requiredEndpointsCount, 4);
+		expect(result).toEqual([null, null, null, { index: 0, required: false }]);
+	});
+
+	it('should handle no endpoints', () => {
+		const endpoints: Array<{ index: number; required: boolean }> = [];
+		const requiredEndpointsCount = endpoints.filter((endpoint) => endpoint.required).length;
+		const result = insertSpacersBetweenEndpoints(endpoints, requiredEndpointsCount, 4);
+		expect(result).toEqual([null, null, null, null]);
+	});
+
+	it('should handle required endpoints greater than min endpoints count', () => {
+		const endpoints = [
+			{ index: 0, required: true },
+			{ index: 1, required: true },
+			{ index: 2, required: true },
+			{ index: 3, required: true },
+			{ index: 4, required: true },
+		];
+		const requiredEndpointsCount = endpoints.filter((endpoint) => endpoint.required).length;
+		const result = insertSpacersBetweenEndpoints(endpoints, requiredEndpointsCount, 4);
+		expect(result).toEqual(endpoints);
+	});
+
+	it('should insert spacers between required and optional endpoints', () => {
+		const endpoints = [{ index: 0, required: true }, { index: 1, required: true }, { index: 2 }];
+		const requiredEndpointsCount = endpoints.filter((endpoint) => endpoint.required).length;
+		const result = insertSpacersBetweenEndpoints(endpoints, requiredEndpointsCount, 4);
+		expect(result).toEqual([
+			{ index: 0, required: true },
+			{ index: 1, required: true },
+			null,
+			{ index: 2 },
+		]);
+	});
+
+	it('should handle required endpoints count greater than endpoints length', () => {
+		const endpoints = [{ index: 0, required: true }];
+		const requiredEndpointsCount = endpoints.filter((endpoint) => endpoint.required).length;
+		const result = insertSpacersBetweenEndpoints(endpoints, requiredEndpointsCount, 4);
+		expect(result).toEqual([{ index: 0, required: true }, null, null, null]);
+	});
+
+	it('should handle min endpoints count less than required endpoints count', () => {
+		const endpoints = [{ index: 0, required: false }];
+		const requiredEndpointsCount = endpoints.filter((endpoint) => endpoint.required).length;
+		const result = insertSpacersBetweenEndpoints(endpoints, requiredEndpointsCount, 0);
+		expect(result).toEqual([{ index: 0, required: false }]);
+	});
+});
diff --git a/packages/editor-ui/src/utils/canvasUtilsV2.ts b/packages/editor-ui/src/utils/canvasUtilsV2.ts
index d08f817d8252f..f8e412caea754 100644
--- a/packages/editor-ui/src/utils/canvasUtilsV2.ts
+++ b/packages/editor-ui/src/utils/canvasUtilsV2.ts
@@ -210,3 +210,23 @@ export function checkOverlap(node1: BoundingBox, node2: BoundingBox) {
 		)
 	);
 }
+
+export function insertSpacersBetweenEndpoints<T>(
+	endpoints: T[],
+	requiredEndpointsCount = 0,
+	minEndpointsCount = 4,
+) {
+	const endpointsWithSpacers: Array<T | null> = [...endpoints];
+	const optionalNonMainInputsCount = endpointsWithSpacers.length - requiredEndpointsCount;
+	const spacerCount = minEndpointsCount - requiredEndpointsCount - optionalNonMainInputsCount;
+
+	// Insert `null` in between required non-main inputs and non-required non-main inputs
+	// to separate them visually if there are less than 4 inputs in total
+	if (endpointsWithSpacers.length < minEndpointsCount) {
+		for (let i = 0; i < spacerCount; i++) {
+			endpointsWithSpacers.splice(requiredEndpointsCount + i, 0, null);
+		}
+	}
+
+	return endpointsWithSpacers;
+}

From fd161a9db3e3cc0e2d3ff098244d3e84f038da41 Mon Sep 17 00:00:00 2001
From: Alex Grozav <alex@grozav.com>
Date: Thu, 19 Dec 2024 17:35:41 +0200
Subject: [PATCH 3/4] feat: update configurable node icon placement

---
 .../canvas/elements/nodes/CanvasNode.vue         | 16 ++++++++--------
 .../nodes/render-types/CanvasNodeDefault.vue     |  6 ++++++
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/packages/editor-ui/src/components/canvas/elements/nodes/CanvasNode.vue b/packages/editor-ui/src/components/canvas/elements/nodes/CanvasNode.vue
index 043591abff00d..ab6177e6664c9 100644
--- a/packages/editor-ui/src/components/canvas/elements/nodes/CanvasNode.vue
+++ b/packages/editor-ui/src/components/canvas/elements/nodes/CanvasNode.vue
@@ -116,9 +116,9 @@ const nonMainInputsWithSpacer = computed(() =>
 
 const mappedInputs = computed(() => {
 	return [
-		...mainInputs.value.map(mainInputsMappingFn).filter(Boolean),
-		...nonMainInputsWithSpacer.value.map(nonMainInputsMappingFn).filter(Boolean),
-	];
+		...mainInputs.value.map(mainInputsMappingFn),
+		...nonMainInputsWithSpacer.value.map(nonMainInputsMappingFn),
+	].filter((endpoint) => !!endpoint);
 });
 
 /**
@@ -127,9 +127,9 @@ const mappedInputs = computed(() => {
 
 const mappedOutputs = computed(() => {
 	return [
-		...mainOutputs.value.map(mainOutputsMappingFn).filter(Boolean),
-		...nonMainOutputs.value.map(nonMainOutputsMappingFn).filter(Boolean),
-	];
+		...mainOutputs.value.map(mainOutputsMappingFn),
+		...nonMainOutputs.value.map(nonMainOutputsMappingFn),
+	].filter((endpoint) => !!endpoint);
 });
 
 /**
@@ -310,7 +310,7 @@ onBeforeUnmount(() => {
 	<div :class="classes" data-test-id="canvas-node" :data-node-type="data.type">
 		<template
 			v-for="source in mappedOutputs"
-			:key="source ? `${source.handleId}(${source.index + 1}/${mappedOutputs.length})` : ''"
+			:key="`${source.handleId}(${source.index + 1}/${mappedOutputs.length})`"
 		>
 			<CanvasHandleRenderer
 				v-if="source"
@@ -327,7 +327,7 @@ onBeforeUnmount(() => {
 
 		<template
 			v-for="target in mappedInputs"
-			:key="target ? `${target.handleId}(${target.index + 1}/${mappedInputs.length})` : ''"
+			:key="`${target.handleId}(${target.index + 1}/${mappedInputs.length})`"
 		>
 			<CanvasHandleRenderer
 				v-if="target"
diff --git a/packages/editor-ui/src/components/canvas/elements/nodes/render-types/CanvasNodeDefault.vue b/packages/editor-ui/src/components/canvas/elements/nodes/render-types/CanvasNodeDefault.vue
index 70a9005adcfaf..ddbbb68a760aa 100644
--- a/packages/editor-ui/src/components/canvas/elements/nodes/render-types/CanvasNodeDefault.vue
+++ b/packages/editor-ui/src/components/canvas/elements/nodes/render-types/CanvasNodeDefault.vue
@@ -201,6 +201,12 @@ function openContextMenu(event: MouseEvent) {
 				var(--configurable-node--input-width)
 		);
 
+		justify-content: flex-start;
+
+		:global(.n8n-node-icon) {
+			margin-left: var(--configurable-node--icon-offset);
+		}
+
 		.description {
 			top: unset;
 			position: relative;

From 22fbd276fcb70f2fe434836dd9877a21b0fd07d1 Mon Sep 17 00:00:00 2001
From: Alex Grozav <alex@grozav.com>
Date: Thu, 19 Dec 2024 17:39:42 +0200
Subject: [PATCH 4/4] chore: remove v-if

---
 .../src/components/canvas/elements/nodes/CanvasNode.vue         | 2 --
 1 file changed, 2 deletions(-)

diff --git a/packages/editor-ui/src/components/canvas/elements/nodes/CanvasNode.vue b/packages/editor-ui/src/components/canvas/elements/nodes/CanvasNode.vue
index ab6177e6664c9..08ae042e11fc2 100644
--- a/packages/editor-ui/src/components/canvas/elements/nodes/CanvasNode.vue
+++ b/packages/editor-ui/src/components/canvas/elements/nodes/CanvasNode.vue
@@ -313,7 +313,6 @@ onBeforeUnmount(() => {
 			:key="`${source.handleId}(${source.index + 1}/${mappedOutputs.length})`"
 		>
 			<CanvasHandleRenderer
-				v-if="source"
 				v-bind="source"
 				:mode="CanvasConnectionMode.Output"
 				:is-read-only="readOnly"
@@ -330,7 +329,6 @@ onBeforeUnmount(() => {
 			:key="`${target.handleId}(${target.index + 1}/${mappedInputs.length})`"
 		>
 			<CanvasHandleRenderer
-				v-if="target"
 				v-bind="target"
 				:mode="CanvasConnectionMode.Input"
 				:is-read-only="readOnly"