From 810a63f808a4811bbb4559d8448b6d251414ebda Mon Sep 17 00:00:00 2001 From: Chenlei Hu Date: Thu, 19 Sep 2024 20:10:43 +0900 Subject: [PATCH] Support async hooks in TreeExplorerNode (#888) * Support async hooks in TreeExplorerNode * rebase * nit * Fix component test failure * Add edit vitest * Add more tests * Add component test --- package-lock.json | 62 +++++++++-- package.json | 1 + src/components/common/EditableText.vue | 2 +- src/components/common/TreeExplorer.vue | 34 ++++-- .../common/TreeExplorerTreeNode.vue | 17 ++- .../__tests__/TreeExplorerTreeNode.spec.ts | 104 +++++++++++++++++- src/hooks/errorHooks.ts | 39 ++++--- src/types/treeExplorerTypes.ts | 16 ++- 8 files changed, 229 insertions(+), 46 deletions(-) diff --git a/package-lock.json b/package-lock.json index 1dd1166de..c49695597 100644 --- a/package-lock.json +++ b/package-lock.json @@ -33,6 +33,7 @@ "@babel/preset-env": "^7.22.20", "@eslint/js": "^9.8.0", "@iconify/json": "^2.2.245", + "@pinia/testing": "^0.1.5", "@playwright/test": "^1.44.1", "@types/jest": "^29.5.12", "@types/lodash": "^4.17.6", @@ -3350,6 +3351,49 @@ "dev": true, "license": "MIT" }, + "node_modules/@pinia/testing": { + "version": "0.1.5", + "resolved": "https://registry.npmjs.org/@pinia/testing/-/testing-0.1.5.tgz", + "integrity": "sha512-AcGzuotkzhRoF00htuxLfIPBBHVE6HjjB3YC5Y3os8vRgKu6ipknK5GBQq9+pduwYQhZ+BcCZDC9TyLAUlUpoQ==", + "dev": true, + "license": "MIT", + "dependencies": { + "vue-demi": "^0.14.10" + }, + "funding": { + "url": "https://github.com/sponsors/posva" + }, + "peerDependencies": { + "pinia": ">=2.2.1" + } + }, + "node_modules/@pinia/testing/node_modules/vue-demi": { + "version": "0.14.10", + "resolved": "https://registry.npmjs.org/vue-demi/-/vue-demi-0.14.10.tgz", + "integrity": "sha512-nMZBOwuzabUO0nLgIcc6rycZEebF6eeUfaiQx9+WSk8e29IbLvPU9feI6tqW4kTo3hvoYAJkMh8n8D0fuISphg==", + "dev": true, + "hasInstallScript": true, + "license": "MIT", + "bin": { + "vue-demi-fix": "bin/vue-demi-fix.js", + "vue-demi-switch": "bin/vue-demi-switch.js" + }, + "engines": { + "node": ">=12" + }, + "funding": { + "url": "https://github.com/sponsors/antfu" + }, + "peerDependencies": { + "@vue/composition-api": "^1.0.0-rc.1", + "vue": "^3.0.0-0 || ^2.6.0" + }, + "peerDependenciesMeta": { + "@vue/composition-api": { + "optional": true + } + } + }, "node_modules/@pkgjs/parseargs": { "version": "0.11.0", "resolved": "https://registry.npmjs.org/@pkgjs/parseargs/-/parseargs-0.11.0.tgz", @@ -10074,12 +10118,13 @@ } }, "node_modules/pinia": { - "version": "2.1.7", - "resolved": "https://registry.npmjs.org/pinia/-/pinia-2.1.7.tgz", - "integrity": "sha512-+C2AHFtcFqjPih0zpYuvof37SFxMQ7OEG2zV9jRI12i9BOy3YQVAHwdKtyyc8pDcDyIc33WCIsZaCFWU7WWxGQ==", + "version": "2.2.2", + "resolved": "https://registry.npmjs.org/pinia/-/pinia-2.2.2.tgz", + "integrity": "sha512-ja2XqFWZC36mupU4z1ZzxeTApV7DOw44cV4dhQ9sGwun+N89v/XP7+j7q6TanS1u1tdbK4r+1BUx7heMaIdagA==", + "license": "MIT", "dependencies": { - "@vue/devtools-api": "^6.5.0", - "vue-demi": ">=0.14.5" + "@vue/devtools-api": "^6.6.3", + "vue-demi": "^0.14.10" }, "funding": { "url": "https://github.com/sponsors/posva" @@ -10099,10 +10144,11 @@ } }, "node_modules/pinia/node_modules/vue-demi": { - "version": "0.14.8", - "resolved": "https://registry.npmjs.org/vue-demi/-/vue-demi-0.14.8.tgz", - "integrity": "sha512-Uuqnk9YE9SsWeReYqK2alDI5YzciATE0r2SkA6iMAtuXvNTMNACJLJEXNXaEy94ECuBe4Sk6RzRU80kjdbIo1Q==", + "version": "0.14.10", + "resolved": "https://registry.npmjs.org/vue-demi/-/vue-demi-0.14.10.tgz", + "integrity": "sha512-nMZBOwuzabUO0nLgIcc6rycZEebF6eeUfaiQx9+WSk8e29IbLvPU9feI6tqW4kTo3hvoYAJkMh8n8D0fuISphg==", "hasInstallScript": true, + "license": "MIT", "bin": { "vue-demi-fix": "bin/vue-demi-fix.js", "vue-demi-switch": "bin/vue-demi-switch.js" diff --git a/package.json b/package.json index 51cf92a1d..05c8eb67f 100644 --- a/package.json +++ b/package.json @@ -25,6 +25,7 @@ "@babel/preset-env": "^7.22.20", "@eslint/js": "^9.8.0", "@iconify/json": "^2.2.245", + "@pinia/testing": "^0.1.5", "@playwright/test": "^1.44.1", "@types/jest": "^29.5.12", "@types/lodash": "^4.17.6", diff --git a/src/components/common/EditableText.vue b/src/components/common/EditableText.vue index 6d767037a..a38c92349 100644 --- a/src/components/common/EditableText.vue +++ b/src/components/common/EditableText.vue @@ -60,7 +60,7 @@ watch( const start = 0 const end = fileName.length const inputElement = inputRef.value.$el - inputElement.setSelectionRange(start, end) + inputElement.setSelectionRange?.(start, end) }) } }, diff --git a/src/components/common/TreeExplorer.vue b/src/components/common/TreeExplorer.vue index f987702b4..893c3f803 100644 --- a/src/components/common/TreeExplorer.vue +++ b/src/components/common/TreeExplorer.vue @@ -92,12 +92,15 @@ const fillNodeInfo = (node: TreeExplorerNode): RenderedTreeExplorerNode => { : children.reduce((acc, child) => acc + child.totalLeaves, 0) } } -const onNodeContentClick = (e: MouseEvent, node: RenderedTreeExplorerNode) => { +const onNodeContentClick = async ( + e: MouseEvent, + node: RenderedTreeExplorerNode +) => { if (!storeSelectionKeys) { selectionKeys.value = {} } if (node.handleClick) { - node.handleClick(node, e) + await node.handleClick(node, e) } emit('nodeClick', node, e) } @@ -118,8 +121,8 @@ const { t } = useI18n() const renameCommand = (node: RenderedTreeExplorerNode) => { renameEditingNode.value = node } -const deleteCommand = (node: RenderedTreeExplorerNode) => { - node.handleDelete?.(node) +const deleteCommand = async (node: RenderedTreeExplorerNode) => { + await node.handleDelete?.(node) emit('nodeDelete', node) } const menuItems = computed(() => @@ -134,12 +137,15 @@ const menuItems = computed(() => label: t('delete'), icon: 'pi pi-trash', command: () => deleteCommand(menuTargetNode.value), - visible: menuTargetNode.value?.handleDelete !== undefined + visible: menuTargetNode.value?.handleDelete !== undefined, + isAsync: true // The delete command can be async }, ...extraMenuItems.value ].map((menuItem) => ({ ...menuItem, - command: wrapCommandWithErrorHandler(menuItem.command) + command: wrapCommandWithErrorHandler(menuItem.command, { + isAsync: menuItem.isAsync ?? false + }) })) ) @@ -153,12 +159,18 @@ const handleContextMenu = (node: RenderedTreeExplorerNode, e: MouseEvent) => { const errorHandling = useErrorHandling() const wrapCommandWithErrorHandler = ( - command: (event: MenuItemCommandEvent) => void + command: (event: MenuItemCommandEvent) => void, + { isAsync = false }: { isAsync: boolean } ) => { - return errorHandling.wrapWithErrorHandling( - command, - menuTargetNode.value?.handleError - ) + return isAsync + ? errorHandling.wrapWithErrorHandlingAsync( + command as (...args: any[]) => Promise, + menuTargetNode.value?.handleError + ) + : errorHandling.wrapWithErrorHandling( + command, + menuTargetNode.value?.handleError + ) } defineExpose({ diff --git a/src/components/common/TreeExplorerTreeNode.vue b/src/components/common/TreeExplorerTreeNode.vue index 23c15afad..82e5ba04d 100644 --- a/src/components/common/TreeExplorerTreeNode.vue +++ b/src/components/common/TreeExplorerTreeNode.vue @@ -46,6 +46,7 @@ import type { TreeExplorerNode } from '@/types/treeExplorerTypes' import EditableText from '@/components/common/EditableText.vue' +import { useErrorHandling } from '@/hooks/errorHooks' const props = defineProps<{ node: RenderedTreeExplorerNode @@ -67,10 +68,14 @@ const renameEditingNode = const isEditing = computed( () => labelEditable.value && renameEditingNode.value?.key === props.node.key ) -const handleRename = (newName: string) => { - props.node.handleRename(props.node, newName) - renameEditingNode.value = null -} +const errorHandling = useErrorHandling() +const handleRename = errorHandling.wrapWithErrorHandlingAsync( + async (newName: string) => { + await props.node.handleRename(props.node, newName) + renameEditingNode.value = null + }, + props.node.handleError +) const container = ref(null) const canDrop = ref(false) const treeNodeElement = ref(null) @@ -83,10 +88,10 @@ onMounted(() => { if (props.node.droppable) { dropTargetCleanup = dropTargetForElements({ element: treeNodeElement.value, - onDrop: (event) => { + onDrop: async (event) => { const dndData = event.source.data as TreeExplorerDragAndDropData if (dndData.type === 'tree-explorer-node') { - props.node.handleDrop?.(props.node, dndData) + await props.node.handleDrop?.(props.node, dndData) canDrop.value = false emit('itemDropped', props.node, dndData.data) } diff --git a/src/components/common/__tests__/TreeExplorerTreeNode.spec.ts b/src/components/common/__tests__/TreeExplorerTreeNode.spec.ts index 10ae5d574..3d6f181ea 100644 --- a/src/components/common/__tests__/TreeExplorerTreeNode.spec.ts +++ b/src/components/common/__tests__/TreeExplorerTreeNode.spec.ts @@ -3,7 +3,20 @@ import { mount } from '@vue/test-utils' import TreeExplorerTreeNode from '@/components/common/TreeExplorerTreeNode.vue' import EditableText from '@/components/common/EditableText.vue' import Badge from 'primevue/badge' +import PrimeVue from 'primevue/config' +import InputText from 'primevue/inputtext' +import { createTestingPinia } from '@pinia/testing' import { RenderedTreeExplorerNode } from '@/types/treeExplorerTypes' +import { createI18n } from 'vue-i18n' +import { createApp } from 'vue' +import { useToastStore } from '@/stores/toastStore' + +// Create a mock i18n instance +const i18n = createI18n({ + legacy: false, + locale: 'en', + messages: {} +}) describe('TreeExplorerTreeNode', () => { const mockNode = { @@ -12,15 +25,28 @@ describe('TreeExplorerTreeNode', () => { leaf: false, totalLeaves: 3, icon: 'pi pi-folder', - type: 'folder' + type: 'folder', + handleRename: () => {} } as RenderedTreeExplorerNode + beforeAll(() => { + // Create a Vue app instance for PrimeVuePrimeVue + const app = createApp({}) + app.use(PrimeVue) + vi.useFakeTimers() + }) + + afterAll(() => { + vi.useRealTimers() + }) + it('renders correctly', () => { const wrapper = mount(TreeExplorerTreeNode, { props: { node: mockNode }, global: { components: { EditableText, Badge }, - provide: { renameEditingNode: { value: null } } + provide: { renameEditingNode: { value: null } }, + plugins: [createTestingPinia(), i18n] } }) @@ -32,4 +58,78 @@ describe('TreeExplorerTreeNode', () => { ) expect(wrapper.findComponent(Badge).props()['value']).toBe(3) }) + + it('makes node label editable when renamingEditingNode matches', async () => { + const wrapper = mount(TreeExplorerTreeNode, { + props: { node: mockNode }, + global: { + components: { EditableText, Badge, InputText }, + provide: { renameEditingNode: { value: { key: '1' } } }, + plugins: [createTestingPinia(), i18n, PrimeVue] + } + }) + + const editableText = wrapper.findComponent(EditableText) + expect(editableText.props('isEditing')).toBe(true) + }) + + it('triggers handleRename callback when editing is finished', async () => { + const handleRenameMock = vi.fn() + const nodeWithMockRename = { + ...mockNode, + handleRename: handleRenameMock + } + + const wrapper = mount(TreeExplorerTreeNode, { + props: { node: nodeWithMockRename }, + global: { + components: { EditableText, Badge, InputText }, + provide: { renameEditingNode: { value: { key: '1' } } }, + plugins: [createTestingPinia(), i18n, PrimeVue] + } + }) + + const editableText = wrapper.findComponent(EditableText) + editableText.vm.$emit('edit', 'New Node Name') + expect(handleRenameMock).toHaveBeenCalledOnce() + }) + + it('shows error toast when handleRename promise rejects', async () => { + const handleRenameMock = vi + .fn() + .mockRejectedValue(new Error('Rename failed')) + const nodeWithMockRename = { + ...mockNode, + handleRename: handleRenameMock + } + + const wrapper = mount(TreeExplorerTreeNode, { + props: { node: nodeWithMockRename }, + global: { + components: { EditableText, Badge, InputText }, + provide: { renameEditingNode: { value: { key: '1' } } }, + plugins: [createTestingPinia(), i18n, PrimeVue] + } + }) + + const toastStore = useToastStore() + const addToastSpy = vi.spyOn(toastStore, 'add') + + const editableText = wrapper.findComponent(EditableText) + editableText.vm.$emit('edit', 'New Node Name') + + // Wait for the promise to reject and the toast to be added + vi.runAllTimers() + + // Wait for any pending promises to resolve + await new Promise(process.nextTick) + + expect(handleRenameMock).toHaveBeenCalledOnce() + expect(addToastSpy).toHaveBeenCalledWith({ + severity: 'error', + summary: 'error', + detail: 'Rename failed', + life: 3000 + }) + }) }) diff --git a/src/hooks/errorHooks.ts b/src/hooks/errorHooks.ts index f14491fa6..b45492cd8 100644 --- a/src/hooks/errorHooks.ts +++ b/src/hooks/errorHooks.ts @@ -1,28 +1,41 @@ -import { useToast } from 'primevue/usetoast' +import { useToastStore } from '@/stores/toastStore' import { useI18n } from 'vue-i18n' export function useErrorHandling() { - const toast = useToast() + const toast = useToastStore() const { t } = useI18n() + const toastErrorHandler = (error: any) => { + toast.add({ + severity: 'error', + summary: t('error'), + detail: error.message, + life: 3000 + }) + } + const wrapWithErrorHandling = (action: (...args: any[]) => any, errorHandler?: (error: any) => void) => (...args: any[]) => { try { return action(...args) } catch (e) { - if (errorHandler) { - errorHandler(e) - } else { - toast.add({ - severity: 'error', - summary: t('error'), - detail: e.message, - life: 3000 - }) - } + ;(errorHandler ?? toastErrorHandler)(e) } } - return { wrapWithErrorHandling } + const wrapWithErrorHandlingAsync = + ( + action: (...args: any[]) => Promise, + errorHandler?: (error: any) => void + ) => + async (...args: any[]) => { + try { + return await action(...args) + } catch (e) { + ;(errorHandler ?? toastErrorHandler)(e) + } + } + + return { wrapWithErrorHandling, wrapWithErrorHandlingAsync } } diff --git a/src/types/treeExplorerTypes.ts b/src/types/treeExplorerTypes.ts index 67056eb59..24b7967ae 100644 --- a/src/types/treeExplorerTypes.ts +++ b/src/types/treeExplorerTypes.ts @@ -9,14 +9,17 @@ export interface TreeExplorerNode { icon?: string getIcon?: (node: TreeExplorerNode) => string // Function to handle renaming the node - handleRename?: (node: TreeExplorerNode, newName: string) => void + handleRename?: ( + node: TreeExplorerNode, + newName: string + ) => void | Promise // Function to handle deleting the node - handleDelete?: (node: TreeExplorerNode) => void + handleDelete?: (node: TreeExplorerNode) => void | Promise // Function to handle adding a child node handleAddChild?: ( node: TreeExplorerNode, child: TreeExplorerNode - ) => void + ) => void | Promise // Whether the node is draggable draggable?: boolean // Whether the node is droppable @@ -25,9 +28,12 @@ export interface TreeExplorerNode { handleDrop?: ( node: TreeExplorerNode, data: TreeExplorerDragAndDropData - ) => void + ) => void | Promise // Function to handle clicking a node - handleClick?: (node: TreeExplorerNode, event: MouseEvent) => void + handleClick?: ( + node: TreeExplorerNode, + event: MouseEvent + ) => void | Promise // Function to handle errors handleError?: (error: Error) => void // Extra context menu items