From 9693142985e0ad8f451eac3b5a7b4f12a0e95142 Mon Sep 17 00:00:00 2001 From: OlegIvaniv Date: Tue, 18 Apr 2023 11:47:08 +0200 Subject: [PATCH] refactor(editor): Refactor history and debounce mixins to composables (no-changelog) (#5930) * refactor(editor): Refactor history and debounce mixins to composables and add unit tests (no-changelog) * Lint fix and use userEvent to fire keydown events * Fix debounce spec --- packages/editor-ui/src/App.vue | 10 +- .../composables/__tests__/useDebounce.test.ts | 72 +++++++++ .../__tests__/useHistoryHelper.test.ts | 94 ++++++++++++ .../editor-ui/src/composables/useDebounce.ts | 38 +++++ .../src/composables/useHistoryHelper.ts | 137 ++++++++++++++++++ packages/editor-ui/src/mixins/history.ts | 121 ---------------- 6 files changed, 346 insertions(+), 126 deletions(-) create mode 100644 packages/editor-ui/src/composables/__tests__/useDebounce.test.ts create mode 100644 packages/editor-ui/src/composables/__tests__/useHistoryHelper.test.ts create mode 100644 packages/editor-ui/src/composables/useDebounce.ts create mode 100644 packages/editor-ui/src/composables/useHistoryHelper.ts delete mode 100644 packages/editor-ui/src/mixins/history.ts diff --git a/packages/editor-ui/src/App.vue b/packages/editor-ui/src/App.vue index 035416a1d..b5baefe06 100644 --- a/packages/editor-ui/src/App.vue +++ b/packages/editor-ui/src/App.vue @@ -45,10 +45,11 @@ import { useUsersStore } from './stores/users'; import { useRootStore } from './stores/n8nRootStore'; import { useTemplatesStore } from './stores/templates'; import { useNodeTypesStore } from './stores/nodeTypes'; -import { historyHelper } from '@/mixins/history'; +import { useHistoryHelper } from '@/composables/useHistoryHelper'; import { newVersions } from '@/mixins/newVersions'; +import { useRoute } from 'vue-router/composables'; -export default mixins(newVersions, showMessage, userHelpers, restApi, historyHelper).extend({ +export default mixins(newVersions, showMessage, userHelpers, restApi).extend({ name: 'App', components: { LoadingView, @@ -56,10 +57,9 @@ export default mixins(newVersions, showMessage, userHelpers, restApi, historyHel Modals, }, setup() { - const { registerCustomAction, unregisterCustomAction } = useGlobalLinkActions(); return { - registerCustomAction, - unregisterCustomAction, + ...useGlobalLinkActions(), + ...useHistoryHelper(useRoute()), }; }, computed: { diff --git a/packages/editor-ui/src/composables/__tests__/useDebounce.test.ts b/packages/editor-ui/src/composables/__tests__/useDebounce.test.ts new file mode 100644 index 000000000..fecac5f4a --- /dev/null +++ b/packages/editor-ui/src/composables/__tests__/useDebounce.test.ts @@ -0,0 +1,72 @@ +import { vi, describe, it, expect } from 'vitest'; +import { useDebounceHelper } from '../useDebounce'; +import { render, screen } from '@testing-library/vue'; + +describe('useDebounceHelper', () => { + const debounceTime = 200; + + const TestComponent = { + template: ` +
+ + + +
+ `, + props: { + mockFn: { + type: Function, + }, + }, + setup() { + const { callDebounced } = useDebounceHelper(); + return { + callDebounced, + debounceTime, + }; + }, + }; + + it('debounces a function call', async () => { + const mockFn = vi.fn(); + render(TestComponent, { props: { mockFn } }); + const button = screen.getByText('Click me'); + + button.click(); + button.click(); + + expect(mockFn).toHaveBeenCalledTimes(1); + }); + + it('supports trailing option', async () => { + const mockFn = vi.fn(); + render(TestComponent, { props: { mockFn } }); + const button = screen.getByText('Click me trailing'); + + button.click(); + button.click(); + + expect(mockFn).toHaveBeenCalledTimes(0); + + await new Promise((resolve) => setTimeout(resolve, debounceTime)); + + expect(mockFn).toHaveBeenCalledTimes(1); + }); + + it('works with async functions', async () => { + const mockAsyncFn = vi.fn(async () => { + await new Promise((resolve) => setTimeout(resolve, 100)); + }); + render(TestComponent, { props: { mockFn: mockAsyncFn } }); + const button = screen.getByText('Click me'); + + button.click(); + button.click(); + + expect(mockAsyncFn).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/editor-ui/src/composables/__tests__/useHistoryHelper.test.ts b/packages/editor-ui/src/composables/__tests__/useHistoryHelper.test.ts new file mode 100644 index 000000000..e31753736 --- /dev/null +++ b/packages/editor-ui/src/composables/__tests__/useHistoryHelper.test.ts @@ -0,0 +1,94 @@ +import { vi, describe, it, expect } from 'vitest'; +import { MAIN_HEADER_TABS } from '@/constants'; +import { render } from '@testing-library/vue'; +import userEvent from '@testing-library/user-event'; +import { useHistoryHelper } from '../useHistoryHelper'; +import { defineComponent } from 'vue'; +import { Route } from 'vue-router'; + +const undoMock = vi.fn(); +const redoMock = vi.fn(); +vi.mock('@/stores/ndv', () => ({ + useNDVStore: () => ({ + activeNodeName: null, + activeNode: {}, + }), +})); +vi.mock('@/stores/history', () => { + return { + useHistoryStore: () => ({ + popUndoableToUndo: undoMock, + popUndoableToRedo: redoMock, + }), + }; +}); +vi.mock('@/stores/ui'); +vi.mock('vue-router/composables', () => ({ + useRoute: () => ({}), +})); + +const TestComponent = defineComponent({ + props: { + route: { + type: Object, + }, + }, + setup(props) { + useHistoryHelper(props.route as Route); + + return {}; + }, + template: '
', +}); + +describe('useHistoryHelper', () => { + beforeEach(() => { + undoMock.mockClear(); + redoMock.mockClear(); + }); + it('should call undo when Ctrl+Z is pressed', async () => { + // @ts-ignore + render(TestComponent, { + props: { + route: { + name: MAIN_HEADER_TABS.WORKFLOW, + meta: { + nodeView: true, + }, + }, + }, + }); + + await userEvent.keyboard('{Control>}z'); + await userEvent.keyboard('{Control>}z'); + + expect(undoMock).toHaveBeenCalledTimes(2); + }); + it('should call redo when Ctrl+Shift+Z is pressed', async () => { + // @ts-ignore + render(TestComponent, { + props: { + route: { + name: MAIN_HEADER_TABS.WORKFLOW, + meta: { + nodeView: true, + }, + }, + }, + }); + + await userEvent.keyboard('{Control>}{Shift>}z'); + await userEvent.keyboard('{Control>}{Shift>}z'); + + expect(redoMock).toHaveBeenCalledTimes(2); + }); + it('should not call undo when Ctrl+Z if not on NodeView', async () => { + // @ts-ignore + render(TestComponent, { props: { route: {} } }); + + await userEvent.keyboard('{Control>}z'); + await userEvent.keyboard('{Control>}z'); + + expect(undoMock).toHaveBeenCalledTimes(0); + }); +}); diff --git a/packages/editor-ui/src/composables/useDebounce.ts b/packages/editor-ui/src/composables/useDebounce.ts new file mode 100644 index 000000000..a5b4c4031 --- /dev/null +++ b/packages/editor-ui/src/composables/useDebounce.ts @@ -0,0 +1,38 @@ +import { ref } from 'vue'; +import { debounce } from 'lodash-es'; + +type DebouncedFunction = (...args: unknown[]) => Promise | void; + +export function useDebounceHelper() { + // Create a ref for the WeakMap to store debounced functions. + const debouncedFunctions = ref(new WeakMap()); + + const callDebounced = async ( + func: DebouncedFunction, + options: { debounceTime: number; trailing?: boolean }, + ...inputParameters: unknown[] + ): Promise => { + const { trailing, debounceTime } = options; + + // Check if a debounced version of the function is already stored in the WeakMap. + let debouncedFunc = debouncedFunctions.value.get(func); + + // If a debounced version is not found, create one and store it in the WeakMap. + if (debouncedFunc === undefined) { + debouncedFunc = debounce( + async (...args: unknown[]) => { + await func(...args); + }, + debounceTime, + trailing ? { trailing } : { leading: true }, + ); + debouncedFunctions.value.set(func, debouncedFunc); + } + + await debouncedFunc(...inputParameters); + }; + + return { + callDebounced, + }; +} diff --git a/packages/editor-ui/src/composables/useHistoryHelper.ts b/packages/editor-ui/src/composables/useHistoryHelper.ts new file mode 100644 index 000000000..79fc5f6d7 --- /dev/null +++ b/packages/editor-ui/src/composables/useHistoryHelper.ts @@ -0,0 +1,137 @@ +import { MAIN_HEADER_TABS } from '@/constants'; +import { useNDVStore } from '@/stores/ndv'; +import { BulkCommand, Undoable } from '@/models/history'; +import { useHistoryStore } from '@/stores/history'; +import { useUIStore } from '@/stores/ui'; + +import { ref, onMounted, onUnmounted, Ref, nextTick, getCurrentInstance } from 'vue'; +import { Command } from '@/models/history'; +import { useDebounceHelper } from './useDebounce'; +import useDeviceSupportHelpers from './useDeviceSupport'; +import { getNodeViewTab } from '@/utils'; +import { Route } from 'vue-router'; + +const UNDO_REDO_DEBOUNCE_INTERVAL = 100; + +export function useHistoryHelper(activeRoute: Route) { + const instance = getCurrentInstance(); + const telemetry = instance?.proxy.$telemetry; + + const ndvStore = useNDVStore(); + const historyStore = useHistoryStore(); + const uiStore = useUIStore(); + + const { callDebounced } = useDebounceHelper(); + const { isCtrlKeyPressed } = useDeviceSupportHelpers(); + + const isNDVOpen = ref(ndvStore.activeNodeName !== null); + + const undo = () => + callDebounced( + async () => { + const command = historyStore.popUndoableToUndo(); + if (!command) { + return; + } + if (command instanceof BulkCommand) { + historyStore.bulkInProgress = true; + const commands = command.commands; + const reverseCommands: Command[] = []; + for (let i = commands.length - 1; i >= 0; i--) { + await commands[i].revert(); + reverseCommands.push(commands[i].getReverseCommand()); + } + historyStore.pushUndoableToRedo(new BulkCommand(reverseCommands)); + await nextTick(); + historyStore.bulkInProgress = false; + } + if (command instanceof Command) { + await command.revert(); + historyStore.pushUndoableToRedo(command.getReverseCommand()); + uiStore.stateIsDirty = true; + } + trackCommand(command, 'undo'); + }, + { debounceTime: UNDO_REDO_DEBOUNCE_INTERVAL }, + ); + + const redo = () => + callDebounced( + async () => { + const command = historyStore.popUndoableToRedo(); + if (!command) { + return; + } + if (command instanceof BulkCommand) { + historyStore.bulkInProgress = true; + const commands = command.commands; + const reverseCommands = []; + for (let i = commands.length - 1; i >= 0; i--) { + await commands[i].revert(); + reverseCommands.push(commands[i].getReverseCommand()); + } + historyStore.pushBulkCommandToUndo(new BulkCommand(reverseCommands), false); + await nextTick(); + historyStore.bulkInProgress = false; + } + if (command instanceof Command) { + await command.revert(); + historyStore.pushCommandToUndo(command.getReverseCommand(), false); + uiStore.stateIsDirty = true; + } + trackCommand(command, 'redo'); + }, + { debounceTime: UNDO_REDO_DEBOUNCE_INTERVAL }, + ); + + function trackCommand(command: Undoable, type: 'undo' | 'redo'): void { + if (command instanceof Command) { + telemetry?.track(`User hit ${type}`, { commands_length: 1, commands: [command.name] }); + } else if (command instanceof BulkCommand) { + telemetry?.track(`User hit ${type}`, { + commands_length: command.commands.length, + commands: command.commands.map((c) => c.name), + }); + } + } + + 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 handleKeyDown(event: KeyboardEvent) { + const currentNodeViewTab = getNodeViewTab(activeRoute); + + if (event.repeat || currentNodeViewTab !== MAIN_HEADER_TABS.WORKFLOW) return; + if (isCtrlKeyPressed(event) && event.key.toLowerCase() === 'z') { + event.preventDefault(); + if (!isNDVOpen.value) { + if (event.shiftKey) { + redo(); + } else { + undo(); + } + } else if (!event.shiftKey) { + trackUndoAttempt(event); + } + } + } + + onMounted(() => { + document.addEventListener('keydown', handleKeyDown); + }); + + onUnmounted(() => { + document.removeEventListener('keydown', handleKeyDown); + }); + + return { + undo, + redo, + }; +} diff --git a/packages/editor-ui/src/mixins/history.ts b/packages/editor-ui/src/mixins/history.ts deleted file mode 100644 index 0726c6a03..000000000 --- a/packages/editor-ui/src/mixins/history.ts +++ /dev/null @@ -1,121 +0,0 @@ -import { MAIN_HEADER_TABS } from './../constants'; -import { useNDVStore } from '@/stores/ndv'; -import { BulkCommand, Undoable } from '@/models/history'; -import { useHistoryStore } from '@/stores/history'; -import { useUIStore } from '@/stores/ui'; -import { useWorkflowsStore } from '@/stores/workflows'; -import { mapStores } from 'pinia'; -import mixins from 'vue-typed-mixins'; -import { Command } from '@/models/history'; -import { debounceHelper } from '@/mixins/debounce'; -import { deviceSupportHelpers } from '@/mixins/deviceSupportHelpers'; -import Vue from 'vue'; -import { getNodeViewTab } from '@/utils'; - -const UNDO_REDO_DEBOUNCE_INTERVAL = 100; - -export const historyHelper = mixins(debounceHelper, deviceSupportHelpers).extend({ - computed: { - ...mapStores(useNDVStore, useHistoryStore, useUIStore, useWorkflowsStore), - isNDVOpen(): boolean { - return this.ndvStore.activeNodeName !== null; - }, - }, - mounted() { - document.addEventListener('keydown', this.handleKeyDown); - }, - destroyed() { - document.removeEventListener('keydown', this.handleKeyDown); - }, - methods: { - handleKeyDown(event: KeyboardEvent) { - const currentNodeViewTab = getNodeViewTab(this.$route); - - if (event.repeat || currentNodeViewTab !== MAIN_HEADER_TABS.WORKFLOW) return; - if (this.isCtrlKeyPressed(event) && event.key.toLowerCase() === 'z') { - event.preventDefault(); - if (!this.isNDVOpen) { - if (event.shiftKey) { - this.callDebounced('redo', { - debounceTime: UNDO_REDO_DEBOUNCE_INTERVAL, - trailing: true, - }); - } else { - this.callDebounced('undo', { - debounceTime: UNDO_REDO_DEBOUNCE_INTERVAL, - trailing: true, - }); - } - } else if (!event.shiftKey) { - this.trackUndoAttempt(event); - } - } - }, - async undo() { - const command = this.historyStore.popUndoableToUndo(); - if (!command) { - return; - } - if (command instanceof BulkCommand) { - this.historyStore.bulkInProgress = true; - const commands = command.commands; - const reverseCommands: Command[] = []; - for (let i = commands.length - 1; i >= 0; i--) { - await commands[i].revert(); - reverseCommands.push(commands[i].getReverseCommand()); - } - this.historyStore.pushUndoableToRedo(new BulkCommand(reverseCommands)); - await Vue.nextTick(); - this.historyStore.bulkInProgress = false; - } - if (command instanceof Command) { - await command.revert(); - this.historyStore.pushUndoableToRedo(command.getReverseCommand()); - this.uiStore.stateIsDirty = true; - } - this.trackCommand(command, 'undo'); - }, - async redo() { - const command = this.historyStore.popUndoableToRedo(); - if (!command) { - return; - } - if (command instanceof BulkCommand) { - this.historyStore.bulkInProgress = true; - const commands = command.commands; - const reverseCommands = []; - for (let i = commands.length - 1; i >= 0; i--) { - await commands[i].revert(); - reverseCommands.push(commands[i].getReverseCommand()); - } - this.historyStore.pushBulkCommandToUndo(new BulkCommand(reverseCommands), false); - await Vue.nextTick(); - this.historyStore.bulkInProgress = false; - } - if (command instanceof Command) { - await command.revert(); - this.historyStore.pushCommandToUndo(command.getReverseCommand(), false); - this.uiStore.stateIsDirty = true; - } - this.trackCommand(command, 'redo'); - }, - trackCommand(command: Undoable, type: 'undo' | 'redo'): void { - if (command instanceof Command) { - this.$telemetry.track(`User hit ${type}`, { commands_length: 1, commands: [command.name] }); - } else if (command instanceof BulkCommand) { - this.$telemetry.track(`User hit ${type}`, { - commands_length: command.commands.length, - commands: command.commands.map((c) => c.name), - }); - } - }, - trackUndoAttempt(event: KeyboardEvent) { - if (this.isNDVOpen && !event.shiftKey) { - const activeNode = this.ndvStore.activeNode; - if (activeNode) { - this.$telemetry.track('User hit undo in NDV', { node_type: activeNode.type }); - } - } - }, - }, -});