mirror of
https://github.com/langgenius/dify.git
synced 2026-10-16 15:35:32 +08:00
fix(workflow): preserve change node menu semantics and focus (#42568)
This commit is contained in:
@@ -3,6 +3,8 @@ import { userEvent } from 'vite-plus/test/browser'
|
||||
import { render } from 'vitest-browser-react'
|
||||
import {
|
||||
Popover,
|
||||
PopoverBackdrop,
|
||||
PopoverClose,
|
||||
PopoverContent,
|
||||
PopoverDescription,
|
||||
PopoverPopup,
|
||||
@@ -11,6 +13,12 @@ import {
|
||||
PopoverTitle,
|
||||
PopoverTrigger,
|
||||
} from '..'
|
||||
import {
|
||||
DropdownMenu,
|
||||
DropdownMenuContent,
|
||||
DropdownMenuItem,
|
||||
DropdownMenuTrigger,
|
||||
} from '../../dropdown-menu'
|
||||
|
||||
const renderWithSafeViewport = (ui: React.ReactNode) =>
|
||||
render(<div style={{ minHeight: '100vh', minWidth: '100vw', padding: '240px' }}>{ui}</div>)
|
||||
@@ -95,6 +103,47 @@ describe('PopoverContent', () => {
|
||||
})
|
||||
|
||||
describe('Popover anatomy', () => {
|
||||
it('places the backdrop above a parent menu and below the interactive popup', async () => {
|
||||
const backdropRef = React.createRef<HTMLDivElement>()
|
||||
const screen = await renderWithSafeViewport(
|
||||
<DropdownMenu modal={false}>
|
||||
<DropdownMenuTrigger>Actions</DropdownMenuTrigger>
|
||||
<DropdownMenuContent>
|
||||
<DropdownMenuItem>Primary action</DropdownMenuItem>
|
||||
<Popover modal="trap-focus">
|
||||
<DropdownMenuItem
|
||||
closeOnClick={false}
|
||||
render={<PopoverTrigger nativeButton={false} render={<div />} />}
|
||||
>
|
||||
More options
|
||||
</DropdownMenuItem>
|
||||
<PopoverPortal>
|
||||
<PopoverBackdrop ref={backdropRef} />
|
||||
<PopoverPositioner placement="right-start">
|
||||
<PopoverPopup>
|
||||
<PopoverTitle>More options</PopoverTitle>
|
||||
<input aria-label="Filter options" />
|
||||
<PopoverClose>Close</PopoverClose>
|
||||
</PopoverPopup>
|
||||
</PopoverPositioner>
|
||||
</PopoverPortal>
|
||||
</Popover>
|
||||
</DropdownMenuContent>
|
||||
</DropdownMenu>,
|
||||
)
|
||||
|
||||
await screen.getByRole('button', { name: 'Actions' }).click()
|
||||
const primaryAction = screen.getByRole('menuitem', { name: 'Primary action' }).element()
|
||||
await screen.getByRole('menuitem', { name: 'More options' }).click()
|
||||
const input = screen.getByRole('textbox', { name: 'Filter options' })
|
||||
|
||||
const { x, y, width, height } = primaryAction.getBoundingClientRect()
|
||||
expect(document.elementFromPoint(x + width / 2, y + height / 2)).toBe(backdropRef.current)
|
||||
await input.click()
|
||||
await input.fill('Example')
|
||||
await expect.element(input).toHaveValue('Example')
|
||||
})
|
||||
|
||||
it('should use the default positioner placement', async () => {
|
||||
const screen = await renderWithSafeViewport(
|
||||
<Popover open>
|
||||
|
||||
@@ -25,6 +25,14 @@ type PopoverCloseProps = BasePopover.Close.Props
|
||||
type PopoverTitleProps = BasePopover.Title.Props
|
||||
type PopoverDescriptionProps = BasePopover.Description.Props
|
||||
|
||||
type PopoverBackdropProps = Omit<BasePopover.Backdrop.Props, 'className'> & {
|
||||
className?: string
|
||||
}
|
||||
|
||||
function PopoverBackdrop({ className, ...props }: PopoverBackdropProps) {
|
||||
return <BasePopover.Backdrop className={cn('fixed inset-0 z-50', className)} {...props} />
|
||||
}
|
||||
|
||||
type PopoverPositionerProps = Omit<BasePopover.Positioner.Props, 'className' | 'side' | 'align'> & {
|
||||
className?: string
|
||||
placement?: Placement
|
||||
@@ -103,6 +111,7 @@ export {
|
||||
createPopoverHandle,
|
||||
Popover,
|
||||
PopoverArrow,
|
||||
PopoverBackdrop,
|
||||
PopoverClose,
|
||||
PopoverContent,
|
||||
PopoverDescription,
|
||||
@@ -114,6 +123,7 @@ export {
|
||||
}
|
||||
export type {
|
||||
PopoverArrowProps,
|
||||
PopoverBackdropProps,
|
||||
PopoverCloseProps,
|
||||
PopoverContentProps,
|
||||
PopoverDescriptionProps,
|
||||
|
||||
@@ -241,7 +241,7 @@ type BlockSelectorContentProps = Pick<
|
||||
searchInputRef: React.RefObject<HTMLInputElement | null>
|
||||
}
|
||||
|
||||
function BlockSelectorContent({
|
||||
export function BlockSelectorContent({
|
||||
allowUserInputSelection,
|
||||
availableBlocksTypes,
|
||||
blocks: blocksFromProps,
|
||||
|
||||
@@ -3,7 +3,7 @@ import {
|
||||
DropdownMenuContent,
|
||||
DropdownMenuTrigger,
|
||||
} from '@langgenius/dify-ui/dropdown-menu'
|
||||
import { screen } from '@testing-library/react'
|
||||
import { screen, waitFor } from '@testing-library/react'
|
||||
import userEvent from '@testing-library/user-event'
|
||||
import { renderWorkflowFlowComponent } from '@/app/components/workflow/__tests__/workflow-test-env'
|
||||
import { useHooksStore } from '@/app/components/workflow/hooks-store'
|
||||
@@ -11,11 +11,13 @@ import useNodes from '@/app/components/workflow/store/workflow/use-nodes'
|
||||
import { BlockEnum, NodeRunningStatus } from '@/app/components/workflow/types'
|
||||
import { useAllWorkflowTools } from '@/service/use-tools'
|
||||
import { FlowType } from '@/types/common'
|
||||
import { BlockClassification } from '../../block-selector/types'
|
||||
import { useAvailableBlocks } from '../../hooks/use-available-blocks'
|
||||
import { useNodesInteractions } from '../../hooks/use-nodes-interactions'
|
||||
import { useNodeMetaData } from '../../hooks/use-nodes-meta-data'
|
||||
import { useIsChatMode, useNodesReadOnly } from '../../hooks/use-workflow'
|
||||
import { NodeActionsDropdownContent } from '../dropdown-content'
|
||||
import { NodeActionsDropdown } from '../index'
|
||||
|
||||
vi.mock('../../hooks/use-available-blocks', async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import('../../hooks/use-available-blocks')>()
|
||||
@@ -64,6 +66,22 @@ vi.mock('@/app/components/workflow/store/workflow/use-nodes', () => ({
|
||||
|
||||
vi.mock('@/service/use-tools', () => ({
|
||||
useAllWorkflowTools: vi.fn(),
|
||||
useAllBuiltInTools: () => ({ data: [] }),
|
||||
useAllCustomTools: () => ({ data: [] }),
|
||||
useAllMCPTools: () => ({ data: [] }),
|
||||
useInvalidateAllBuiltInTools: () => vi.fn(),
|
||||
}))
|
||||
|
||||
vi.mock('@/service/use-plugins', () => ({
|
||||
useFeaturedToolsRecommendations: () => ({ plugins: [], isLoading: false }),
|
||||
useFeaturedTriggersRecommendations: () => ({ plugins: [], isLoading: false }),
|
||||
}))
|
||||
vi.mock('@/app/components/plugins/marketplace/query', () => ({
|
||||
useMarketplacePlugins: () => ({ data: undefined }),
|
||||
}))
|
||||
vi.mock('@/service/use-triggers', () => ({
|
||||
useAllTriggerPlugins: () => ({ data: [] }),
|
||||
useInvalidateAllTriggerPlugins: () => vi.fn(),
|
||||
}))
|
||||
|
||||
const mockUseAvailableBlocks = vi.mocked(useAvailableBlocks)
|
||||
@@ -143,6 +161,22 @@ describe('node actions menu details', () => {
|
||||
selector({
|
||||
configsMap: { flowType: FlowType.appFlow },
|
||||
accessControl: { canRun: true },
|
||||
availableNodesMetaData: {
|
||||
nodes: [
|
||||
{
|
||||
metaData: {
|
||||
type: BlockEnum.HttpRequest,
|
||||
title: 'HTTP Request',
|
||||
classification: BlockClassification.Default,
|
||||
sort: 0,
|
||||
author: 'Dify',
|
||||
description: 'Send a request',
|
||||
},
|
||||
defaultValue: {},
|
||||
checkValid: () => ({ isValid: true }),
|
||||
},
|
||||
],
|
||||
},
|
||||
}),
|
||||
)
|
||||
mockUseNodes.mockReturnValue([
|
||||
@@ -151,6 +185,46 @@ describe('node actions menu details', () => {
|
||||
mockUseAllWorkflowTools.mockReturnValue({ data: [] } as any)
|
||||
})
|
||||
|
||||
it('keeps the menu after Escape and closes both surfaces after changing a node', async () => {
|
||||
const user = userEvent.setup()
|
||||
renderWorkflowFlowComponent(
|
||||
<NodeActionsDropdown
|
||||
id="node-1"
|
||||
data={{ type: BlockEnum.Code, title: 'Code Node', desc: '' }}
|
||||
/>,
|
||||
{
|
||||
nodes: [],
|
||||
edges: [{ id: 'edge-1', source: 'node-0', target: 'node-1', sourceHandle: 'branch-a' }],
|
||||
},
|
||||
)
|
||||
|
||||
await user.click(screen.getByRole('button', { name: 'common.operation.more' }))
|
||||
const changeNode = screen.getByRole('menuitem', { name: 'workflow.panel.changeBlock' })
|
||||
await user.click(changeNode)
|
||||
const search = screen.getByRole('searchbox', { name: 'workflow.tabs.searchBlock' })
|
||||
await waitFor(() => expect(search).toHaveFocus())
|
||||
await user.keyboard('{ArrowDown}')
|
||||
expect(search).toHaveFocus()
|
||||
await user.keyboard('{Escape}')
|
||||
await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument())
|
||||
expect(screen.getByRole('menu')).toBeInTheDocument()
|
||||
expect(changeNode).toHaveFocus()
|
||||
|
||||
await user.keyboard('{Enter}')
|
||||
await user.click(await screen.findByRole('button', { name: 'HTTP Request' }))
|
||||
|
||||
expect(handleNodeChange).toHaveBeenCalledExactlyOnceWith(
|
||||
'node-1',
|
||||
BlockEnum.HttpRequest,
|
||||
'branch-a',
|
||||
undefined,
|
||||
)
|
||||
await waitFor(() => {
|
||||
expect(screen.queryByRole('menu')).not.toBeInTheDocument()
|
||||
expect(screen.queryByRole('dialog')).not.toBeInTheDocument()
|
||||
})
|
||||
})
|
||||
|
||||
it('should run, copy, duplicate, delete, and expose the help link', async () => {
|
||||
const user = userEvent.setup()
|
||||
const { store } = renderDropdownContent()
|
||||
|
||||
@@ -41,8 +41,8 @@ vi.mock('@/service/use-tools', () => ({
|
||||
useAllWorkflowTools: vi.fn(),
|
||||
}))
|
||||
|
||||
vi.mock('../change-block-menu-trigger', () => ({
|
||||
ChangeBlockMenuTrigger: () => <div data-testid="node-actions-change-block" />,
|
||||
vi.mock('../change-block-popup', () => ({
|
||||
ChangeBlockPopup: () => <div data-testid="node-actions-change-block" />,
|
||||
}))
|
||||
|
||||
const mockUseNodeMetaData = vi.mocked(useNodeMetaData)
|
||||
|
||||
+40
-26
@@ -1,8 +1,9 @@
|
||||
import type { CommonNodeType, Node, OnSelectBlock } from '@/app/components/workflow/types'
|
||||
import { PopoverClose, PopoverPopup, PopoverTitle } from '@langgenius/dify-ui/popover'
|
||||
import { intersection } from 'es-toolkit/array'
|
||||
import { useCallback, useMemo } from 'react'
|
||||
import { useCallback, useMemo, useRef } from 'react'
|
||||
import { useTranslation } from 'react-i18next'
|
||||
import BlockSelector from '@/app/components/workflow/block-selector'
|
||||
import { BlockSelectorContent } from '@/app/components/workflow/block-selector'
|
||||
import { useHooksStore } from '@/app/components/workflow/hooks-store'
|
||||
import useNodes from '@/app/components/workflow/store/workflow/use-nodes'
|
||||
import { BlockEnum, isTriggerNode } from '@/app/components/workflow/types'
|
||||
@@ -12,18 +13,22 @@ import { useAvailableBlocks } from '../hooks/use-available-blocks'
|
||||
import { useNodesInteractions } from '../hooks/use-nodes-interactions'
|
||||
import { useIsChatMode } from '../hooks/use-workflow'
|
||||
|
||||
type ChangeBlockMenuTriggerProps = {
|
||||
type ChangeBlockPopupProps = {
|
||||
nodeId: string
|
||||
nodeData: Node['data']
|
||||
sourceHandle: string
|
||||
onComplete: () => void
|
||||
}
|
||||
|
||||
export function ChangeBlockMenuTrigger({
|
||||
export function ChangeBlockPopup({
|
||||
nodeId,
|
||||
nodeData,
|
||||
sourceHandle,
|
||||
}: ChangeBlockMenuTriggerProps) {
|
||||
onComplete,
|
||||
}: ChangeBlockPopupProps) {
|
||||
const { t } = useTranslation()
|
||||
const searchInputRef = useRef<HTMLInputElement>(null)
|
||||
const completedRef = useRef(false)
|
||||
const { handleNodeChange } = useNodesInteractions()
|
||||
const nodeCatalogType = getNodeCatalogType(nodeData)
|
||||
const { availablePrevBlocks, availableNextBlocks } = useAvailableBlocks(
|
||||
@@ -53,33 +58,42 @@ export function ChangeBlockMenuTrigger({
|
||||
return availableNextBlocks
|
||||
}, [availablePrevBlocks, availableNextBlocks])
|
||||
|
||||
const handleComplete = useCallback(() => {
|
||||
completedRef.current = true
|
||||
onComplete()
|
||||
}, [onComplete])
|
||||
|
||||
const handleSelect = useCallback<OnSelectBlock>(
|
||||
(type, pluginDefaultValue) => {
|
||||
handleComplete()
|
||||
handleNodeChange(nodeId, type, sourceHandle, pluginDefaultValue)
|
||||
},
|
||||
[handleNodeChange, nodeId, sourceHandle],
|
||||
)
|
||||
|
||||
const triggerElement = (
|
||||
<button
|
||||
type="button"
|
||||
className="mx-1 flex h-8 w-[calc(100%-8px)] cursor-pointer items-center rounded-lg border-0 bg-transparent px-2 text-left text-sm text-text-secondary outline-hidden select-none hover:bg-state-base-hover focus-visible:bg-state-base-hover focus-visible:outline-hidden"
|
||||
>
|
||||
{t(($) => $['panel.changeBlock'], { ns: 'workflow' })}
|
||||
</button>
|
||||
[handleNodeChange, nodeId, sourceHandle, handleComplete],
|
||||
)
|
||||
|
||||
return (
|
||||
<BlockSelector
|
||||
onSelect={handleSelect}
|
||||
trigger={triggerElement}
|
||||
popupClassName="min-w-[240px]"
|
||||
availableBlocksTypes={availableNodes}
|
||||
showStartTab={showStartTab}
|
||||
ignoreNodeIds={ignoreNodeIds}
|
||||
forceEnableStartTab={nodeData.type === BlockEnum.Start}
|
||||
allowUserInputSelection={allowStartNodeSelection}
|
||||
isolateKeyboardEvents
|
||||
/>
|
||||
<PopoverPopup
|
||||
initialFocus={searchInputRef}
|
||||
finalFocus={() => !completedRef.current}
|
||||
className="w-100 overflow-hidden rounded-xl border-[0.5px] border-components-panel-border bg-components-panel-bg shadow-lg"
|
||||
onKeyDown={(event) => event.stopPropagation()}
|
||||
>
|
||||
<PopoverTitle className="sr-only">
|
||||
{t(($) => $['panel.changeBlock'], { ns: 'workflow' })}
|
||||
</PopoverTitle>
|
||||
<BlockSelectorContent
|
||||
onSelect={handleSelect}
|
||||
onRequestClose={handleComplete}
|
||||
searchInputRef={searchInputRef}
|
||||
availableBlocksTypes={availableNodes}
|
||||
showStartTab={showStartTab}
|
||||
ignoreNodeIds={ignoreNodeIds}
|
||||
forceEnableStartTab={nodeData.type === BlockEnum.Start}
|
||||
allowUserInputSelection={allowStartNodeSelection}
|
||||
/>
|
||||
<PopoverClose className="sr-only" tabIndex={-1}>
|
||||
{t(($) => $['operation.close'], { ns: 'common' })}
|
||||
</PopoverClose>
|
||||
</PopoverPopup>
|
||||
)
|
||||
}
|
||||
@@ -5,8 +5,15 @@ import {
|
||||
ContextMenuLinkItem,
|
||||
ContextMenuSeparator,
|
||||
} from '@langgenius/dify-ui/context-menu'
|
||||
import {
|
||||
Popover,
|
||||
PopoverBackdrop,
|
||||
PopoverPortal,
|
||||
PopoverPositioner,
|
||||
PopoverTrigger,
|
||||
} from '@langgenius/dify-ui/popover'
|
||||
import { useTranslation } from 'react-i18next'
|
||||
import { ChangeBlockMenuTrigger } from './change-block-menu-trigger'
|
||||
import { ChangeBlockPopup } from './change-block-popup'
|
||||
import {
|
||||
NODE_ACTIONS_MENU_DELETE_ITEM_CLASS_NAME,
|
||||
NODE_ACTIONS_MENU_ITEM_WITH_SHORTCUT_CLASS_NAME,
|
||||
@@ -33,11 +40,26 @@ export function NodeActionsContextMenuContent(props: NodeActionsMenuProps) {
|
||||
<ContextMenuItem onClick={model.handleRun}>{singleRunActionLabel}</ContextMenuItem>
|
||||
)}
|
||||
{model.canChangeBlock && (
|
||||
<ChangeBlockMenuTrigger
|
||||
nodeId={model.id}
|
||||
nodeData={model.data}
|
||||
sourceHandle={model.sourceHandle}
|
||||
/>
|
||||
<Popover modal="trap-focus">
|
||||
<ContextMenuItem
|
||||
closeOnClick={false}
|
||||
render={<PopoverTrigger nativeButton={false} render={<div />} />}
|
||||
className="data-popup-open:bg-state-base-hover"
|
||||
>
|
||||
{t(($) => $['panel.changeBlock'], { ns: 'workflow' })}
|
||||
</ContextMenuItem>
|
||||
<PopoverPortal>
|
||||
<PopoverBackdrop />
|
||||
<PopoverPositioner placement="right-start" positionMethod="fixed">
|
||||
<ChangeBlockPopup
|
||||
nodeId={model.id}
|
||||
nodeData={model.data}
|
||||
sourceHandle={model.sourceHandle}
|
||||
onComplete={props.onClose}
|
||||
/>
|
||||
</PopoverPositioner>
|
||||
</PopoverPortal>
|
||||
</Popover>
|
||||
)}
|
||||
</ContextMenuGroup>
|
||||
)}
|
||||
|
||||
@@ -5,8 +5,15 @@ import {
|
||||
DropdownMenuLinkItem,
|
||||
DropdownMenuSeparator,
|
||||
} from '@langgenius/dify-ui/dropdown-menu'
|
||||
import {
|
||||
Popover,
|
||||
PopoverBackdrop,
|
||||
PopoverPortal,
|
||||
PopoverPositioner,
|
||||
PopoverTrigger,
|
||||
} from '@langgenius/dify-ui/popover'
|
||||
import { useTranslation } from 'react-i18next'
|
||||
import { ChangeBlockMenuTrigger } from './change-block-menu-trigger'
|
||||
import { ChangeBlockPopup } from './change-block-popup'
|
||||
import {
|
||||
NODE_ACTIONS_MENU_DELETE_ITEM_CLASS_NAME,
|
||||
NODE_ACTIONS_MENU_ITEM_WITH_SHORTCUT_CLASS_NAME,
|
||||
@@ -33,11 +40,26 @@ export function NodeActionsDropdownContent(props: NodeActionsMenuProps) {
|
||||
<DropdownMenuItem onClick={model.handleRun}>{singleRunActionLabel}</DropdownMenuItem>
|
||||
)}
|
||||
{model.canChangeBlock && (
|
||||
<ChangeBlockMenuTrigger
|
||||
nodeId={model.id}
|
||||
nodeData={model.data}
|
||||
sourceHandle={model.sourceHandle}
|
||||
/>
|
||||
<Popover modal="trap-focus">
|
||||
<DropdownMenuItem
|
||||
closeOnClick={false}
|
||||
render={<PopoverTrigger nativeButton={false} render={<div />} />}
|
||||
className="data-popup-open:bg-state-base-hover"
|
||||
>
|
||||
{t(($) => $['panel.changeBlock'], { ns: 'workflow' })}
|
||||
</DropdownMenuItem>
|
||||
<PopoverPortal>
|
||||
<PopoverBackdrop />
|
||||
<PopoverPositioner placement="right-start" positionMethod="fixed">
|
||||
<ChangeBlockPopup
|
||||
nodeId={model.id}
|
||||
nodeData={model.data}
|
||||
sourceHandle={model.sourceHandle}
|
||||
onComplete={props.onClose}
|
||||
/>
|
||||
</PopoverPositioner>
|
||||
</PopoverPortal>
|
||||
</Popover>
|
||||
)}
|
||||
</DropdownMenuGroup>
|
||||
)}
|
||||
|
||||
Reference in New Issue
Block a user