From 8aaa0d3c572fb125f6dcc4d291a8b0473ec683e1 Mon Sep 17 00:00:00 2001 From: Michael Kret Date: Wed, 27 Nov 2024 14:11:53 +0200 Subject: [PATCH 1/6] fix double popup --- packages/editor-ui/src/stores/workflows.store.ts | 1 + packages/editor-ui/src/utils/executionUtils.ts | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/editor-ui/src/stores/workflows.store.ts b/packages/editor-ui/src/stores/workflows.store.ts index f49beaed84781..e22a0724f71ff 100644 --- a/packages/editor-ui/src/stores/workflows.store.ts +++ b/packages/editor-ui/src/stores/workflows.store.ts @@ -1726,5 +1726,6 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { setNodes, setConnections, markExecutionAsStopped, + formPopupWindow, }; }); diff --git a/packages/editor-ui/src/utils/executionUtils.ts b/packages/editor-ui/src/utils/executionUtils.ts index 014f3492b638d..71adcc880b759 100644 --- a/packages/editor-ui/src/utils/executionUtils.ts +++ b/packages/editor-ui/src/utils/executionUtils.ts @@ -131,7 +131,9 @@ 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') { + useWorkflowsStore().formPopupWindow = openPopUpWindow(testUrl); + } } } } From a3958dafcbf822ca64d68e6875c13bf3598d3cdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Wed, 27 Nov 2024 13:27:04 +0100 Subject: [PATCH 2/6] alternative proposal for node-2026 --- .../editor-ui/src/stores/workflows.store.ts | 13 ++------- .../editor-ui/src/utils/executionUtils.ts | 29 ++++++++++--------- 2 files changed, 19 insertions(+), 23 deletions(-) diff --git a/packages/editor-ui/src/stores/workflows.store.ts b/packages/editor-ui/src/stores/workflows.store.ts index f49beaed84781..58452eaf34249 100644 --- 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 & { settings: NonNullable } = { @@ -143,7 +143,6 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { const chatMessages = ref([]); const isChatPanelOpen = ref(false); const isLogsPanelOpen = ref(false); - const formPopupWindow = ref(null); const workflowName = computed(() => workflow.value.name); @@ -1319,12 +1318,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') { @@ -1577,8 +1571,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.ts b/packages/editor-ui/src/utils/executionUtils.ts index 014f3492b638d..d8db7b8f8e950 100644 --- 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); } } } From 38c64fea1216178967608df544823007e9375543 Mon Sep 17 00:00:00 2001 From: Michael Kret Date: Wed, 27 Nov 2024 14:33:48 +0200 Subject: [PATCH 3/6] update --- .../editor-ui/src/stores/workflows.store.ts | 18 +++++++------ .../src/utils/executionUtils.test.ts | 25 ++++++------------- .../editor-ui/src/utils/executionUtils.ts | 2 +- 3 files changed, 19 insertions(+), 26 deletions(-) diff --git a/packages/editor-ui/src/stores/workflows.store.ts b/packages/editor-ui/src/stores/workflows.store.ts index e22a0724f71ff..92cb680a89169 100644 --- a/packages/editor-ui/src/stores/workflows.store.ts +++ b/packages/editor-ui/src/stores/workflows.store.ts @@ -1278,6 +1278,15 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { } } + function openFormWindow(url: string) { + if (!formPopupWindow.value || formPopupWindow.value.closed) { + formPopupWindow.value = openPopUpWindow(url); + } else { + formPopupWindow.value.location = url; + formPopupWindow.value.focus(); + } + } + function getFormResumeUrl(node: INode, executionId: string) { const { webhookSuffix } = (node.parameters.options ?? {}) as IDataObject; const suffix = webhookSuffix && typeof webhookSuffix !== 'object' ? `/${webhookSuffix}` : ''; @@ -1319,12 +1328,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(); - } + openFormWindow(testUrl); } } else { if (tasksData.length && tasksData[tasksData.length - 1].executionStatus === 'waiting') { @@ -1726,6 +1730,6 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { setNodes, setConnections, markExecutionAsStopped, - formPopupWindow, + openFormWindow, }; }); diff --git a/packages/editor-ui/src/utils/executionUtils.test.ts b/packages/editor-ui/src/utils/executionUtils.test.ts index abcab9509fcc4..f32f4d2a6699d 100644 --- a/packages/editor-ui/src/utils/executionUtils.test.ts +++ b/packages/editor-ui/src/utils/executionUtils.test.ts @@ -1,24 +1,12 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { - displayForm, - openPopUpWindow, - executionFilterToQueryFilter, - waitingNodeTooltip, -} from './executionUtils'; +import { displayForm, executionFilterToQueryFilter, waitingNodeTooltip } from './executionUtils'; import type { INode, IRunData, IPinData } from 'n8n-workflow'; import { type INodeUi } from '../Interface'; +import { useWorkflowsStore } from '@/stores/workflows.store'; const FORM_TRIGGER_NODE_TYPE = 'formTrigger'; const WAIT_NODE_TYPE = 'waitNode'; -vi.mock('./executionUtils', async () => { - const actual = await vi.importActual('./executionUtils'); - return { - ...actual, - openPopUpWindow: vi.fn(), - }; -}); - vi.mock('@/stores/root.store', () => ({ useRootStore: () => ({ formWaitingUrl: 'http://localhost:5678/form-waiting', @@ -29,6 +17,7 @@ vi.mock('@/stores/root.store', () => ({ vi.mock('@/stores/workflows.store', () => ({ useWorkflowsStore: () => ({ activeExecutionId: '123', + openFormWindow: vi.fn(), }), })); @@ -53,7 +42,7 @@ describe('displayForm', () => { vi.clearAllMocks(); }); - it('should not call openPopUpWindow if node has already run or is pinned', () => { + it('should not call openFormWindow if node has already run or is pinned', () => { const nodes: INode[] = [ { id: '1', @@ -86,7 +75,7 @@ describe('displayForm', () => { getTestUrl: getTestUrlMock, }); - expect(openPopUpWindow).not.toHaveBeenCalled(); + expect(useWorkflowsStore().openFormWindow).not.toHaveBeenCalled(); }); it('should skip nodes if destinationNode does not match and node is not a directParentNode', () => { @@ -119,7 +108,7 @@ describe('displayForm', () => { getTestUrl: getTestUrlMock, }); - expect(openPopUpWindow).not.toHaveBeenCalled(); + expect(useWorkflowsStore().openFormWindow).not.toHaveBeenCalled(); }); it('should not open pop-up if source is "RunData.ManualChatMessage"', () => { @@ -146,7 +135,7 @@ describe('displayForm', () => { getTestUrl: getTestUrlMock, }); - expect(openPopUpWindow).not.toHaveBeenCalled(); + expect(useWorkflowsStore().openFormWindow).not.toHaveBeenCalled(); }); describe('executionFilterToQueryFilter()', () => { diff --git a/packages/editor-ui/src/utils/executionUtils.ts b/packages/editor-ui/src/utils/executionUtils.ts index 71adcc880b759..06cafc5e8b6a0 100644 --- a/packages/editor-ui/src/utils/executionUtils.ts +++ b/packages/editor-ui/src/utils/executionUtils.ts @@ -132,7 +132,7 @@ export function displayForm({ let testUrl = ''; if (node.type === FORM_TRIGGER_NODE_TYPE) testUrl = getTestUrl(node); if (testUrl && source !== 'RunData.ManualChatMessage') { - useWorkflowsStore().formPopupWindow = openPopUpWindow(testUrl); + useWorkflowsStore().openFormWindow(testUrl); } } } From 0c00d36640e68c53b9a302b2525995875ab52fb9 Mon Sep 17 00:00:00 2001 From: Michael Kret Date: Wed, 27 Nov 2024 14:44:38 +0200 Subject: [PATCH 4/6] Revert "update" This reverts commit 38c64fea1216178967608df544823007e9375543. --- .../editor-ui/src/stores/workflows.store.ts | 18 ++++++------- .../src/utils/executionUtils.test.ts | 25 +++++++++++++------ .../editor-ui/src/utils/executionUtils.ts | 2 +- 3 files changed, 26 insertions(+), 19 deletions(-) diff --git a/packages/editor-ui/src/stores/workflows.store.ts b/packages/editor-ui/src/stores/workflows.store.ts index 92cb680a89169..e22a0724f71ff 100644 --- a/packages/editor-ui/src/stores/workflows.store.ts +++ b/packages/editor-ui/src/stores/workflows.store.ts @@ -1278,15 +1278,6 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { } } - function openFormWindow(url: string) { - if (!formPopupWindow.value || formPopupWindow.value.closed) { - formPopupWindow.value = openPopUpWindow(url); - } else { - formPopupWindow.value.location = url; - formPopupWindow.value.focus(); - } - } - function getFormResumeUrl(node: INode, executionId: string) { const { webhookSuffix } = (node.parameters.options ?? {}) as IDataObject; const suffix = webhookSuffix && typeof webhookSuffix !== 'object' ? `/${webhookSuffix}` : ''; @@ -1328,7 +1319,12 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { (node.type === WAIT_NODE_TYPE && node.parameters.resume === 'form') ) { const testUrl = getFormResumeUrl(node, executionId); - openFormWindow(testUrl); + if (!formPopupWindow.value || formPopupWindow.value.closed) { + formPopupWindow.value = openPopUpWindow(testUrl); + } else { + formPopupWindow.value.location = testUrl; + formPopupWindow.value.focus(); + } } } else { if (tasksData.length && tasksData[tasksData.length - 1].executionStatus === 'waiting') { @@ -1730,6 +1726,6 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { setNodes, setConnections, markExecutionAsStopped, - openFormWindow, + formPopupWindow, }; }); diff --git a/packages/editor-ui/src/utils/executionUtils.test.ts b/packages/editor-ui/src/utils/executionUtils.test.ts index f32f4d2a6699d..abcab9509fcc4 100644 --- a/packages/editor-ui/src/utils/executionUtils.test.ts +++ b/packages/editor-ui/src/utils/executionUtils.test.ts @@ -1,12 +1,24 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { displayForm, executionFilterToQueryFilter, waitingNodeTooltip } from './executionUtils'; +import { + displayForm, + openPopUpWindow, + executionFilterToQueryFilter, + waitingNodeTooltip, +} from './executionUtils'; import type { INode, IRunData, IPinData } from 'n8n-workflow'; import { type INodeUi } from '../Interface'; -import { useWorkflowsStore } from '@/stores/workflows.store'; const FORM_TRIGGER_NODE_TYPE = 'formTrigger'; const WAIT_NODE_TYPE = 'waitNode'; +vi.mock('./executionUtils', async () => { + const actual = await vi.importActual('./executionUtils'); + return { + ...actual, + openPopUpWindow: vi.fn(), + }; +}); + vi.mock('@/stores/root.store', () => ({ useRootStore: () => ({ formWaitingUrl: 'http://localhost:5678/form-waiting', @@ -17,7 +29,6 @@ vi.mock('@/stores/root.store', () => ({ vi.mock('@/stores/workflows.store', () => ({ useWorkflowsStore: () => ({ activeExecutionId: '123', - openFormWindow: vi.fn(), }), })); @@ -42,7 +53,7 @@ describe('displayForm', () => { vi.clearAllMocks(); }); - it('should not call openFormWindow if node has already run or is pinned', () => { + it('should not call openPopUpWindow if node has already run or is pinned', () => { const nodes: INode[] = [ { id: '1', @@ -75,7 +86,7 @@ describe('displayForm', () => { getTestUrl: getTestUrlMock, }); - expect(useWorkflowsStore().openFormWindow).not.toHaveBeenCalled(); + expect(openPopUpWindow).not.toHaveBeenCalled(); }); it('should skip nodes if destinationNode does not match and node is not a directParentNode', () => { @@ -108,7 +119,7 @@ describe('displayForm', () => { getTestUrl: getTestUrlMock, }); - expect(useWorkflowsStore().openFormWindow).not.toHaveBeenCalled(); + expect(openPopUpWindow).not.toHaveBeenCalled(); }); it('should not open pop-up if source is "RunData.ManualChatMessage"', () => { @@ -135,7 +146,7 @@ describe('displayForm', () => { getTestUrl: getTestUrlMock, }); - expect(useWorkflowsStore().openFormWindow).not.toHaveBeenCalled(); + expect(openPopUpWindow).not.toHaveBeenCalled(); }); describe('executionFilterToQueryFilter()', () => { diff --git a/packages/editor-ui/src/utils/executionUtils.ts b/packages/editor-ui/src/utils/executionUtils.ts index 06cafc5e8b6a0..71adcc880b759 100644 --- a/packages/editor-ui/src/utils/executionUtils.ts +++ b/packages/editor-ui/src/utils/executionUtils.ts @@ -132,7 +132,7 @@ export function displayForm({ let testUrl = ''; if (node.type === FORM_TRIGGER_NODE_TYPE) testUrl = getTestUrl(node); if (testUrl && source !== 'RunData.ManualChatMessage') { - useWorkflowsStore().openFormWindow(testUrl); + useWorkflowsStore().formPopupWindow = openPopUpWindow(testUrl); } } } From 55fbafe12194323f13d1fc99fd4b849528fe92cc Mon Sep 17 00:00:00 2001 From: Michael Kret Date: Wed, 27 Nov 2024 14:44:44 +0200 Subject: [PATCH 5/6] Revert "fix double popup" This reverts commit 8aaa0d3c572fb125f6dcc4d291a8b0473ec683e1. --- packages/editor-ui/src/stores/workflows.store.ts | 1 - packages/editor-ui/src/utils/executionUtils.ts | 4 +--- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/editor-ui/src/stores/workflows.store.ts b/packages/editor-ui/src/stores/workflows.store.ts index e22a0724f71ff..f49beaed84781 100644 --- a/packages/editor-ui/src/stores/workflows.store.ts +++ b/packages/editor-ui/src/stores/workflows.store.ts @@ -1726,6 +1726,5 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { setNodes, setConnections, markExecutionAsStopped, - formPopupWindow, }; }); diff --git a/packages/editor-ui/src/utils/executionUtils.ts b/packages/editor-ui/src/utils/executionUtils.ts index 71adcc880b759..014f3492b638d 100644 --- a/packages/editor-ui/src/utils/executionUtils.ts +++ b/packages/editor-ui/src/utils/executionUtils.ts @@ -131,9 +131,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') { - useWorkflowsStore().formPopupWindow = openPopUpWindow(testUrl); - } + if (testUrl && source !== 'RunData.ManualChatMessage') openPopUpWindow(testUrl); } } } From 2488e142f5a72e24e1a0692741a7e66b616a85cd Mon Sep 17 00:00:00 2001 From: Michael Kret Date: Wed, 27 Nov 2024 14:49:32 +0200 Subject: [PATCH 6/6] test fix --- packages/editor-ui/src/utils/executionUtils.test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/editor-ui/src/utils/executionUtils.test.ts b/packages/editor-ui/src/utils/executionUtils.test.ts index abcab9509fcc4..a4f87cf3f1099 100644 --- 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()', () => {