From 9c41c5c6d411c40ca4d1bf6b3f08bca80a6bf0a0 Mon Sep 17 00:00:00 2001 From: Alex Grozav Date: Mon, 7 Oct 2024 18:03:03 +0300 Subject: [PATCH] fix(editor): Disable all modifying keybindings when the canvas is in read-only mode (no-changelog) (#11137) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ --- .../src/components/canvas/Canvas.vue | 52 ++++++----- .../__tests__/useKeybindings.test.ts | 89 ++++++++++++++++++- .../src/composables/useKeybindings.spec.ts | 85 ------------------ .../src/composables/useKeybindings.ts | 8 +- 4 files changed, 118 insertions(+), 116 deletions(-) delete mode 100644 packages/editor-ui/src/composables/useKeybindings.spec.ts diff --git a/packages/editor-ui/src/components/canvas/Canvas.vue b/packages/editor-ui/src/components/canvas/Canvas.vue index 7d066384fe..c68e9fac69 100644 --- a/packages/editor-ui/src/components/canvas/Canvas.vue +++ b/packages/editor-ui/src/components/canvas/Canvas.vue @@ -138,30 +138,34 @@ onKeyUp(panningKeyCode, () => { isPanningEnabled.value = false; }); -useKeybindings( - { - ctrl_c: emitWithSelectedNodes((ids) => emit('copy:nodes', ids)), - ctrl_x: emitWithSelectedNodes((ids) => emit('cut:nodes', ids)), - 'delete|backspace': emitWithSelectedNodes((ids) => emit('delete:nodes', ids)), - ctrl_d: emitWithSelectedNodes((ids) => emit('duplicate:nodes', ids)), - d: emitWithSelectedNodes((ids) => emit('update:nodes:enabled', ids)), - p: emitWithSelectedNodes((ids) => emit('update:nodes:pin', ids, 'keyboard-shortcut')), - enter: emitWithLastSelectedNode((id) => onSetNodeActive(id)), - f2: emitWithLastSelectedNode((id) => emit('update:node:name', id)), - tab: () => emit('create:node', 'tab'), - shift_s: () => emit('create:sticky'), - ctrl_alt_n: () => emit('create:workflow'), - ctrl_enter: () => emit('run:workflow'), - ctrl_s: () => emit('save:workflow'), - ctrl_a: () => addSelectedNodes(graphNodes.value), - '+|=': async () => await onZoomIn(), - '-|_': async () => await onZoomOut(), - 0: async () => await onResetZoom(), - 1: async () => await onFitView(), - // @TODO implement arrow key shortcuts to modify selection - }, - { disabled: disableKeyBindings }, -); +const keyMap = computed(() => ({ + ctrl_c: emitWithSelectedNodes((ids) => emit('copy:nodes', ids)), + enter: emitWithLastSelectedNode((id) => onSetNodeActive(id)), + ctrl_a: () => addSelectedNodes(graphNodes.value), + '+|=': async () => await onZoomIn(), + '-|_': async () => await onZoomOut(), + 0: async () => await onResetZoom(), + 1: async () => await onFitView(), + // @TODO implement arrow key shortcuts to modify selection + + ...(props.readOnly + ? {} + : { + ctrl_x: emitWithSelectedNodes((ids) => emit('cut:nodes', ids)), + 'delete|backspace': emitWithSelectedNodes((ids) => emit('delete:nodes', ids)), + ctrl_d: emitWithSelectedNodes((ids) => emit('duplicate:nodes', ids)), + d: emitWithSelectedNodes((ids) => emit('update:nodes:enabled', ids)), + p: emitWithSelectedNodes((ids) => emit('update:nodes:pin', ids, 'keyboard-shortcut')), + f2: emitWithLastSelectedNode((id) => emit('update:node:name', id)), + tab: () => emit('create:node', 'tab'), + shift_s: () => emit('create:sticky'), + ctrl_alt_n: () => emit('create:workflow'), + ctrl_enter: () => emit('run:workflow'), + ctrl_s: () => emit('save:workflow'), + }), +})); + +useKeybindings(keyMap, { disabled: disableKeyBindings }); /** * Nodes diff --git a/packages/editor-ui/src/composables/__tests__/useKeybindings.test.ts b/packages/editor-ui/src/composables/__tests__/useKeybindings.test.ts index 5383b9deeb..54c042c79a 100644 --- a/packages/editor-ui/src/composables/__tests__/useKeybindings.test.ts +++ b/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) => { @@ -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); + }); }); diff --git a/packages/editor-ui/src/composables/useKeybindings.spec.ts b/packages/editor-ui/src/composables/useKeybindings.spec.ts deleted file mode 100644 index b002c95dc0..0000000000 --- a/packages/editor-ui/src/composables/useKeybindings.spec.ts +++ /dev/null @@ -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); - }); -}); diff --git a/packages/editor-ui/src/composables/useKeybindings.ts b/packages/editor-ui/src/composables/useKeybindings.ts index ffee421d84..4aa460c562 100644 --- a/packages/editor-ui/src/composables/useKeybindings.ts +++ b/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 { computed, toValue, type MaybeRefOrGetter, unref } from 'vue'; +import type { MaybeRef, Ref } from 'vue'; +import { computed, unref } from 'vue'; type KeyMap = Record void>; export const useKeybindings = ( - keymap: MaybeRefOrGetter, + keymap: Ref, options?: { disabled: MaybeRef; }, @@ -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]);