fix(editor): Disable all modifying keybindings when the canvas is in read-only mode (no-changelog) (#11137)

Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <aditya@netroy.in>

packages/editor-ui/src/components/canvas/Canvas.vue

@@ -138,30 +138,34 @@ onKeyUp(panningKeyCode, () => {
 	isPanningEnabled.value = false;
 });
 
-useKeybindings(
+const keyMap = computed(() => ({
-	{
+	ctrl_c: emitWithSelectedNodes((ids) => emit('copy:nodes', ids)),
-		ctrl_c: emitWithSelectedNodes((ids) => emit('copy:nodes', ids)),
+	enter: emitWithLastSelectedNode((id) => onSetNodeActive(id)),
-		ctrl_x: emitWithSelectedNodes((ids) => emit('cut:nodes', ids)),
+	ctrl_a: () => addSelectedNodes(graphNodes.value),
-		'delete|backspace': emitWithSelectedNodes((ids) => emit('delete:nodes', ids)),
+	'+|=': async () => await onZoomIn(),
-		ctrl_d: emitWithSelectedNodes((ids) => emit('duplicate:nodes', ids)),
+	'-|_': async () => await onZoomOut(),
-		d: emitWithSelectedNodes((ids) => emit('update:nodes:enabled', ids)),
+	0: async () => await onResetZoom(),
-		p: emitWithSelectedNodes((ids) => emit('update:nodes:pin', ids, 'keyboard-shortcut')),
+	1: async () => await onFitView(),
-		enter: emitWithLastSelectedNode((id) => onSetNodeActive(id)),
+	// @TODO implement arrow key shortcuts to modify selection
-		f2: emitWithLastSelectedNode((id) => emit('update:node:name', id)),
+
-		tab: () => emit('create:node', 'tab'),
+	...(props.readOnly
-		shift_s: () => emit('create:sticky'),
+		? {}
-		ctrl_alt_n: () => emit('create:workflow'),
+		: {
-		ctrl_enter: () => emit('run:workflow'),
+				ctrl_x: emitWithSelectedNodes((ids) => emit('cut:nodes', ids)),
-		ctrl_s: () => emit('save:workflow'),
+				'delete|backspace': emitWithSelectedNodes((ids) => emit('delete:nodes', ids)),
-		ctrl_a: () => addSelectedNodes(graphNodes.value),
+				ctrl_d: emitWithSelectedNodes((ids) => emit('duplicate:nodes', ids)),
-		'+|=': async () => await onZoomIn(),
+				d: emitWithSelectedNodes((ids) => emit('update:nodes:enabled', ids)),
-		'-|_': async () => await onZoomOut(),
+				p: emitWithSelectedNodes((ids) => emit('update:nodes:pin', ids, 'keyboard-shortcut')),
-		0: async () => await onResetZoom(),
+				f2: emitWithLastSelectedNode((id) => emit('update:node:name', id)),
-		1: async () => await onFitView(),
+				tab: () => emit('create:node', 'tab'),
-		// @TODO implement arrow key shortcuts to modify selection
+				shift_s: () => emit('create:sticky'),
-	},
+				ctrl_alt_n: () => emit('create:workflow'),
-	{ disabled: disableKeyBindings },
+				ctrl_enter: () => emit('run:workflow'),
-);
+				ctrl_s: () => emit('save:workflow'),
+			}),
+}));
+
+useKeybindings(keyMap, { disabled: disableKeyBindings });
 
 /**
  * Nodes

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

@@ -1,7 +1,7 @@
 import { renderComponent } from '@/__tests__/render';
 import userEvent from '@testing-library/user-event';
 import { describe, expect, it, vi } from 'vitest';
-import { defineComponent, h } from 'vue';
+import { defineComponent, h, ref } from 'vue';
 import { useKeybindings } from '../useKeybindings';
 
 const renderTestComponent = async (...args: Parameters<typeof useKeybindings>) => {
@@ -19,7 +19,8 @@ describe('useKeybindings', () => {
 	it('should trigger case-insensitive keyboard shortcuts', async () => {
 		const saveSpy = vi.fn();
 		const saveAllSpy = vi.fn();
-		await renderTestComponent({ Ctrl_s: saveSpy, ctrl_Shift_S: saveAllSpy });
+		const keymap = ref({ Ctrl_s: saveSpy, ctrl_Shift_S: saveAllSpy });
+		await renderTestComponent(keymap);
 		await userEvent.keyboard('{Control>}s');
 		expect(saveSpy).toHaveBeenCalled();
 		expect(saveAllSpy).not.toHaveBeenCalled();
@@ -31,7 +32,8 @@ describe('useKeybindings', () => {
 	it('should not trigger shortcuts when an input element has focus', async () => {
 		const saveSpy = vi.fn();
 		const saveAllSpy = vi.fn();
-		const { getByRole } = await renderTestComponent({ Ctrl_s: saveSpy, ctrl_Shift_S: saveAllSpy });
+		const keymap = ref({ Ctrl_s: saveSpy, ctrl_Shift_S: saveAllSpy });
+		const { getByRole } = await renderTestComponent(keymap);
 
 		getByRole('textbox').focus();
 
@@ -41,4 +43,85 @@ describe('useKeybindings', () => {
 		expect(saveSpy).not.toHaveBeenCalled();
 		expect(saveAllSpy).not.toHaveBeenCalled();
 	});
+
+	it('should call the correct handler for a single key press', async () => {
+		const handler = vi.fn();
+		const keymap = ref({ a: handler });
+
+		useKeybindings(keymap);
+
+		const event = new KeyboardEvent('keydown', { key: 'a' });
+		document.dispatchEvent(event);
+
+		expect(handler).toHaveBeenCalled();
+	});
+
+	it('should call the correct handler for a combination key press', async () => {
+		const handler = vi.fn();
+		const keymap = ref({ 'ctrl+a': handler });
+
+		useKeybindings(keymap);
+
+		const event = new KeyboardEvent('keydown', { key: 'a', ctrlKey: true });
+		document.dispatchEvent(event);
+
+		expect(handler).toHaveBeenCalled();
+	});
+
+	it('should not call handler if key press is ignored', async () => {
+		const handler = vi.fn();
+		const keymap = ref({ a: handler });
+
+		useKeybindings(keymap);
+
+		const input = document.createElement('input');
+		document.body.appendChild(input);
+		input.focus();
+
+		const event = new KeyboardEvent('keydown', { key: 'a' });
+		document.dispatchEvent(event);
+
+		expect(handler).not.toHaveBeenCalled();
+		document.body.removeChild(input);
+	});
+
+	it('should not call handler if disabled', async () => {
+		const handler = vi.fn();
+		const keymap = ref({ a: handler });
+		const disabled = ref(true);
+
+		useKeybindings(keymap, { disabled });
+
+		const event = new KeyboardEvent('keydown', { key: 'a' });
+		document.dispatchEvent(event);
+
+		expect(handler).not.toHaveBeenCalled();
+	});
+
+	it('should normalize shortcut strings correctly', async () => {
+		const handler = vi.fn();
+		const keymap = ref({ 'ctrl+shift+a': handler });
+
+		useKeybindings(keymap);
+
+		const event = new KeyboardEvent('keydown', { key: 'A', ctrlKey: true, shiftKey: true });
+		document.dispatchEvent(event);
+
+		expect(handler).toHaveBeenCalled();
+	});
+
+	it('should normalize shortcut string alternatives correctly', async () => {
+		const handler = vi.fn();
+		const keymap = ref({ 'a|b': handler });
+
+		useKeybindings(keymap);
+
+		const eventA = new KeyboardEvent('keydown', { key: 'A' });
+		document.dispatchEvent(eventA);
+		expect(handler).toHaveBeenCalled();
+
+		const eventB = new KeyboardEvent('keydown', { key: 'B' });
+		document.dispatchEvent(eventB);
+		expect(handler).toHaveBeenCalledTimes(2);
+	});
 });

packages/editor-ui/src/composables/useKeybindings.spec.ts

@@ -1,85 +0,0 @@
-import { useKeybindings } from '@/composables/useKeybindings';
-import { ref } from 'vue';
-
-describe('useKeybindings', () => {
-	it('should call the correct handler for a single key press', async () => {
-		const handler = vi.fn();
-		const keymap = ref({ a: handler });
-
-		useKeybindings(keymap);
-
-		const event = new KeyboardEvent('keydown', { key: 'a' });
-		document.dispatchEvent(event);
-
-		expect(handler).toHaveBeenCalled();
-	});
-
-	it('should call the correct handler for a combination key press', async () => {
-		const handler = vi.fn();
-		const keymap = ref({ 'ctrl+a': handler });
-
-		useKeybindings(keymap);
-
-		const event = new KeyboardEvent('keydown', { key: 'a', ctrlKey: true });
-		document.dispatchEvent(event);
-
-		expect(handler).toHaveBeenCalled();
-	});
-
-	it('should not call handler if key press is ignored', async () => {
-		const handler = vi.fn();
-		const keymap = ref({ a: handler });
-
-		useKeybindings(keymap);
-
-		const input = document.createElement('input');
-		document.body.appendChild(input);
-		input.focus();
-
-		const event = new KeyboardEvent('keydown', { key: 'a' });
-		document.dispatchEvent(event);
-
-		expect(handler).not.toHaveBeenCalled();
-		document.body.removeChild(input);
-	});
-
-	it('should not call handler if disabled', async () => {
-		const handler = vi.fn();
-		const keymap = ref({ a: handler });
-		const disabled = ref(true);
-
-		useKeybindings(keymap, { disabled });
-
-		const event = new KeyboardEvent('keydown', { key: 'a' });
-		document.dispatchEvent(event);
-
-		expect(handler).not.toHaveBeenCalled();
-	});
-
-	it('should normalize shortcut strings correctly', async () => {
-		const handler = vi.fn();
-		const keymap = ref({ 'ctrl+shift+a': handler });
-
-		useKeybindings(keymap);
-
-		const event = new KeyboardEvent('keydown', { key: 'A', ctrlKey: true, shiftKey: true });
-		document.dispatchEvent(event);
-
-		expect(handler).toHaveBeenCalled();
-	});
-
-	it('should normalize shortcut string alternatives correctly', async () => {
-		const handler = vi.fn();
-		const keymap = ref({ 'a|b': handler });
-
-		useKeybindings(keymap);
-
-		const eventA = new KeyboardEvent('keydown', { key: 'A' });
-		document.dispatchEvent(eventA);
-		expect(handler).toHaveBeenCalled();
-
-		const eventB = new KeyboardEvent('keydown', { key: 'B' });
-		document.dispatchEvent(eventB);
-		expect(handler).toHaveBeenCalledTimes(2);
-	});
-});

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

@@ -1,12 +1,12 @@
 import { useActiveElement, useEventListener } from '@vueuse/core';
 import { useDeviceSupport } from 'n8n-design-system';
-import type { MaybeRef } from 'vue';
+import type { MaybeRef, Ref } from 'vue';
-import { computed, toValue, type MaybeRefOrGetter, unref } from 'vue';
+import { computed, unref } from 'vue';
 
 type KeyMap = Record<string, (event: KeyboardEvent) => void>;
 
 export const useKeybindings = (
-	keymap: MaybeRefOrGetter<KeyMap>,
+	keymap: Ref<KeyMap>,
 	options?: {
 		disabled: MaybeRef<boolean>;
 	},
@@ -29,7 +29,7 @@ export const useKeybindings = (
 
 	const normalizedKeymap = computed(() =>
 		Object.fromEntries(
-			Object.entries(toValue(keymap))
+			Object.entries(keymap.value)
 				.map(([shortcut, handler]) => {
 					const shortcuts = shortcut.split('|');
 					return shortcuts.map((s) => [normalizeShortcutString(s), handler]);