fix(editor): Prevent canvas undo/redo when NDV is open (#8118)

## Summary
Preventing canvas undo/redo while NDV or any modal is open. We already
had a NDV open check in place but looks like it was broken by unreactive
ref inside `useHistoryHelper` composable.
This PR fixes this by using store getter directly inside the helper
method and adds modal open check.

## Related tickets and issues
Fixes ADO-657

## Review / Merge checklist
- [ ] PR title and summary are descriptive. **Remember, the title
automatically goes into the changelog. Use `(no-changelog)` otherwise.**
([conventions](https://github.com/n8n-io/n8n/blob/master/.github/pull_request_title_conventions.md))
- [ ] [Docs updated](https://github.com/n8n-io/n8n-docs) or follow-up
ticket created.
- [ ] Tests included.
> A bug is not considered fixed, unless a test is added to prevent it
from happening again.
   > A feature is not complete without tests.

cypress/e2e/10-undo-redo.cy.ts

@@ -1,12 +1,14 @@
 import { CODE_NODE_NAME, SET_NODE_NAME, EDIT_FIELDS_SET_NODE_NAME } from './../constants';
 import { SCHEDULE_TRIGGER_NODE_NAME } from '../constants';
 import { WorkflowPage as WorkflowPageClass } from '../pages/workflow';
+import { MessageBox as MessageBoxClass } from '../pages/modals/message-box';
 import { NDV } from '../pages/ndv';
 
 // Suite-specific constants
 const CODE_NODE_NEW_NAME = 'Something else';
 
 const WorkflowPage = new WorkflowPageClass();
+const messageBox = new MessageBoxClass();
 const ndv = new NDV();
 
 describe('Undo/Redo', () => {
@@ -354,4 +356,36 @@ describe('Undo/Redo', () => {
 					.should('have.css', 'left', `637px`)
 					.should('have.css', 'top', `501px`);
 	});
+
+	it('should not undo/redo when NDV or a modal is open', () => {
+		WorkflowPage.actions.addInitialNodeToCanvas(SCHEDULE_TRIGGER_NODE_NAME, { keepNdvOpen: true });
+		// Try while NDV is open
+		WorkflowPage.actions.hitUndo();
+		WorkflowPage.getters.canvasNodes().should('have.have.length', 1);
+		ndv.getters.backToCanvas().click();
+		// Try while modal is open
+		cy.getByTestId('menu-item').contains('About n8n').click({ force: true });
+		cy.getByTestId('about-modal').should('be.visible');
+		WorkflowPage.actions.hitUndo();
+		WorkflowPage.getters.canvasNodes().should('have.have.length', 1);
+		cy.getByTestId('close-about-modal-button').click();
+		// Should work now
+		WorkflowPage.actions.hitUndo();
+		WorkflowPage.getters.canvasNodes().should('have.have.length', 0);
+	});
+
+	it('should not undo/redo when NDV or a prompt is open', () => {
+		WorkflowPage.actions.addInitialNodeToCanvas(SCHEDULE_TRIGGER_NODE_NAME, { keepNdvOpen: false });
+		WorkflowPage.getters.workflowMenu().click();
+		WorkflowPage.getters.workflowMenuItemImportFromURLItem().should('be.visible');
+		WorkflowPage.getters.workflowMenuItemImportFromURLItem().click();
+		// Try while prompt is open
+		messageBox.getters.header().click();
+		WorkflowPage.actions.hitUndo();
+		WorkflowPage.getters.canvasNodes().should('have.have.length', 1);
+		// Close prompt and try again
+		messageBox.actions.cancel();
+		WorkflowPage.actions.hitUndo();
+		WorkflowPage.getters.canvasNodes().should('have.have.length', 0);
+	});
 });

packages/editor-ui/src/components/AboutModal.vue

@@ -47,7 +47,12 @@
 
 		<template #footer>
 			<div class="action-buttons">
-				<n8n-button @click="closeDialog" float="right" :label="$locale.baseText('about.close')" />
+				<n8n-button
+					@click="closeDialog"
+					float="right"
+					:label="$locale.baseText('about.close')"
+					data-test-id="close-about-modal-button"
+				/>
 			</div>
 		</template>
 	</Modal>

packages/editor-ui/src/composables/__tests__/useHistoryHelper.test.ts

@@ -22,7 +22,13 @@ vi.mock('@/stores/history.store', () => {
 		}),
 	};
 });
-vi.mock('@/stores/ui.store');
+vi.mock('@/stores/ui.store', () => {
+	return {
+		useUIStore: () => ({
+			isAnyModalOpen: false,
+		}),
+	};
+});
 vi.mock('vue-router', () => ({
 	useRoute: () => ({}),
 }));

packages/editor-ui/src/composables/useHistoryHelper.ts

@@ -5,13 +5,14 @@ import { BulkCommand, Command } from '@/models/history';
 import { useHistoryStore } from '@/stores/history.store';
 import { useUIStore } from '@/stores/ui.store';
 
-import { ref, onMounted, onUnmounted, nextTick, getCurrentInstance } from 'vue';
+import { onMounted, onUnmounted, nextTick, getCurrentInstance } from 'vue';
 import { useDebounceHelper } from './useDebounce';
 import { useDeviceSupport } from 'n8n-design-system/composables/useDeviceSupport';
 import { getNodeViewTab } from '@/utils/canvasUtils';
 import type { Route } from 'vue-router';
 
 const UNDO_REDO_DEBOUNCE_INTERVAL = 100;
+const ELEMENT_UI_OVERLAY_SELECTOR = '.el-overlay';
 
 export function useHistoryHelper(activeRoute: Route) {
 	const instance = getCurrentInstance();
@@ -24,8 +25,6 @@ export function useHistoryHelper(activeRoute: Route) {
 	const { callDebounced } = useDebounceHelper();
 	const { isCtrlKeyPressed } = useDeviceSupport();
 
-	const isNDVOpen = ref<boolean>(ndvStore.activeNodeName !== null);
-
 	const undo = async () =>
 		callDebounced(
 			async () => {
@@ -95,29 +94,43 @@ export function useHistoryHelper(activeRoute: Route) {
 		}
 	}
 
-	function trackUndoAttempt(event: KeyboardEvent) {
-		if (isNDVOpen.value && !event.shiftKey) {
-			const activeNode = ndvStore.activeNode;
-			if (activeNode) {
-				telemetry?.track('User hit undo in NDV', { node_type: activeNode.type });
-			}
+	function trackUndoAttempt() {
+		const activeNode = ndvStore.activeNode;
+		if (activeNode) {
+			telemetry?.track('User hit undo in NDV', { node_type: activeNode.type });
 		}
 	}
 
+	/**
+	 * Checks if there is a Element UI dialog open by querying
+	 * for the visible overlay element.
+	 */
+	function isMessageDialogOpen(): boolean {
+		return (
+			document.querySelector(`${ELEMENT_UI_OVERLAY_SELECTOR}:not([style*="display: none"])`) !==
+			null
+		);
+	}
+
 	function handleKeyDown(event: KeyboardEvent) {
 		const currentNodeViewTab = getNodeViewTab(activeRoute);
+		const isNDVOpen = ndvStore.isNDVOpen;
+		const isAnyModalOpen = uiStore.isAnyModalOpen || isMessageDialogOpen();
+		const undoKeysPressed = isCtrlKeyPressed(event) && event.key.toLowerCase() === 'z';
 
 		if (event.repeat || currentNodeViewTab !== MAIN_HEADER_TABS.WORKFLOW) return;
-		if (isCtrlKeyPressed(event) && event.key.toLowerCase() === 'z') {
+		if (isNDVOpen || isAnyModalOpen) {
+			if (isNDVOpen && undoKeysPressed && !event.shiftKey) {
+				trackUndoAttempt();
+			}
+			return;
+		}
+		if (undoKeysPressed) {
 			event.preventDefault();
-			if (!isNDVOpen.value) {
-				if (event.shiftKey) {
-					void redo();
-				} else {
-					void undo();
-				}
-			} else if (!event.shiftKey) {
-				trackUndoAttempt(event);
+			if (event.shiftKey) {
+				void redo();
+			} else {
+				void undo();
 			}
 		}
 	}

packages/editor-ui/src/stores/ndv.store.ts

@@ -139,6 +139,9 @@ export const useNDVStore = defineStore(STORES.NDV, {
 
 			return null;
 		},
+		isNDVOpen(): boolean {
+			return this.activeNodeName !== null;
+		},
 	},
 	actions: {
 		setActiveNodeName(nodeName: string | null): void {

packages/editor-ui/src/stores/ui.store.ts

@@ -410,6 +410,9 @@ export const useUIStore = defineStore(STORES.UI, {
 			const style = getComputedStyle(document.body);
 			return Number(style.getPropertyValue('--header-height'));
 		},
+		isAnyModalOpen(): boolean {
+			return this.modalStack.length > 0;
+		},
 	},
 	actions: {
 		setTheme(theme: ThemeOption): void {