diff --git a/web/app/components/workflow/skill/constants.ts b/web/app/components/workflow/skill/constants.ts index 0bf8f42e37..7787219c72 100644 --- a/web/app/components/workflow/skill/constants.ts +++ b/web/app/components/workflow/skill/constants.ts @@ -11,14 +11,6 @@ export const START_TAB_ID = '__start__' as const // Drag type identifier for internal tree node dragging export const INTERNAL_NODE_DRAG_TYPE = 'application/x-dify-tree-node' -// Context menu trigger types (describes WHERE user clicked) -export const CONTEXT_MENU_TYPE = { - BLANK: 'blank', - NODE: 'node', -} as const - -export type ContextMenuType = (typeof CONTEXT_MENU_TYPE)[keyof typeof CONTEXT_MENU_TYPE] - // Node menu types (determines which menu options to show) export const NODE_MENU_TYPE = { ROOT: 'root', diff --git a/web/app/components/workflow/skill/file-tree/tree/file-tree.spec.tsx b/web/app/components/workflow/skill/file-tree/tree/file-tree.spec.tsx index 4f60fad145..db5e67badf 100644 --- a/web/app/components/workflow/skill/file-tree/tree/file-tree.spec.tsx +++ b/web/app/components/workflow/skill/file-tree/tree/file-tree.spec.tsx @@ -1,7 +1,7 @@ -import type { ReactNode, Ref } from 'react' +import type { HTMLAttributes, ReactNode, Ref } from 'react' import type { AppAssetTreeView } from '@/types/app-asset' import { fireEvent, render, screen } from '@testing-library/react' -import { CONTEXT_MENU_TYPE, ROOT_ID } from '../../constants' +import { ROOT_ID } from '../../constants' import FileTree from './file-tree' type MockWorkflowState = { @@ -17,7 +17,6 @@ type MockWorkflowActions = { openTab: (id: string, options: { pinned: boolean }) => void setSelectedNodeIds: (ids: string[]) => void clearSelection: () => void - setContextMenu: (menu: { top: number, left: number, type: string } | null) => void setDragInsertTarget: (target: { parentId: string | null, index: number } | null) => void setFileTreeSearchTerm: (term: string) => void } @@ -142,7 +141,6 @@ const mocks = vi.hoisted(() => ({ openTab: vi.fn(), setSelectedNodeIds: vi.fn(), clearSelection: vi.fn(), - setContextMenu: vi.fn(), setDragInsertTarget: vi.fn(), setFileTreeSearchTerm: vi.fn(), } as MockWorkflowActions, @@ -168,14 +166,14 @@ const mocks = vi.hoisted(() => ({ })) vi.mock('react-arborist', async () => { - const React = await vi.importActual('react') - type MockTreeComponentProps = { children?: ReactNode + ref?: Ref } & Record - const Tree = React.forwardRef((props: MockTreeComponentProps, ref: Ref) => { - mocks.treeProps = props as unknown as CapturedTreeProps + const Tree = (props: MockTreeComponentProps) => { + const { ref, ...rest } = props + mocks.treeProps = rest as unknown as CapturedTreeProps if (typeof ref === 'function') ref(mocks.treeApi) @@ -183,7 +181,7 @@ vi.mock('react-arborist', async () => { (ref as { current: unknown }).current = mocks.treeApi return
- }) + } return { Tree } }) @@ -267,7 +265,27 @@ vi.mock('./upload-status-tooltip', () => ({ })) vi.mock('./tree-context-menu', () => ({ - default: () =>
, + default: ({ + children, + treeRef, + onContextMenu, + ...props + }: HTMLAttributes & { + children?: ReactNode + treeRef?: { current: { deselectAll: () => void } | null } + }) => ( +
{ + treeRef?.current?.deselectAll() + mocks.actions.clearSelection() + onContextMenu?.(event) + }} + {...props} + > + {children} +
+ ), })) function getCapturedTreeProps(): CapturedTreeProps { @@ -399,18 +417,13 @@ describe('FileTree', () => { expect(mocks.actions.clearSelection).toHaveBeenCalledTimes(1) }) - it('should open blank context menu with pointer position on right click', () => { + it('should clear selection when blank area is right clicked', () => { render() fireEvent.contextMenu(getTreeDropZone(), { clientX: 64, clientY: 128 }) expect(mocks.treeApi.deselectAll).toHaveBeenCalledTimes(1) expect(mocks.actions.clearSelection).toHaveBeenCalledTimes(1) - expect(mocks.actions.setContextMenu).toHaveBeenCalledWith({ - top: 128, - left: 64, - type: CONTEXT_MENU_TYPE.BLANK, - }) }) it('should forward root drag events to root file drop handlers', () => { diff --git a/web/app/components/workflow/skill/file-tree/tree/file-tree.tsx b/web/app/components/workflow/skill/file-tree/tree/file-tree.tsx index 5a6729adfb..6b415043f1 100644 --- a/web/app/components/workflow/skill/file-tree/tree/file-tree.tsx +++ b/web/app/components/workflow/skill/file-tree/tree/file-tree.tsx @@ -15,7 +15,7 @@ import SearchMenu from '@/app/components/base/icons/src/vender/knowledge/SearchM import Loading from '@/app/components/base/loading' import { useStore, useWorkflowStore } from '@/app/components/workflow/store' import { cn } from '@/utils/classnames' -import { CONTEXT_MENU_TYPE, ROOT_ID } from '../../constants' +import { ROOT_ID } from '../../constants' import { useSkillAssetTreeData } from '../../hooks/file-tree/data/use-skill-asset-tree' import { useSkillTreeCollaboration } from '../../hooks/file-tree/data/use-skill-tree-collaboration' import { useRootFileDrop } from '../../hooks/file-tree/dnd/use-root-file-drop' @@ -181,17 +181,6 @@ const FileTree = ({ className }: FileTreeProps) => { storeApi.getState().clearSelection() }, [storeApi, treeRef]) - const handleBlankAreaContextMenu = useCallback((e: React.MouseEvent) => { - e.preventDefault() - treeRef.current?.deselectAll() - storeApi.getState().clearSelection() - storeApi.getState().setContextMenu({ - top: e.clientY, - left: e.clientX, - type: CONTEXT_MENU_TYPE.BLANK, - }) - }, [storeApi, treeRef]) - // Node move API (for internal drag-drop) const { executeMoveNode } = useNodeMove() const { executeReorderNode } = useNodeReorder() @@ -387,14 +376,14 @@ const FileTree = ({ className }: FileTreeProps) => { className, )} > -
{ > {renderTreeNode} -
+ {dragOverFolderId ? : } />}
- ) } diff --git a/web/app/components/workflow/skill/file-tree/tree/menu-item.spec.tsx b/web/app/components/workflow/skill/file-tree/tree/menu-item.spec.tsx index 1eb3aa3b0c..a3004a0e75 100644 --- a/web/app/components/workflow/skill/file-tree/tree/menu-item.spec.tsx +++ b/web/app/components/workflow/skill/file-tree/tree/menu-item.spec.tsx @@ -5,6 +5,7 @@ import MenuItem from './menu-item' const MockIcon = (props: React.SVGProps) => const defaultProps: MenuItemProps = { + menuType: 'dropdown', icon: MockIcon, label: 'Rename', onClick: vi.fn(), diff --git a/web/app/components/workflow/skill/file-tree/tree/menu-item.tsx b/web/app/components/workflow/skill/file-tree/tree/menu-item.tsx index 21b16a33c2..a7a36d1e63 100644 --- a/web/app/components/workflow/skill/file-tree/tree/menu-item.tsx +++ b/web/app/components/workflow/skill/file-tree/tree/menu-item.tsx @@ -3,7 +3,17 @@ import type { VariantProps } from 'class-variance-authority' import { cva } from 'class-variance-authority' import * as React from 'react' -import Tooltip from '@/app/components/base/tooltip-plus' +import { + ContextMenuItem, +} from '@/app/components/base/ui/context-menu' +import { + DropdownMenuItem, +} from '@/app/components/base/ui/dropdown-menu' +import { + Tooltip, + TooltipContent, + TooltipTrigger, +} from '@/app/components/base/ui/tooltip' import ShortcutsName from '@/app/components/workflow/shortcuts-name' import { cn } from '@/utils/classnames' @@ -51,26 +61,33 @@ const labelVariants = cva('text-text-secondary system-sm-regular', { }) export type MenuItemProps = { + menuType: 'dropdown' | 'context' icon: React.ElementType | string label: string kbd?: readonly string[] - onClick: React.MouseEventHandler + onClick: () => void disabled?: boolean tooltip?: string } & VariantProps -const MenuItem = ({ icon: Icon, label, kbd, onClick, disabled, variant, tooltip }: MenuItemProps) => { - const handleClick = React.useCallback((event: React.MouseEvent) => { - event.stopPropagation() - onClick(event) - }, [onClick]) +const MenuItem = ({ + menuType, + icon: Icon, + label, + kbd, + onClick, + disabled, + variant, + tooltip, +}: MenuItemProps) => { + const ItemComponent = menuType === 'dropdown' ? DropdownMenuItem : ContextMenuItem return ( - + ) } diff --git a/web/app/components/workflow/skill/file-tree/tree/node-menu.spec.tsx b/web/app/components/workflow/skill/file-tree/tree/node-menu.spec.tsx index f3913921fd..12ecae82d0 100644 --- a/web/app/components/workflow/skill/file-tree/tree/node-menu.spec.tsx +++ b/web/app/components/workflow/skill/file-tree/tree/node-menu.spec.tsx @@ -29,6 +29,7 @@ type MockFileOperations = { type RenderNodeMenuProps = { type?: 'root' | 'folder' | 'file' + menuType?: 'dropdown' | 'context' nodeId?: string onClose?: () => void treeRef?: RefObject | null> @@ -95,6 +96,7 @@ vi.mock('../../hooks/file-tree/operations/use-file-operations', () => ({ const renderNodeMenu = ({ type = NODE_MENU_TYPE.FOLDER, + menuType = 'dropdown', nodeId = 'node-1', onClose = vi.fn(), treeRef, @@ -103,6 +105,7 @@ const renderNodeMenu = ({ render( import('../../start-tab/import-skill-moda ssr: false, }) -const MENU_CONTAINER_STYLES = [ - 'min-w-[180px] rounded-xl border-[0.5px] border-components-panel-border', - 'bg-components-panel-bg-blur p-1 shadow-lg backdrop-blur-[5px]', -] as const - const KBD_CUT = ['ctrl', 'x'] as const const KBD_PASTE = ['ctrl', 'v'] as const type NodeMenuProps = { type: NodeMenuType + menuType: 'dropdown' | 'context' nodeId?: string onClose: () => void - className?: string treeRef?: React.RefObject | null> node?: NodeApi } const NodeMenu = ({ type, + menuType, nodeId, onClose, - className, treeRef, node, }: NodeMenuProps) => { @@ -101,9 +101,10 @@ const NodeMenu = ({ const deleteConfirmContent = isFolder ? t('skillSidebar.menu.deleteConfirmContent') : t('skillSidebar.menu.fileDeleteConfirmContent') + const Separator = menuType === 'dropdown' ? DropdownMenuSeparator : ContextMenuSeparator return ( -
+ <> {isFolder && ( <> handleNewFile()} disabled={isLoading} /> handleNewFolder()} disabled={isLoading} /> -
+ fileInputRef.current?.click()} disabled={isLoading} /> folderInputRef.current?.click()} @@ -154,8 +159,9 @@ const NodeMenu = ({ {isRoot && ( <> -
+ setIsImportModalOpen(true)} @@ -165,25 +171,27 @@ const NodeMenu = ({ )} - {(showRenameDelete || hasClipboard) &&
} + {(showRenameDelete || hasClipboard) && } )} {!isFolder && ( <> -
+ )} {!isRoot && ( <> -
+ handleRename()} disabled={isLoading} /> handleDeleteClick()} disabled={isLoading} variant="destructive" /> @@ -257,7 +268,7 @@ const NodeMenu = ({ isOpen={isImportModalOpen} onClose={() => setIsImportModalOpen(false)} /> -
+ ) } diff --git a/web/app/components/workflow/skill/file-tree/tree/tree-context-menu.spec.tsx b/web/app/components/workflow/skill/file-tree/tree/tree-context-menu.spec.tsx index 188f08f598..6c3f75398d 100644 --- a/web/app/components/workflow/skill/file-tree/tree/tree-context-menu.spec.tsx +++ b/web/app/components/workflow/skill/file-tree/tree/tree-context-menu.spec.tsx @@ -1,174 +1,61 @@ -import type { ReactNode } from 'react' -import type { ContextMenuState } from '@/app/components/workflow/store/workflow/skill-editor/types' -import { act, fireEvent, render, screen } from '@testing-library/react' -import { CONTEXT_MENU_TYPE, NODE_MENU_TYPE, ROOT_ID } from '../../constants' +import { fireEvent, render, screen } from '@testing-library/react' +import { ROOT_ID } from '../../constants' import TreeContextMenu from './tree-context-menu' -type MockWorkflowState = { - contextMenu: ContextMenuState | null -} - -type FloatingOptions = { - open: boolean - onOpenChange: (open: boolean) => void - position: { - x: number - y: number - } -} - const mocks = vi.hoisted(() => ({ - storeState: { - contextMenu: null, - } as MockWorkflowState, - setContextMenu: vi.fn(), - floatingOptions: null as FloatingOptions | null, - getFloatingProps: vi.fn(() => ({ 'data-floating-props': 'applied' })), + clearSelection: vi.fn(), + deselectAll: vi.fn(), })) vi.mock('@/app/components/workflow/store', () => ({ - useStore: (selector: (state: MockWorkflowState) => unknown) => selector(mocks.storeState), useWorkflowStore: () => ({ getState: () => ({ - setContextMenu: mocks.setContextMenu, + clearSelection: mocks.clearSelection, }), }), })) -vi.mock('@floating-ui/react', () => ({ - FloatingPortal: ({ children }: { children: ReactNode }) => ( -
{children}
- ), -})) - -vi.mock('@/app/components/base/portal-to-follow-elem-plus/use-context-menu-floating', () => ({ - useContextMenuFloating: (options: FloatingOptions) => { - mocks.floatingOptions = options - return { - refs: { - setFloating: vi.fn(), - }, - floatingStyles: { - left: `${options.position.x}px`, - top: `${options.position.y}px`, - }, - getFloatingProps: mocks.getFloatingProps, - isPositioned: true, - } - }, -})) - vi.mock('./node-menu', () => ({ - default: ({ - type, - nodeId, - onClose, - }: { - type: string - nodeId?: string - onClose: () => void - }) => ( -
- -
+ default: ({ type, menuType, nodeId }: { type: string, menuType: string, nodeId?: string }) => ( +
), })) -const setContextMenuState = (contextMenu: ContextMenuState | null) => { - mocks.storeState.contextMenu = contextMenu -} - describe('TreeContextMenu', () => { beforeEach(() => { vi.clearAllMocks() - mocks.floatingOptions = null - setContextMenuState(null) }) - // Rendering should depend on context-menu state in the workflow store. describe('Rendering', () => { - it('should render nothing when context menu state is null', () => { - render() + it('should render trigger children', () => { + render( + +
blank area
+
, + ) - expect(screen.queryByTestId('node-menu')).not.toBeInTheDocument() - expect(screen.queryByTestId('floating-portal')).not.toBeInTheDocument() - }) - - it('should render file menu with node id when node context is on a file', () => { - setContextMenuState({ - top: 40, - left: 24, - type: CONTEXT_MENU_TYPE.NODE, - nodeId: 'file-1', - isFolder: false, - }) - - render() - - const menu = screen.getByTestId('node-menu') - expect(menu).toHaveAttribute('data-type', NODE_MENU_TYPE.FILE) - expect(menu).toHaveAttribute('data-node-id', 'file-1') - expect(menu.parentElement).toHaveStyle({ - left: '24px', - top: '40px', - visibility: 'visible', - }) - expect(mocks.getFloatingProps).toHaveBeenCalledTimes(1) - expect(mocks.floatingOptions?.open).toBe(true) - expect(mocks.floatingOptions?.position).toEqual({ x: 24, y: 40 }) - }) - - it('should render root menu with root id when context is blank area', () => { - setContextMenuState({ - top: 100, - left: 80, - type: CONTEXT_MENU_TYPE.BLANK, - }) - - render() - - const menu = screen.getByTestId('node-menu') - expect(menu).toHaveAttribute('data-type', NODE_MENU_TYPE.ROOT) - expect(menu).toHaveAttribute('data-node-id', ROOT_ID) + expect(screen.getByText('blank area')).toBeInTheDocument() }) }) - // Close events from floating layer and menu should reset store context menu. - describe('Closing behavior', () => { - it('should clear context menu when floating layer requests close', () => { - setContextMenuState({ - top: 12, - left: 16, - type: CONTEXT_MENU_TYPE.NODE, - nodeId: 'file-1', - isFolder: false, - }) + describe('Interactions', () => { + it('should clear selection and open root menu when blank area is right clicked', () => { + render( + +
blank area
+
, + ) - render() + fireEvent.contextMenu(screen.getByText('blank area')) - act(() => { - mocks.floatingOptions?.onOpenChange(false) - }) - - expect(mocks.setContextMenu).toHaveBeenCalledTimes(1) - expect(mocks.setContextMenu).toHaveBeenCalledWith(null) - }) - - it('should clear context menu when node menu closes', () => { - setContextMenuState({ - top: 12, - left: 16, - type: CONTEXT_MENU_TYPE.NODE, - nodeId: 'file-1', - isFolder: false, - }) - - render() - - fireEvent.click(screen.getByRole('button', { name: 'close' })) - - expect(mocks.setContextMenu).toHaveBeenCalledTimes(1) - expect(mocks.setContextMenu).toHaveBeenCalledWith(null) + expect(mocks.deselectAll).toHaveBeenCalledTimes(1) + expect(mocks.clearSelection).toHaveBeenCalledTimes(1) + expect(screen.getByTestId('node-menu-context')).toHaveAttribute('data-type', 'root') + expect(screen.getByTestId('node-menu-context')).toHaveAttribute('data-node-id', ROOT_ID) }) }) }) diff --git a/web/app/components/workflow/skill/file-tree/tree/tree-context-menu.tsx b/web/app/components/workflow/skill/file-tree/tree/tree-context-menu.tsx index cf00b1afa9..54552ee91b 100644 --- a/web/app/components/workflow/skill/file-tree/tree/tree-context-menu.tsx +++ b/web/app/components/workflow/skill/file-tree/tree/tree-context-menu.tsx @@ -2,62 +2,63 @@ import type { TreeApi } from 'react-arborist' import type { TreeNodeData } from '../../type' -import { FloatingPortal } from '@floating-ui/react' import * as React from 'react' -import { useCallback, useMemo } from 'react' -import { useContextMenuFloating } from '@/app/components/base/portal-to-follow-elem-plus/use-context-menu-floating' -import { useStore, useWorkflowStore } from '@/app/components/workflow/store' -import { getMenuNodeId, getNodeMenuType } from '../../utils/tree-utils' +import { + ContextMenu, + ContextMenuContent, + ContextMenuTrigger, +} from '@/app/components/base/ui/context-menu' +import { useWorkflowStore } from '@/app/components/workflow/store' +import { NODE_MENU_TYPE, ROOT_ID } from '../../constants' import NodeMenu from './node-menu' -type TreeContextMenuProps = { +type TreeContextMenuProps = Omit< + React.ComponentPropsWithoutRef, + 'children' | 'onContextMenu' +> & { treeRef: React.RefObject | null> + triggerRef?: React.Ref + children: React.ReactNode } -const TreeContextMenu = ({ treeRef }: TreeContextMenuProps) => { - const contextMenu = useStore(s => s.contextMenu) +const TreeContextMenu = ({ + treeRef, + triggerRef, + children, + ...props +}: TreeContextMenuProps) => { const storeApi = useWorkflowStore() - const handleClose = useCallback(() => { - storeApi.getState().setContextMenu(null) - }, [storeApi]) + const handleContextMenu = React.useCallback((event: React.MouseEvent) => { + const target = event.target as HTMLElement + if (target.closest('[role="treeitem"]')) + return - const position = useMemo(() => ({ - x: contextMenu?.left ?? 0, - y: contextMenu?.top ?? 0, - }), [contextMenu?.left, contextMenu?.top]) + treeRef.current?.deselectAll() + storeApi.getState().clearSelection() + }, [storeApi, treeRef]) - const { refs, floatingStyles, getFloatingProps, isPositioned } = useContextMenuFloating({ - open: !!contextMenu, - onOpenChange: (open) => { - if (!open) - handleClose() - }, - position, - }) - - if (!contextMenu) - return null + const handleMenuClose = React.useCallback(() => {}, []) return ( - -
+ + {children} + + -
-
+ + ) } diff --git a/web/app/components/workflow/skill/file-tree/tree/tree-node.spec.tsx b/web/app/components/workflow/skill/file-tree/tree/tree-node.spec.tsx index 1c228cef26..c898143789 100644 --- a/web/app/components/workflow/skill/file-tree/tree/tree-node.spec.tsx +++ b/web/app/components/workflow/skill/file-tree/tree/tree-node.spec.tsx @@ -5,9 +5,6 @@ import TreeNode from './tree-node' type MockWorkflowSelectorState = { dirtyContents: Set - contextMenu: { - nodeId?: string - } | null isCutNode: (nodeId: string) => boolean } @@ -27,7 +24,6 @@ type NodeState = { const workflowState = vi.hoisted(() => ({ dirtyContents: new Set(), cutNodeIds: new Set(), - contextMenuNodeId: null as string | null, dragOverFolderId: null as string | null, })) @@ -40,7 +36,6 @@ const handlerMocks = vi.hoisted(() => ({ handleClick: vi.fn(), handleDoubleClick: vi.fn(), handleToggle: vi.fn(), - handleContextMenu: vi.fn(), handleKeyDown: vi.fn(), })) @@ -56,9 +51,6 @@ const dndMocks = vi.hoisted(() => ({ vi.mock('@/app/components/workflow/store', () => ({ useStore: (selector: (state: MockWorkflowSelectorState) => unknown) => selector({ dirtyContents: workflowState.dirtyContents, - contextMenu: workflowState.contextMenuNodeId - ? { nodeId: workflowState.contextMenuNodeId } - : null, isCutNode: (nodeId: string) => workflowState.cutNodeIds.has(nodeId), }), useWorkflowStore: () => ({ @@ -80,7 +72,6 @@ vi.mock('../../hooks/file-tree/interaction/use-tree-node-handlers', () => ({ handleClick: handlerMocks.handleClick, handleDoubleClick: handlerMocks.handleDoubleClick, handleToggle: handlerMocks.handleToggle, - handleContextMenu: handlerMocks.handleContextMenu, handleKeyDown: handlerMocks.handleKeyDown, }), })) @@ -99,8 +90,8 @@ vi.mock('../../hooks/file-tree/dnd/use-folder-file-drop', () => ({ })) vi.mock('./node-menu', () => ({ - default: ({ type, onClose }: { type: string, onClose: () => void }) => ( -
+ default: ({ type, menuType, onClose }: { type: string, menuType: string, onClose: () => void }) => ( +
), @@ -136,6 +127,7 @@ const createNode = (overrides: Partial = {}): NodeApi = willReceiveDrop: resolved.willReceiveDrop, isEditing: resolved.isEditing, level: resolved.level, + select: vi.fn(), } as unknown as NodeApi } @@ -155,7 +147,6 @@ describe('TreeNode', () => { workflowState.dirtyContents.clear() workflowState.cutNodeIds.clear() - workflowState.contextMenuNodeId = null workflowState.dragOverFolderId = null dndMocks.isDragOver = false @@ -164,8 +155,7 @@ describe('TreeNode', () => { // Core rendering should reflect selection, folder expansion, and store-driven visual states. describe('Rendering', () => { - it('should render file node with context-menu highlight and action button label', () => { - workflowState.contextMenuNodeId = 'file-1' + it('should render file node with action button label', () => { const props = buildProps({ id: 'file-1', name: 'readme.md', nodeType: 'file' }) render() @@ -173,7 +163,6 @@ describe('TreeNode', () => { const treeItem = screen.getByRole('treeitem') expect(treeItem).toHaveAttribute('aria-selected', 'false') expect(treeItem).not.toHaveAttribute('aria-expanded') - expect(treeItem).toHaveClass('bg-state-base-hover') expect(screen.getByText('readme.md')).toBeInTheDocument() expect(screen.getByRole('button', { name: /workflow\.skillSidebar\.menu\.moreActions/i })).toBeInTheDocument() }) @@ -229,7 +218,7 @@ describe('TreeNode', () => { expect(handlerMocks.handleDoubleClick).toHaveBeenCalled() }) - it('should call keyboard and context-menu handlers on tree item', () => { + it('should call keyboard handler and open context menu on tree item right click', () => { const props = buildProps({ id: 'file-1', name: 'readme.md', nodeType: 'file' }) render() @@ -239,7 +228,7 @@ describe('TreeNode', () => { fireEvent.contextMenu(treeItem) expect(handlerMocks.handleKeyDown).toHaveBeenCalledTimes(1) - expect(handlerMocks.handleContextMenu).toHaveBeenCalledTimes(1) + expect(screen.getByTestId('node-menu-context')).toHaveAttribute('data-type', 'file') }) it('should attach folder drag handlers only when node is a folder', () => { @@ -274,20 +263,16 @@ describe('TreeNode', () => { expect(dndMocks.onDragLeave).not.toHaveBeenCalled() }) - it('should open and close dropdown menu when more actions button is toggled', () => { + it('should open dropdown menu when more actions button is clicked', () => { const props = buildProps({ id: 'file-1', name: 'readme.md', nodeType: 'file' }) render() - expect(screen.queryByTestId('node-menu')).not.toBeInTheDocument() + expect(screen.queryByTestId('node-menu-dropdown')).not.toBeInTheDocument() fireEvent.click(screen.getByRole('button', { name: /workflow\.skillSidebar\.menu\.moreActions/i })) - expect(screen.getByTestId('node-menu')).toHaveAttribute('data-type', 'file') - - fireEvent.click(screen.getByRole('button', { name: 'close-menu' })) - - expect(screen.queryByTestId('node-menu')).not.toBeInTheDocument() + expect(screen.getByTestId('node-menu-dropdown')).toHaveAttribute('data-type', 'file') }) }) diff --git a/web/app/components/workflow/skill/file-tree/tree/tree-node.tsx b/web/app/components/workflow/skill/file-tree/tree/tree-node.tsx index 10ac2810ce..ede8cf0acb 100644 --- a/web/app/components/workflow/skill/file-tree/tree/tree-node.tsx +++ b/web/app/components/workflow/skill/file-tree/tree/tree-node.tsx @@ -3,13 +3,18 @@ import type { NodeRendererProps } from 'react-arborist' import type { TreeNodeData } from '../../type' import * as React from 'react' -import { useCallback, useEffect, useRef, useState } from 'react' +import { useCallback, useEffect, useRef } from 'react' import { useTranslation } from 'react-i18next' import { - PortalToFollowElem, - PortalToFollowElemContent, - PortalToFollowElemTrigger, -} from '@/app/components/base/portal-to-follow-elem-plus' + ContextMenu, + ContextMenuContent, + ContextMenuTrigger, +} from '@/app/components/base/ui/context-menu' +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuTrigger, +} from '@/app/components/base/ui/dropdown-menu' import { useStore, useWorkflowStore } from '@/app/components/workflow/store' import { cn } from '@/utils/classnames' import { useFolderFileDrop } from '../../hooks/file-tree/dnd/use-folder-file-drop' @@ -29,49 +34,44 @@ const TreeNode = ({ node, style, dragHandle, treeChildren }: TreeNodeProps) => { const isSelected = node.isSelected const isDirty = useStore(s => s.dirtyContents.has(node.data.id)) const isCut = useStore(s => s.isCutNode(node.data.id)) - const contextMenuNodeId = useStore(s => s.contextMenu?.nodeId) - const hasContextMenu = contextMenuNodeId === node.data.id const storeApi = useWorkflowStore() - const [showDropdown, setShowDropdown] = useState(false) - // Sync react-arborist drag state to Zustand for DragActionTooltip - const prevIsDragging = useRef(node.isDragging) + const prevIsDraggingRef = useRef(node.isDragging) useEffect(() => { // When drag starts - if (node.isDragging && !prevIsDragging.current) + if (node.isDragging && !prevIsDraggingRef.current) storeApi.getState().setCurrentDragType('move') // When drag ends - if (!node.isDragging && prevIsDragging.current) { + if (!node.isDragging && prevIsDraggingRef.current) { storeApi.getState().setCurrentDragType(null) storeApi.getState().setDragOverFolderId(null) } - prevIsDragging.current = node.isDragging + prevIsDraggingRef.current = node.isDragging }, [node.isDragging, storeApi]) // Sync react-arborist willReceiveDrop to Zustand for DragActionTooltip - const prevWillReceiveDrop = useRef(node.willReceiveDrop) + const prevWillReceiveDropRef = useRef(node.willReceiveDrop) useEffect(() => { // When willReceiveDrop becomes true, set dragOverFolderId - if (isFolder && node.willReceiveDrop && !prevWillReceiveDrop.current) + if (isFolder && node.willReceiveDrop && !prevWillReceiveDropRef.current) storeApi.getState().setDragOverFolderId(node.data.id) // When willReceiveDrop becomes false, clear if this node was the target - if (isFolder && !node.willReceiveDrop && prevWillReceiveDrop.current) { + if (isFolder && !node.willReceiveDrop && prevWillReceiveDropRef.current) { const currentDragOverId = storeApi.getState().dragOverFolderId if (currentDragOverId === node.data.id) storeApi.getState().setDragOverFolderId(null) } - prevWillReceiveDrop.current = node.willReceiveDrop + prevWillReceiveDropRef.current = node.willReceiveDrop }, [isFolder, node.willReceiveDrop, node.data.id, storeApi]) const { handleClick, handleDoubleClick, handleToggle, - handleContextMenu, handleKeyDown, } = useTreeNodeHandlers({ node }) @@ -83,105 +83,115 @@ const TreeNode = ({ node, style, dragHandle, treeChildren }: TreeNodeProps) => { const handleMoreClick = useCallback((e: React.MouseEvent) => { e.stopPropagation() - setShowDropdown(prev => !prev) }, []) + const handleContextMenu = useCallback((e: React.MouseEvent) => { + e.stopPropagation() + node.select() + }, [node]) + + const handleMenuClose = useCallback(() => {}, []) + return ( -
- - {/* Main content area - isolated click/double-click handling */} -
+ -
- + +
+
+ +
+ + {node.isEditing + ? ( + + ) + : ( + + {node.data.name} + + )}
- {node.isEditing - ? ( - - ) - : ( - - {node.data.name} - - )} -
- - {/* More button - separate from main content click handling */} - - - - - - setShowDropdown(false)} - node={node} - /> - - -
+ + + + + + + + + + ) } diff --git a/web/app/components/workflow/skill/hooks/file-tree/interaction/use-tree-node-handlers.spec.tsx b/web/app/components/workflow/skill/hooks/file-tree/interaction/use-tree-node-handlers.spec.tsx index fbb0ce08b1..5878c090a2 100644 --- a/web/app/components/workflow/skill/hooks/file-tree/interaction/use-tree-node-handlers.spec.tsx +++ b/web/app/components/workflow/skill/hooks/file-tree/interaction/use-tree-node-handlers.spec.tsx @@ -6,11 +6,9 @@ import { useTreeNodeHandlers } from './use-tree-node-handlers' const { mockClearArtifactSelection, mockOpenTab, - mockSetContextMenu, } = vi.hoisted(() => ({ mockClearArtifactSelection: vi.fn(), mockOpenTab: vi.fn(), - mockSetContextMenu: vi.fn(), })) vi.mock('es-toolkit/function', () => ({ @@ -22,7 +20,6 @@ vi.mock('@/app/components/workflow/store', () => ({ getState: () => ({ clearArtifactSelection: mockClearArtifactSelection, openTab: mockOpenTab, - setContextMenu: mockSetContextMenu, }), }), })) @@ -54,8 +51,6 @@ const createMouseEvent = (params: { shiftKey?: boolean ctrlKey?: boolean metaKey?: boolean - clientX?: number - clientY?: number } = {}) => { return { stopPropagation: vi.fn(), @@ -63,8 +58,6 @@ const createMouseEvent = (params: { shiftKey: params.shiftKey ?? false, ctrlKey: params.ctrlKey ?? false, metaKey: params.metaKey ?? false, - clientX: params.clientX ?? 0, - clientY: params.clientY ?? 0, } as unknown as React.MouseEvent } @@ -183,29 +176,8 @@ describe('useTreeNodeHandlers', () => { }) }) - // Scenario: context menu and keyboard handlers update menu state and open/toggle actions. - describe('context menu and keyboard', () => { - it('should select node and set context menu payload on right click', () => { - const node = createNode({ id: 'folder-1', nodeType: 'folder' }) - const { result } = renderHook(() => useTreeNodeHandlers({ node })) - const event = createMouseEvent({ clientX: 120, clientY: 45 }) - - act(() => { - result.current.handleContextMenu(event) - }) - - expect(event.preventDefault).toHaveBeenCalledTimes(1) - expect(event.stopPropagation).toHaveBeenCalledTimes(1) - expect(node.select).toHaveBeenCalledTimes(1) - expect(mockSetContextMenu).toHaveBeenCalledWith({ - top: 45, - left: 120, - type: 'node', - nodeId: 'folder-1', - isFolder: true, - }) - }) - + // Scenario: keyboard handlers should toggle folders and open files without relying on overlay state. + describe('keyboard', () => { it('should toggle folder on Enter key', () => { const node = createNode({ nodeType: 'folder' }) const { result } = renderHook(() => useTreeNodeHandlers({ node })) diff --git a/web/app/components/workflow/skill/hooks/file-tree/interaction/use-tree-node-handlers.ts b/web/app/components/workflow/skill/hooks/file-tree/interaction/use-tree-node-handlers.ts index 6b620ad45d..ca2a360340 100644 --- a/web/app/components/workflow/skill/hooks/file-tree/interaction/use-tree-node-handlers.ts +++ b/web/app/components/workflow/skill/hooks/file-tree/interaction/use-tree-node-handlers.ts @@ -15,7 +15,6 @@ type UseTreeNodeHandlersReturn = { handleClick: (e: React.MouseEvent) => void handleDoubleClick: (e: React.MouseEvent) => void handleToggle: (e: React.MouseEvent) => void - handleContextMenu: (e: React.MouseEvent) => void handleKeyDown: (e: React.KeyboardEvent) => void } @@ -75,19 +74,6 @@ export function useTreeNodeHandlers({ throttledToggle() }, [throttledToggle]) - const handleContextMenu = useCallback((e: React.MouseEvent) => { - e.preventDefault() - e.stopPropagation() - node.select() - storeApi.getState().setContextMenu({ - top: e.clientY, - left: e.clientX, - type: 'node', - nodeId: node.data.id, - isFolder, - }) - }, [isFolder, node, storeApi]) - const handleKeyDown = useCallback((e: React.KeyboardEvent) => { if (e.key === 'Enter' || e.key === ' ') { e.preventDefault() @@ -105,7 +91,6 @@ export function useTreeNodeHandlers({ handleClick, handleDoubleClick, handleToggle, - handleContextMenu, handleKeyDown, } } diff --git a/web/app/components/workflow/skill/skill-body/sidebar-search-add.tsx b/web/app/components/workflow/skill/skill-body/sidebar-search-add.tsx index d176ca73a5..465871a52e 100644 --- a/web/app/components/workflow/skill/skill-body/sidebar-search-add.tsx +++ b/web/app/components/workflow/skill/skill-body/sidebar-search-add.tsx @@ -98,12 +98,14 @@ const SidebarSearchAdd = () => { /> {
fileInputRef.current?.click()} disabled={isLoading} /> folderInputRef.current?.click()} @@ -128,6 +132,7 @@ const SidebarSearchAdd = () => {
setIsImportModalOpen(true)} diff --git a/web/app/components/workflow/skill/utils/tree-utils.ts b/web/app/components/workflow/skill/utils/tree-utils.ts index d1076119e9..25c09ce74e 100644 --- a/web/app/components/workflow/skill/utils/tree-utils.ts +++ b/web/app/components/workflow/skill/utils/tree-utils.ts @@ -1,6 +1,5 @@ -import type { ContextMenuType, NodeMenuType } from '../constants' import type { AppAssetTreeView } from '@/types/app-asset' -import { CONTEXT_MENU_TYPE, NODE_MENU_TYPE, ROOT_ID } from '../constants' +import { ROOT_ID } from '../constants' // Root utilities @@ -12,24 +11,6 @@ export function toApiParentId(folderId: string | null | undefined): string | nul return isRootId(folderId) ? null : folderId! } -export function getNodeMenuType( - contextType: ContextMenuType, - isFolder?: boolean, -): NodeMenuType { - if (contextType === CONTEXT_MENU_TYPE.BLANK) - return NODE_MENU_TYPE.ROOT - return isFolder ? NODE_MENU_TYPE.FOLDER : NODE_MENU_TYPE.FILE -} - -export function getMenuNodeId( - contextType: ContextMenuType, - nodeId?: string, -): string { - return contextType === CONTEXT_MENU_TYPE.BLANK - ? ROOT_ID - : (nodeId ?? ROOT_ID) -} - // Tree utilities export function buildNodeMap(nodes: AppAssetTreeView[]): Map { diff --git a/web/app/components/workflow/store/workflow/skill-editor/file-operations-menu-slice.ts b/web/app/components/workflow/store/workflow/skill-editor/file-operations-menu-slice.ts deleted file mode 100644 index 1a8e4ff68e..0000000000 --- a/web/app/components/workflow/store/workflow/skill-editor/file-operations-menu-slice.ts +++ /dev/null @@ -1,17 +0,0 @@ -import type { StateCreator } from 'zustand' -import type { FileOperationsMenuSliceShape, SkillEditorSliceShape } from './types' - -export type { FileOperationsMenuSliceShape } from './types' - -export const createFileOperationsMenuSlice: StateCreator< - SkillEditorSliceShape, - [], - [], - FileOperationsMenuSliceShape -> = set => ({ - contextMenu: null, - - setContextMenu: (contextMenu) => { - set({ contextMenu }) - }, -}) diff --git a/web/app/components/workflow/store/workflow/skill-editor/index.ts b/web/app/components/workflow/store/workflow/skill-editor/index.ts index 56abb6591a..1c183b0dc8 100644 --- a/web/app/components/workflow/store/workflow/skill-editor/index.ts +++ b/web/app/components/workflow/store/workflow/skill-editor/index.ts @@ -4,7 +4,6 @@ import { START_TAB_ID } from '@/app/components/workflow/skill/constants' import { createArtifactSlice } from './artifact-slice' import { createClipboardSlice } from './clipboard-slice' import { createDirtySlice } from './dirty-slice' -import { createFileOperationsMenuSlice } from './file-operations-menu-slice' import { createFileTreeSlice } from './file-tree-slice' import { createMetadataSlice } from './metadata-slice' import { createTabSlice } from './tab-slice' @@ -13,7 +12,6 @@ import { createUploadSlice } from './upload-slice' export type { ArtifactSliceShape } from './artifact-slice' export type { ClipboardSliceShape } from './clipboard-slice' export type { DirtySliceShape } from './dirty-slice' -export type { FileOperationsMenuSliceShape } from './file-operations-menu-slice' export type { FileTreeSliceShape } from './file-tree-slice' export type { MetadataSliceShape } from './metadata-slice' export type { OpenTabOptions, TabSliceShape } from './tab-slice' @@ -26,7 +24,6 @@ export const createSkillEditorSlice: StateCreator = (...a ...createClipboardSlice(...args), ...createDirtySlice(...args), ...createMetadataSlice(...args), - ...createFileOperationsMenuSlice(...args), ...createUploadSlice(...args), ...createArtifactSlice(...args), @@ -48,7 +45,6 @@ export const createSkillEditorSlice: StateCreator = (...a dirtyContents: new Map(), fileMetadata: new Map>(), dirtyMetadataIds: new Set(), - contextMenu: null, fileTreeSearchTerm: '', uploadStatus: 'idle', uploadProgress: { uploaded: 0, total: 0, failed: 0 }, diff --git a/web/app/components/workflow/store/workflow/skill-editor/types.ts b/web/app/components/workflow/store/workflow/skill-editor/types.ts index 0806d0558d..4d5c108153 100644 --- a/web/app/components/workflow/store/workflow/skill-editor/types.ts +++ b/web/app/components/workflow/store/workflow/skill-editor/types.ts @@ -1,5 +1,3 @@ -import type { ContextMenuType } from '@/app/components/workflow/skill/constants' - export type OpenTabOptions = { pinned?: boolean autoFocusEditor?: boolean @@ -92,19 +90,6 @@ export type MetadataSliceShape = { getFileMetadata: (fileId: string) => Record | undefined } -export type ContextMenuState = { - top: number - left: number - type: ContextMenuType - nodeId?: string - isFolder?: boolean -} - -export type FileOperationsMenuSliceShape = { - contextMenu: ContextMenuState | null - setContextMenu: (menu: ContextMenuState | null) => void -} - export type UploadStatus = 'idle' | 'uploading' | 'success' | 'partial_error' export type UploadProgress = { @@ -133,7 +118,6 @@ export type SkillEditorSliceShape & ClipboardSliceShape & DirtySliceShape & MetadataSliceShape - & FileOperationsMenuSliceShape & UploadSliceShape & ArtifactSliceShape & {