diff --git a/web/app/components/app-sidebar/dataset-info/__tests__/dropdown-callbacks.spec.tsx b/web/app/components/app-sidebar/dataset-info/__tests__/dropdown-callbacks.spec.tsx index 6ed10609e9..b514b6e095 100644 --- a/web/app/components/app-sidebar/dataset-info/__tests__/dropdown-callbacks.spec.tsx +++ b/web/app/components/app-sidebar/dataset-info/__tests__/dropdown-callbacks.spec.tsx @@ -137,14 +137,6 @@ vi.mock('@/app/components/datasets/rename-modal', () => ({ }, })) -vi.mock('@/app/components/base/portal-to-follow-elem', () => ({ - PortalToFollowElem: ({ children }: { children: React.ReactNode }) =>
{children}
, - PortalToFollowElemTrigger: ({ children, onClick }: { children: React.ReactNode, onClick?: () => void }) => ( -
{children}
- ), - PortalToFollowElemContent: ({ children }: { children: React.ReactNode }) =>
{children}
, -})) - describe('Dropdown callback coverage', () => { beforeEach(() => { vi.clearAllMocks() @@ -159,7 +151,7 @@ describe('Dropdown callback coverage', () => { const user = userEvent.setup() render() - await user.click(screen.getByTestId('portal-trigger')) + await user.click(screen.getByRole('button')) await user.click(screen.getByText('common.operation.edit')) expect(screen.getByTestId('rename-modal')).toBeInTheDocument() @@ -175,7 +167,7 @@ describe('Dropdown callback coverage', () => { const user = userEvent.setup() render() - await user.click(screen.getByTestId('portal-trigger')) + await user.click(screen.getByRole('button')) await user.click(screen.getByText('common.operation.edit')) expect(screen.getByTestId('rename-modal')).toBeInTheDocument() @@ -190,7 +182,7 @@ describe('Dropdown callback coverage', () => { const user = userEvent.setup() render() - await user.click(screen.getByTestId('portal-trigger')) + await user.click(screen.getByRole('button')) await user.click(screen.getByText('common.operation.delete')) await waitFor(() => { @@ -210,7 +202,7 @@ describe('Dropdown callback coverage', () => { render() - await user.click(screen.getByTestId('portal-trigger')) + await user.click(screen.getByRole('button')) await user.click(screen.getByText('common.operation.delete')) await waitFor(() => { @@ -224,7 +216,7 @@ describe('Dropdown callback coverage', () => { render() - await user.click(screen.getByTestId('portal-trigger')) + await user.click(screen.getByRole('button')) await user.click(screen.getByText('datasetPipeline.operations.exportPipeline')) await waitFor(() => { @@ -232,6 +224,27 @@ describe('Dropdown callback coverage', () => { }) }) + it('should not attempt export when the dataset has no pipeline id', async () => { + const user = userEvent.setup() + mockDataset = createDataset({ pipeline_id: '' }) + + render() + + await user.click(screen.getByRole('button')) + await user.click(screen.getByText('datasetPipeline.operations.exportPipeline')) + + expect(mockExportPipeline).not.toHaveBeenCalled() + }) + + it('should render and open correctly when collapsed', async () => { + const user = userEvent.setup() + render() + + await user.click(screen.getByRole('button')) + + expect(screen.getByText('common.operation.edit')).toBeInTheDocument() + }) + it('should surface the backend message when checking app usage fails', async () => { const user = userEvent.setup() mockCheckIsUsedInApp.mockRejectedValueOnce({ @@ -240,7 +253,7 @@ describe('Dropdown callback coverage', () => { render() - await user.click(screen.getByTestId('portal-trigger')) + await user.click(screen.getByRole('button')) await user.click(screen.getByText('common.operation.delete')) await waitFor(() => { diff --git a/web/app/components/app-sidebar/dataset-info/__tests__/index.spec.tsx b/web/app/components/app-sidebar/dataset-info/__tests__/index.spec.tsx index bb85e00c14..e6d3f94e2a 100644 --- a/web/app/components/app-sidebar/dataset-info/__tests__/index.spec.tsx +++ b/web/app/components/app-sidebar/dataset-info/__tests__/index.spec.tsx @@ -1,5 +1,4 @@ import type { DataSet } from '@/models/datasets' -import { RiEditLine } from '@remixicon/react' import { createEvent, fireEvent, render, screen, waitFor } from '@testing-library/react' import userEvent from '@testing-library/user-event' import * as React from 'react' @@ -22,6 +21,7 @@ const mockInvalidDatasetDetail = vi.fn() const mockExportPipeline = vi.fn() const mockCheckIsUsedInApp = vi.fn() const mockDeleteDataset = vi.fn() +const TestEditIcon = () => const createDataset = (overrides: Partial = {}): DataSet => ({ id: 'dataset-1', @@ -210,7 +210,7 @@ describe('MenuItem', () => { const user = userEvent.setup() const handleClick = vi.fn() // Arrange - render() + render() // Act await user.click(screen.getByText('Edit')) @@ -225,7 +225,7 @@ describe('MenuItem', () => { render(
- +
, ) @@ -236,7 +236,7 @@ describe('MenuItem', () => { }) it('should not crash when no click handler is provided', () => { - render() + render() const event = createEvent.click(screen.getByText('Edit')) fireEvent(screen.getByText('Edit'), event) diff --git a/web/app/components/app-sidebar/dataset-info/dropdown.tsx b/web/app/components/app-sidebar/dataset-info/dropdown.tsx index 9abebfcc88..e69b3d7e32 100644 --- a/web/app/components/app-sidebar/dataset-info/dropdown.tsx +++ b/web/app/components/app-sidebar/dataset-info/dropdown.tsx @@ -1,6 +1,5 @@ import type { DataSet } from '@/models/datasets' import { cn } from '@langgenius/dify-ui/cn' -import { RiMoreFill } from '@remixicon/react' import * as React from 'react' import { useCallback, useState } from 'react' import { useTranslation } from 'react-i18next' @@ -14,7 +13,6 @@ import { useInvalid } from '@/service/use-base' import { useExportPipelineDSL } from '@/service/use-pipeline' import { downloadBlob } from '@/utils/download' import ActionButton from '../../base/action-button' -import { PortalToFollowElem, PortalToFollowElemContent, PortalToFollowElemTrigger } from '../../base/portal-to-follow-elem' import { AlertDialog, AlertDialogActions, @@ -24,6 +22,11 @@ import { AlertDialogDescription, AlertDialogTitle, } from '../../base/ui/alert-dialog' +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuTrigger, +} from '../../base/ui/dropdown-menu' import RenameDatasetModal from '../../datasets/rename-modal' import Menu from './menu' @@ -44,10 +47,6 @@ const DropDown = ({ const isCurrentWorkspaceDatasetOperator = useAppContextWithSelector(state => state.isCurrentWorkspaceDatasetOperator) const dataset = useDatasetDetailContextWithSelector(state => state.dataset) as DataSet - const handleTrigger = useCallback(() => { - setOpen(prev => !prev) - }, []) - const invalidDatasetList = useInvalidDatasetList() const invalidDatasetDetail = useInvalid([...datasetDetailQueryKeyPrefix, dataset.id]) @@ -57,9 +56,11 @@ const DropDown = ({ }, [invalidDatasetDetail, invalidDatasetList]) const openRenameModal = useCallback(() => { - setShowRenameModal(true) - handleTrigger() - }, [handleTrigger]) + setOpen(false) + queueMicrotask(() => { + setShowRenameModal(true) + }) + }, []) const { mutateAsync: exportPipelineConfig } = useExportPipelineDSL() @@ -67,7 +68,7 @@ const DropDown = ({ const { pipeline_id, name } = dataset if (!pipeline_id) return - handleTrigger() + setOpen(false) try { const { data } = await exportPipelineConfig({ pipelineId: pipeline_id, @@ -79,9 +80,10 @@ const DropDown = ({ catch { toast(t('exportFailed', { ns: 'app' }), { type: 'error' }) } - }, [dataset, exportPipelineConfig, handleTrigger, t]) + }, [dataset, exportPipelineConfig, t]) const detectIsUsedByApp = useCallback(async () => { + setOpen(false) try { const { is_using: isUsedByApp } = await checkIsUsedInApp(dataset.id) setConfirmMessage(isUsedByApp ? t('datasetUsedByApp', { ns: 'dataset' })! : t('deleteDatasetConfirmContent', { ns: 'dataset' })!) @@ -91,10 +93,7 @@ const DropDown = ({ const res = await e.json() toast(res?.message || 'Unknown error', { type: 'error' }) } - finally { - handleTrigger() - } - }, [dataset.id, handleTrigger, t]) + }, [dataset.id, t]) const onConfirmDelete = useCallback(async () => { try { @@ -109,32 +108,27 @@ const DropDown = ({ }, [dataset.id, replace, invalidDatasetList, t]) return ( - - - - + }> + + - - + + - + {showRenameModal && ( - + ) } diff --git a/web/app/components/app/configuration/debug/debug-with-multiple-model/__tests__/debug-item.spec.tsx b/web/app/components/app/configuration/debug/debug-with-multiple-model/__tests__/debug-item.spec.tsx index 19c5fbf3fa..bc433a8f9d 100644 --- a/web/app/components/app/configuration/debug/debug-with-multiple-model/__tests__/debug-item.spec.tsx +++ b/web/app/components/app/configuration/debug/debug-with-multiple-model/__tests__/debug-item.spec.tsx @@ -1,7 +1,7 @@ import type { CSSProperties } from 'react' import type { ModelAndParameter } from '../../types' -import type { Item } from '@/app/components/base/dropdown' -import { fireEvent, render, screen } from '@testing-library/react' +import { render, screen } from '@testing-library/react' +import userEvent from '@testing-library/user-event' import { ModelStatusEnum } from '@/app/components/header/account-setting/model-provider-page/declarations' import { AppModeEnum } from '@/types/app' import DebugItem from '../debug-item' @@ -10,12 +10,6 @@ const mockUseDebugConfigurationContext = vi.fn() const mockUseDebugWithMultipleModelContext = vi.fn() const mockUseProviderContext = vi.fn() -let capturedDropdownProps: { - onSelect: (item: Item) => void - items: Item[] - secondItems?: Item[] -} | null = null - let capturedModelParameterTriggerProps: { modelAndParameter: ModelAndParameter } | null = null @@ -51,34 +45,6 @@ vi.mock('../model-parameter-trigger', () => ({ }, })) -vi.mock('@/app/components/base/dropdown', () => ({ - default: (props: { onSelect: (item: Item) => void, items: Item[], secondItems?: Item[] }) => { - capturedDropdownProps = props - return ( -
- {props.items.map(item => ( - - ))} - {props.secondItems?.map(item => ( - - ))} -
- ) - }, -})) - const createModelAndParameter = (overrides: Partial = {}): ModelAndParameter => ({ id: 'model-1', model: 'gpt-3.5-turbo', @@ -117,7 +83,6 @@ const renderComponent = (props: Partial = {}) => { describe('DebugItem', () => { beforeEach(() => { vi.clearAllMocks() - capturedDropdownProps = null capturedModelParameterTriggerProps = null mockUseDebugConfigurationContext.mockReturnValue({ @@ -137,12 +102,18 @@ describe('DebugItem', () => { }) }) + const openMenu = async () => { + const user = userEvent.setup() + await user.click(screen.getByRole('button')) + return user + } + describe('rendering', () => { it('should render with basic props', () => { renderComponent() expect(screen.getByTestId('model-parameter-trigger')).toBeInTheDocument() - expect(screen.getByTestId('dropdown')).toBeInTheDocument() + expect(screen.getByRole('button')).toBeInTheDocument() }) it('should display correct index number', () => { @@ -280,7 +251,7 @@ describe('DebugItem', () => { }) describe('dropdown menu', () => { - it('should show duplicate option when less than 4 models', () => { + it('should show duplicate option when less than 4 models', async () => { mockUseDebugWithMultipleModelContext.mockReturnValue({ multipleModelConfigs: [createModelAndParameter()], onMultipleModelConfigsChange: vi.fn(), @@ -288,13 +259,12 @@ describe('DebugItem', () => { }) renderComponent() + await openMenu() - expect(capturedDropdownProps?.items).toContainEqual( - expect.objectContaining({ value: 'duplicate' }), - ) + expect(screen.getByText('appDebug.duplicateModel')).toBeInTheDocument() }) - it('should hide duplicate option when 4 or more models', () => { + it('should hide duplicate option when 4 or more models', async () => { mockUseDebugWithMultipleModelContext.mockReturnValue({ multipleModelConfigs: [ createModelAndParameter({ id: '1' }), @@ -307,52 +277,48 @@ describe('DebugItem', () => { }) renderComponent() + await openMenu() - expect(capturedDropdownProps?.items).not.toContainEqual( - expect.objectContaining({ value: 'duplicate' }), - ) + expect(screen.queryByText('appDebug.duplicateModel')).not.toBeInTheDocument() }) - it('should show debug-as-single-model option when provider and model are set', () => { + it('should show debug-as-single-model option when provider and model are set', async () => { renderComponent({ modelAndParameter: createModelAndParameter({ provider: 'openai', model: 'gpt-3.5-turbo', }), }) + await openMenu() - expect(capturedDropdownProps?.items).toContainEqual( - expect.objectContaining({ value: 'debug-as-single-model' }), - ) + expect(screen.getByText('appDebug.debugAsSingleModel')).toBeInTheDocument() }) - it('should hide debug-as-single-model option when provider is missing', () => { + it('should hide debug-as-single-model option when provider is missing', async () => { renderComponent({ modelAndParameter: createModelAndParameter({ provider: '', model: 'gpt-3.5-turbo', }), }) + await openMenu() - expect(capturedDropdownProps?.items).not.toContainEqual( - expect.objectContaining({ value: 'debug-as-single-model' }), - ) + expect(screen.queryByText('appDebug.debugAsSingleModel')).not.toBeInTheDocument() }) - it('should hide debug-as-single-model option when model is missing', () => { + it('should hide debug-as-single-model option when model is missing', async () => { renderComponent({ modelAndParameter: createModelAndParameter({ provider: 'openai', model: '', }), }) + await openMenu() - expect(capturedDropdownProps?.items).not.toContainEqual( - expect.objectContaining({ value: 'debug-as-single-model' }), - ) + expect(screen.queryByText('appDebug.debugAsSingleModel')).not.toBeInTheDocument() }) - it('should show remove option in secondItems when more than 2 models', () => { + it('should show remove option in secondItems when more than 2 models', async () => { mockUseDebugWithMultipleModelContext.mockReturnValue({ multipleModelConfigs: [ createModelAndParameter({ id: '1' }), @@ -364,13 +330,12 @@ describe('DebugItem', () => { }) renderComponent() + await openMenu() - expect(capturedDropdownProps?.secondItems).toContainEqual( - expect.objectContaining({ value: 'remove' }), - ) + expect(screen.getByText('common.operation.remove')).toBeInTheDocument() }) - it('should not show remove option when 2 or fewer models', () => { + it('should not show remove option when 2 or fewer models', async () => { mockUseDebugWithMultipleModelContext.mockReturnValue({ multipleModelConfigs: [ createModelAndParameter({ id: '1' }), @@ -381,13 +346,14 @@ describe('DebugItem', () => { }) renderComponent() + await openMenu() - expect(capturedDropdownProps?.secondItems).toBeUndefined() + expect(screen.queryByText('common.operation.remove')).not.toBeInTheDocument() }) }) describe('dropdown actions', () => { - it('should duplicate model when duplicate is selected', () => { + it('should duplicate model when duplicate is selected', async () => { const onMultipleModelConfigsChange = vi.fn() const originalModel = createModelAndParameter({ id: 'original' }) @@ -399,7 +365,8 @@ describe('DebugItem', () => { renderComponent({ modelAndParameter: originalModel }) - fireEvent.click(screen.getByTestId('dropdown-item-duplicate')) + const user = await openMenu() + await user.click(screen.getByText('appDebug.duplicateModel')) expect(onMultipleModelConfigsChange).toHaveBeenCalledWith( true, @@ -414,7 +381,7 @@ describe('DebugItem', () => { ) }) - it('should not duplicate when already at 4 models', () => { + it('should not duplicate when already at 4 models', async () => { const onMultipleModelConfigsChange = vi.fn() const models = [ createModelAndParameter({ id: '1' }), @@ -430,14 +397,13 @@ describe('DebugItem', () => { }) renderComponent({ modelAndParameter: models[0] }) - - // Since duplicate is not shown when >= 4 models, we need to manually call handleSelect - capturedDropdownProps?.onSelect({ value: 'duplicate', text: 'Duplicate' }) + await openMenu() expect(onMultipleModelConfigsChange).not.toHaveBeenCalled() + expect(screen.queryByText('appDebug.duplicateModel')).not.toBeInTheDocument() }) - it('should call onDebugWithMultipleModelChange when debug-as-single-model is selected', () => { + it('should call onDebugWithMultipleModelChange when debug-as-single-model is selected', async () => { const onDebugWithMultipleModelChange = vi.fn() const modelAndParameter = createModelAndParameter() @@ -449,12 +415,13 @@ describe('DebugItem', () => { renderComponent({ modelAndParameter }) - fireEvent.click(screen.getByTestId('dropdown-item-debug-as-single-model')) + const user = await openMenu() + await user.click(screen.getByText('appDebug.debugAsSingleModel')) expect(onDebugWithMultipleModelChange).toHaveBeenCalledWith(modelAndParameter) }) - it('should remove model when remove is selected', () => { + it('should remove model when remove is selected', async () => { const onMultipleModelConfigsChange = vi.fn() const models = [ createModelAndParameter({ id: '1' }), @@ -470,7 +437,8 @@ describe('DebugItem', () => { renderComponent({ modelAndParameter: models[1] }) - fireEvent.click(screen.getByTestId('dropdown-second-item-remove')) + const user = await openMenu() + await user.click(screen.getByText('common.operation.remove')) expect(onMultipleModelConfigsChange).toHaveBeenCalledWith( true, @@ -478,7 +446,7 @@ describe('DebugItem', () => { ) }) - it('should insert duplicated model at correct position', () => { + it('should insert duplicated model at correct position', async () => { const onMultipleModelConfigsChange = vi.fn() const models = [ createModelAndParameter({ id: '1' }), @@ -495,7 +463,8 @@ describe('DebugItem', () => { // Duplicate the second model renderComponent({ modelAndParameter: models[1] }) - fireEvent.click(screen.getByTestId('dropdown-item-duplicate')) + const user = await openMenu() + await user.click(screen.getByText('appDebug.duplicateModel')) expect(onMultipleModelConfigsChange).toHaveBeenCalledWith( true, diff --git a/web/app/components/app/configuration/debug/debug-with-multiple-model/debug-item.tsx b/web/app/components/app/configuration/debug/debug-with-multiple-model/debug-item.tsx index f7a3112225..4b21616d46 100644 --- a/web/app/components/app/configuration/debug/debug-with-multiple-model/debug-item.tsx +++ b/web/app/components/app/configuration/debug/debug-with-multiple-model/debug-item.tsx @@ -1,9 +1,15 @@ import type { CSSProperties, FC } from 'react' import type { ModelAndParameter } from '../types' -import type { Item } from '@/app/components/base/dropdown' -import { memo } from 'react' +import { memo, useState } from 'react' import { useTranslation } from 'react-i18next' -import Dropdown from '@/app/components/base/dropdown' +import ActionButton from '@/app/components/base/action-button' +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuSeparator, + DropdownMenuTrigger, +} from '@/app/components/base/ui/dropdown-menu' import { ModelStatusEnum } from '@/app/components/header/account-setting/model-provider-page/declarations' import { useDebugConfigurationContext } from '@/context/debug-configuration' import { useProviderContext } from '@/context/provider-context' @@ -35,34 +41,43 @@ const DebugItem: FC = ({ const index = multipleModelConfigs.findIndex(v => v.id === modelAndParameter.id) const currentProvider = textGenerationModelList.find(item => item.provider === modelAndParameter.provider) const currentModel = currentProvider?.models.find(item => item.model === modelAndParameter.model) + const [open, setOpen] = useState(false) - const handleSelect = (item: Item) => { - if (item.value === 'duplicate') { - if (multipleModelConfigs.length >= 4) - return + const handleDuplicate = () => { + setOpen(false) + if (multipleModelConfigs.length >= 4) + return - onMultipleModelConfigsChange( - true, - [ - ...multipleModelConfigs.slice(0, index + 1), - { - ...modelAndParameter, - id: `${Date.now()}`, - }, - ...multipleModelConfigs.slice(index + 1), - ], - ) - } - if (item.value === 'debug-as-single-model') - onDebugWithMultipleModelChange(modelAndParameter) - if (item.value === 'remove') { - onMultipleModelConfigsChange( - true, - multipleModelConfigs.filter(item => item.id !== modelAndParameter.id), - ) - } + onMultipleModelConfigsChange( + true, + [ + ...multipleModelConfigs.slice(0, index + 1), + { + ...modelAndParameter, + id: `${Date.now()}`, + }, + ...multipleModelConfigs.slice(index + 1), + ], + ) } + const handleDebugAsSingleModel = () => { + setOpen(false) + onDebugWithMultipleModelChange(modelAndParameter) + } + + const handleRemove = () => { + setOpen(false) + onMultipleModelConfigsChange( + true, + multipleModelConfigs.filter(item => item.id !== modelAndParameter.id), + ) + } + + const showDuplicate = multipleModelConfigs.length <= 3 + const showDebugAsSingleModel = !!(modelAndParameter.provider && modelAndParameter.model) + const showRemove = multipleModelConfigs.length > 2 + return (
= ({ - 2 - ? [ - { - value: 'remove', - text: t('operation.remove', { ns: 'common' }) as string, - }, - ] - : undefined - } - /> + + }> + + + + + + {showDuplicate && ( + + {t('duplicateModel', { ns: 'appDebug' })} + + )} + {showDebugAsSingleModel && ( + + {t('debugAsSingleModel', { ns: 'appDebug' })} + + )} + {showRemove && ( + <> + {(showDuplicate || showDebugAsSingleModel) && } + + {t('operation.remove', { ns: 'common' })} + + + )} + +
{ diff --git a/web/app/components/base/action-button/index.tsx b/web/app/components/base/action-button/index.tsx index ec84fa06d8..86c8933a8a 100644 --- a/web/app/components/base/action-button/index.tsx +++ b/web/app/components/base/action-button/index.tsx @@ -30,7 +30,7 @@ const actionButtonVariants = cva( }, ) -export type ActionButtonProps = { +type ActionButtonProps = { size?: 'xs' | 's' | 'm' | 'l' | 'xl' state?: ActionButtonState styleCss?: CSSProperties @@ -73,4 +73,4 @@ const ActionButton = ({ className, size, state = ActionButtonState.Default, styl ActionButton.displayName = 'ActionButton' export default ActionButton -export { ActionButton, ActionButtonState, actionButtonVariants } +export { ActionButton, ActionButtonState } diff --git a/web/app/components/base/chat/chat-with-history/header/__tests__/mobile-operation-dropdown.spec.tsx b/web/app/components/base/chat/chat-with-history/header/__tests__/mobile-operation-dropdown.spec.tsx index 295bebecac..525f9b89a5 100644 --- a/web/app/components/base/chat/chat-with-history/header/__tests__/mobile-operation-dropdown.spec.tsx +++ b/web/app/components/base/chat/chat-with-history/header/__tests__/mobile-operation-dropdown.spec.tsx @@ -1,4 +1,4 @@ -import { render, screen } from '@testing-library/react' +import { render, screen, waitFor } from '@testing-library/react' import userEvent from '@testing-library/user-event' import * as React from 'react' import { beforeEach, describe, expect, it, vi } from 'vitest' @@ -53,11 +53,16 @@ describe('MobileOperationDropdown Component', () => { // Reset Chat await user.click(screen.getByText('share.chat.resetChat')) - expect(defaultProps.handleResetChat).toHaveBeenCalledTimes(1) + await waitFor(() => { + expect(defaultProps.handleResetChat).toHaveBeenCalledTimes(1) + }) + await user.click(screen.getByRole('button')) // View Chat Settings await user.click(screen.getByText('share.chat.viewChatSettings')) - expect(defaultProps.handleViewChatSettings).toHaveBeenCalledTimes(1) + await waitFor(() => { + expect(defaultProps.handleViewChatSettings).toHaveBeenCalledTimes(1) + }) }) it('applies hover state to ActionButton when open', async () => { @@ -72,4 +77,16 @@ describe('MobileOperationDropdown Component', () => { await user.click(trigger) expect(trigger).toHaveClass('action-btn-hover') }) + + it('closes the menu after clicking an action', async () => { + const user = userEvent.setup() + render() + + await user.click(screen.getByRole('button')) + await user.click(screen.getByText('share.chat.resetChat')) + + await waitFor(() => { + expect(screen.queryByText('share.chat.resetChat')).not.toBeInTheDocument() + }) + }) }) diff --git a/web/app/components/base/chat/chat-with-history/header/__tests__/operation.spec.tsx b/web/app/components/base/chat/chat-with-history/header/__tests__/operation.spec.tsx index 454f20066e..294d5eebc5 100644 --- a/web/app/components/base/chat/chat-with-history/header/__tests__/operation.spec.tsx +++ b/web/app/components/base/chat/chat-with-history/header/__tests__/operation.spec.tsx @@ -1,4 +1,4 @@ -import { render, screen } from '@testing-library/react' +import { render, screen, waitFor } from '@testing-library/react' import userEvent from '@testing-library/user-event' import * as React from 'react' import { beforeEach, describe, expect, it, vi } from 'vitest' @@ -74,12 +74,18 @@ describe('Operation Component', () => { expect(defaultProps.togglePin).toHaveBeenCalledTimes(1) // Rename + await user.click(screen.getByText('Chat Title')) await user.click(screen.getByText('explore.sidebar.action.rename')) - expect(defaultProps.onRenameConversation).toHaveBeenCalledTimes(1) + await waitFor(() => { + expect(defaultProps.onRenameConversation).toHaveBeenCalledTimes(1) + }) // Delete + await user.click(screen.getByText('Chat Title')) await user.click(screen.getByText('explore.sidebar.action.delete')) - expect(defaultProps.onDelete).toHaveBeenCalledTimes(1) + await waitFor(() => { + expect(defaultProps.onDelete).toHaveBeenCalledTimes(1) + }) }) it('applies hover background when open', async () => { diff --git a/web/app/components/base/chat/chat-with-history/header/mobile-operation-dropdown.tsx b/web/app/components/base/chat/chat-with-history/header/mobile-operation-dropdown.tsx index f8101e1b3a..5d80db7ac3 100644 --- a/web/app/components/base/chat/chat-with-history/header/mobile-operation-dropdown.tsx +++ b/web/app/components/base/chat/chat-with-history/header/mobile-operation-dropdown.tsx @@ -1,7 +1,12 @@ -import { useState } from 'react' +import { useCallback, useState } from 'react' import { useTranslation } from 'react-i18next' import ActionButton, { ActionButtonState } from '@/app/components/base/action-button' -import { PortalToFollowElem, PortalToFollowElemContent, PortalToFollowElemTrigger } from '@/app/components/base/portal-to-follow-elem' +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuTrigger, +} from '@/app/components/base/ui/dropdown-menu' type Props = { handleResetChat: () => void @@ -16,40 +21,45 @@ const MobileOperationDropdown = ({ }: Props) => { const { t } = useTranslation() const [open, setOpen] = useState(false) + const handleMenuAction = useCallback((callback: () => void) => { + setOpen(false) + queueMicrotask(callback) + }, []) return ( - - setOpen(v => !v)} + } data-testid="mobile-more-btn" >
- - -
+ + handleMenuAction(handleResetChat)} > -
- {t('chat.resetChat', { ns: 'share' })} -
- {!hideViewChatSettings && ( -
- {t('chat.viewChatSettings', { ns: 'share' })} -
- )} -
-
- + {t('chat.resetChat', { ns: 'share' })} + + {!hideViewChatSettings && ( + handleMenuAction(handleViewChatSettings)} + > + {t('chat.viewChatSettings', { ns: 'share' })} + + )} + + ) } diff --git a/web/app/components/base/chat/chat-with-history/header/operation.tsx b/web/app/components/base/chat/chat-with-history/header/operation.tsx index d61af1f6a1..a6dd6a0a9e 100644 --- a/web/app/components/base/chat/chat-with-history/header/operation.tsx +++ b/web/app/components/base/chat/chat-with-history/header/operation.tsx @@ -1,14 +1,16 @@ 'use client' -import type { Placement } from '@floating-ui/react' import type { FC } from 'react' +import type { Placement } from '@/app/components/base/ui/placement' import { cn } from '@langgenius/dify-ui/cn' -import { - RiArrowDownSLine, -} from '@remixicon/react' import * as React from 'react' -import { useState } from 'react' +import { useCallback, useState } from 'react' import { useTranslation } from 'react-i18next' -import { PortalToFollowElem, PortalToFollowElemContent, PortalToFollowElemTrigger } from '@/app/components/base/portal-to-follow-elem' +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuTrigger, +} from '@/app/components/base/ui/dropdown-menu' type Props = { title: string @@ -33,42 +35,51 @@ const Operation: FC = ({ }) => { const { t } = useTranslation() const [open, setOpen] = useState(false) + const handleDeferredAction = useCallback((action: () => void) => { + setOpen(false) + queueMicrotask(action) + }, []) return ( - - setOpen(v => !v)} + } >
{title}
- +
-
- -
-
- {isPinned ? t('sidebar.action.unpin', { ns: 'explore' }) : t('sidebar.action.pin', { ns: 'explore' })} -
- {isShowRenameConversation && ( -
- {t('sidebar.action.rename', { ns: 'explore' })} -
- )} - {isShowDelete && ( -
- {t('sidebar.action.delete', { ns: 'explore' })} -
- )} -
-
-
+ + + + {isPinned ? t('sidebar.action.unpin', { ns: 'explore' }) : t('sidebar.action.pin', { ns: 'explore' })} + + {isShowRenameConversation && ( + onRenameConversation && handleDeferredAction(onRenameConversation)} + > + {t('sidebar.action.rename', { ns: 'explore' })} + + )} + {isShowDelete && ( + handleDeferredAction(onDelete)} + > + {t('sidebar.action.delete', { ns: 'explore' })} + + )} + + ) } export default React.memo(Operation) diff --git a/web/app/components/base/chat/chat-with-history/sidebar/__tests__/operation.spec.tsx b/web/app/components/base/chat/chat-with-history/sidebar/__tests__/operation.spec.tsx index e46b54872e..5aa8da7965 100644 --- a/web/app/components/base/chat/chat-with-history/sidebar/__tests__/operation.spec.tsx +++ b/web/app/components/base/chat/chat-with-history/sidebar/__tests__/operation.spec.tsx @@ -1,16 +1,9 @@ -import { render, screen } from '@testing-library/react' +import { render, screen, waitFor } from '@testing-library/react' import userEvent from '@testing-library/user-event' import * as React from 'react' import { beforeEach, describe, expect, it, vi } from 'vitest' import Operation from '../operation' -// Mock PortalToFollowElem components to render children in place -vi.mock('@/app/components/base/portal-to-follow-elem', () => ({ - PortalToFollowElem: ({ children, open }: { children: React.ReactNode, open: boolean }) =>
{children}
, - PortalToFollowElemTrigger: ({ children, onClick }: { children: React.ReactNode, onClick?: () => void }) =>
{children}
, - PortalToFollowElemContent: ({ children }: { children: React.ReactNode }) =>
{children}
, -})) - describe('Operation', () => { const defaultProps = { isActive: false, @@ -72,7 +65,9 @@ describe('Operation', () => { await user.click(screen.getByRole('button')) await user.click(screen.getByText('explore.sidebar.action.rename')) - expect(defaultProps.onRenameConversation).toHaveBeenCalled() + await waitFor(() => { + expect(defaultProps.onRenameConversation).toHaveBeenCalled() + }) }) it('should call onDelete when delete is clicked', async () => { @@ -82,7 +77,9 @@ describe('Operation', () => { await user.click(screen.getByRole('button')) await user.click(screen.getByText('explore.sidebar.action.delete')) - expect(defaultProps.onDelete).toHaveBeenCalled() + await waitFor(() => { + expect(defaultProps.onDelete).toHaveBeenCalled() + }) }) it('should respect visibility props', async () => { @@ -108,8 +105,7 @@ describe('Operation', () => { await user.click(screen.getByRole('button')) - const portalContent = screen.getByTestId('portal-content') - expect(portalContent).toBeInTheDocument() + expect(screen.getByText('explore.sidebar.action.pin')).toBeInTheDocument() }) it('should close dropdown when item hovering stops', async () => { @@ -120,5 +116,60 @@ describe('Operation', () => { expect(screen.getByText('explore.sidebar.action.pin')).toBeInTheDocument() rerender() + + await waitFor(() => { + expect(screen.queryByText('explore.sidebar.action.pin')).not.toBeInTheDocument() + }) + }) + + it('should keep the trigger mounted while visually hidden', () => { + render() + + const trigger = screen.getByRole('button') + expect(trigger).toHaveClass('pointer-events-none') + expect(trigger).toHaveClass('opacity-0') + }) + + it('should safely ignore rename clicks when callback is missing', async () => { + const user = userEvent.setup() + render() + + await user.click(screen.getByRole('button')) + await user.click(screen.getByText('explore.sidebar.action.rename')) + + await waitFor(() => { + expect(screen.queryByText('explore.sidebar.action.rename')).not.toBeInTheDocument() + }) + }) + + it('should not bubble trigger clicks to the parent container', async () => { + const user = userEvent.setup() + const parentClick = vi.fn() + + render( +
+ +
, + ) + + await user.click(screen.getByRole('button')) + + expect(parentClick).not.toHaveBeenCalled() + }) + + it('should not bubble popup clicks to the parent container', async () => { + const user = userEvent.setup() + const parentClick = vi.fn() + + render( +
+ +
, + ) + + await user.click(screen.getByRole('button')) + await user.click(screen.getByRole('menu')) + + expect(parentClick).not.toHaveBeenCalled() }) }) diff --git a/web/app/components/base/chat/chat-with-history/sidebar/operation.tsx b/web/app/components/base/chat/chat-with-history/sidebar/operation.tsx index 18ff3e4d62..611d2bb1b9 100644 --- a/web/app/components/base/chat/chat-with-history/sidebar/operation.tsx +++ b/web/app/components/base/chat/chat-with-history/sidebar/operation.tsx @@ -1,19 +1,17 @@ 'use client' import type { FC } from 'react' import { cn } from '@langgenius/dify-ui/cn' -import { - RiDeleteBinLine, - RiEditLine, - RiMoreFill, - RiPushpinLine, - RiUnpinLine, -} from '@remixicon/react' import { useBoolean } from 'ahooks' import * as React from 'react' -import { useEffect, useRef, useState } from 'react' +import { useCallback, useEffect, useState } from 'react' import { useTranslation } from 'react-i18next' import ActionButton, { ActionButtonState } from '@/app/components/base/action-button' -import { PortalToFollowElem, PortalToFollowElemContent, PortalToFollowElemTrigger } from '@/app/components/base/portal-to-follow-elem' +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuTrigger, +} from '@/app/components/base/ui/dropdown-menu' type Props = { isActive?: boolean @@ -38,24 +36,29 @@ const Operation: FC = ({ }) => { const { t } = useTranslation() const [open, setOpen] = useState(false) - const ref = useRef(null) const [isHovering, { setTrue: setIsHovering, setFalse: setNotHovering }] = useBoolean(false) useEffect(() => { if (!isItemHovering && !isHovering) setOpen(false) }, [isItemHovering, isHovering]) + const handleDeferredAction = useCallback((action?: () => void) => { + if (!action) + return + setOpen(false) + queueMicrotask(action) + }, []) return ( - - setOpen(v => !v)} + } + onClick={e => e.stopPropagation()} > = ({ : ActionButtonState.Default } > - + - - -
+ e.stopPropagation(), + }} + > + { e.stopPropagation() + togglePin() }} > -
- {isPinned && } - {!isPinned && } - {isPinned ? t('sidebar.action.unpin', { ns: 'explore' }) : t('sidebar.action.pin', { ns: 'explore' })} -
- {isShowRenameConversation && ( -
- - {t('sidebar.action.rename', { ns: 'explore' })} -
- )} - {isShowDelete && ( -
- - {t('sidebar.action.delete', { ns: 'explore' })} -
- )} -
-
-
+ {isPinned && } + {!isPinned && } + {isPinned ? t('sidebar.action.unpin', { ns: 'explore' }) : t('sidebar.action.pin', { ns: 'explore' })} + + {isShowRenameConversation && ( + { + e.stopPropagation() + handleDeferredAction(onRenameConversation) + }} + > + + {t('sidebar.action.rename', { ns: 'explore' })} + + )} + {isShowDelete && ( + { + e.stopPropagation() + handleDeferredAction(onDelete) + }} + > + + {t('sidebar.action.delete', { ns: 'explore' })} + + )} + + ) } export default React.memo(Operation) diff --git a/web/app/components/base/dropdown/__tests__/index.spec.tsx b/web/app/components/base/dropdown/__tests__/index.spec.tsx deleted file mode 100644 index 9820554e3d..0000000000 --- a/web/app/components/base/dropdown/__tests__/index.spec.tsx +++ /dev/null @@ -1,225 +0,0 @@ -import { act, cleanup, fireEvent, render, screen } from '@testing-library/react' -import Dropdown from '../index' - -describe('Dropdown Component', () => { - const mockItems = [ - { value: 'option1', text: 'Option 1' }, - { value: 'option2', text: 'Option 2' }, - ] - const mockSecondItems = [ - { value: 'option3', text: 'Option 3' }, - ] - const onSelect = vi.fn() - - afterEach(() => { - cleanup() - vi.clearAllMocks() - }) - - it('renders default trigger properly', () => { - const { container } = render( - , - ) - const trigger = container.querySelector('button') - expect(trigger).toBeInTheDocument() - }) - - it('renders custom trigger when provided', () => { - render( - } - />, - ) - const trigger = screen.getByTestId('custom-trigger') - expect(trigger).toBeInTheDocument() - expect(trigger).toHaveTextContent('Closed') - }) - - it('opens dropdown menu on trigger click and shows items', async () => { - render( - , - ) - const trigger = screen.getByRole('button') - - await act(async () => { - fireEvent.click(trigger) - }) - - // Dropdown items are rendered in a portal (document.body) - expect(screen.getByText('Option 1')).toBeInTheDocument() - expect(screen.getByText('Option 2')).toBeInTheDocument() - }) - - it('calls onSelect and closes dropdown when an item is clicked', async () => { - render( - , - ) - const trigger = screen.getByRole('button') - - await act(async () => { - fireEvent.click(trigger) - }) - - const option1 = screen.getByText('Option 1') - await act(async () => { - fireEvent.click(option1) - }) - - expect(onSelect).toHaveBeenCalledWith(mockItems[0]) - expect(screen.queryByText('Option 1')).not.toBeInTheDocument() - }) - - it('calls onSelect and closes dropdown when a second item is clicked', async () => { - render( - , - ) - - await act(async () => { - fireEvent.click(screen.getByRole('button')) - }) - - const option3 = screen.getByText('Option 3') - await act(async () => { - fireEvent.click(option3) - }) - expect(onSelect).toHaveBeenCalledWith(mockSecondItems[0]) - expect(screen.queryByText('Option 3')).not.toBeInTheDocument() - }) - - it('renders second items and divider when provided', async () => { - render( - , - ) - const trigger = screen.getByRole('button') - - await act(async () => { - fireEvent.click(trigger) - }) - - expect(screen.getByText('Option 1')).toBeInTheDocument() - expect(screen.getByText('Option 3')).toBeInTheDocument() - - // Check for divider (h-px bg-divider-regular) - const divider = document.body.querySelector('.bg-divider-regular.h-px') - expect(divider).toBeInTheDocument() - }) - - it('applies custom classNames', async () => { - const popupClass = 'custom-popup' - const itemClass = 'custom-item' - const secondItemClass = 'custom-second-item' - - render( - , - ) - - await act(async () => { - fireEvent.click(screen.getByRole('button')) - }) - - const popup = document.body.querySelector(`.${popupClass}`) - expect(popup).toBeInTheDocument() - - const items = screen.getAllByText('Option 1') - expect(items[0]).toHaveClass(itemClass) - - const secondItems = screen.getAllByText('Option 3') - expect(secondItems[0]).toHaveClass(secondItemClass) - }) - - it('applies open class to trigger when menu is open', async () => { - render() - const trigger = screen.getByRole('button') - await act(async () => { - fireEvent.click(trigger) - }) - expect(trigger).toHaveClass('bg-divider-regular') - }) - - it('handles JSX elements as item text', async () => { - const itemsWithJSX = [ - { value: 'jsx', text: JSX Content }, - ] - render( - , - ) - - await act(async () => { - fireEvent.click(screen.getByRole('button')) - }) - - expect(screen.getByTestId('jsx-item')).toBeInTheDocument() - expect(screen.getByText('JSX Content')).toBeInTheDocument() - }) - - it('does not render items section if items list is empty', async () => { - render( - , - ) - - await act(async () => { - fireEvent.click(screen.getByRole('button')) - }) - - const p1Divs = document.body.querySelectorAll('.p-1') - expect(p1Divs.length).toBe(1) - expect(screen.queryByText('Option 1')).not.toBeInTheDocument() - expect(screen.getByText('Option 3')).toBeInTheDocument() - }) - - it('does not render divider if only one section is provided', async () => { - const { rerender } = render( - , - ) - await act(async () => { - fireEvent.click(screen.getByRole('button')) - }) - expect(document.body.querySelector('.bg-divider-regular.h-px')).not.toBeInTheDocument() - - await act(async () => { - rerender( - , - ) - }) - expect(document.body.querySelector('.bg-divider-regular.h-px')).not.toBeInTheDocument() - }) - - it('renders nothing if both item lists are empty', async () => { - render() - await act(async () => { - fireEvent.click(screen.getByRole('button')) - }) - const popup = document.body.querySelector('.bg-components-panel-bg') - expect(popup?.children.length).toBe(0) - }) - - it('passes triggerProps to ActionButton and applies custom className', () => { - render( - , - ) - const trigger = screen.getByLabelText('dropdown-trigger') - expect(trigger).toBeDisabled() - expect(trigger).toHaveClass('custom-trigger-class') - }) -}) diff --git a/web/app/components/base/dropdown/index.stories.tsx b/web/app/components/base/dropdown/index.stories.tsx deleted file mode 100644 index 7cb7f820f6..0000000000 --- a/web/app/components/base/dropdown/index.stories.tsx +++ /dev/null @@ -1,88 +0,0 @@ -import type { Meta, StoryObj } from '@storybook/nextjs-vite' -import type { Item } from '.' -import { useState } from 'react' -import { fn } from 'storybook/test' -import Dropdown from '.' - -const PRIMARY_ITEMS: Item[] = [ - { value: 'rename', text: 'Rename' }, - { value: 'duplicate', text: 'Duplicate' }, -] - -const SECONDARY_ITEMS: Item[] = [ - { value: 'archive', text: Archive }, - { value: 'delete', text: Delete }, -] - -const meta = { - title: 'Base/Navigation/Dropdown', - component: Dropdown, - parameters: { - docs: { - description: { - component: 'Small contextual menu with optional destructive section. Uses portal positioning utilities for precise placement.', - }, - }, - }, - tags: ['autodocs'], - args: { - items: PRIMARY_ITEMS, - secondItems: SECONDARY_ITEMS, - }, -} satisfies Meta - -export default meta -type Story = StoryObj - -const DropdownDemo = (props: React.ComponentProps) => { - const [lastAction, setLastAction] = useState('None') - - return ( -
- { - setLastAction(String(item.value)) - props.onSelect?.(item) - }} - /> -
- Last action: - {' '} - {lastAction} -
-
- ) -} - -export const Playground: Story = { - render: args => , - args: { - items: PRIMARY_ITEMS, - secondItems: SECONDARY_ITEMS, - onSelect: fn(), - }, -} - -export const CustomTrigger: Story = { - render: args => ( - ( - - )} - /> - ), - args: { - items: PRIMARY_ITEMS, - onSelect: fn(), - }, -} diff --git a/web/app/components/base/dropdown/index.tsx b/web/app/components/base/dropdown/index.tsx deleted file mode 100644 index f9a19f34ea..0000000000 --- a/web/app/components/base/dropdown/index.tsx +++ /dev/null @@ -1,122 +0,0 @@ -import type { FC } from 'react' -import type { ActionButtonProps } from '@/app/components/base/action-button' -import { cn } from '@langgenius/dify-ui/cn' -import { - RiMoreFill, -} from '@remixicon/react' -import { useState } from 'react' -import ActionButton from '@/app/components/base/action-button' -import { - PortalToFollowElem, - PortalToFollowElemContent, - PortalToFollowElemTrigger, -} from '@/app/components/base/portal-to-follow-elem' - -export type Item = { - value: string | number - text: string | React.JSX.Element -} -type DropdownProps = { - items: Item[] - secondItems?: Item[] - onSelect: (item: Item) => void - renderTrigger?: (open: boolean) => React.ReactNode - triggerProps?: ActionButtonProps - popupClassName?: string - itemClassName?: string - secondItemClassName?: string -} -const Dropdown: FC = ({ - items, - onSelect, - secondItems, - renderTrigger, - triggerProps, - popupClassName, - itemClassName, - secondItemClassName, -}) => { - const [open, setOpen] = useState(false) - - const handleSelect = (item: Item) => { - setOpen(false) - onSelect(item) - } - - return ( - - setOpen(v => !v)}> - { - renderTrigger - ? renderTrigger(open) - : ( - - - - ) - } - - -
- { - !!items.length && ( -
- { - items.map(item => ( -
handleSelect(item)} - > - {item.text} -
- )) - } -
- ) - } - { - (!!items.length && !!secondItems?.length) && ( -
- ) - } - { - !!secondItems?.length && ( -
- { - secondItems.map(item => ( -
handleSelect(item)} - > - {item.text} -
- )) - } -
- ) - } -
- - - ) -} - -export default Dropdown diff --git a/web/app/components/datasets/documents/create-from-pipeline/data-source/online-drive/file-list/header/breadcrumbs/dropdown/__tests__/index.spec.tsx b/web/app/components/datasets/documents/create-from-pipeline/data-source/online-drive/file-list/header/breadcrumbs/dropdown/__tests__/index.spec.tsx index 0157d3cf79..aef07e9be7 100644 --- a/web/app/components/datasets/documents/create-from-pipeline/data-source/online-drive/file-list/header/breadcrumbs/dropdown/__tests__/index.spec.tsx +++ b/web/app/components/datasets/documents/create-from-pipeline/data-source/online-drive/file-list/header/breadcrumbs/dropdown/__tests__/index.spec.tsx @@ -31,10 +31,10 @@ describe('Dropdown', () => { const { container } = render() - // Assert - Button should have RiMoreFill icon (rendered as svg) + // Assert - Button should have the more icon const button = screen.getByRole('button') expect(button).toBeInTheDocument() - expect(container.querySelector('svg')).toBeInTheDocument() + expect(container.querySelector('.i-ri-more-fill')).toBeInTheDocument() }) it('should render separator after dropdown', () => { diff --git a/web/app/components/datasets/documents/create-from-pipeline/data-source/online-drive/file-list/header/breadcrumbs/dropdown/index.tsx b/web/app/components/datasets/documents/create-from-pipeline/data-source/online-drive/file-list/header/breadcrumbs/dropdown/index.tsx index 7178b45b34..c6a90824ab 100644 --- a/web/app/components/datasets/documents/create-from-pipeline/data-source/online-drive/file-list/header/breadcrumbs/dropdown/index.tsx +++ b/web/app/components/datasets/documents/create-from-pipeline/data-source/online-drive/file-list/header/breadcrumbs/dropdown/index.tsx @@ -1,12 +1,11 @@ import { cn } from '@langgenius/dify-ui/cn' -import { RiMoreFill } from '@remixicon/react' import * as React from 'react' import { useCallback, useState } from 'react' import { - PortalToFollowElem, - PortalToFollowElemContent, - PortalToFollowElemTrigger, -} from '@/app/components/base/portal-to-follow-elem' + DropdownMenu, + DropdownMenuContent, + DropdownMenuTrigger, +} from '@/app/components/base/ui/dropdown-menu' import Menu from './menu' type DropdownProps = { @@ -22,26 +21,17 @@ const Dropdown = ({ }: DropdownProps) => { const [open, setOpen] = useState(false) - const handleTrigger = useCallback(() => { - setOpen(prev => !prev) - }, []) - const handleBreadCrumbClick = useCallback((index: number) => { onBreadcrumbClick(index) setOpen(false) }, [onBreadcrumbClick]) return ( - - + }> - - + + - + / - + ) } diff --git a/web/app/components/header/account-setting/data-source-page-new/__tests__/operator.spec.tsx b/web/app/components/header/account-setting/data-source-page-new/__tests__/operator.spec.tsx index 0140626dd7..12042acfa1 100644 --- a/web/app/components/header/account-setting/data-source-page-new/__tests__/operator.spec.tsx +++ b/web/app/components/header/account-setting/data-source-page-new/__tests__/operator.spec.tsx @@ -1,5 +1,6 @@ import type { DataSourceCredential } from '../types' -import { fireEvent, render, screen } from '@testing-library/react' +import { fireEvent, render, screen, waitFor } from '@testing-library/react' +import userEvent from '@testing-library/user-event' import { CredentialTypeEnum } from '@/app/components/plugins/plugin-auth/types' import Operator from '../operator' @@ -9,10 +10,6 @@ import Operator from '../operator' */ // Helper to open dropdown -const openDropdown = () => { - fireEvent.click(screen.getByRole('button')) -} - describe('Operator Component', () => { const mockOnAction = vi.fn() const mockOnRename = vi.fn() @@ -37,7 +34,7 @@ describe('Operator Component', () => { // Act render() - openDropdown() + await userEvent.setup().click(screen.getByRole('button')) // Assert expect(await screen.findByText('plugin.auth.setDefault')).toBeInTheDocument() @@ -53,7 +50,7 @@ describe('Operator Component', () => { // Act render() - openDropdown() + await userEvent.setup().click(screen.getByRole('button')) // Assert expect(await screen.findByText('plugin.auth.setDefault')).toBeInTheDocument() @@ -71,11 +68,13 @@ describe('Operator Component', () => { render() // Act - openDropdown() + await userEvent.setup().click(screen.getByRole('button')) fireEvent.click(await screen.findByText('common.operation.rename')) // Assert - expect(mockOnRename).toHaveBeenCalledTimes(1) + await waitFor(() => { + expect(mockOnRename).toHaveBeenCalledTimes(1) + }) expect(mockOnAction).not.toHaveBeenCalled() }) @@ -85,7 +84,7 @@ describe('Operator Component', () => { render() // Act & Assert - openDropdown() + await userEvent.setup().click(screen.getByRole('button')) const renameBtn = await screen.findByText('common.operation.rename') expect(() => fireEvent.click(renameBtn)).not.toThrow() }) @@ -96,11 +95,13 @@ describe('Operator Component', () => { render() // Act - openDropdown() + await userEvent.setup().click(screen.getByRole('button')) fireEvent.click(await screen.findByText('plugin.auth.setDefault')) // Assert - expect(mockOnAction).toHaveBeenCalledWith('setDefault', credential) + await waitFor(() => { + expect(mockOnAction).toHaveBeenCalledWith('setDefault', credential) + }) }) it('should call onAction for "edit" action', async () => { @@ -109,11 +110,13 @@ describe('Operator Component', () => { render() // Act - openDropdown() + await userEvent.setup().click(screen.getByRole('button')) fireEvent.click(await screen.findByText('common.operation.edit')) // Assert - expect(mockOnAction).toHaveBeenCalledWith('edit', credential) + await waitFor(() => { + expect(mockOnAction).toHaveBeenCalledWith('edit', credential) + }) }) it('should call onAction for "change" action', async () => { @@ -122,11 +125,13 @@ describe('Operator Component', () => { render() // Act - openDropdown() + await userEvent.setup().click(screen.getByRole('button')) fireEvent.click(await screen.findByText('common.dataSource.notion.changeAuthorizedPages')) // Assert - expect(mockOnAction).toHaveBeenCalledWith('change', credential) + await waitFor(() => { + expect(mockOnAction).toHaveBeenCalledWith('change', credential) + }) }) it('should call onAction for "delete" action', async () => { @@ -135,11 +140,13 @@ describe('Operator Component', () => { render() // Act - openDropdown() + await userEvent.setup().click(screen.getByRole('button')) fireEvent.click(await screen.findByText('common.operation.remove')) // Assert - expect(mockOnAction).toHaveBeenCalledWith('delete', credential) + await waitFor(() => { + expect(mockOnAction).toHaveBeenCalledWith('delete', credential) + }) }) }) }) diff --git a/web/app/components/header/account-setting/data-source-page-new/operator.tsx b/web/app/components/header/account-setting/data-source-page-new/operator.tsx index 22303fb8f0..b8aea9fb1d 100644 --- a/web/app/components/header/account-setting/data-source-page-new/operator.tsx +++ b/web/app/components/header/account-setting/data-source-page-new/operator.tsx @@ -1,21 +1,20 @@ import type { DataSourceCredential, } from './types' -import type { Item } from '@/app/components/base/dropdown' -import { - RiDeleteBinLine, - RiEditLine, - RiEqualizer2Line, - RiHome9Line, - RiStickyNoteAddLine, -} from '@remixicon/react' import { memo, useCallback, - useMemo, + useState, } from 'react' import { useTranslation } from 'react-i18next' -import Dropdown from '@/app/components/base/dropdown' +import ActionButton from '@/app/components/base/action-button' +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuSeparator, + DropdownMenuTrigger, +} from '@/app/components/base/ui/dropdown-menu' import { CredentialTypeEnum } from '@/app/components/plugins/plugin-auth/types' type OperatorProps = { @@ -29,106 +28,60 @@ const Operator = ({ onRename, }: OperatorProps) => { const { t } = useTranslation() + const [open, setOpen] = useState(false) const { type, } = credentialItem - const items = useMemo(() => { - const commonItems = [ - { - value: 'setDefault', - text: ( -
- -
{t('auth.setDefault', { ns: 'plugin' })}
-
- ), - }, - ...( - type === CredentialTypeEnum.OAUTH2 - ? [ - { - value: 'rename', - text: ( -
- -
{t('operation.rename', { ns: 'common' })}
-
- ), - }, - ] - : [] - ), - ...( - type === CredentialTypeEnum.API_KEY - ? [ - { - value: 'edit', - text: ( -
- -
{t('operation.edit', { ns: 'common' })}
-
- ), - }, - ] - : [] - ), - ] - if (type === CredentialTypeEnum.OAUTH2) { - const oAuthItems = [ - { - value: 'change', - text: ( -
- -
{t('dataSource.notion.changeAuthorizedPages', { ns: 'common' })}
-
- ), - }, - ] - commonItems.push(...oAuthItems) - } - return commonItems - }, [t, type]) - - const secondItems = useMemo(() => { - return [ - { - value: 'delete', - text: ( -
- -
- {t('operation.remove', { ns: 'common' })} -
-
- ), - }, - ] - }, []) - const handleSelect = useCallback((item: Item) => { - if (item.value === 'rename') { - onRename?.() - return - } - onAction( - item.value as string, - credentialItem, - ) - }, [onAction, credentialItem, onRename]) + const handleAction = useCallback((action: string) => { + setOpen(false) + queueMicrotask(() => { + if (action === 'rename') { + onRename?.() + return + } + onAction(action, credentialItem) + }) + }, [credentialItem, onAction, onRename]) return ( - + + }> + + + + + + handleAction('setDefault')}> + +
{t('auth.setDefault', { ns: 'plugin' })}
+
+ {type === CredentialTypeEnum.OAUTH2 && ( + handleAction('rename')}> + +
{t('operation.rename', { ns: 'common' })}
+
+ )} + {type === CredentialTypeEnum.API_KEY && ( + handleAction('edit')}> + +
{t('operation.edit', { ns: 'common' })}
+
+ )} + {type === CredentialTypeEnum.OAUTH2 && ( + handleAction('change')}> + +
{t('dataSource.notion.changeAuthorizedPages', { ns: 'common' })}
+
+ )} + + handleAction('delete')}> + +
+ {t('operation.remove', { ns: 'common' })} +
+
+
+
) } diff --git a/web/app/components/plugins/plugin-page/plugin-tasks/__tests__/index.spec.tsx b/web/app/components/plugins/plugin-page/plugin-tasks/__tests__/index.spec.tsx index 56992b377f..8c33894929 100644 --- a/web/app/components/plugins/plugin-page/plugin-tasks/__tests__/index.spec.tsx +++ b/web/app/components/plugins/plugin-page/plugin-tasks/__tests__/index.spec.tsx @@ -680,6 +680,26 @@ describe('PluginTasks Component', () => { }) }) + it('should close the menu after clearing the last non-running plugins', async () => { + setupMocks([ + createMockPlugin({ status: TaskStatus.success, plugin_unique_identifier: 'success-1' }), + ]) + + render() + + fireEvent.click(document.getElementById('plugin-task-trigger')!) + + await waitFor(() => { + expect(document.querySelector('.w-\\[360px\\]')).toBeInTheDocument() + }) + + fireEvent.click(screen.getByRole('button', { name: /task\.clearAll/i })) + + await waitFor(() => { + expect(document.querySelector('.w-\\[360px\\]')).not.toBeInTheDocument() + }) + }) + it('should clear only error plugins when onClearErrors is called', async () => { const { mockMutateAsync } = setupMocks([ createMockPlugin({ status: TaskStatus.failed, plugin_unique_identifier: 'error-1' }), @@ -792,6 +812,30 @@ describe('PluginTasks Component', () => { expect(document.querySelector('.w-\\[360px\\]')).toBeInTheDocument() }) + + it('should open for installing-with-success state', () => { + setupMocks([ + createMockPlugin({ status: TaskStatus.running, plugin_unique_identifier: 'running-1' }), + createMockPlugin({ status: TaskStatus.success, plugin_unique_identifier: 'success-1' }), + ]) + + render() + fireEvent.click(document.getElementById('plugin-task-trigger')!) + + expect(document.querySelector('.w-\\[360px\\]')).toBeInTheDocument() + }) + + it('should open for installing-with-error state', () => { + setupMocks([ + createMockPlugin({ status: TaskStatus.running, plugin_unique_identifier: 'running-1' }), + createMockPlugin({ status: TaskStatus.failed, plugin_unique_identifier: 'failed-1' }), + ]) + + render() + fireEvent.click(document.getElementById('plugin-task-trigger')!) + + expect(document.querySelector('.w-\\[360px\\]')).toBeInTheDocument() + }) }) }) diff --git a/web/app/components/plugins/plugin-page/plugin-tasks/index.tsx b/web/app/components/plugins/plugin-page/plugin-tasks/index.tsx index b96b1ec07c..5acd193a82 100644 --- a/web/app/components/plugins/plugin-page/plugin-tasks/index.tsx +++ b/web/app/components/plugins/plugin-page/plugin-tasks/index.tsx @@ -5,10 +5,10 @@ import { } from 'react' import { useTranslation } from 'react-i18next' import { - PortalToFollowElem, - PortalToFollowElemContent, - PortalToFollowElemTrigger, -} from '@/app/components/base/portal-to-follow-elem' + DropdownMenu, + DropdownMenuContent, + DropdownMenuTrigger, +} from '@/app/components/base/ui/dropdown-menu' import useGetIcon from '@/app/components/plugins/install-plugin/base/use-get-icon' import PluginTaskList from './components/plugin-task-list' import TaskStatusIndicator from './components/task-status-indicator' @@ -96,16 +96,14 @@ const PluginTasks = () => { return (
- - + } + onClick={handleTriggerClick} + > { totalPluginsLength={totalPluginsLength} onClick={() => {}} /> - - + + { onClearErrors={handleClearErrors} onClearSingle={handleClearSingle} /> - - + +
) } diff --git a/web/app/components/share/text-generation/__tests__/menu-dropdown.spec.tsx b/web/app/components/share/text-generation/__tests__/menu-dropdown.spec.tsx index 7ccd788cb0..edad871855 100644 --- a/web/app/components/share/text-generation/__tests__/menu-dropdown.spec.tsx +++ b/web/app/components/share/text-generation/__tests__/menu-dropdown.spec.tsx @@ -3,6 +3,27 @@ import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-libra import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import MenuDropdown from '../menu-dropdown' +vi.mock('../info-modal', () => ({ + default: ({ + isShow, + onClose, + data, + }: { + isShow: boolean + onClose: () => void + data?: SiteInfo + }) => { + if (!isShow) + return null + return ( +
+ {data?.title} + +
+ ) + }, +})) + const mockReplace = vi.fn() const mockPathname = '/test-path' vi.mock('@/next/navigation', () => ({ @@ -191,6 +212,25 @@ describe('MenuDropdown', () => { expect(screen.getByText('Test App')).toBeInTheDocument() }) }) + + it('should close InfoModal when the close handler runs', async () => { + render() + + fireEvent.click(screen.getByRole('button')) + await waitFor(() => { + expect(screen.getByText('common.userProfile.about')).toBeInTheDocument() + }) + + fireEvent.click(screen.getByText('common.userProfile.about')) + await waitFor(() => { + expect(screen.getByTestId('info-modal')).toBeInTheDocument() + }) + + fireEvent.click(screen.getByText('Close Info')) + await waitFor(() => { + expect(screen.queryByTestId('info-modal')).not.toBeInTheDocument() + }) + }) }) describe('forceClose prop', () => { diff --git a/web/app/components/share/text-generation/menu-dropdown.tsx b/web/app/components/share/text-generation/menu-dropdown.tsx index bc8323676c..d50a0d77de 100644 --- a/web/app/components/share/text-generation/menu-dropdown.tsx +++ b/web/app/components/share/text-generation/menu-dropdown.tsx @@ -1,26 +1,25 @@ 'use client' -import type { Placement } from '@floating-ui/react' import type { FC } from 'react' +import type { Placement } from '@/app/components/base/ui/placement' import type { SiteInfo } from '@/models/share' import { cn } from '@langgenius/dify-ui/cn' -import { - RiEqualizer2Line, -} from '@remixicon/react' import * as React from 'react' -import { useCallback, useEffect, useRef, useState } from 'react' +import { useCallback, useEffect, useState } from 'react' import { useTranslation } from 'react-i18next' import ActionButton from '@/app/components/base/action-button' -import { - PortalToFollowElem, - PortalToFollowElemContent, - PortalToFollowElemTrigger, -} from '@/app/components/base/portal-to-follow-elem' import ThemeSwitcher from '@/app/components/base/theme-switcher' +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuLinkItem, + DropdownMenuSeparator, + DropdownMenuTrigger, +} from '@/app/components/base/ui/dropdown-menu' import { useWebAppStore } from '@/context/web-app-context' import { AccessMode } from '@/models/access-control' import { usePathname, useRouter } from '@/next/navigation' import { webAppLogout } from '@/service/webapp-auth' -import Divider from '../../base/divider' import InfoModal from './info-modal' type Props = { @@ -40,24 +39,22 @@ const MenuDropdown: FC = ({ const router = useRouter() const pathname = usePathname() const { t } = useTranslation() - const [open, doSetOpen] = useState(false) - const openRef = useRef(open) - const setOpen = useCallback((v: boolean) => { - doSetOpen(v) - openRef.current = v - }, [doSetOpen]) - - const handleTrigger = useCallback(() => { - setOpen(!openRef.current) - }, [setOpen]) + const [open, setOpen] = useState(false) const shareCode = useWebAppStore(s => s.shareCode) const handleLogout = useCallback(async () => { + setOpen(false) await webAppLogout(shareCode!) router.replace(`/webapp-signin?redirect_url=${pathname}`) - }, [router, pathname, webAppLogout, shareCode]) + }, [pathname, router, setOpen, shareCode]) const [show, setShow] = useState(false) + const handleOpenInfoModal = useCallback(() => { + setOpen(false) + queueMicrotask(() => { + setShow(true) + }) + }, []) useEffect(() => { if (forceClose) @@ -66,60 +63,56 @@ const MenuDropdown: FC = ({ return ( <> - - -
- - - -
-
- -
-
-
-
{t('theme.theme', { ns: 'common' })}
- -
+ } + aria-label={t('operation.more', { ns: 'common' })} + > + + + + + +
+
+
{t('theme.theme', { ns: 'common' })}
+
- -
- {data?.privacy_policy && ( - - {t('chat.privacyPolicyMiddle', { ns: 'share' })} - - )} -
{ - handleTrigger() - setShow(true) - }} - className="cursor-pointer rounded-lg px-3 py-1.5 system-md-regular text-text-secondary hover:bg-state-base-hover" - > - {t('userProfile.about', { ns: 'common' })} -
-
- {!(hideLogout || webAppAccessMode === AccessMode.EXTERNAL_MEMBERS || webAppAccessMode === AccessMode.PUBLIC) && ( -
-
- {t('userProfile.logout', { ns: 'common' })} -
-
- )}
- - + + {data?.privacy_policy && ( + + {t('chat.privacyPolicyMiddle', { ns: 'share' })} + + )} + + {t('userProfile.about', { ns: 'common' })} + + {!(hideLogout || webAppAccessMode === AccessMode.EXTERNAL_MEMBERS || webAppAccessMode === AccessMode.PUBLIC) && ( + + {t('userProfile.logout', { ns: 'common' })} + + )} +
+ {show && ( ({ + useTheme: () => ({ + theme: 'light', + }), +})) + +vi.mock('@/service/use-plugins', () => ({ + useDownloadPlugin: vi.fn(), +})) + +vi.mock('@/utils/download', () => ({ + downloadBlob: (...args: unknown[]) => mockDownloadBlob(...args), +})) + +vi.mock('@/utils/var', () => ({ + getMarketplaceUrl: (path: string) => `https://marketplace.example${path}`, +})) + +const createQueryClient = () => new QueryClient({ + defaultOptions: { + queries: { + retry: false, + }, + }, +}) + +const renderComponent = (props?: Partial>) => { + const queryClient = createQueryClient() + vi.spyOn(queryClient, 'removeQueries').mockImplementation(((...args) => { + return mockRemoveQueries(...args) + }) as typeof queryClient.removeQueries) + + return render( + + + , + ) +} + +describe('OperationDropdown', () => { + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(useDownloadPlugin).mockImplementation((_, enabled) => ({ + data: enabled ? null : null, + isLoading: false, + }) as unknown as ReturnType) + }) + + it('should render download and view details actions when opened', async () => { + renderComponent({ open: true }) + + expect(screen.getByText('common.operation.download')).toBeInTheDocument() + expect(screen.getByText('common.operation.viewDetails')).toBeInTheDocument() + }) + + it('should request a download when download is clicked', async () => { + const onOpenChange = vi.fn() + renderComponent({ open: true, onOpenChange }) + + await userEvent.setup().click(screen.getByText('common.operation.download')) + + expect(onOpenChange).toHaveBeenCalledWith(false) + expect(mockRemoveQueries).toHaveBeenCalled() + }) + + it('should skip download when already loading', async () => { + vi.mocked(useDownloadPlugin).mockReturnValue({ + data: null, + isLoading: true, + } as unknown as ReturnType) + + renderComponent({ open: true }) + + await userEvent.setup().click(screen.getByText('common.operation.download')) + + expect(mockRemoveQueries).not.toHaveBeenCalled() + }) + + it('should download the blob when the hook returns data', async () => { + vi.mocked(useDownloadPlugin).mockImplementation((_, enabled) => ({ + data: enabled ? new Blob(['plugin zip'], { type: 'application/zip' }) : null, + isLoading: false, + }) as unknown as ReturnType) + + renderComponent({ open: true }) + + await userEvent.setup().click(screen.getByText('common.operation.download')) + + await waitFor(() => { + expect(mockDownloadBlob).toHaveBeenCalledWith({ + data: expect.any(Blob), + fileName: 'langgenius-test-plugin_1.0.0.zip', + }) + }) + expect(mockRemoveQueries).toHaveBeenCalled() + }) + + it('should link to the marketplace detail page', () => { + renderComponent({ open: true }) + + expect(screen.getByRole('menuitem', { name: 'common.operation.viewDetails' })).toHaveAttribute( + 'href', + 'https://marketplace.example/plugins/langgenius/test-plugin', + ) + }) +}) diff --git a/web/app/components/workflow/block-selector/market-place-plugin/action.tsx b/web/app/components/workflow/block-selector/market-place-plugin/action.tsx index 4ae623ffc1..a058e8c051 100644 --- a/web/app/components/workflow/block-selector/market-place-plugin/action.tsx +++ b/web/app/components/workflow/block-selector/market-place-plugin/action.tsx @@ -1,19 +1,19 @@ 'use client' import type { FC } from 'react' import { cn } from '@langgenius/dify-ui/cn' -import { RiMoreFill } from '@remixicon/react' import { useQueryClient } from '@tanstack/react-query' import { useTheme } from 'next-themes' import * as React from 'react' -import { useCallback, useEffect, useMemo, useRef, useState } from 'react' +import { useCallback, useEffect, useMemo, useState } from 'react' import { useTranslation } from 'react-i18next' import ActionButton from '@/app/components/base/action-button' -// import { Button } from '@/app/components/base/ui/button' import { - PortalToFollowElem, - PortalToFollowElemContent, - PortalToFollowElemTrigger, -} from '@/app/components/base/portal-to-follow-elem' + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuLinkItem, + DropdownMenuTrigger, +} from '@/app/components/base/ui/dropdown-menu' import { useDownloadPlugin } from '@/service/use-plugins' import { downloadBlob } from '@/utils/download' import { getMarketplaceUrl } from '@/utils/var' @@ -36,16 +36,10 @@ const OperationDropdown: FC = ({ const { t } = useTranslation() const { theme } = useTheme() const queryClient = useQueryClient() - const openRef = useRef(open) - const setOpen = useCallback((v: boolean) => { - onOpenChange(v) - openRef.current = v + const setOpen = useCallback((value: boolean) => { + onOpenChange(value) }, [onOpenChange]) - const handleTrigger = useCallback(() => { - setOpen(!openRef.current) - }, [setOpen]) - const [needDownload, setNeedDownload] = useState(false) const downloadInfo = useMemo(() => ({ organization: author, @@ -56,12 +50,13 @@ const OperationDropdown: FC = ({ const handleDownload = useCallback(() => { if (isLoading) return + setOpen(false) queryClient.removeQueries({ queryKey: ['plugins', 'downloadPlugin', downloadInfo], exact: true, }) setNeedDownload(true) - }, [downloadInfo, isLoading, queryClient]) + }, [downloadInfo, isLoading, queryClient, setOpen]) useEffect(() => { if (!needDownload || !blob) @@ -75,27 +70,33 @@ const OperationDropdown: FC = ({ }) }, [author, blob, downloadInfo, name, needDownload, queryClient, version]) return ( - - + }> - + - - -
-
{t('operation.download', { ns: 'common' })}
- {t('operation.viewDetails', { ns: 'common' })} -
-
-
+ + + + {t('operation.download', { ns: 'common' })} + + + {t('operation.viewDetails', { ns: 'common' })} + + + ) } export default React.memo(OperationDropdown) diff --git a/web/docs/overlay-migration.md b/web/docs/overlay-migration.md index 6762c2bfb1..339db8945b 100644 --- a/web/docs/overlay-migration.md +++ b/web/docs/overlay-migration.md @@ -9,7 +9,6 @@ This document tracks the migration away from legacy overlay APIs. - `@/app/components/base/tooltip` - `@/app/components/base/modal` - `@/app/components/base/select` (including `custom` / `pure`) - - `@/app/components/base/dropdown` - `@/app/components/base/dialog` - `@/app/components/base/toast` (including `context`) - Replacement primitives: diff --git a/web/eslint.constants.mjs b/web/eslint.constants.mjs index fb912d6524..57a14fc22d 100644 --- a/web/eslint.constants.mjs +++ b/web/eslint.constants.mjs @@ -52,13 +52,6 @@ export const OVERLAY_RESTRICTED_IMPORT_PATTERNS = [ ], message: 'Deprecated: use @/app/components/base/ui/select instead. See issue #32767.', }, - { - group: [ - '**/base/dropdown', - '**/base/dropdown/index', - ], - message: 'Deprecated: use @/app/components/base/ui/dropdown-menu instead. See issue #32767.', - }, { group: [ '**/base/dialog', @@ -80,7 +73,6 @@ export const OVERLAY_MIGRATION_LEGACY_BASE_FILES = [ 'app/components/base/chip/index.tsx', 'app/components/base/date-and-time-picker/date-picker/index.tsx', 'app/components/base/date-and-time-picker/time-picker/index.tsx', - 'app/components/base/dropdown/index.tsx', 'app/components/base/features/new-feature-panel/file-upload/setting-modal.tsx', 'app/components/base/features/new-feature-panel/text-to-speech/voice-settings.tsx', 'app/components/base/file-uploader/file-from-link-or-local/index.tsx',