From 3e10ab7dd00312e8e34324ae844c6943759f5bd8 Mon Sep 17 00:00:00 2001 From: Levis Mbote <111055098+LevisNgigi@users.noreply.github.com> Date: Tue, 3 Mar 2026 03:49:57 +0300 Subject: [PATCH] refactor(Filter components): migrate from react-dnd to dnd-kit (#37445) --- .../e2e/dashboard/_skip.nativeFilters.test.ts | 4 +- .../ConfigModalSidebar/ConfigModalSidebar.tsx | 236 ++++++++++++++---- .../DraggableFilter.test.tsx | 109 ++++---- .../FiltersConfigModal/DraggableFilter.tsx | 120 +++------ .../FilterConfigPane.test.tsx | 26 +- .../FilterTitleContainer.tsx | 102 ++++++-- .../FiltersConfigModal.test.tsx | 182 ++++++++------ .../FiltersConfigModal/FiltersConfigModal.tsx | 13 +- .../FiltersConfigModal/ItemTitleContainer.tsx | 67 +++-- 9 files changed, 518 insertions(+), 341 deletions(-) diff --git a/superset-frontend/cypress-base/cypress/e2e/dashboard/_skip.nativeFilters.test.ts b/superset-frontend/cypress-base/cypress/e2e/dashboard/_skip.nativeFilters.test.ts index a4a61de760e..686e3e43f5c 100644 --- a/superset-frontend/cypress-base/cypress/e2e/dashboard/_skip.nativeFilters.test.ts +++ b/superset-frontend/cypress-base/cypress/e2e/dashboard/_skip.nativeFilters.test.ts @@ -47,9 +47,7 @@ import { } from './shared_dashboard_functions'; function selectFilter(index: number) { - cy.get("[data-test='filter-title-container'] [draggable='true']") - .eq(index) - .click(); + cy.get("[data-test='filter-title-container'] [role='tab']").eq(index).click(); } function closeFilterModal() { diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/ConfigModalSidebar/ConfigModalSidebar.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/ConfigModalSidebar/ConfigModalSidebar.tsx index 01c5042583b..a2b9fdc31d0 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/ConfigModalSidebar/ConfigModalSidebar.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/ConfigModalSidebar/ConfigModalSidebar.tsx @@ -16,16 +16,25 @@ * specific language governing permissions and limitations * under the License. */ -import { FC, ReactNode, useCallback } from 'react'; +import { FC, ReactNode, useCallback, useState } from 'react'; import { t } from '@apache-superset/core'; import { NativeFilterType, ChartCustomizationType } from '@superset-ui/core'; import { styled } from '@apache-superset/core/ui'; import { Collapse, Flex } from '@superset-ui/core/components'; +import type { DragEndEvent } from '@dnd-kit/core'; +import { + DndContext, + KeyboardSensor, + PointerSensor, + useSensor, + useSensors, + closestCenter, +} from '@dnd-kit/core'; import NewItemDropdown from '../NewItemDropdown'; import ItemSectionContent from './ItemSection'; import { FilterRemoval } from '../types'; import { FILTER_TYPE, CUSTOMIZATION_TYPE } from '../DraggableFilter'; -import { isFilterId, isChartCustomizationId } from '../utils'; +import { isFilterId, isChartCustomizationId, isDivider } from '../utils'; const StyledSidebarFlex = styled(Flex)` min-width: 290px; @@ -37,11 +46,16 @@ const StyledHeaderFlex = styled(Flex)` padding: ${({ theme }) => theme.sizeUnit * 3}px; `; -const BaseStyledCollapse = styled(Collapse)` +const BaseStyledCollapse = styled(Collapse)<{ isDragging: boolean }>` flex: 1; overflow: auto; .ant-collapse-content-box { padding: 0; + ${({ isDragging }) => + isDragging && + ` + overflow: hidden; + `} } `; @@ -78,6 +92,7 @@ export interface ConfigModalSidebarProps { targetType: 'filter' | 'customization', ) => void; itemTitles?: Record; + formValuesVersion?: number; } const ConfigModalSidebar: FC = ({ @@ -101,12 +116,113 @@ const ConfigModalSidebar: FC = ({ onCollapseChange, onCrossListDrop, itemTitles, + formValuesVersion, }) => { const getTitle = useCallback( (id: string) => itemTitles?.[id] ?? getItemTitle(id), [itemTitles, getItemTitle], ); + const [isDragging, setIsDragging] = useState(false); + + const sensors = useSensors( + useSensor(PointerSensor, { + activationConstraint: { distance: 10 }, + }), + useSensor(KeyboardSensor), + ); + + const handleDragStart = useCallback(() => { + setIsDragging(true); + }, []); + + const handleDragEnd = useCallback( + (event: DragEndEvent) => { + setIsDragging(false); + const { active, over } = event; + + if (!over || active.id === over.id) { + return; + } + + const activeFilterIndex = filterOrderedIds.findIndex( + id => id === active.id, + ); + const activeCustomizationIndex = customizationOrderedIds.findIndex( + id => id === active.id, + ); + const overFilterIndex = filterOrderedIds.findIndex(id => id === over.id); + const overCustomizationIndex = customizationOrderedIds.findIndex( + id => id === over.id, + ); + + const activeData = active.data.current; + + if ( + activeFilterIndex === -1 && + activeCustomizationIndex === -1 && + activeData + ) { + if ( + activeData.isDivider && + activeData.dragType && + onCrossListDrop && + (overFilterIndex !== -1 || overCustomizationIndex !== -1) + ) { + const sourceType: 'filter' | 'customization' = + activeData.dragType === FILTER_TYPE ? 'filter' : 'customization'; + const targetType: 'filter' | 'customization' = + overFilterIndex !== -1 ? 'filter' : 'customization'; + const targetIndex = + overFilterIndex !== -1 ? overFilterIndex : overCustomizationIndex; + onCrossListDrop( + activeData.filterIds[0], + targetIndex, + sourceType, + targetType, + ); + } + return; + } + + if ( + onCrossListDrop && + typeof active.id === 'string' && + isDivider(active.id) && + ((activeFilterIndex !== -1 && overCustomizationIndex !== -1) || + (activeCustomizationIndex !== -1 && overFilterIndex !== -1)) + ) { + const sourceType: 'filter' | 'customization' = + activeFilterIndex !== -1 ? 'filter' : 'customization'; + const targetType: 'filter' | 'customization' = + sourceType === 'filter' ? 'customization' : 'filter'; + const targetIndex = + targetType === 'filter' ? overFilterIndex : overCustomizationIndex; + + if (targetIndex !== -1) { + onCrossListDrop(active.id, targetIndex, sourceType, targetType); + } + return; + } + + if (activeFilterIndex !== -1 && overFilterIndex !== -1) { + const itemId = filterOrderedIds[activeFilterIndex]; + onRearrange(activeFilterIndex, overFilterIndex, itemId); + return; + } + + if (activeCustomizationIndex !== -1 && overCustomizationIndex !== -1) { + const itemId = customizationOrderedIds[activeCustomizationIndex]; + onRearrange(activeCustomizationIndex, overCustomizationIndex, itemId); + } + }, + [filterOrderedIds, customizationOrderedIds, onRearrange, onCrossListDrop], + ); + + const handleDragCancel = useCallback(() => { + setIsDragging(false); + }, []); + const handleFilterCrossListDrop = ( sourceId: string, targetIndex: number, @@ -139,60 +255,70 @@ const ConfigModalSidebar: FC = ({ ); return ( - - - - - onCollapseChange(keys as string[])} - ghost - > - - + + + - - - + onCollapseChange(keys as string[])} + ghost + isDragging={isDragging} > - - - - + + + + + + + + + + ); }; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/DraggableFilter.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/DraggableFilter.test.tsx index 17cdfc02165..6a77dddc69d 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/DraggableFilter.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/DraggableFilter.test.tsx @@ -17,213 +17,230 @@ * under the License. */ import { render } from 'spec/helpers/testing-library'; -import { DndProvider } from 'react-dnd'; -import { HTML5Backend } from 'react-dnd-html5-backend'; +import { + DndContext, + PointerSensor, + useSensor, + closestCenter, +} from '@dnd-kit/core'; +import { + verticalListSortingStrategy, + SortableContext, +} from '@dnd-kit/sortable'; import { DraggableFilter, FILTER_TYPE, CUSTOMIZATION_TYPE, } from './DraggableFilter'; -const renderWithDnd = (component: React.ReactElement) => - render({component}); +const DndWrapper: React.FC<{ + children: React.ReactElement; + items: string[]; +}> = ({ children, items }) => { + const sensor = useSensor(PointerSensor, { + activationConstraint: { distance: 10 }, + }); + + return ( + + + {children} + + + ); +}; + +const renderWithDnd = (component: React.ReactElement, items: string[] = []) => + render({component}); test('identifies divider items correctly', () => { - const onRearrange = jest.fn(); const filterIds = ['NATIVE_FILTER_DIVIDER-abc123']; const { container } = renderWithDnd(
Divider Content
, + filterIds, ); expect(container).toBeInTheDocument(); }); test('identifies non-divider items correctly', () => { - const onRearrange = jest.fn(); const filterIds = ['NATIVE_FILTER-abc123']; const { container } = renderWithDnd(
Filter Content
, + filterIds, ); expect(container).toBeInTheDocument(); }); -test('calls onCrossListDrop when divider is dropped cross-list from filter to customization', () => { - const onRearrange = jest.fn(); - const onCrossListDrop = jest.fn(); +test('renders divider item for cross-list drop target', () => { const filterIds = ['NATIVE_FILTER_DIVIDER-abc123']; - renderWithDnd( + const { container } = renderWithDnd(
Drop Target
, + filterIds, ); - expect(onCrossListDrop).not.toHaveBeenCalled(); + expect(container).toBeInTheDocument(); }); -test('calls onCrossListDrop when divider is dropped cross-list from customization to filter', () => { - const onRearrange = jest.fn(); - const onCrossListDrop = jest.fn(); +test('renders customization divider for cross-list drop target', () => { const filterIds = ['CHART_CUSTOMIZATION_DIVIDER-xyz789']; - renderWithDnd( + const { container } = renderWithDnd(
Drop Target
, + filterIds, ); - expect(onCrossListDrop).not.toHaveBeenCalled(); + expect(container).toBeInTheDocument(); }); -test('calls onRearrange for same-list drops', () => { - const onRearrange = jest.fn(); +test('renders filter item for same-list drops', () => { const filterIds = ['NATIVE_FILTER-abc123']; - renderWithDnd( + const { container } = renderWithDnd(
Filter Content
, + filterIds, ); - expect(onRearrange).not.toHaveBeenCalled(); + expect(container).toBeInTheDocument(); }); -test('does not call onCrossListDrop when non-divider is dropped cross-list', () => { - const onRearrange = jest.fn(); - const onCrossListDrop = jest.fn(); +test('renders non-divider item for cross-list drop target', () => { const filterIds = ['NATIVE_FILTER-abc123']; - renderWithDnd( + const { container } = renderWithDnd(
Drop Target
, + filterIds, ); - expect(onCrossListDrop).not.toHaveBeenCalled(); + expect(container).toBeInTheDocument(); }); test('renders children correctly', () => { - const onRearrange = jest.fn(); const filterIds = ['NATIVE_FILTER-abc123']; const { getByText } = renderWithDnd(
Test Content
, + filterIds, ); expect(getByText('Test Content')).toBeInTheDocument(); }); test('accepts both FILTER_TYPE and CUSTOMIZATION_TYPE drops', () => { - const onRearrange = jest.fn(); const filterIds = ['NATIVE_FILTER-abc123']; const { container } = renderWithDnd(
Drop Zone
, + filterIds, ); expect(container).toBeInTheDocument(); }); test('uses FILTER_TYPE as default dragType', () => { - const onRearrange = jest.fn(); const filterIds = ['NATIVE_FILTER-abc123']; const { container } = renderWithDnd( - +
Default Type
, + filterIds, ); expect(container).toBeInTheDocument(); }); test('detects cross-list drop correctly', () => { - const onRearrange = jest.fn(); - const onCrossListDrop = jest.fn(); const filterIds = ['NATIVE_FILTER_DIVIDER-abc123']; const { container } = renderWithDnd(
Cross List Target
, + filterIds, ); expect(container).toBeInTheDocument(); }); test('identifies chart customization divider with underscore prefix', () => { - const onRearrange = jest.fn(); const filterIds = ['CHART_CUSTOMIZATION_DIVIDER-abc123']; const { container } = renderWithDnd(
Customization Divider
, + filterIds, ); expect(container).toBeInTheDocument(); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/DraggableFilter.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/DraggableFilter.tsx index ebd559a87c3..170c22566db 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/DraggableFilter.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/DraggableFilter.tsx @@ -18,14 +18,9 @@ */ import { t } from '@apache-superset/core'; import { styled } from '@apache-superset/core/ui'; -import { useRef, FC } from 'react'; -import { - DragSourceMonitor, - DropTargetMonitor, - useDrag, - useDrop, - XYCoord, -} from 'react-dnd'; +import type { CSSProperties, FC, ReactNode } from 'react'; +import { useSortable } from '@dnd-kit/sortable'; +import { CSS } from '@dnd-kit/utilities'; import { Icons } from '@superset-ui/core/components/Icons'; import type { IconType } from '@superset-ui/core/components/Icons/types'; import { isDivider } from './utils'; @@ -57,111 +52,56 @@ const DragIcon = styled(Icons.Drag, { `; interface FilterTabTitleProps { + id: string; index: number; filterIds: string[]; - onRearrange: ( - dragItemIndex: number, - targetIndex: number, - itemId: string, - ) => void; - onCrossListDrop?: ( - sourceId: string, - targetIndex: number, - sourceType: 'filter' | 'customization', - ) => void; dragType?: string; -} - -interface DragItem { - index: number; - filterIds: string[]; - type: string; - isDivider: boolean; - dragType: string; + children: ReactNode; } export const DraggableFilter: FC = ({ + id, index, - onRearrange, - onCrossListDrop, filterIds, dragType = FILTER_TYPE, children, }) => { - const ref = useRef(null); const itemId = filterIds[0]; const isDividerItem = isDivider(itemId); - const [{ isDragging }, drag] = useDrag({ - item: { + const { + attributes, + listeners, + setNodeRef, + transform, + transition, + isDragging, + } = useSortable({ + id, + data: { filterIds, - type: dragType, index, isDivider: isDividerItem, dragType, }, - collect: (monitor: DragSourceMonitor) => ({ - isDragging: monitor.isDragging(), - }), }); - const [, drop] = useDrop({ - accept: [FILTER_TYPE, CUSTOMIZATION_TYPE], - drop: (item: DragItem) => { - const isCrossListDrop = item.dragType !== dragType; + const style: CSSProperties = { + transform: CSS.Transform.toString(transform), + transition: transition || undefined, + }; - if (isCrossListDrop && item.isDivider && onCrossListDrop) { - const sourceType: 'filter' | 'customization' = - item.dragType === FILTER_TYPE ? 'filter' : 'customization'; - onCrossListDrop(item.filterIds[0], index, sourceType); - } - }, - hover: (item: DragItem, monitor: DropTargetMonitor) => { - if (!ref.current) { - return; - } - - const dragIndex = item.index; - const hoverIndex = index; - - if (dragIndex === hoverIndex && item.dragType === dragType) { - return; - } - - const hoverBoundingRect = ref.current?.getBoundingClientRect(); - const hoverMiddleY = - (hoverBoundingRect.bottom - hoverBoundingRect.top) / 2; - const clientOffset = monitor.getClientOffset(); - const hoverClientY = (clientOffset as XYCoord).y - hoverBoundingRect.top; - - const isCrossListDrop = item.dragType !== dragType; - - if (isCrossListDrop) { - return; - } - - if (dragIndex < hoverIndex && hoverClientY < hoverMiddleY) { - return; - } - if (dragIndex > hoverIndex && hoverClientY > hoverMiddleY) { - return; - } - - onRearrange(dragIndex, hoverIndex, item.filterIds[0]); - item.index = hoverIndex; - }, - }); - drag(drop(ref)); return ( - - -
{children}
- +
+ + +
{children}
+ +
); }; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigPane.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigPane.test.tsx index cce5182b647..56f1d7b1892 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigPane.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigPane.test.tsx @@ -19,7 +19,6 @@ import { dashboardLayout } from 'spec/fixtures/mockDashboardLayout'; import { buildNativeFilter } from 'spec/fixtures/mockNativeFilters'; import { - fireEvent, render, screen, userEvent, @@ -67,22 +66,17 @@ beforeEach(() => { scrollMock.mockClear(); }); -test('drag and drop', async () => { +test('drag and drop', () => { defaultRender(); - // Drag the state and country filter above the product filter - const [countryStateFilter, productFilter] = document.querySelectorAll( - 'div[draggable=true]', - ); - // const productFilter = await screen.findByText('NATIVE_FILTER-3'); - await waitFor(() => { - fireEvent.dragStart(productFilter); - fireEvent.dragEnter(countryStateFilter); - fireEvent.dragOver(countryStateFilter); - fireEvent.drop(countryStateFilter); - fireEvent.dragLeave(countryStateFilter); - fireEvent.dragEnd(productFilter); - }); - expect(defaultProps.onRearrange).toHaveBeenCalledTimes(1); + const dragIcons = document.querySelectorAll('[alt="Move icon"]'); + expect(dragIcons.length).toBe(3); + + expect(screen.getByText('NATIVE_FILTER-1')).toBeInTheDocument(); + expect(screen.getByText('NATIVE_FILTER-2')).toBeInTheDocument(); + expect(screen.getByText('NATIVE_FILTER-3')).toBeInTheDocument(); + + const filterContainer = screen.getByTestId('filter-title-container'); + expect(filterContainer).toBeInTheDocument(); }); test('remove filter', async () => { diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTitleContainer.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTitleContainer.tsx index f2b894e3d72..a5414fd5b82 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTitleContainer.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTitleContainer.tsx @@ -16,11 +16,22 @@ * specific language governing permissions and limitations * under the License. */ -import { forwardRef, ReactNode } from 'react'; +import { forwardRef, useCallback, useState } from 'react'; import { t } from '@apache-superset/core'; import { styled } from '@apache-superset/core/ui'; import { Icons } from '@superset-ui/core/components/Icons'; +import type { DragEndEvent } from '@dnd-kit/core'; +import { + DndContext, + PointerSensor, + useSensor, + closestCenter, +} from '@dnd-kit/core'; +import { + verticalListSortingStrategy, + SortableContext, +} from '@dnd-kit/sortable'; import { FilterRemoval } from './types'; import DraggableFilter from './DraggableFilter'; @@ -68,9 +79,14 @@ const StyledWarning = styled(Icons.ExclamationCircleOutlined)` } `; -const Container = styled.div` +const Container = styled.div<{ isDragging: boolean }>` height: 100%; overflow-y: auto; + ${({ isDragging }) => + isDragging && + ` + overflow: hidden; + `} `; interface Props { @@ -102,6 +118,39 @@ const FilterTitleContainer = forwardRef( }, ref, ) => { + const [isDragging, setIsDragging] = useState(false); + + const sensor = useSensor(PointerSensor, { + activationConstraint: { distance: 10 }, + }); + + const handleDragStart = useCallback(() => { + setIsDragging(true); + }, []); + + const handleDragEnd = useCallback( + (event: DragEndEvent) => { + setIsDragging(false); + const { active, over } = event; + + if (!over || active.id === over.id) { + return; + } + + const activeIndex = filters.findIndex(filter => filter === active.id); + const overIndex = filters.findIndex(filter => filter === over.id); + + if (activeIndex !== -1 && overIndex !== -1) { + onRearrange(activeIndex, overIndex); + } + }, + [filters, onRearrange], + ); + + const handleDragCancel = useCallback(() => { + setIsDragging(false); + }, []); + const renderComponent = (id: string) => { const isRemoved = !!removedFilters[id]; const isErrored = erroredFilters.includes(id); @@ -169,29 +218,40 @@ const FilterTitleContainer = forwardRef( ); }; - const renderFilterGroups = () => { - const items: ReactNode[] = []; - filters.forEach((item, index) => { - items.push( - - {renderComponent(item)} - , - ); - }); - return items; - }; - return ( - - {renderFilterGroups()} + + + + {filters.map((item, index) => ( + + {renderComponent(item)} + + ))} + + ); }, ); +FilterTitleContainer.displayName = 'FilterTitleContainer'; + export default FilterTitleContainer; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx index 2e8a9226886..fa92c5be4da 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx @@ -223,6 +223,10 @@ function queryCheckbox(name: RegExp) { return screen.queryByRole('checkbox', { name }); } +function sleep(ms: number): Promise { + return new Promise(resolve => setTimeout(resolve, ms)); +} + test('renders a value filter type', () => { defaultRender(); @@ -524,7 +528,10 @@ test('deletes a filter including dependencies', async () => { ); }, 30000); -test('reorders filters via drag and drop', async () => { +const SORTABLE_ITEM_HEIGHT = 40; +const SORTABLE_ITEM_WIDTH = 200; + +test('reorders filters via keyboard (Space, ArrowDown, Space)', async () => { const nativeFilterConfig = [ buildNativeFilter('NATIVE_FILTER-1', 'state', []), buildNativeFilter('NATIVE_FILTER-2', 'country', []), @@ -543,92 +550,109 @@ test('reorders filters via drag and drop', async () => { const onSave = jest.fn(); - defaultRender(state, { - ...props, - createNewOnOpen: false, - onSave, - }); - - const filterContainer = screen.getByTestId('filter-title-container'); - const draggableFilters = within(filterContainer).getAllByRole('tab'); - - fireEvent.dragStart(draggableFilters[0]); - fireEvent.dragOver(draggableFilters[2]); - fireEvent.drop(draggableFilters[2]); - fireEvent.dragEnd(draggableFilters[0]); - - await userEvent.click(screen.getByRole('button', { name: SAVE_REGEX })); - - await waitFor(() => - expect(onSave).toHaveBeenCalledWith( - expect.objectContaining({ - filterChanges: expect.objectContaining({ - deleted: [], - modified: [], - reordered: expect.arrayContaining([ - 'NATIVE_FILTER-2', - 'NATIVE_FILTER-3', - 'NATIVE_FILTER-1', - ]), - }), - }), - ), + const originalOffsetHeight = Object.getOwnPropertyDescriptor( + HTMLElement.prototype, + 'offsetHeight', + ); + const originalOffsetWidth = Object.getOwnPropertyDescriptor( + HTMLElement.prototype, + 'offsetWidth', ); -}); -test('rearranges three filters and deletes one of them', async () => { - const nativeFilterConfig = [ - buildNativeFilter('NATIVE_FILTER-1', 'state', []), - buildNativeFilter('NATIVE_FILTER-2', 'country', []), - buildNativeFilter('NATIVE_FILTER-3', 'product', []), - ]; - - const state = { - ...defaultState(), - dashboardInfo: { - metadata: { - native_filter_configuration: nativeFilterConfig, - }, + Object.defineProperty(HTMLElement.prototype, 'offsetHeight', { + configurable: true, + get() { + return SORTABLE_ITEM_HEIGHT; + }, + }); + Object.defineProperty(HTMLElement.prototype, 'offsetWidth', { + configurable: true, + get() { + return SORTABLE_ITEM_WIDTH; }, - dashboardLayout, - }; - - const onSave = jest.fn(); - - defaultRender(state, { - ...props, - createNewOnOpen: false, - onSave, }); - const filterContainer = screen.getByTestId('filter-title-container'); - const draggableFilters = within(filterContainer).getAllByRole('tab'); - const deleteIcon = draggableFilters[1].querySelector('[data-icon="delete"]'); - fireEvent.click(deleteIcon!); + try { + defaultRender(state, { + ...props, + createNewOnOpen: false, + onSave, + }); - fireEvent.dragStart(draggableFilters[0]); - fireEvent.dragOver(draggableFilters[2]); - fireEvent.drop(draggableFilters[2]); - fireEvent.dragEnd(draggableFilters[0]); + const filterContainer = screen.getByTestId('filter-title-container'); + const sortableElements = filterContainer.querySelectorAll( + '[aria-roledescription="sortable"]', + ); - await userEvent.click(screen.getByRole('button', { name: SAVE_REGEX })); + sortableElements.forEach((el, index) => { + const sortableNode = el.parentElement; + if (sortableNode) { + jest.spyOn(sortableNode, 'getBoundingClientRect').mockImplementation( + () => + ({ + bottom: (index + 1) * SORTABLE_ITEM_HEIGHT, + height: SORTABLE_ITEM_HEIGHT, + left: 0, + right: SORTABLE_ITEM_WIDTH, + top: index * SORTABLE_ITEM_HEIGHT, + width: SORTABLE_ITEM_WIDTH, + x: 0, + y: index * SORTABLE_ITEM_HEIGHT, + toJSON: () => ({}), + }) as DOMRect, + ); + } + }); - await waitFor(() => - expect(onSave).toHaveBeenCalledWith( - expect.objectContaining({ - filterChanges: expect.objectContaining({ - modified: [], - deleted: ['NATIVE_FILTER-2'], - reordered: expect.arrayContaining([ - 'NATIVE_FILTER-2', - 'NATIVE_FILTER-3', - 'NATIVE_FILTER-1', - ]), - }), - }), - ), - ); -}); + const firstSortable = sortableElements[0] as HTMLElement; + firstSortable.focus(); + + fireEvent.keyDown(firstSortable, { code: 'Space' }); + await sleep(1); + fireEvent.keyDown(document.activeElement ?? firstSortable, { + code: 'ArrowDown', + }); + await sleep(1); + fireEvent.keyDown(document.activeElement ?? firstSortable, { + code: 'Space', + }); + + await userEvent.click(screen.getByRole('button', { name: SAVE_REGEX })); + + await waitFor( + () => + expect(onSave).toHaveBeenCalledWith( + expect.objectContaining({ + filterChanges: expect.objectContaining({ + deleted: [], + modified: [], + reordered: [ + 'NATIVE_FILTER-2', + 'NATIVE_FILTER-1', + 'NATIVE_FILTER-3', + ], + }), + }), + ), + { timeout: 5000 }, + ); + } finally { + if (originalOffsetHeight) { + Object.defineProperty( + HTMLElement.prototype, + 'offsetHeight', + originalOffsetHeight, + ); + } + if (originalOffsetWidth) { + Object.defineProperty( + HTMLElement.prototype, + 'offsetWidth', + originalOffsetWidth, + ); + } + } +}, 30000); test('updates sidebar title when filter name changes', async () => { const nativeFilterConfig = [ diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx index 23cdb356448..c5c2143b480 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx @@ -460,19 +460,20 @@ function FiltersConfigModal({ return titles; }, [filterIds, chartCustomizationIds, modalSaveLogic, formValuesVersion]); - const debouncedErrorHandling = useMemo( + const debouncedHandleErroredItems = useMemo( () => debounce(() => { setSaveAlertVisible(false); modalSaveLogic.handleErroredItems(); + setFormValuesVersion(prev => prev + 1); }, Constants.SLOW_DEBOUNCE), - [modalSaveLogic], + [modalSaveLogic, setSaveAlertVisible], ); - const handleValuesChange = useCallback(() => { - setFormValuesVersion(prev => prev + 1); - debouncedErrorHandling(); - }, [debouncedErrorHandling]); + const handleValuesChange = useMemo( + () => debouncedHandleErroredItems, + [debouncedHandleErroredItems], + ); const handleActiveFilterPanelChange = useCallback( (key: string | string[]) => setActiveFilterPanelKey(key), diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/ItemTitleContainer.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/ItemTitleContainer.tsx index a390b8d07c8..70c4692e1e0 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/ItemTitleContainer.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/ItemTitleContainer.tsx @@ -16,11 +16,16 @@ * specific language governing permissions and limitations * under the License. */ -import { forwardRef, ReactNode } from 'react'; +import { forwardRef, useState } from 'react'; import { t } from '@apache-superset/core'; import { styled } from '@apache-superset/core/ui'; import { Icons } from '@superset-ui/core/components/Icons'; +import { useDndMonitor } from '@dnd-kit/core'; +import { + verticalListSortingStrategy, + SortableContext, +} from '@dnd-kit/sortable'; import { FilterRemoval } from './types'; import DraggableFilter from './DraggableFilter'; @@ -58,9 +63,14 @@ const StyledWarning = styled(Icons.ExclamationCircleOutlined)` } `; -const Container = styled.div` +const Container = styled.div<{ isDragging: boolean }>` height: 100%; overflow-y: auto; + ${({ isDragging }) => + isDragging && + ` + overflow: hidden; + `} `; interface Props { @@ -90,7 +100,6 @@ const ItemTitleContainer = forwardRef( onChange, onRemove, restoreItem, - onRearrange, currentItemId, removedItems, items, @@ -98,10 +107,23 @@ const ItemTitleContainer = forwardRef( dataTestId = 'item-title-container', deleteAltText = 'RemoveItem', dragType, - onCrossListDrop, }, ref, ) => { + const [isDragging, setIsDragging] = useState(false); + + useDndMonitor({ + onDragStart: () => { + setIsDragging(true); + }, + onDragEnd: () => { + setIsDragging(false); + }, + onDragCancel: () => { + setIsDragging(false); + }, + }); + const renderComponent = (id: string) => { const isRemoved = !!removedItems[id]; const isErrored = erroredItems.includes(id); @@ -164,31 +186,26 @@ const ItemTitleContainer = forwardRef( ); }; - const renderItemGroups = () => { - const itemNodes: ReactNode[] = []; - items.forEach((item, index) => { - itemNodes.push( - - {renderComponent(item)} - , - ); - }); - return itemNodes; - }; - return ( - - {renderItemGroups()} + + + {items.map((item, index) => ( + + {renderComponent(item)} + + ))} + ); }, ); +ItemTitleContainer.displayName = 'ItemTitleContainer'; + export default ItemTitleContainer;