From 0a59002ef878ff8836d3ca63956f7a444d329d0b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milorad=20FIlipovi=C4=87?= <milorad@n8n.io>
Date: Fri, 31 Mar 2023 13:59:09 +0200
Subject: [PATCH] feat(editor): Enable saving workflow when node details view
 is open (#5856)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* feat(editor): Enable saving workflow when node details view is open
* ⚡ Preventing event propagation
* ⚡ Move save event handler to `NodeDetailsView`
* ✅ Added e2e tests
* 👕 Fixing linting and removing unnecessary event logic

---------

Co-authored-by: Oleg Ivaniv <oleg@n8n.io>
---
 cypress/e2e/5-ndv.cy.ts                       |  8 ++++
 .../src/components/NodeDetailsView.vue        | 24 ++++++++++-
 .../src/plugins/i18n/locales/en.json          |  1 +
 packages/editor-ui/src/views/NodeView.vue     | 43 +++++++++++++------
 4 files changed, 63 insertions(+), 13 deletions(-)

diff --git a/cypress/e2e/5-ndv.cy.ts b/cypress/e2e/5-ndv.cy.ts
index 67b318da6d..ea4533725c 100644
--- a/cypress/e2e/5-ndv.cy.ts
+++ b/cypress/e2e/5-ndv.cy.ts
@@ -90,6 +90,14 @@ describe('NDV', () => {
 		});
 	});
 
+	it('should save workflow using keyboard shortcut from NDV', () => {
+		workflowPage.actions.addNodeToCanvas('Manual');
+		workflowPage.actions.addNodeToCanvas('Set', true, true);
+		ndv.getters.container().should('be.visible');
+		workflowPage.actions.saveWorkflowUsingKeyboardShortcut();
+		workflowPage.getters.isWorkflowSaved();
+	})
+
 	describe('test output schema view', () => {
 		const schemaKeys = ['id', 'name', 'email', 'notes', 'country', 'created', 'objectValue', 'prop1', 'prop2'];
 		function setupSchemaWorkflow() {
diff --git a/packages/editor-ui/src/components/NodeDetailsView.vue b/packages/editor-ui/src/components/NodeDetailsView.vue
index 07e27bbfba..0b4dd3de86 100644
--- a/packages/editor-ui/src/components/NodeDetailsView.vue
+++ b/packages/editor-ui/src/components/NodeDetailsView.vue
@@ -28,7 +28,13 @@
 			</div>
 		</n8n-tooltip>
 
-		<div class="data-display" v-if="activeNode">
+		<div
+			v-if="activeNode"
+			class="data-display"
+			ref="container"
+			@keydown.capture="onKeyDown"
+			tabindex="0"
+		>
 			<div @click="close" :class="$style.modalBackground"></div>
 			<NDVDraggablePanels
 				:isTriggerNode="isTriggerNode"
@@ -156,6 +162,7 @@ import { useNDVStore } from '@/stores/ndv';
 import { useNodeTypesStore } from '@/stores/nodeTypes';
 import { useUIStore } from '@/stores/ui';
 import { useSettingsStore } from '@/stores/settings';
+import useDeviceSupport from '@/composables/useDeviceSupport';
 
 export default mixins(
 	externalHooks,
@@ -184,6 +191,11 @@ export default mixins(
 			default: false,
 		},
 	},
+	setup() {
+		return {
+			...useDeviceSupport(),
+		};
+	},
 	data() {
 		return {
 			settingsEventBus: new Vue(),
@@ -469,6 +481,16 @@ export default mixins(
 		},
 	},
 	methods: {
+		onKeyDown(e: KeyboardEvent) {
+			if (e.key === 's' && this.isCtrlKeyPressed(e)) {
+				e.stopPropagation();
+				e.preventDefault();
+
+				if (this.readOnly) return;
+
+				this.$emit('saveKeyboardShortcut', e);
+			}
+		},
 		onInputItemHover(e: { itemIndex: number; outputIndex: number } | null) {
 			if (!this.inputNodeName) {
 				return;
diff --git a/packages/editor-ui/src/plugins/i18n/locales/en.json b/packages/editor-ui/src/plugins/i18n/locales/en.json
index eee57392ef..74552f5a89 100644
--- a/packages/editor-ui/src/plugins/i18n/locales/en.json
+++ b/packages/editor-ui/src/plugins/i18n/locales/en.json
@@ -40,6 +40,7 @@
 	"generic.unsavedWork.confirmMessage.confirmButtonText": "Save",
 	"generic.unsavedWork.confirmMessage.cancelButtonText": "Leave without saving",
 	"generic.workflow": "Workflow",
+	"generic.workflowSaved": "Workflow changes saved",
 	"generic.editor": "Editor",
 	"about.aboutN8n": "About n8n",
 	"about.close": "Close",
diff --git a/packages/editor-ui/src/views/NodeView.vue b/packages/editor-ui/src/views/NodeView.vue
index e6d1413722..1083ba92b0 100644
--- a/packages/editor-ui/src/views/NodeView.vue
+++ b/packages/editor-ui/src/views/NodeView.vue
@@ -92,6 +92,7 @@
 				:isProductionExecutionPreview="isProductionExecutionPreview"
 				@valueChanged="valueChanged"
 				@stopExecution="stopExecution"
+				@saveKeyboardShortcut="onSaveKeyboardShortcut"
 			/>
 			<node-creation
 				v-if="!isReadOnly"
@@ -752,9 +753,24 @@ export default mixins(
 
 			return uniqueName;
 		},
-		async onSaveKeyboardShortcut() {
-			const saved = await this.saveCurrentWorkflow();
+		async onSaveKeyboardShortcut(e: KeyboardEvent) {
+			let saved = await this.saveCurrentWorkflow();
 			if (saved) await this.settingsStore.fetchPromptsData();
+			if (this.activeNode) {
+				// If NDV is open, save will not work from editable input fields
+				// so don't show success message if this is true
+				if (e.target instanceof HTMLInputElement) {
+					saved = e.target.readOnly;
+				} else {
+					saved = true;
+				}
+				if (saved) {
+					this.$showMessage({
+						title: this.$locale.baseText('generic.workflowSaved'),
+						type: 'success',
+					});
+				}
+			}
 		},
 		showTriggerCreator(source: NodeCreatorOpenSource) {
 			if (this.createNodeActive) return;
@@ -994,6 +1010,19 @@ export default mixins(
 			}
 		},
 		async keyDown(e: KeyboardEvent) {
+			if (e.key === 's' && this.isCtrlKeyPressed(e)) {
+				e.stopPropagation();
+				e.preventDefault();
+
+				if (this.isReadOnly) {
+					return;
+				}
+
+				this.callDebounced('onSaveKeyboardShortcut', { debounceTime: 1000 }, e);
+
+				return;
+			}
+
 			// @ts-ignore
 			const path = e.path || (e.composedPath && e.composedPath());
 
@@ -1082,16 +1111,6 @@ export default mixins(
 					title: this.$locale.baseText('nodeView.showMessage.keyDown.title'),
 					type: 'success',
 				});
-			} else if (e.key === 's' && this.isCtrlKeyPressed(e)) {
-				// Save workflow
-				e.stopPropagation();
-				e.preventDefault();
-
-				if (this.isReadOnly) {
-					return;
-				}
-
-				this.callDebounced('onSaveKeyboardShortcut', { debounceTime: 1000 });
 			} else if (e.key === 'Enter') {
 				// Activate the last selected node
 				const lastSelectedNode = this.lastSelectedNode;