refactor(draggable filters): react-dnd to dnd-kit migration for DraggableFilters

superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/DraggableFilter.tsx

@@ -17,25 +17,21 @@
  * under the License.
  */
 import { styled } from '@apache-superset/core/ui';
-import { useRef, FC } from 'react';
-import {
-  DragSourceMonitor,
-  DropTargetMonitor,
-  useDrag,
-  useDrop,
-  XYCoord,
-} from 'react-dnd';
+import { FC } from 'react';
+import { CSS } from '@dnd-kit/utilities';
+import { useSortable } from '@dnd-kit/sortable';
+
 import { Icons } from '@superset-ui/core/components/Icons';
 import type { IconType } from '@superset-ui/core/components/Icons/types';
 
 interface TitleContainerProps {
   readonly isDragging: boolean;
 }
 
-const FILTER_TYPE = 'FILTER';
-
-const Container = styled.div<TitleContainerProps>`
-  ${({ isDragging, theme }) => `
+const Container = styled('div', {
+  shouldForwardProp: propName => propName !== 'isDragging',
+})<TitleContainerProps>`
+  ${({ isDragging }) => `
     opacity: ${isDragging ? 0.3 : 1};
     cursor: ${isDragging ? 'grabbing' : 'pointer'};
     width: 100%;
@@ -54,92 +50,32 @@ const DragIcon = styled(Icons.Drag, {
 `;
 
 interface FilterTabTitleProps {
-  index: number;
-  filterIds: string[];
-  onRearrange: (dragItemIndex: number, targetIndex: number) => void;
-}
-
-interface DragItem {
-  index: number;
-  filterIds: string[];
-  type: string;
+  filterId: string;
 }
 
 export const DraggableFilter: FC<FilterTabTitleProps> = ({
-  index,
-  onRearrange,
-  filterIds,
+  filterId,
   children,
 }) => {
-  const ref = useRef<HTMLDivElement>(null);
-  const [{ isDragging }, drag] = useDrag({
-    item: { filterIds, type: FILTER_TYPE, index },
-    collect: (monitor: DragSourceMonitor) => ({
-      isDragging: monitor.isDragging(),
-    }),
-  });
-  const [, drop] = useDrop({
-    accept: FILTER_TYPE,
-    hover: (item: DragItem, monitor: DropTargetMonitor) => {
-      if (!ref.current) {
-        return;
-      }
-
-      const dragIndex = item.index;
-      const hoverIndex = index;
-
-      // Don't replace items with themselves
-      if (dragIndex === hoverIndex) {
-        return;
-      }
-      // Determine rectangle on screen
-      const hoverBoundingRect = ref.current?.getBoundingClientRect();
-
-      // Get vertical middle
-      const hoverMiddleY =
-        (hoverBoundingRect.bottom - hoverBoundingRect.top) / 2;
-
-      // Determine mouse position
-      const clientOffset = monitor.getClientOffset();
-
-      // Get pixels to the top
-      const hoverClientY = (clientOffset as XYCoord).y - hoverBoundingRect.top;
-
-      // Only perform the move when the mouse has crossed half of the items height
-      // When dragging downwards, only move when the cursor is below 50%
-      // When dragging upwards, only move when the cursor is above 50%
 
-      // Dragging downwards
-      if (dragIndex < hoverIndex && hoverClientY < hoverMiddleY) {
-        return;
-      }
+  const { attributes, listeners, setNodeRef, isDragging, transform, transition } = useSortable({ id: filterId });
 
-      // Dragging upwards
-      if (dragIndex > hoverIndex && hoverClientY > hoverMiddleY) {
-        return;
-      }
+  const style = {
+    transform: transform ? CSS.Transform.toString(transform) : undefined,
+    transition,
+  };
 
-      onRearrange(dragIndex, hoverIndex);
-      // Note: we're mutating the monitor item here.
-      // Generally it's better to avoid mutations,
-      // but it's good here for the sake of performance
-      // to avoid expensive index searches.
-      // eslint-disable-next-line no-param-reassign
-      item.index = hoverIndex;
-    },
-  });
-  drag(drop(ref));
   return (
-    <Container ref={ref} isDragging={isDragging}>
-      <DragIcon
-        isDragging={isDragging}
-        alt="Move icon"
-        className="dragIcon"
-        viewBox="4 4 16 16"
-      />
-      <div css={{ flex: 1 }}>{children}</div>
+    <Container ref={setNodeRef} style={style} isDragging={isDragging} {...listeners} {...attributes}>
+        <DragIcon
+            isDragging={isDragging}
+            aria-label='Move filter'
+            className="dragIcon"
+            viewBox="4 4 16 16"
+        />
+        <div css={{ flex: 1 }}>{children}</div>
     </Container>
-  );
+  )
 };
 
 export default DraggableFilter;

superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTitleContainer.tsx

@@ -16,13 +16,25 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { forwardRef, ReactNode } from 'react';
+import { forwardRef } from 'react';
 
 import { t } from '@superset-ui/core';
 import { styled } from '@apache-superset/core/ui';
 import { Icons } from '@superset-ui/core/components/Icons';
 import { FilterRemoval } from './types';
 import DraggableFilter from './DraggableFilter';
+import {
+  DndContext,
+  DragEndEvent,
+  PointerSensor,
+  closestCenter,
+  useSensor,
+  useSensors,
+} from '@dnd-kit/core';
+import {
+  SortableContext,
+  verticalListSortingStrategy,
+} from '@dnd-kit/sortable';
 
 export const FilterTitle = styled.div`
   ${({ theme }) => `
@@ -152,27 +164,54 @@ const FilterTitleContainer = forwardRef<HTMLDivElement, Props>(
       );
     };
 
-    const renderFilterGroups = () => {
-      const items: ReactNode[] = [];
-      filters.forEach((item, index) => {
-        items.push(
+    const handleDragEnd = (event: DragEndEvent) => {
+      const { active, over } = event;
+      const activeId = active.id;
+      const overId = over?.id;
+      if (activeId == null || overId == null) return;
+      const from = filters.indexOf(String(activeId));
+      const to = filters.indexOf(String(overId));
+      if (from === -1 || to === -1) return;
+      onRearrange(from, to);
+    };
+
+    /*
+     * We need sensor in this case because we have other listeners in the same component
+     * and dnd-kit listeners blocks other listeners preventing actions like undo and delete.
+     * Using sensors we can set activation constraint and avoid blocking other actions.
+     */
+    const sensors = useSensors(
+      useSensor(PointerSensor, {
+        activationConstraint: {
+          distance: 5,
+        },
+      }),
+    );
+
+    const renderFilterGroups = () => (
+      <SortableContext items={filters} strategy={verticalListSortingStrategy}>
+        {filters.map(item => (
           <DraggableFilter
-            key={index}
-            onRearrange={onRearrange}
-            index={index}
-            filterIds={[item]}
+            // Need to pass key here to smoothly handle reordering of items
+            key={item}
+            filterId={item}
           >
             {renderComponent(item)}
-          </DraggableFilter>,
-        );
-      });
-      return items;
-    };
+          </DraggableFilter>
+        ))}
+      </SortableContext>
+    );
 
     return (
-      <Container data-test="filter-title-container" ref={ref}>
-        {renderFilterGroups()}
-      </Container>
+      <DndContext
+        collisionDetection={closestCenter}
+        onDragEnd={handleDragEnd}
+        sensors={sensors}
+      >
+        <Container data-test="filter-title-container" ref={ref}>
+          {renderFilterGroups()}
+        </Container>
+      </DndContext>
     );
   },
 );