diff --git a/web/app/components/datasets/list/dataset-card/__tests__/index.spec.tsx b/web/app/components/datasets/list/dataset-card/__tests__/index.spec.tsx index 21ddda5ce6..29139da114 100644 --- a/web/app/components/datasets/list/dataset-card/__tests__/index.spec.tsx +++ b/web/app/components/datasets/list/dataset-card/__tests__/index.spec.tsx @@ -7,8 +7,6 @@ import { ChunkingMode, DatasetPermission, DataSourceType } from '@/models/datase import DatasetCardFooter from '../components/dataset-card-footer' import Description from '../components/description' import DatasetCard from '../index' -import OperationItem from '../operation-item' -import Operations from '../operations' // Mock external hooks only vi.mock('@/hooks/use-format-time-from-now', () => ({ @@ -62,8 +60,8 @@ vi.mock('../components/tag-area', () => ({
)), })) -vi.mock('../components/operations-popover', () => ({ - default: () =>
, +vi.mock('../components/operations-dropdown', () => ({ + default: () =>
, })) // Factory function for DataSet mock data @@ -233,152 +231,6 @@ describe('DatasetCard Integration', () => { }) }) }) - - // Integration tests for OperationItem component - describe('OperationItem', () => { - const MockIcon = ({ className }: { className?: string }) => ( - - ) - - describe('Rendering', () => { - it('should render icon and name', () => { - render() - expect(screen.getByText('Edit')).toBeInTheDocument() - expect(screen.getByTestId('mock-icon')).toBeInTheDocument() - }) - }) - - describe('User Interactions', () => { - it('should call handleClick when clicked', () => { - const handleClick = vi.fn() - render() - - const item = screen.getByText('Delete').closest('div') - fireEvent.click(item!) - - expect(handleClick).toHaveBeenCalledTimes(1) - }) - - it('should prevent default and stop propagation on click', () => { - const handleClick = vi.fn() - render() - - const item = screen.getByText('Action').closest('div') - const event = new MouseEvent('click', { bubbles: true, cancelable: true }) - const preventDefaultSpy = vi.spyOn(event, 'preventDefault') - const stopPropagationSpy = vi.spyOn(event, 'stopPropagation') - - item!.dispatchEvent(event) - - expect(preventDefaultSpy).toHaveBeenCalled() - expect(stopPropagationSpy).toHaveBeenCalled() - }) - }) - - describe('Edge Cases', () => { - it('should not throw when handleClick is undefined', () => { - render() - const item = screen.getByText('No handler').closest('div') - expect(() => { - fireEvent.click(item!) - }).not.toThrow() - }) - - it('should handle empty name', () => { - render() - expect(screen.getByTestId('mock-icon')).toBeInTheDocument() - }) - }) - }) - - // Integration tests for Operations component - describe('Operations', () => { - const defaultProps = { - showDelete: true, - showExportPipeline: true, - openRenameModal: vi.fn(), - handleExportPipeline: vi.fn(), - detectIsUsedByApp: vi.fn(), - } - - describe('Rendering', () => { - it('should always render edit operation', () => { - render() - expect(screen.getByText(/operation\.edit/)).toBeInTheDocument() - }) - - it('should render export pipeline when showExportPipeline is true', () => { - render() - expect(screen.getByText(/exportPipeline/)).toBeInTheDocument() - }) - - it('should not render export pipeline when showExportPipeline is false', () => { - render() - expect(screen.queryByText(/exportPipeline/)).not.toBeInTheDocument() - }) - - it('should render delete when showDelete is true', () => { - render() - expect(screen.getByText(/operation\.delete/)).toBeInTheDocument() - }) - - it('should not render delete when showDelete is false', () => { - render() - expect(screen.queryByText(/operation\.delete/)).not.toBeInTheDocument() - }) - }) - - describe('User Interactions', () => { - it('should call openRenameModal when edit is clicked', () => { - const openRenameModal = vi.fn() - render() - - const editItem = screen.getByText(/operation\.edit/).closest('div') - fireEvent.click(editItem!) - - expect(openRenameModal).toHaveBeenCalledTimes(1) - }) - - it('should call handleExportPipeline when export is clicked', () => { - const handleExportPipeline = vi.fn() - render() - - const exportItem = screen.getByText(/exportPipeline/).closest('div') - fireEvent.click(exportItem!) - - expect(handleExportPipeline).toHaveBeenCalledTimes(1) - }) - - it('should call detectIsUsedByApp when delete is clicked', () => { - const detectIsUsedByApp = vi.fn() - render() - - const deleteItem = screen.getByText(/operation\.delete/).closest('div') - fireEvent.click(deleteItem!) - - expect(detectIsUsedByApp).toHaveBeenCalledTimes(1) - }) - }) - - describe('Edge Cases', () => { - it('should render only edit when both showDelete and showExportPipeline are false', () => { - render() - expect(screen.getByText(/operation\.edit/)).toBeInTheDocument() - expect(screen.queryByText(/exportPipeline/)).not.toBeInTheDocument() - expect(screen.queryByText(/operation\.delete/)).not.toBeInTheDocument() - }) - - it('should render divider before delete section when showDelete is true', () => { - const { container } = render() - expect(container.querySelector('.bg-divider-subtle')).toBeInTheDocument() - }) - - it('should not render divider when showDelete is false', () => { - const { container } = render() - expect(container.querySelector('.bg-divider-subtle')).toBeNull() - }) - }) - }) }) describe('DatasetCard Component', () => { diff --git a/web/app/components/datasets/list/dataset-card/__tests__/operations.spec.tsx b/web/app/components/datasets/list/dataset-card/__tests__/operations.spec.tsx index edb54cba0c..a6765c0ebe 100644 --- a/web/app/components/datasets/list/dataset-card/__tests__/operations.spec.tsx +++ b/web/app/components/datasets/list/dataset-card/__tests__/operations.spec.tsx @@ -1,7 +1,16 @@ import { fireEvent, render, screen } from '@testing-library/react' import { describe, expect, it, vi } from 'vitest' +import { DropdownMenu } from '@/app/components/base/ui/dropdown-menu' import Operations from '../operations' +function renderInMenu(ui: React.ReactElement) { + return render( + + {ui} + , + ) +} + describe('Operations', () => { const defaultProps = { showDelete: true, @@ -17,100 +26,65 @@ describe('Operations', () => { describe('Rendering', () => { it('should render without crashing', () => { - render() - // Edit operation should always be visible + renderInMenu() expect(screen.getByText(/operation\.edit/)).toBeInTheDocument() }) it('should render edit operation', () => { - render() + renderInMenu() expect(screen.getByText(/operation\.edit/)).toBeInTheDocument() }) it('should render export pipeline operation when showExportPipeline is true', () => { - render() + renderInMenu() expect(screen.getByText(/exportPipeline/)).toBeInTheDocument() }) it('should not render export pipeline operation when showExportPipeline is false', () => { - render() + renderInMenu() expect(screen.queryByText(/exportPipeline/)).not.toBeInTheDocument() }) it('should render delete operation when showDelete is true', () => { - render() + renderInMenu() expect(screen.getByText(/operation\.delete/)).toBeInTheDocument() }) it('should not render delete operation when showDelete is false', () => { - render() + renderInMenu() expect(screen.queryByText(/operation\.delete/)).not.toBeInTheDocument() }) }) - describe('Props', () => { - it('should render divider when showDelete is true', () => { - const { container } = render() - const divider = container.querySelector('.bg-divider-subtle') - expect(divider).toBeInTheDocument() - }) - - it('should not render divider when showDelete is false', () => { - const { container } = render() - // Should not have the divider-subtle one (the separator before delete) - expect(container.querySelector('.bg-divider-subtle')).toBeNull() - }) - }) - describe('User Interactions', () => { it('should call openRenameModal when edit is clicked', () => { const openRenameModal = vi.fn() - render() - - const editItem = screen.getByText(/operation\.edit/).closest('div') - fireEvent.click(editItem!) + renderInMenu() + fireEvent.click(screen.getByText(/operation\.edit/)) expect(openRenameModal).toHaveBeenCalledTimes(1) }) it('should call handleExportPipeline when export is clicked', () => { const handleExportPipeline = vi.fn() - render() - - const exportItem = screen.getByText(/exportPipeline/).closest('div') - fireEvent.click(exportItem!) + renderInMenu() + fireEvent.click(screen.getByText(/exportPipeline/)) expect(handleExportPipeline).toHaveBeenCalledTimes(1) }) it('should call detectIsUsedByApp when delete is clicked', () => { const detectIsUsedByApp = vi.fn() - render() - - const deleteItem = screen.getByText(/operation\.delete/).closest('div') - fireEvent.click(deleteItem!) + renderInMenu() + fireEvent.click(screen.getByText(/operation\.delete/)) expect(detectIsUsedByApp).toHaveBeenCalledTimes(1) }) }) - describe('Styles', () => { - it('should have correct container styling', () => { - const { container } = render() - const operationsContainer = container.firstChild - expect(operationsContainer).toHaveClass( - 'relative', - 'flex', - 'w-full', - 'flex-col', - 'rounded-xl', - ) - }) - }) - describe('Edge Cases', () => { it('should render only edit when both showDelete and showExportPipeline are false', () => { - render() + renderInMenu() expect(screen.getByText(/operation\.edit/)).toBeInTheDocument() expect(screen.queryByText(/exportPipeline/)).not.toBeInTheDocument() expect(screen.queryByText(/operation\.delete/)).not.toBeInTheDocument() diff --git a/web/app/components/datasets/list/dataset-card/components/__tests__/corner-labels.spec.tsx b/web/app/components/datasets/list/dataset-card/components/__tests__/corner-labels.spec.tsx index 7fff6f4dc1..b68f018e72 100644 --- a/web/app/components/datasets/list/dataset-card/components/__tests__/corner-labels.spec.tsx +++ b/web/app/components/datasets/list/dataset-card/components/__tests__/corner-labels.spec.tsx @@ -80,7 +80,7 @@ describe('CornerLabels', () => { const dataset = createMockDataset({ embedding_available: false }) const { container } = render() const labelContainer = container.firstChild as HTMLElement - expect(labelContainer).toHaveClass('absolute', 'right-0', 'top-0', 'z-10') + expect(labelContainer).toHaveClass('absolute', 'right-0', 'top-0', 'z-5') }) it('should have correct positioning for pipeline label', () => { @@ -90,7 +90,7 @@ describe('CornerLabels', () => { }) const { container } = render() const labelContainer = container.firstChild as HTMLElement - expect(labelContainer).toHaveClass('absolute', 'right-0', 'top-0', 'z-10') + expect(labelContainer).toHaveClass('absolute', 'right-0', 'top-0', 'z-5') }) }) diff --git a/web/app/components/datasets/list/dataset-card/components/__tests__/operations-popover.spec.tsx b/web/app/components/datasets/list/dataset-card/components/__tests__/operations-dropdown.spec.tsx similarity index 51% rename from web/app/components/datasets/list/dataset-card/components/__tests__/operations-popover.spec.tsx rename to web/app/components/datasets/list/dataset-card/components/__tests__/operations-dropdown.spec.tsx index 0d53bb1315..690e01113f 100644 --- a/web/app/components/datasets/list/dataset-card/components/__tests__/operations-popover.spec.tsx +++ b/web/app/components/datasets/list/dataset-card/components/__tests__/operations-dropdown.spec.tsx @@ -1,11 +1,11 @@ import type { DataSet } from '@/models/datasets' -import { fireEvent, render } from '@testing-library/react' +import { fireEvent, render, screen } from '@testing-library/react' import { describe, expect, it, vi } from 'vitest' import { IndexingType } from '@/app/components/datasets/create/step-two' import { ChunkingMode, DatasetPermission, DataSourceType } from '@/models/datasets' -import OperationsPopover from '../operations-popover' +import OperationsDropdown from '../operations-dropdown' -describe('OperationsPopover', () => { +describe('OperationsDropdown', () => { const createMockDataset = (overrides: Partial = {}): DataSet => ({ id: 'dataset-1', name: 'Test Dataset', @@ -42,102 +42,143 @@ describe('OperationsPopover', () => { describe('Rendering', () => { it('should render without crashing', () => { - const { container } = render() + const { container } = render() expect(container.firstChild).toBeInTheDocument() }) it('should render the more icon button', () => { - const { container } = render() - const moreIcon = container.querySelector('svg') + const { container } = render() + const moreIcon = container.querySelector('.i-ri-more-fill') expect(moreIcon).toBeInTheDocument() }) it('should render in hidden state initially (group-hover)', () => { - const { container } = render() + const { container } = render() const wrapper = container.firstChild as HTMLElement - expect(wrapper).toHaveClass('hidden', 'group-hover:block') + expect(wrapper).toHaveClass( + 'invisible', + 'pointer-events-none', + 'group-hover:visible', + 'group-hover:pointer-events-auto', + ) }) }) describe('Props', () => { it('should show delete option when not workspace dataset operator', () => { - render() + render() const triggerButton = document.querySelector('[class*="cursor-pointer"]') if (triggerButton) fireEvent.click(triggerButton) - - // showDelete should be true (inverse of isCurrentWorkspaceDatasetOperator) - // This means delete operation will be visible }) it('should hide delete option when is workspace dataset operator', () => { - render() + render() const triggerButton = document.querySelector('[class*="cursor-pointer"]') if (triggerButton) fireEvent.click(triggerButton) - - // showDelete should be false }) it('should show export pipeline when runtime_mode is rag_pipeline', () => { const dataset = createMockDataset({ runtime_mode: 'rag_pipeline' }) - render() + render() const triggerButton = document.querySelector('[class*="cursor-pointer"]') if (triggerButton) fireEvent.click(triggerButton) - - // showExportPipeline should be true }) it('should hide export pipeline when runtime_mode is not rag_pipeline', () => { const dataset = createMockDataset({ runtime_mode: 'general' }) - render() + render() const triggerButton = document.querySelector('[class*="cursor-pointer"]') if (triggerButton) fireEvent.click(triggerButton) - - // showExportPipeline should be false }) }) describe('Styles', () => { it('should have correct positioning styles', () => { - const { container } = render() + const { container } = render() const wrapper = container.firstChild as HTMLElement - expect(wrapper).toHaveClass('absolute', 'right-2', 'top-2', 'z-15') + expect(wrapper).toHaveClass('absolute', 'right-2', 'top-2', 'z-5') + }) + + it('should keep the trigger mounted when closed so menu exit animations retain an anchor', () => { + const { container } = render() + const wrapper = container.firstChild as HTMLElement + const trigger = container.querySelector('[aria-label="Dataset operations"]') + + expect(wrapper).not.toHaveClass('hidden') + expect(trigger).toBeInTheDocument() }) it('should have icon with correct size classes', () => { - const { container } = render() - const icon = container.querySelector('svg') + const { container } = render() + const icon = container.querySelector('.i-ri-more-fill') expect(icon).toHaveClass('h-5', 'w-5', 'text-text-tertiary') }) + + it('should have aria-label on trigger for accessibility', () => { + const { container } = render() + const trigger = container.querySelector('[aria-label="Dataset operations"]') + expect(trigger).toBeInTheDocument() + }) + + it('should expose visible keyboard focus styles on the trigger', () => { + const { container } = render() + const trigger = container.querySelector('[aria-label="Dataset operations"]') + expect(trigger).toHaveClass( + 'focus-visible:outline-hidden', + 'focus-visible:ring-1', + 'focus-visible:ring-inset', + 'focus-visible:ring-components-input-border-hover', + ) + }) + + it('should use a solid trigger background without backdrop blur on hover states', () => { + const { container } = render() + const trigger = container.querySelector('[aria-label="Dataset operations"]') + expect(trigger).toHaveClass('bg-components-button-secondary-bg') + expect(trigger).not.toHaveClass('hover:backdrop-blur-[5px]', 'backdrop-blur-[5px]') + }) }) describe('User Interactions', () => { + it('should keep outside interactions available when the menu is open', () => { + const onOutsideClick = vi.fn() + + render( +
+ + +
, + ) + + fireEvent.click(screen.getByLabelText('Dataset operations')) + fireEvent.click(screen.getByRole('button', { name: 'Outside action' })) + + expect(onOutsideClick).toHaveBeenCalledTimes(1) + }) + it('should pass openRenameModal to Operations', () => { const openRenameModal = vi.fn() - render() - - // The openRenameModal should be passed to Operations component - expect(openRenameModal).not.toHaveBeenCalled() // Initially not called + render() + expect(openRenameModal).not.toHaveBeenCalled() }) it('should pass handleExportPipeline to Operations', () => { const handleExportPipeline = vi.fn() - render() - + render() expect(handleExportPipeline).not.toHaveBeenCalled() }) it('should pass detectIsUsedByApp to Operations', () => { const detectIsUsedByApp = vi.fn() - render() - + render() expect(detectIsUsedByApp).not.toHaveBeenCalled() }) }) @@ -145,13 +186,13 @@ describe('OperationsPopover', () => { describe('Edge Cases', () => { it('should handle dataset with external provider', () => { const dataset = createMockDataset({ provider: 'external' }) - const { container } = render() + const { container } = render() expect(container.firstChild).toBeInTheDocument() }) it('should handle dataset with undefined runtime_mode', () => { const dataset = createMockDataset({ runtime_mode: undefined }) - const { container } = render() + const { container } = render() expect(container.firstChild).toBeInTheDocument() }) }) diff --git a/web/app/components/datasets/list/dataset-card/components/corner-labels.tsx b/web/app/components/datasets/list/dataset-card/components/corner-labels.tsx index 03ca543ee7..2fbb840526 100644 --- a/web/app/components/datasets/list/dataset-card/components/corner-labels.tsx +++ b/web/app/components/datasets/list/dataset-card/components/corner-labels.tsx @@ -14,7 +14,7 @@ const CornerLabels = ({ dataset }: CornerLabelsProps) => { return ( ) @@ -24,7 +24,7 @@ const CornerLabels = ({ dataset }: CornerLabelsProps) => { return ( ) diff --git a/web/app/components/datasets/list/dataset-card/components/operations-dropdown.tsx b/web/app/components/datasets/list/dataset-card/components/operations-dropdown.tsx new file mode 100644 index 0000000000..37a73d5d8f --- /dev/null +++ b/web/app/components/datasets/list/dataset-card/components/operations-dropdown.tsx @@ -0,0 +1,68 @@ +import type { DataSet } from '@/models/datasets' +import * as React from 'react' +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuTrigger, +} from '@/app/components/base/ui/dropdown-menu' +import { cn } from '@/utils/classnames' +import Operations from '../operations' + +type OperationsDropdownProps = { + dataset: DataSet + isCurrentWorkspaceDatasetOperator: boolean + openRenameModal: () => void + handleExportPipeline: (include?: boolean) => void + detectIsUsedByApp: () => void +} + +const OperationsDropdown = ({ + dataset, + isCurrentWorkspaceDatasetOperator, + openRenameModal, + handleExportPipeline, + detectIsUsedByApp, +}: OperationsDropdownProps) => { + const [open, setOpen] = React.useState(false) + + return ( +
e.stopPropagation()} + > + + + + + + + + +
+ ) +} + +export default React.memo(OperationsDropdown) diff --git a/web/app/components/datasets/list/dataset-card/components/operations-popover.tsx b/web/app/components/datasets/list/dataset-card/components/operations-popover.tsx deleted file mode 100644 index 643242a24f..0000000000 --- a/web/app/components/datasets/list/dataset-card/components/operations-popover.tsx +++ /dev/null @@ -1,52 +0,0 @@ -import type { DataSet } from '@/models/datasets' -import { RiMoreFill } from '@remixicon/react' -import * as React from 'react' -import CustomPopover from '@/app/components/base/popover' -import { cn } from '@/utils/classnames' -import Operations from '../operations' - -type OperationsPopoverProps = { - dataset: DataSet - isCurrentWorkspaceDatasetOperator: boolean - openRenameModal: () => void - handleExportPipeline: (include?: boolean) => void - detectIsUsedByApp: () => void -} - -const OperationsPopover = ({ - dataset, - isCurrentWorkspaceDatasetOperator, - openRenameModal, - handleExportPipeline, - detectIsUsedByApp, -}: OperationsPopoverProps) => ( -
- - )} - className="z-20 min-w-[186px]" - popupClassName="rounded-xl bg-none shadow-none ring-0 min-w-[186px]" - position="br" - trigger="click" - btnElement={( -
- -
- )} - btnClassName={open => - cn( - 'size-9 cursor-pointer justify-center radius-lg border-[0.5px] border-components-actionbar-border bg-components-actionbar-bg p-0 shadow-lg shadow-shadow-shadow-5 ring-2 ring-inset ring-components-actionbar-bg hover:border-components-actionbar-border', - open ? 'border-components-actionbar-border bg-state-base-hover' : '', - )} - /> -
-) - -export default React.memo(OperationsPopover) diff --git a/web/app/components/datasets/list/dataset-card/index.tsx b/web/app/components/datasets/list/dataset-card/index.tsx index 2a22255eda..5bd032d151 100644 --- a/web/app/components/datasets/list/dataset-card/index.tsx +++ b/web/app/components/datasets/list/dataset-card/index.tsx @@ -9,7 +9,7 @@ import DatasetCardFooter from './components/dataset-card-footer' import DatasetCardHeader from './components/dataset-card-header' import DatasetCardModals from './components/dataset-card-modals' import Description from './components/description' -import OperationsPopover from './components/operations-popover' +import OperationsDropdown from './components/operations-dropdown' import TagArea from './components/tag-area' import { useDatasetCardState } from './hooks/use-dataset-card-state' @@ -82,7 +82,7 @@ const DatasetCard = ({ onClick={handleTagAreaClick} /> - -
- - {showExportPipeline && ( - - )} -
+ <> + + + {t('operation.edit', { ns: 'common' })} + + {showExportPipeline && ( + + + {t('operations.exportPipeline', { ns: 'datasetPipeline' })} + + )} {showDelete && ( <> - -
- -
+ + + + {t('operation.delete', { ns: 'common' })} + )} -
+ ) } diff --git a/web/app/components/datasets/list/index.tsx b/web/app/components/datasets/list/index.tsx index fae469dc9e..ffc9950f46 100644 --- a/web/app/components/datasets/list/index.tsx +++ b/web/app/components/datasets/list/index.tsx @@ -5,7 +5,6 @@ import { useBoolean, useDebounceFn } from 'ahooks' import { useState } from 'react' import { useTranslation } from 'react-i18next' -import { ApiConnectionMod } from '@/app/components/base/icons/src/vender/solid/development' import Input from '@/app/components/base/input' import TagManagementModal from '@/app/components/base/tag-management' import TagFilter from '@/app/components/base/tag-management/filter' @@ -85,11 +84,11 @@ const List = () => { }
diff --git a/web/eslint-suppressions.json b/web/eslint-suppressions.json index 2160c4bcf2..3b32e4e7e0 100644 --- a/web/eslint-suppressions.json +++ b/web/eslint-suppressions.json @@ -4503,11 +4503,6 @@ "count": 3 } }, - "app/components/datasets/list/dataset-card/components/corner-labels.tsx": { - "tailwindcss/enforce-consistent-class-order": { - "count": 2 - } - }, "app/components/datasets/list/dataset-card/components/dataset-card-footer.tsx": { "no-restricted-imports": { "count": 1 @@ -4526,14 +4521,6 @@ "count": 1 } }, - "app/components/datasets/list/dataset-card/components/operations-popover.tsx": { - "no-restricted-imports": { - "count": 1 - }, - "tailwindcss/enforce-consistent-class-order": { - "count": 2 - } - }, "app/components/datasets/list/dataset-card/components/tag-area.tsx": { "tailwindcss/enforce-consistent-class-order": { "count": 1