diff --git a/superset-frontend/src/components/Datasource/FoldersEditor/TreeItem.styles.ts b/superset-frontend/src/components/Datasource/FoldersEditor/TreeItem.styles.ts index d7d678a999a..86d7bfaba1f 100644 --- a/superset-frontend/src/components/Datasource/FoldersEditor/TreeItem.styles.ts +++ b/superset-frontend/src/components/Datasource/FoldersEditor/TreeItem.styles.ts @@ -30,7 +30,7 @@ export const TreeItemContainer = styled.div<{ }>` ${({ theme, depth, isDragging, isOverlay }) => ` margin: 0 ${theme.marginMD}px; - margin-left: ${isOverlay ? ITEM_INDENTATION_WIDTH : (depth - 1) * FOLDER_INDENTATION_WIDTH + ITEM_INDENTATION_WIDTH}px; + margin-left: ${Math.max(0, (depth - 1) * FOLDER_INDENTATION_WIDTH + ITEM_INDENTATION_WIDTH)}px; padding-left: ${theme.paddingSM}px; display: flex; align-items: center; diff --git a/superset-frontend/src/components/Datasource/FoldersEditor/VirtualizedTreeItem.tsx b/superset-frontend/src/components/Datasource/FoldersEditor/VirtualizedTreeItem.tsx index 39fa42d19df..9bfda10c8a3 100644 --- a/superset-frontend/src/components/Datasource/FoldersEditor/VirtualizedTreeItem.tsx +++ b/superset-frontend/src/components/Datasource/FoldersEditor/VirtualizedTreeItem.tsx @@ -61,6 +61,7 @@ export interface VirtualizedTreeItemData { metricsMap: Map; columnsMap: Map; activeId: UniqueIdentifier | null; + draggedFolderChildIds: Set; forbiddenDropFolderIds: Set; currentDropTargetId: string | null; onToggleCollapse: (id: string) => void; @@ -151,6 +152,7 @@ function VirtualizedTreeItemComponent({ metricsMap, columnsMap, activeId, + draggedFolderChildIds, forbiddenDropFolderIds, currentDropTargetId, onToggleCollapse, @@ -185,6 +187,12 @@ function VirtualizedTreeItemComponent({ ); } + // Hidden descendants of the dragged folder — not droppable. + // handleDragEnd uses lastValidOverIdRef when dropping in this dead zone. + if (draggedFolderChildIds.has(item.uuid)) { + return
; + } + const childCount = isFolder ? (folderChildCounts.get(item.uuid) ?? 0) : 0; const showEmptyState = isFolder && childCount === 0; diff --git a/superset-frontend/src/components/Datasource/FoldersEditor/VirtualizedTreeList.tsx b/superset-frontend/src/components/Datasource/FoldersEditor/VirtualizedTreeList.tsx index 1920b296198..eeb2dad76b5 100644 --- a/superset-frontend/src/components/Datasource/FoldersEditor/VirtualizedTreeList.tsx +++ b/superset-frontend/src/components/Datasource/FoldersEditor/VirtualizedTreeList.tsx @@ -48,6 +48,7 @@ interface VirtualizedTreeListProps { columnsMap: Map; isDragging: boolean; activeId: UniqueIdentifier | null; + draggedFolderChildIds: Set; forbiddenDropFolderIds: Set; currentDropTargetId: string | null; onToggleCollapse: (id: string) => void; @@ -73,6 +74,7 @@ export function VirtualizedTreeList({ columnsMap, isDragging, activeId, + draggedFolderChildIds, forbiddenDropFolderIds, currentDropTargetId, onToggleCollapse, @@ -180,6 +182,7 @@ export function VirtualizedTreeList({ metricsMap, columnsMap, activeId, + draggedFolderChildIds, forbiddenDropFolderIds, currentDropTargetId, onToggleCollapse, @@ -199,6 +202,7 @@ export function VirtualizedTreeList({ metricsMap, columnsMap, activeId, + draggedFolderChildIds, forbiddenDropFolderIds, currentDropTargetId, onToggleCollapse, diff --git a/superset-frontend/src/components/Datasource/FoldersEditor/components/DragOverlayContent.test.tsx b/superset-frontend/src/components/Datasource/FoldersEditor/components/DragOverlayContent.test.tsx new file mode 100644 index 00000000000..ba921ef4600 --- /dev/null +++ b/superset-frontend/src/components/Datasource/FoldersEditor/components/DragOverlayContent.test.tsx @@ -0,0 +1,124 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { render, screen } from 'spec/helpers/testing-library'; +import { FlattenedTreeItem } from '../constants'; +import { FoldersEditorItemType } from '../../types'; +import { DragOverlayContent } from './DragOverlayContent'; + +// Mock TreeItem to avoid dnd-kit hook dependencies +jest.mock('../TreeItem', () => ({ + TreeItem: ({ name, id }: { name: string; id: string }) => ( +
{name}
+ ), +})); + +const makeItem = ( + uuid: string, + type: FoldersEditorItemType, + depth = 0, + parentId: string | null = null, +): FlattenedTreeItem => ({ + uuid, + type, + name: uuid, + depth, + parentId, + index: 0, +}); + +const defaultProps = { + dragOverlayWidth: 400, + selectedItemIds: new Set(), + metricsMap: new Map(), + columnsMap: new Map(), + itemSeparatorInfo: new Map(), +}; + +test('returns null when dragOverlayItems is empty', () => { + const { container } = render( + , + ); + // The wrapper div rendered by testing-library's render should be empty + expect(container.querySelector('[data-test]')).toBeNull(); +}); + +test('renders folder block overlay for folder drag with children', () => { + const items: FlattenedTreeItem[] = [ + makeItem('folder-1', FoldersEditorItemType.Folder, 0), + makeItem('metric-1', FoldersEditorItemType.Metric, 1, 'folder-1'), + makeItem('metric-2', FoldersEditorItemType.Metric, 1, 'folder-1'), + ]; + + render(); + + expect(screen.getByTestId('tree-item-folder-1')).toBeInTheDocument(); + expect(screen.getByTestId('tree-item-metric-1')).toBeInTheDocument(); + expect(screen.getByTestId('tree-item-metric-2')).toBeInTheDocument(); + expect(screen.queryByText(/and \d+ more/)).not.toBeInTheDocument(); +}); + +test('truncates folder overlay and shows "... and N more" for large folders', () => { + const MAX_FOLDER_OVERLAY_CHILDREN = 8; + const totalChildren = MAX_FOLDER_OVERLAY_CHILDREN + 5; + const items: FlattenedTreeItem[] = [ + makeItem('folder-1', FoldersEditorItemType.Folder, 0), + ...Array.from({ length: totalChildren }, (_, i) => + makeItem(`item-${i}`, FoldersEditorItemType.Metric, 1, 'folder-1'), + ), + ]; + + render(); + + // folder header + MAX_FOLDER_OVERLAY_CHILDREN are visible + expect(screen.getByTestId('tree-item-folder-1')).toBeInTheDocument(); + for (let i = 0; i < MAX_FOLDER_OVERLAY_CHILDREN; i += 1) { + expect(screen.getByTestId(`tree-item-item-${i}`)).toBeInTheDocument(); + } + + // Items beyond the limit are not rendered + expect( + screen.queryByTestId(`tree-item-item-${MAX_FOLDER_OVERLAY_CHILDREN}`), + ).not.toBeInTheDocument(); + + // "... and N more" indicator shown with the remaining count + expect(screen.getByText(/and 5 more/)).toBeInTheDocument(); +}); + +test('renders stacked overlay for single non-folder item drag', () => { + const items: FlattenedTreeItem[] = [ + makeItem('metric-1', FoldersEditorItemType.Metric, 1, 'folder-1'), + ]; + + render(); + + expect(screen.getByTestId('tree-item-metric-1')).toBeInTheDocument(); +}); + +test('renders stacked overlay for folder with no children', () => { + const items: FlattenedTreeItem[] = [ + makeItem('folder-1', FoldersEditorItemType.Folder, 0), + ]; + + render(); + + expect(screen.getByTestId('tree-item-folder-1')).toBeInTheDocument(); + // Single folder should use the stacked overlay, not folder block + expect(screen.queryByText(/and \d+ more/)).not.toBeInTheDocument(); +}); diff --git a/superset-frontend/src/components/Datasource/FoldersEditor/components/DragOverlayContent.tsx b/superset-frontend/src/components/Datasource/FoldersEditor/components/DragOverlayContent.tsx index b14ab8d3344..1fd6a4d5251 100644 --- a/superset-frontend/src/components/Datasource/FoldersEditor/components/DragOverlayContent.tsx +++ b/superset-frontend/src/components/Datasource/FoldersEditor/components/DragOverlayContent.tsx @@ -18,12 +18,21 @@ */ import { memo } from 'react'; +import { t } from '@apache-superset/core'; import { Metric } from '@superset-ui/core'; import { ColumnMeta } from '@superset-ui/chart-controls'; import { FoldersEditorItemType } from '../../types'; -import { FlattenedTreeItem } from '../constants'; +import { FlattenedTreeItem, isDefaultFolder } from '../constants'; import { TreeItem } from '../TreeItem'; -import { DragOverlayStack, DragOverlayItem } from '../styles'; +import { + DragOverlayStack, + DragOverlayItem, + DragOverlayFolderBlock, + FolderBlockSlot, + MoreItemsIndicator, +} from '../styles'; + +const MAX_FOLDER_OVERLAY_CHILDREN = 8; interface DragOverlayContentProps { dragOverlayItems: FlattenedTreeItem[]; @@ -31,6 +40,7 @@ interface DragOverlayContentProps { selectedItemIds: Set; metricsMap: Map; columnsMap: Map; + itemSeparatorInfo: Map; } function DragOverlayContentInner({ @@ -39,11 +49,63 @@ function DragOverlayContentInner({ selectedItemIds, metricsMap, columnsMap, + itemSeparatorInfo, }: DragOverlayContentProps) { if (dragOverlayItems.length === 0) { return null; } + const firstItem = dragOverlayItems[0]; + const isFolderDrag = firstItem.type === FoldersEditorItemType.Folder; + + // Folder drag: folder header + children + if (isFolderDrag && dragOverlayItems.length > 1) { + const maxVisible = 1 + MAX_FOLDER_OVERLAY_CHILDREN; // folder header + children + const visibleItems = dragOverlayItems.slice(0, maxVisible); + const remainingCount = dragOverlayItems.length - maxVisible; + const baseDepth = firstItem.depth; + + return ( + + {visibleItems.map((item, index) => { + const isItemFolder = item.type === FoldersEditorItemType.Folder; + const separatorType = itemSeparatorInfo.get(item.uuid); + // No separator on the very last visible item + const isLastVisible = + index === visibleItems.length - 1 && remainingCount === 0; + const effectiveSeparator = isLastVisible ? undefined : separatorType; + return ( + + + + ); + })} + {remainingCount > 0 && ( + + {t('... and %d more', remainingCount)} + + )} + + ); + } + + // Multi-select or single item drag: stacked card behavior return ( {[...dragOverlayItems].reverse().map((item, index) => { diff --git a/superset-frontend/src/components/Datasource/FoldersEditor/constants.ts b/superset-frontend/src/components/Datasource/FoldersEditor/constants.ts index 913ee905fb6..599e16ca512 100644 --- a/superset-frontend/src/components/Datasource/FoldersEditor/constants.ts +++ b/superset-frontend/src/components/Datasource/FoldersEditor/constants.ts @@ -36,6 +36,9 @@ export const DEFAULT_FOLDERS_COUNT = 2; export const DRAG_INDENTATION_WIDTH = 64; export const MAX_DEPTH = 3; +// Base row height for tree items +export const ITEM_BASE_HEIGHT = 32; + // Type definitions export type TreeItem = DatasourceFolder | DatasourceFolderItem; diff --git a/superset-frontend/src/components/Datasource/FoldersEditor/hooks/useDragHandlers.test.ts b/superset-frontend/src/components/Datasource/FoldersEditor/hooks/useDragHandlers.test.ts new file mode 100644 index 00000000000..45ed8865b49 --- /dev/null +++ b/superset-frontend/src/components/Datasource/FoldersEditor/hooks/useDragHandlers.test.ts @@ -0,0 +1,164 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { renderHook, act } from '@testing-library/react-hooks'; +import type { DragStartEvent } from '@dnd-kit/core'; +import { FlattenedTreeItem } from '../constants'; +import { FoldersEditorItemType } from '../../types'; +import { useDragHandlers } from './useDragHandlers'; + +const makeItem = ( + uuid: string, + type: FoldersEditorItemType, + depth: number, + parentId: string | null = null, + index = 0, +): FlattenedTreeItem => ({ + uuid, + type, + name: uuid, + depth, + parentId, + index, +}); + +/** + * Flat list representing: + * folder-a (depth 0) + * metric-1 (depth 1) + * subfolder-b (depth 1) + * metric-2 (depth 2) + * metric-3 (depth 1) + * folder-c (depth 0) + * column-1 (depth 1) + */ +const flatItems: FlattenedTreeItem[] = [ + makeItem('folder-a', FoldersEditorItemType.Folder, 0, null, 0), + makeItem('metric-1', FoldersEditorItemType.Metric, 1, 'folder-a', 1), + makeItem('subfolder-b', FoldersEditorItemType.Folder, 1, 'folder-a', 2), + makeItem('metric-2', FoldersEditorItemType.Metric, 2, 'subfolder-b', 3), + makeItem('metric-3', FoldersEditorItemType.Metric, 1, 'folder-a', 4), + makeItem('folder-c', FoldersEditorItemType.Folder, 0, null, 5), + makeItem('column-1', FoldersEditorItemType.Column, 1, 'folder-c', 6), +]; + +function makeDragStartEvent(id: string): DragStartEvent { + return { + active: { + id, + rect: { current: { initial: null, translated: null } }, + data: { current: {} }, + }, + } as unknown as DragStartEvent; +} + +function setup(selectedIds: Set = new Set()) { + return renderHook(() => + useDragHandlers({ + setItems: jest.fn(), + computeFlattenedItems: () => flatItems, + fullFlattenedItems: flatItems, + selectedItemIds: selectedIds, + onChange: jest.fn(), + addWarningToast: jest.fn(), + }), + ); +} + +test('folder drag collects all visible descendants into draggedFolderChildIds', () => { + const { result } = setup(); + + act(() => { + result.current.handleDragStart(makeDragStartEvent('folder-a')); + }); + + const childIds = result.current.draggedFolderChildIds; + expect(childIds.has('metric-1')).toBe(true); + expect(childIds.has('subfolder-b')).toBe(true); + expect(childIds.has('metric-2')).toBe(true); + expect(childIds.has('metric-3')).toBe(true); + // Items outside the folder are not included + expect(childIds.has('folder-c')).toBe(false); + expect(childIds.has('column-1')).toBe(false); +}); + +test('non-folder drag leaves draggedFolderChildIds empty', () => { + const { result } = setup(); + + act(() => { + result.current.handleDragStart(makeDragStartEvent('metric-1')); + }); + + expect(result.current.draggedFolderChildIds.size).toBe(0); +}); + +test('projectionItems (flattenedItems) excludes folder descendants during folder drag', () => { + const { result } = setup(); + + act(() => { + result.current.handleDragStart(makeDragStartEvent('folder-a')); + }); + + const itemIds = result.current.flattenedItems.map( + (item: FlattenedTreeItem) => item.uuid, + ); + // The stable snapshot includes all items (captured before activeId is set), + // but projectionItems filtering is internal. We verify the hook returns + // the full stable snapshot since it's what the virtualized list needs. + expect(itemIds).toContain('folder-a'); + expect(itemIds).toContain('folder-c'); + expect(itemIds).toContain('column-1'); +}); + +test('subfolder drag collects only its own descendants', () => { + const { result } = setup(); + + act(() => { + result.current.handleDragStart(makeDragStartEvent('subfolder-b')); + }); + + const childIds = result.current.draggedFolderChildIds; + expect(childIds.has('metric-2')).toBe(true); + // Items outside subfolder-b + expect(childIds.has('metric-1')).toBe(false); + expect(childIds.has('metric-3')).toBe(false); + expect(childIds.has('folder-a')).toBe(false); +}); + +test('draggedItemIds contains selected items when active item is selected', () => { + const selected = new Set(['metric-1', 'metric-3']); + const { result } = setup(selected); + + act(() => { + result.current.handleDragStart(makeDragStartEvent('metric-1')); + }); + + expect(result.current.draggedItemIds).toEqual(selected); +}); + +test('draggedItemIds contains only active item when not in selection', () => { + const selected = new Set(['metric-1', 'metric-3']); + const { result } = setup(selected); + + act(() => { + result.current.handleDragStart(makeDragStartEvent('column-1')); + }); + + expect(result.current.draggedItemIds).toEqual(new Set(['column-1'])); +}); diff --git a/superset-frontend/src/components/Datasource/FoldersEditor/hooks/useDragHandlers.ts b/superset-frontend/src/components/Datasource/FoldersEditor/hooks/useDragHandlers.ts index 388da41cfcd..ddeca6884e1 100644 --- a/superset-frontend/src/components/Datasource/FoldersEditor/hooks/useDragHandlers.ts +++ b/superset-frontend/src/components/Datasource/FoldersEditor/hooks/useDragHandlers.ts @@ -66,6 +66,11 @@ export function useDragHandlers({ null, ); const [draggedItemIds, setDraggedItemIds] = useState>(new Set()); + const [draggedFolderChildIds, setDraggedFolderChildIds] = useState< + Set + >(new Set()); + // Last non-null overId — fallback target when dropping in dead zones + const lastValidOverIdRef = useRef(null); // Store the flattened items at drag start to keep them stable during drag // This prevents react-window from re-rendering due to flattenedItems reference changes @@ -88,13 +93,19 @@ export function useDragHandlers({ [activeId, computedFlattenedItems], ); - const flattenedItemsIndexMap = useMemo(() => { + // Exclude dragged folder children — they'd skew getProjection's minDepth calc + const projectionItems = useMemo(() => { + if (draggedFolderChildIds.size === 0) return flattenedItems; + return flattenedItems.filter(item => !draggedFolderChildIds.has(item.uuid)); + }, [flattenedItems, draggedFolderChildIds]); + + const projectionIndexMap = useMemo(() => { const map = new Map(); - flattenedItems.forEach((item, index) => { + projectionItems.forEach((item, index) => { map.set(item.uuid, index); }); return map; - }, [flattenedItems]); + }, [projectionItems]); // Shared lookup maps for O(1) access - used by handleDragEnd and forbiddenDropFolderIds const fullItemsByUuid = useMemo(() => { @@ -152,8 +163,10 @@ export function useDragHandlers({ setActiveId(null); setOverId(null); offsetLeftRef.current = 0; + lastValidOverIdRef.current = null; setCurrentDropTargetId(null); setDraggedItemIds(new Set()); + setDraggedFolderChildIds(new Set()); setDragOverlayWidth(null); // Clear the stable snapshot so next render uses fresh computed items dragStartFlattenedItemsRef.current = null; @@ -162,7 +175,8 @@ export function useDragHandlers({ const handleDragStart = ({ active }: DragStartEvent) => { // Capture the current flattened items BEFORE setting activeId // This ensures the list stays stable during the entire drag operation - dragStartFlattenedItemsRef.current = computeFlattenedItems(null); + const snapshot = computeFlattenedItems(null); + dragStartFlattenedItemsRef.current = snapshot; setActiveId(active.id); @@ -176,6 +190,23 @@ export function useDragHandlers({ } else { setDraggedItemIds(new Set([active.id as string])); } + + // Collect descendant IDs for hiding from list / showing in overlay + const activeIndex = snapshot.findIndex( + item => item.uuid === (active.id as string), + ); + const activeItem = snapshot[activeIndex]; + if (activeItem?.type === FoldersEditorItemType.Folder) { + const descendantIds = new Set(); + for (let i = activeIndex + 1; i < snapshot.length; i += 1) { + if (snapshot[i].depth > activeItem.depth) { + descendantIds.add(snapshot[i].uuid); + } else { + break; + } + } + setDraggedFolderChildIds(descendantIds); + } }; const handleDragMove = useCallback( @@ -190,23 +221,26 @@ export function useDragHandlers({ } const projection = getProjection( - flattenedItems, + projectionItems, activeId, overId, delta.x, DRAG_INDENTATION_WIDTH, - flattenedItemsIndexMap, + projectionIndexMap, ); const newParentId = projection?.parentId ?? null; setCurrentDropTargetId(newParentId); } }, - [activeId, overId, flattenedItems, flattenedItemsIndexMap], + [activeId, overId, projectionItems, projectionIndexMap], ); const handleDragOver = useCallback( ({ over }: DragOverEvent) => { setOverId(over?.id ?? null); + if (over) { + lastValidOverIdRef.current = over.id; + } if (activeId && over) { if (typeof over.id === 'string' && over.id.endsWith('-empty')) { @@ -216,12 +250,12 @@ export function useDragHandlers({ } const projection = getProjection( - flattenedItems, + projectionItems, activeId, over.id, offsetLeftRef.current, DRAG_INDENTATION_WIDTH, - flattenedItemsIndexMap, + projectionIndexMap, ); const newParentId = projection?.parentId ?? null; setCurrentDropTargetId(newParentId); @@ -229,22 +263,32 @@ export function useDragHandlers({ setCurrentDropTargetId(null); } }, - [activeId, flattenedItems, flattenedItemsIndexMap], + [activeId, projectionItems, projectionIndexMap], ); const handleDragEnd = ({ active, over }: DragEndEvent) => { const itemsBeingDragged = Array.from(draggedItemIds); + const folderChildIds = draggedFolderChildIds; const finalOffsetLeft = offsetLeftRef.current; + // Capture fallback overId before reset (for dead-zone drops) + const fallbackOverId = lastValidOverIdRef.current; resetDragState(); - if (!over || itemsBeingDragged.length === 0) { + // Folder drags only: hidden children create dead zones where over is null. + // Regular drags with null over just cancel. + const effectiveOver = + over ?? + (folderChildIds.size > 0 && fallbackOverId + ? { id: fallbackOverId } + : null); + if (!effectiveOver || itemsBeingDragged.length === 0) { return; } - let targetOverId = over.id; + let targetOverId = effectiveOver.id; let isEmptyDrop = false; - if (typeof over.id === 'string' && over.id.endsWith('-empty')) { - targetOverId = over.id.replace('-empty', ''); + if (typeof targetOverId === 'string' && targetOverId.endsWith('-empty')) { + targetOverId = targetOverId.replace('-empty', ''); isEmptyDrop = true; if (itemsBeingDragged.includes(targetOverId as string)) { @@ -252,6 +296,11 @@ export function useDragHandlers({ } } + // Dropping onto a descendant of the dragged folder is a no-op + if (folderChildIds.has(targetOverId as string)) { + return; + } + const activeIndex = fullItemsIndexMap.get(active.id as string) ?? -1; const overIndex = fullItemsIndexMap.get(targetOverId as string) ?? -1; @@ -266,12 +315,12 @@ export function useDragHandlers({ ); let projectedPosition = getProjection( - flattenedItems, + projectionItems, active.id, targetOverId, finalOffsetLeft, DRAG_INDENTATION_WIDTH, - flattenedItemsIndexMap, + projectionIndexMap, ); if (isEmptyDrop) { @@ -636,8 +685,6 @@ export function useDragHandlers({ } else { insertionIndex = overItemInRemaining; } - } else if (projectedPosition.depth > overItem.depth) { - insertionIndex = overItemInRemaining + 1; } else { insertionIndex = overItemInRemaining + 1; } @@ -680,12 +727,34 @@ export function useDragHandlers({ const dragOverlayItems = useMemo(() => { if (!activeId || draggedItemIds.size === 0) return []; + const activeItem = fullItemsByUuid.get(activeId as string); + + // Folder drag: include folder + visible descendants + if ( + activeItem?.type === FoldersEditorItemType.Folder && + draggedFolderChildIds.size > 0 + ) { + const activeIdStr = activeId as string; + return flattenedItems.filter( + (item: FlattenedTreeItem) => + item.uuid === activeIdStr || draggedFolderChildIds.has(item.uuid), + ); + } + + // Multi-select / single item: stacked overlay const draggedItems = fullFlattenedItems.filter((item: FlattenedTreeItem) => draggedItemIds.has(item.uuid), ); return draggedItems.slice(0, 3); - }, [activeId, draggedItemIds, fullFlattenedItems]); + }, [ + activeId, + draggedItemIds, + draggedFolderChildIds, + flattenedItems, + fullFlattenedItems, + fullItemsByUuid, + ]); const forbiddenDropFolderIds = useMemo(() => { const forbidden = new Set(); @@ -788,6 +857,7 @@ export function useDragHandlers({ isDragging: activeId !== null, activeId, draggedItemIds, + draggedFolderChildIds, dragOverlayWidth, flattenedItems, dragOverlayItems, diff --git a/superset-frontend/src/components/Datasource/FoldersEditor/hooks/useItemHeights.ts b/superset-frontend/src/components/Datasource/FoldersEditor/hooks/useItemHeights.ts index 4654637b292..e45382a663f 100644 --- a/superset-frontend/src/components/Datasource/FoldersEditor/hooks/useItemHeights.ts +++ b/superset-frontend/src/components/Datasource/FoldersEditor/hooks/useItemHeights.ts @@ -23,6 +23,7 @@ import { FOLDER_INDENTATION_WIDTH, ITEM_INDENTATION_WIDTH, } from '../TreeItem.styles'; +import { ITEM_BASE_HEIGHT } from '../constants'; export interface ItemHeights { /** Height of a regular item (metric/column) including margins */ @@ -49,13 +50,13 @@ export interface ItemHeights { * The spacing is built into the height calculation, NOT the CSS margins, * to avoid double-spacing issues with absolute positioning. */ -function calculateItemHeights(theme: SupersetTheme): ItemHeights { +export function calculateItemHeights(theme: SupersetTheme): ItemHeights { // Regular item height - just the row height, minimal spacing // The OptionControlContainer sets the actual content height - const regularItem = 32; + const regularItem = ITEM_BASE_HEIGHT; // Folder header - base height + vertical padding (for taller highlight) + bottom spacing - const folderHeader = 32 + theme.paddingSM + theme.marginXS; + const folderHeader = ITEM_BASE_HEIGHT + theme.paddingSM + theme.marginXS; // Separator visible: 1px line + vertical margins (marginSM above and below) const separatorVisible = 1 + theme.marginSM * 2; diff --git a/superset-frontend/src/components/Datasource/FoldersEditor/index.tsx b/superset-frontend/src/components/Datasource/FoldersEditor/index.tsx index a7a775ecc3e..d10c5f9a3f0 100644 --- a/superset-frontend/src/components/Datasource/FoldersEditor/index.tsx +++ b/superset-frontend/src/components/Datasource/FoldersEditor/index.tsx @@ -52,6 +52,7 @@ import { pointerSensorOptions, measuringConfig, autoScrollConfig, + getCollisionDetection, } from './sensors'; import { FoldersContainer, FoldersContent } from './styles'; import { FoldersEditorProps } from './types'; @@ -160,6 +161,7 @@ export default function FoldersEditor({ const { isDragging, activeId, + draggedFolderChildIds, dragOverlayWidth, flattenedItems, dragOverlayItems, @@ -412,9 +414,20 @@ export default function FoldersEditor({ return separators; }, [flattenedItems, lastChildIds]); + // Exclude dragged folder children so SortableContext skips hidden items const sortableItemIds = useMemo( - () => flattenedItems.map(({ uuid }) => uuid), - [flattenedItems], + () => + draggedFolderChildIds.size > 0 + ? flattenedItems + .filter(item => !draggedFolderChildIds.has(item.uuid)) + .map(({ uuid }) => uuid) + : flattenedItems.map(({ uuid }) => uuid), + [flattenedItems, draggedFolderChildIds], + ); + + const collisionDetection = useMemo( + () => getCollisionDetection(activeId), + [activeId], ); const selectedMetricsCount = useMemo(() => { @@ -465,6 +478,7 @@ export default function FoldersEditor({ diff --git a/superset-frontend/src/components/Datasource/FoldersEditor/sensors.test.ts b/superset-frontend/src/components/Datasource/FoldersEditor/sensors.test.ts new file mode 100644 index 00000000000..b1b32b30adc --- /dev/null +++ b/superset-frontend/src/components/Datasource/FoldersEditor/sensors.test.ts @@ -0,0 +1,120 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { rectIntersection, pointerWithin, closestCenter } from '@dnd-kit/core'; +import type { CollisionDescriptor } from '@dnd-kit/core'; +import { getCollisionDetection } from './sensors'; + +jest.mock('@dnd-kit/core', () => { + const actual = jest.requireActual('@dnd-kit/core'); + return { + ...actual, + rectIntersection: jest.fn(), + pointerWithin: jest.fn(), + closestCenter: jest.fn(), + }; +}); + +const mockRectIntersection = rectIntersection as jest.Mock; +const mockPointerWithin = pointerWithin as jest.Mock; +const mockClosestCenter = closestCenter as jest.Mock; + +const collision = (id: string): CollisionDescriptor => ({ + id, + data: { droppableContainer: { id } as any, value: 0 }, +}); + +const dummyArgs = {} as any; + +test('returns rectIntersection when activeId is null', () => { + const detector = getCollisionDetection(null); + expect(detector).toBe(rectIntersection); +}); + +test('returns rectIntersection result when best match is not the active item', () => { + const detector = getCollisionDetection('active-1'); + const collisions = [collision('other-item'), collision('active-1')]; + mockRectIntersection.mockReturnValue(collisions); + + const result = detector(dummyArgs); + + expect(result).toBe(collisions); + expect(mockPointerWithin).not.toHaveBeenCalled(); + expect(mockClosestCenter).not.toHaveBeenCalled(); +}); + +test('returns rectIntersection result when collisions array is empty', () => { + const detector = getCollisionDetection('active-1'); + mockRectIntersection.mockReturnValue([]); + + const result = detector(dummyArgs); + + expect(result).toEqual([]); + expect(mockPointerWithin).not.toHaveBeenCalled(); +}); + +test('falls back to pointerWithin when rectIntersection picks the active item', () => { + const detector = getCollisionDetection('active-1'); + const activeCollision = collision('active-1'); + const otherCollision = collision('other-item'); + mockRectIntersection.mockReturnValue([activeCollision]); + mockPointerWithin.mockReturnValue([otherCollision]); + + const result = detector(dummyArgs); + + expect(result).toEqual([otherCollision, activeCollision]); + expect(mockClosestCenter).not.toHaveBeenCalled(); +}); + +test('keeps rectIntersection result when pointerWithin also finds only the active item', () => { + const detector = getCollisionDetection('active-1'); + const activeCollision = collision('active-1'); + mockRectIntersection.mockReturnValue([activeCollision]); + mockPointerWithin.mockReturnValue([collision('active-1')]); + + const result = detector(dummyArgs); + + expect(result).toEqual([activeCollision]); + expect(mockClosestCenter).not.toHaveBeenCalled(); +}); + +test('falls back to closestCenter when pointerWithin returns empty', () => { + const detector = getCollisionDetection('active-1'); + const activeCollision = collision('active-1'); + const centerCollision = collision('nearby-item'); + mockRectIntersection.mockReturnValue([activeCollision]); + mockPointerWithin.mockReturnValue([]); + mockClosestCenter.mockReturnValue([centerCollision]); + + const result = detector(dummyArgs); + + expect(result).toEqual([centerCollision, activeCollision]); +}); + +test('returns rectIntersection result when all fallbacks only find the active item', () => { + const detector = getCollisionDetection('active-1'); + const activeCollision = collision('active-1'); + mockRectIntersection.mockReturnValue([activeCollision]); + mockPointerWithin.mockReturnValue([]); + mockClosestCenter.mockReturnValue([collision('active-1')]); + + const result = detector(dummyArgs); + + expect(result).toEqual([activeCollision]); +}); diff --git a/superset-frontend/src/components/Datasource/FoldersEditor/sensors.ts b/superset-frontend/src/components/Datasource/FoldersEditor/sensors.ts index 3c8cdc467e9..3b5ba987b4d 100644 --- a/superset-frontend/src/components/Datasource/FoldersEditor/sensors.ts +++ b/superset-frontend/src/components/Datasource/FoldersEditor/sensors.ts @@ -21,27 +21,71 @@ import { PointerSensorOptions, MeasuringConfiguration, MeasuringStrategy, + rectIntersection, + pointerWithin, + closestCenter, + CollisionDetection, + UniqueIdentifier, } from '@dnd-kit/core'; +/** + * Collision detection that deprioritizes the active (dragged) item. + * + * rectIntersection can match the DragPlaceholder at the original position, + * preventing repositioning. Falls back through pointerWithin (actual pointer + * position) then closestCenter (gaps between droppable rects). + */ +export function getCollisionDetection( + activeId: UniqueIdentifier | null, +): CollisionDetection { + if (!activeId) return rectIntersection; + + return args => { + const collisions = rectIntersection(args); + + // Best match isn't the active item — use as-is + if (collisions.length === 0 || collisions[0]?.id !== activeId) { + return collisions; + } + + // rectIntersection picked the active item — try pointer position instead + const pointerCollisions = pointerWithin(args); + const nonActivePointer = pointerCollisions.find(c => c.id !== activeId); + if (nonActivePointer) { + return [nonActivePointer, ...collisions]; + } + + // Pointer is over the DragPlaceholder — keep it for horizontal depth changes + if (pointerCollisions.length > 0) { + return collisions; + } + + // Gap between droppable rects — fall back to closestCenter + const centerCollisions = closestCenter(args); + const nonActiveCenter = centerCollisions.find(c => c.id !== activeId); + if (nonActiveCenter) { + return [nonActiveCenter, ...collisions]; + } + + return collisions; + }; +} + export const pointerSensorOptions: PointerSensorOptions = { activationConstraint: { distance: 8, }, }; -// Use BeforeDragging strategy to measure items once at drag start rather than continuously. -// This is critical for virtualized lists where items get unmounted during scroll. -// MeasuringStrategy.Always causes issues because dnd-kit loses track of items -// that are unmounted by react-window during auto-scroll. +// Measure once at drag start — MeasuringStrategy.Always breaks with virtualization +// because react-window unmounts items during scroll. export const measuringConfig: MeasuringConfiguration = { droppable: { strategy: MeasuringStrategy.BeforeDragging, }, }; -// Disable auto-scroll because it conflicts with virtualization. -// When auto-scroll moves the viewport, react-window unmounts items that scroll out of view, -// which causes dnd-kit to lose track of the dragged item and reset the drag operation. +// Disabled — auto-scroll + react-window unmounting causes dnd-kit to lose the drag. export const autoScrollConfig = { enabled: false, }; diff --git a/superset-frontend/src/components/Datasource/FoldersEditor/styles.tsx b/superset-frontend/src/components/Datasource/FoldersEditor/styles.tsx index 7bd04b7a577..95e91a0343a 100644 --- a/superset-frontend/src/components/Datasource/FoldersEditor/styles.tsx +++ b/superset-frontend/src/components/Datasource/FoldersEditor/styles.tsx @@ -17,6 +17,7 @@ * under the License. */ import { styled, css } from '@apache-superset/core/ui'; +import { calculateItemHeights } from './hooks/useItemHeights'; export const FoldersContainer = styled.div` display: flex; @@ -83,6 +84,55 @@ export const DragOverlayStack = styled.div<{ width?: number }>` will-change: transform; `; +export const DragOverlayFolderBlock = styled.div<{ width?: number }>` + ${({ theme, width }) => ` + width: ${width ? `${width}px` : '100%'}; + will-change: transform; + background: ${theme.colorBgContainer}; + border-radius: ${theme.borderRadius}px; + box-shadow: ${theme.boxShadowSecondary}; + pointer-events: none; + overflow: hidden; + opacity: 0.95; + `} +`; + +// Matches react-window slot heights so the overlay lines up with the list. +export const FolderBlockSlot = styled.div<{ + variant: 'folder' | 'item'; + separatorType?: 'visible' | 'transparent'; +}>` + ${({ theme, variant, separatorType }) => { + const heights = calculateItemHeights(theme); + let minHeight = + variant === 'folder' ? heights.folderHeader : heights.regularItem; + if (separatorType === 'visible') { + minHeight += heights.separatorVisible; + } else if (separatorType === 'transparent') { + minHeight += heights.separatorTransparent; + } + return ` + min-height: ${minHeight}px; + display: flex; + align-items: stretch; + + > * { + flex: 1; + min-width: 0; + } + `; + }} +`; + +export const MoreItemsIndicator = styled.div` + ${({ theme }) => ` + padding: ${theme.paddingXS}px ${theme.paddingMD}px; + color: ${theme.colorTextSecondary}; + font-size: ${theme.fontSizeSM}px; + text-align: center; + `} +`; + export const DragOverlayItem = styled.div<{ stackIndex: number; totalItems: number;