From 2b5b21b310b24f406400ccc2fcc349c7cf6905eb Mon Sep 17 00:00:00 2001 From: Alex Grozav Date: Thu, 29 Aug 2024 13:26:07 +0300 Subject: [PATCH 1/3] fix: prevent keyboard shortcuts when ndv is open --- .../src/components/canvas/Canvas.vue | 55 +++++++++++-------- .../src/composables/useKeybindings.ts | 14 ++++- packages/editor-ui/src/views/NodeView.v2.vue | 5 ++ 3 files changed, 49 insertions(+), 25 deletions(-) diff --git a/packages/editor-ui/src/components/canvas/Canvas.vue b/packages/editor-ui/src/components/canvas/Canvas.vue index e7277c6761162..a981595ee42b5 100644 --- a/packages/editor-ui/src/components/canvas/Canvas.vue +++ b/packages/editor-ui/src/components/canvas/Canvas.vue @@ -19,7 +19,7 @@ import { Background } from '@vue-flow/background'; import { MiniMap } from '@vue-flow/minimap'; import Node from './elements/nodes/CanvasNode.vue'; import Edge from './elements/edges/CanvasEdge.vue'; -import { computed, onMounted, onUnmounted, provide, ref, useCssModule, watch } from 'vue'; +import { computed, onMounted, onUnmounted, provide, ref, toRef, useCssModule, watch } from 'vue'; import type { EventBus } from 'n8n-design-system'; import { createEventBus } from 'n8n-design-system'; import { useContextMenu, type ContextMenuAction } from '@/composables/useContextMenu'; @@ -73,6 +73,7 @@ const props = withDefaults( controlsPosition?: PanelPosition; eventBus?: EventBus; readOnly?: boolean; + keyBindings?: boolean; }>(), { id: 'canvas', @@ -81,6 +82,7 @@ const props = withDefaults( controlsPosition: PanelPosition.BottomLeft, eventBus: () => createEventBus(), readOnly: false, + keyBindings: true, }, ); @@ -101,27 +103,36 @@ const { findNode, } = useVueFlow({ id: props.id, deleteKeyCode: null }); -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 -}); +/** + * Key bindings + */ + +const disableKeyBindings = computed(() => !props.keyBindings); + +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 contextMenu = useContextMenu(); diff --git a/packages/editor-ui/src/composables/useKeybindings.ts b/packages/editor-ui/src/composables/useKeybindings.ts index 62d3e0fc7f3da..ffee421d84771 100644 --- a/packages/editor-ui/src/composables/useKeybindings.ts +++ b/packages/editor-ui/src/composables/useKeybindings.ts @@ -1,13 +1,21 @@ import { useActiveElement, useEventListener } from '@vueuse/core'; import { useDeviceSupport } from 'n8n-design-system'; -import { computed, toValue, type MaybeRefOrGetter } from 'vue'; +import type { MaybeRef } from 'vue'; +import { computed, toValue, type MaybeRefOrGetter, unref } from 'vue'; type KeyMap = Record void>; -export const useKeybindings = (keymap: MaybeRefOrGetter) => { +export const useKeybindings = ( + keymap: MaybeRefOrGetter, + options?: { + disabled: MaybeRef; + }, +) => { const activeElement = useActiveElement(); const { isCtrlKeyPressed } = useDeviceSupport(); + const isDisabled = computed(() => unref(options?.disabled)); + const ignoreKeyPresses = computed(() => { if (!activeElement.value) return false; @@ -60,7 +68,7 @@ export const useKeybindings = (keymap: MaybeRefOrGetter) => { } function onKeyDown(event: KeyboardEvent) { - if (ignoreKeyPresses.value) return; + if (ignoreKeyPresses.value || isDisabled.value) return; const shortcutString = toShortcutString(event); diff --git a/packages/editor-ui/src/views/NodeView.v2.vue b/packages/editor-ui/src/views/NodeView.v2.vue index 7846b09dc420a..327ee259e596a 100644 --- a/packages/editor-ui/src/views/NodeView.v2.vue +++ b/packages/editor-ui/src/views/NodeView.v2.vue @@ -221,6 +221,10 @@ const fallbackNodes = computed(() => ], ); +const keyBindingsEnabled = computed(() => { + return !ndvStore.activeNode; +}); + /** * Initialization */ @@ -1483,6 +1487,7 @@ onDeactivated(() => { :fallback-nodes="fallbackNodes" :event-bus="canvasEventBus" :read-only="isCanvasReadOnly" + :key-bindings="keyBindingsEnabled" @update:nodes:position="onUpdateNodesPosition" @update:node:position="onUpdateNodePosition" @update:node:active="onSetNodeActive" From 8166cb048bd050ecab4ec9c4edcbe56f54e2f732 Mon Sep 17 00:00:00 2001 From: Alex Grozav Date: Thu, 29 Aug 2024 13:36:38 +0300 Subject: [PATCH 2/3] test: add tests for useKeybindings composable --- .../src/composables/useKeybindings.spec.ts | 85 +++++++++++++++++++ 1 file changed, 85 insertions(+) create mode 100644 packages/editor-ui/src/composables/useKeybindings.spec.ts diff --git a/packages/editor-ui/src/composables/useKeybindings.spec.ts b/packages/editor-ui/src/composables/useKeybindings.spec.ts new file mode 100644 index 0000000000000..b002c95dc05af --- /dev/null +++ b/packages/editor-ui/src/composables/useKeybindings.spec.ts @@ -0,0 +1,85 @@ +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); + }); +}); From a0cbcb02906445b70a1f5f5129dbb1afd7426ea5 Mon Sep 17 00:00:00 2001 From: Alex Grozav Date: Thu, 29 Aug 2024 13:37:12 +0300 Subject: [PATCH 3/3] chore: remove import --- packages/editor-ui/src/components/canvas/Canvas.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/editor-ui/src/components/canvas/Canvas.vue b/packages/editor-ui/src/components/canvas/Canvas.vue index a981595ee42b5..b456ec0df15d4 100644 --- a/packages/editor-ui/src/components/canvas/Canvas.vue +++ b/packages/editor-ui/src/components/canvas/Canvas.vue @@ -19,7 +19,7 @@ import { Background } from '@vue-flow/background'; import { MiniMap } from '@vue-flow/minimap'; import Node from './elements/nodes/CanvasNode.vue'; import Edge from './elements/edges/CanvasEdge.vue'; -import { computed, onMounted, onUnmounted, provide, ref, toRef, useCssModule, watch } from 'vue'; +import { computed, onMounted, onUnmounted, provide, ref, useCssModule, watch } from 'vue'; import type { EventBus } from 'n8n-design-system'; import { createEventBus } from 'n8n-design-system'; import { useContextMenu, type ContextMenuAction } from '@/composables/useContextMenu';