fix(n8n Form Node): Duplicate popup in manual mode (#11925)

Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <[email protected]>
n8n-io · Nov 28, 2024 · 5e07a2b · 5e07a2b
1 parent 601f7fe
commit 5e07a2b
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 28 deletions.
diff --git a/packages/editor-ui/src/stores/workflows.store.ts b/packages/editor-ui/src/stores/workflows.store.ts
@@ -84,7 +84,7 @@ import { TelemetryHelpers } from 'n8n-workflow';
 import { useWorkflowHelpers } from '@/composables/useWorkflowHelpers';
 import { useRouter } from 'vue-router';
 import { useSettingsStore } from './settings.store';
-import { openPopUpWindow } from '@/utils/executionUtils';
+import { closeFormPopupWindow, openFormPopupWindow } from '@/utils/executionUtils';
 import { useNodeHelpers } from '@/composables/useNodeHelpers';
 
 const defaults: Omit<IWorkflowDb, 'id'> & { settings: NonNullable<IWorkflowDb['settings']> } = {
@@ -143,7 +143,6 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => {
 	const chatMessages = ref<string[]>([]);
 	const isChatPanelOpen = ref(false);
 	const isLogsPanelOpen = ref(false);
-	const formPopupWindow = ref<Window | null>(null);
 
 	const workflowName = computed(() => workflow.value.name);
 
@@ -1323,12 +1322,7 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => {
 				(node.type === WAIT_NODE_TYPE && node.parameters.resume === 'form')
 			) {
 				const testUrl = getFormResumeUrl(node, executionId);
-				if (!formPopupWindow.value || formPopupWindow.value.closed) {
-					formPopupWindow.value = openPopUpWindow(testUrl);
-				} else {
-					formPopupWindow.value.location = testUrl;
-					formPopupWindow.value.focus();
-				}
+				openFormPopupWindow(testUrl);
 			}
 		} else {
 			if (tasksData.length && tasksData[tasksData.length - 1].executionStatus === 'waiting') {
@@ -1581,8 +1575,7 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => {
 		uiStore.removeActiveAction('workflowRunning');
 		workflowHelpers.setDocumentTitle(workflowName.value, 'IDLE');
 
-		formPopupWindow.value?.close();
-		formPopupWindow.value = null;
+		closeFormPopupWindow();
 
 		const runData = workflowExecutionData.value?.data?.resultData.runData ?? {};
 		for (const nodeName in runData) {

diff --git a/packages/editor-ui/src/utils/executionUtils.test.ts b/packages/editor-ui/src/utils/executionUtils.test.ts
@@ -1,7 +1,7 @@
 import { describe, it, expect, vi, beforeEach } from 'vitest';
 import {
 	displayForm,
-	openPopUpWindow,
+	openFormPopupWindow,
 	executionFilterToQueryFilter,
 	waitingNodeTooltip,
 } from './executionUtils';
@@ -15,7 +15,7 @@ vi.mock('./executionUtils', async () => {
 	const actual = await vi.importActual('./executionUtils');
 	return {
 		...actual,
-		openPopUpWindow: vi.fn(),
+		openFormPopupWindow: vi.fn(),
 	};
 });
 
@@ -86,7 +86,7 @@ describe('displayForm', () => {
 			getTestUrl: getTestUrlMock,
 		});
 
-		expect(openPopUpWindow).not.toHaveBeenCalled();
+		expect(openFormPopupWindow).not.toHaveBeenCalled();
 	});
 
 	it('should skip nodes if destinationNode does not match and node is not a directParentNode', () => {
@@ -119,7 +119,7 @@ describe('displayForm', () => {
 			getTestUrl: getTestUrlMock,
 		});
 
-		expect(openPopUpWindow).not.toHaveBeenCalled();
+		expect(openFormPopupWindow).not.toHaveBeenCalled();
 	});
 
 	it('should not open pop-up if source is "RunData.ManualChatMessage"', () => {
@@ -146,7 +146,7 @@ describe('displayForm', () => {
 			getTestUrl: getTestUrlMock,
 		});
 
-		expect(openPopUpWindow).not.toHaveBeenCalled();
+		expect(openFormPopupWindow).not.toHaveBeenCalled();
 	});
 
 	describe('executionFilterToQueryFilter()', () => {

diff --git a/packages/editor-ui/src/utils/executionUtils.ts b/packages/editor-ui/src/utils/executionUtils.ts
@@ -82,25 +82,28 @@ export const executionFilterToQueryFilter = (
 	return queryFilter;
 };
 
-export const openPopUpWindow = (
-	url: string,
-	options?: { width?: number; height?: number; alwaysInNewTab?: boolean },
-) => {
-	const windowWidth = window.innerWidth;
-	const smallScreen = windowWidth <= 800;
-	if (options?.alwaysInNewTab || smallScreen) {
-		return window.open(url, '_blank');
-	} else {
-		const height = options?.width || 700;
-		const width = options?.height || window.innerHeight - 50;
+let formPopupWindow: Window | null = null;
+
+export const openFormPopupWindow = (url: string) => {
+	if (!formPopupWindow || formPopupWindow.closed) {
+		const height = 700;
+		const width = window.innerHeight - 50;
 		const left = (window.innerWidth - height) / 2;
 		const top = 50;
 		const features = `width=${height},height=${width},left=${left},top=${top},resizable=yes,scrollbars=yes`;
 		const windowName = `form-waiting-since-${Date.now()}`;
-		return window.open(url, windowName, features);
+		formPopupWindow = window.open(url, windowName, features);
+	} else {
+		formPopupWindow.location = url;
+		formPopupWindow.focus();
 	}
 };
 
+export const closeFormPopupWindow = () => {
+	formPopupWindow?.close();
+	formPopupWindow = null;
+};
+
 export function displayForm({
 	nodes,
 	runData,
@@ -131,7 +134,7 @@ export function displayForm({
 		if (node.name === destinationNode || !node.disabled) {
 			let testUrl = '';
 			if (node.type === FORM_TRIGGER_NODE_TYPE) testUrl = getTestUrl(node);
-			if (testUrl && source !== 'RunData.ManualChatMessage') openPopUpWindow(testUrl);
+			if (testUrl && source !== 'RunData.ManualChatMessage') openFormPopupWindow(testUrl);
 		}
 	}
 }