chore: action filter to dnd sortable (#17422)

frontend/src/lib/sortable.ts

@@ -0,0 +1,71 @@
+// Adapted from https://github.com/clauderic/dnd-kit/pull/805 to fix an issue where variable
+// height items in a sortable container were not always firing collisions correctly.
+// Should be possible to remove this custom collision detection algorithm once a proper fix
+// is merged into dnd-kit.
+
+import { CollisionDetection, DroppableContainer, UniqueIdentifier } from '@dnd-kit/core'
+
+export const verticalSortableListCollisionDetection: CollisionDetection = (args) => {
+    if (args.collisionRect.top < (args.active.rect.current?.initial?.top ?? 0)) {
+        return highestDroppableContainerMajorityCovered(args)
+    } else {
+        return lowestDroppableContainerMajorityCovered(args)
+    }
+}
+
+// Look for the first (/ furthest up / highest) droppable container that is at least
+// 50% covered by the top edge of the dragging container.
+const highestDroppableContainerMajorityCovered: CollisionDetection = ({ droppableContainers, collisionRect }) => {
+    const ascendingDroppabaleContainers = droppableContainers.sort(sortByRectTop)
+
+    for (const droppableContainer of ascendingDroppabaleContainers) {
+        const {
+            rect: { current: droppableRect },
+        } = droppableContainer
+
+        if (droppableRect) {
+            const coveredPercentage =
+                (droppableRect.top + droppableRect.height - collisionRect.top) / droppableRect.height
+
+            if (coveredPercentage > 0.5) {
+                return [collision(droppableContainer)]
+            }
+        }
+    }
+
+    // if we haven't found anything then we are off the top, so return the first item
+    return [collision(ascendingDroppabaleContainers[0])]
+}
+
+// Look for the last (/ furthest down / lowest) droppable container that is at least
+// 50% covered by the bottom edge of the dragging container.
+const lowestDroppableContainerMajorityCovered: CollisionDetection = ({ droppableContainers, collisionRect }) => {
+    const descendingDroppabaleContainers = droppableContainers.sort(sortByRectTop).reverse()
+
+    for (const droppableContainer of descendingDroppabaleContainers) {
+        const {
+            rect: { current: droppableRect },
+        } = droppableContainer
+
+        if (droppableRect) {
+            const coveredPercentage = (collisionRect.bottom - droppableRect.top) / droppableRect.height
+
+            if (coveredPercentage > 0.5) {
+                return [collision(droppableContainer)]
+            }
+        }
+    }
+
+    // if we haven't found anything then we are off the bottom, so return the last item
+    return [collision(descendingDroppabaleContainers[0])]
+}
+
+const sortByRectTop = (a: DroppableContainer, b: DroppableContainer): number =>
+    (a?.rect.current?.top || 0) - (b?.rect.current?.top || 0)
+
+const collision = (dropppableContainer?: DroppableContainer): { id: UniqueIdentifier; value?: DroppableContainer } => {
+    return {
+        id: dropppableContainer?.id ?? '',
+        value: dropppableContainer,
+    }
+}

frontend/src/scenes/experiments/SecondaryMetrics.tsx

@@ -196,15 +196,15 @@ export function SecondaryMetrics({
                                         buttonCopy="Add funnel step"
                                         seriesIndicatorType="numeric"
                                         sortable
-                                        showNestedArrow={true}
+                                        showNestedArrow
                                         propertiesTaxonomicGroupTypes={[
                                             TaxonomicFilterGroupType.EventProperties,
                                             TaxonomicFilterGroupType.PersonProperties,
                                             TaxonomicFilterGroupType.EventFeatureFlags,
                                             TaxonomicFilterGroupType.Cohorts,
                                             TaxonomicFilterGroupType.Elements,
                                         ]}
-                                        readOnly={true}
+                                        readOnly
                                     />
                                 )}
                                 {metric.filters.insight === InsightType.TRENDS && (

frontend/src/scenes/insights/EditorFilters/FunnelsQuerySteps.tsx

@@ -58,7 +58,7 @@ export function FunnelsQuerySteps({ insightProps }: EditorFilterProps): JSX.Elem
                 seriesIndicatorType="numeric"
                 entitiesLimit={FUNNEL_STEP_COUNT_LIMIT}
                 sortable
-                showNestedArrow={true}
+                showNestedArrow
                 propertiesTaxonomicGroupTypes={[
                     TaxonomicFilterGroupType.EventProperties,
                     TaxonomicFilterGroupType.PersonProperties,

frontend/src/scenes/insights/filters/ActionFilter/ActionFilter.tsx

@@ -10,14 +10,17 @@ import {
     InsightType,
     Optional,
 } from '~/types'
-import { SortableActionFilterContainer, SortableActionFilterRow } from './ActionFilterRow/SortableActionFilterRow'
 import { TaxonomicFilterGroupType } from 'lib/components/TaxonomicFilter/types'
 import { RenameModal } from 'scenes/insights/filters/ActionFilter/RenameModal'
 import { eventUsageLogic } from 'lib/utils/eventUsageLogic'
 import { teamLogic } from '../../../teamLogic'
 import clsx from 'clsx'
 import { LemonButton, LemonButtonProps } from 'lib/lemon-ui/LemonButton'
 import { IconPlusMini } from 'lib/lemon-ui/icons'
+import { SortableContext, verticalListSortingStrategy } from '@dnd-kit/sortable'
+import { DndContext } from '@dnd-kit/core'
+import { restrictToParentElement, restrictToVerticalAxis } from '@dnd-kit/modifiers'
+import { verticalSortableListCollisionDetection } from 'lib/sortable'
 
 export interface ActionFilterProps {
     setFilters: (filters: FilterType) => void
@@ -159,6 +162,7 @@ export const ActionFilter = React.forwardRef<HTMLDivElement, ActionFilterProps>(
     }
 
     const reachedLimit: boolean = Boolean(entitiesLimit && localFilters.length >= entitiesLimit)
+    const sortedItemIds = localFilters.map((i) => i.uuid)
 
     return (
         <div
@@ -173,36 +177,40 @@ export const ActionFilter = React.forwardRef<HTMLDivElement, ActionFilterProps>(
                 </BindLogic>
             )}
             {localFilters ? (
-                sortable ? (
-                    <SortableActionFilterContainer onSortEnd={onSortEnd} lockAxis="y" distance={5} useDragHandle>
-                        {localFilters.map((filter, index) => (
-                            <SortableActionFilterRow
-                                key={index}
-                                typeKey={typeKey}
-                                filter={filter as ActionFilterType}
-                                index={index}
-                                filterIndex={index}
-                                filterCount={localFilters.length}
-                                showNestedArrow={showNestedArrow}
-                                {...commonProps}
-                            />
-                        ))}
-                    </SortableActionFilterContainer>
-                ) : (
-                    localFilters.map((filter, index) => (
-                        <ActionFilterRow
-                            filter={filter as ActionFilterType}
-                            index={index}
-                            key={index}
-                            typeKey={typeKey}
-                            singleFilter={singleFilter}
-                            hideFilter={hideFilter || readOnly}
-                            filterCount={localFilters.length}
-                            showNestedArrow={showNestedArrow}
-                            {...commonProps}
-                        />
-                    ))
-                )
+                <ul>
+                    <DndContext
+                        onDragEnd={({ active, over }) => {
+                            if (over && active.id !== over.id) {
+                                onSortEnd({
+                                    oldIndex: sortedItemIds.indexOf(active.id.toString()),
+                                    newIndex: sortedItemIds.indexOf(over.id.toString()),
+                                })
+                            }
+                        }}
+                        modifiers={[restrictToVerticalAxis, restrictToParentElement]}
+                        collisionDetection={verticalSortableListCollisionDetection}
+                    >
+                        <SortableContext
+                            disabled={!sortable}
+                            items={sortedItemIds}
+                            strategy={verticalListSortingStrategy}
+                        >
+                            {localFilters.map((filter, index) => (
+                                <ActionFilterRow
+                                    key={filter.uuid}
+                                    typeKey={typeKey}
+                                    filter={filter}
+                                    index={index}
+                                    filterCount={localFilters.length}
+                                    showNestedArrow={showNestedArrow}
+                                    singleFilter={singleFilter}
+                                    hideFilter={hideFilter || readOnly}
+                                    {...commonProps}
+                                />
+                            ))}
+                        </SortableContext>
+                    </DndContext>
+                </ul>
             ) : null}
             {!singleFilter && (
                 <div className="ActionFilter-footer">

frontend/src/scenes/insights/filters/ActionFilter/ActionFilterRow/ActionFilterRow.scss

@@ -1,4 +1,6 @@
 .ActionFilterRow {
+    background: var(--bg-3000);
+
     .ActionFilterRow-content {
         display: flex;
         align-items: flex-start;

frontend/src/scenes/insights/filters/ActionFilter/ActionFilterRow/ActionFilterRow.tsx

@@ -30,7 +30,6 @@ import { actionsModel } from '~/models/actionsModel'
 import { PropertyKeyInfo } from 'lib/components/PropertyKeyInfo'
 import { TaxonomicPopover, TaxonomicStringPopover } from 'lib/components/TaxonomicPopover/TaxonomicPopover'
 import { IconCopy, IconDelete, IconEdit, IconFilter, IconWithCount } from 'lib/lemon-ui/icons'
-import { SortableHandle as sortableHandle } from 'react-sortable-hoc'
 import { SortableDragIcon } from 'lib/lemon-ui/icons'
 import { LemonButton } from 'lib/lemon-ui/LemonButton'
 import { LemonSelect, LemonSelectOption, LemonSelectOptions } from '@posthog/lemon-ui'
@@ -40,12 +39,16 @@ import { LemonDropdown } from 'lib/lemon-ui/LemonDropdown'
 import { HogQLEditor } from 'lib/components/HogQLEditor/HogQLEditor'
 import { entityFilterLogicType } from '../entityFilterLogicType'
 import { isAllEventsEntityFilter } from 'scenes/insights/utils'
+import { useSortable } from '@dnd-kit/sortable'
+import { CSS } from '@dnd-kit/utilities'
+import { LocalFilter } from '../entityFilterLogic'
+import { DraggableSyntheticListeners } from '@dnd-kit/core'
 
-const DragHandle = sortableHandle(() => (
-    <span className="ActionFilterRowDragHandle">
+const DragHandle = (props: DraggableSyntheticListeners | undefined): JSX.Element => (
+    <span className="ActionFilterRowDragHandle" {...props}>
         <SortableDragIcon />
     </span>
-))
+)
 
 export enum MathAvailability {
     All,
@@ -68,7 +71,7 @@ const getValue = (
 
 export interface ActionFilterRowProps {
     logic: BuiltLogic<entityFilterLogicType>
-    filter: ActionFilter
+    filter: LocalFilter
     index: number
     typeKey: string
     mathAvailability: MathAvailability
@@ -149,6 +152,8 @@ export function ActionFilterRow({
 
     const [isHogQLDropdownVisible, setIsHogQLDropdownVisible] = useState(false)
 
+    const { setNodeRef, attributes, transform, transition, listeners, isDragging } = useSortable({ id: filter.uuid })
+
     const propertyFiltersVisible = typeof filter.order === 'number' ? entityFilterVisible[filter.order] : false
 
     let name: string | null | undefined, value: PropertyFilterValue
@@ -302,7 +307,7 @@ export function ActionFilterRow({
     )
 
     const rowStartElements = [
-        sortable && filterCount > 1 ? <DragHandle /> : null,
+        sortable && filterCount > 1 ? <DragHandle {...listeners} /> : null,
         showSeriesIndicator && <div key="series-indicator">{seriesIndicator}</div>,
     ].filter(Boolean)
 
@@ -316,7 +321,17 @@ export function ActionFilterRow({
         : []
 
     return (
-        <div className={'ActionFilterRow'}>
+        <li
+            className={'ActionFilterRow'}
+            ref={setNodeRef}
+            {...attributes}
+            style={{
+                position: 'relative',
+                zIndex: isDragging ? 1 : undefined,
+                transform: CSS.Translate.toString(transform),
+                transition,
+            }}
+        >
             <div className="ActionFilterRow-content">
                 {renderRow ? (
                     renderRow({
@@ -461,7 +476,7 @@ export function ActionFilterRow({
                     />
                 </div>
             )}
-        </div>
+        </li>
     )
 }
 

frontend/src/scenes/insights/filters/ActionFilter/entityFilterLogic.test.ts

@@ -5,11 +5,13 @@ import filtersJson from './__mocks__/filters.json'
 import eventDefinitionsJson from './__mocks__/event_definitions.json'
 import { FilterType } from '~/types'
 import { useMocks } from '~/mocks/jest'
+import * as libUtils from 'lib/utils'
 
 describe('entityFilterLogic', () => {
     let logic: ReturnType<typeof entityFilterLogic.build>
 
     beforeEach(() => {
+        ;(libUtils as any).uuid = jest.fn().mockReturnValue('generated-uuid')
         useMocks({
             get: {
                 '/api/projects/:team/actions/': {

frontend/src/scenes/insights/filters/ActionFilter/entityFilterLogic.ts

@@ -2,10 +2,11 @@ import { kea, props, key, path, connect, actions, reducers, selectors, listeners
 import { EntityTypes, FilterType, Entity, EntityType, ActionFilter, EntityFilter, AnyPropertyFilter } from '~/types'
 import type { entityFilterLogicType } from './entityFilterLogicType'
 import { eventUsageLogic, GraphSeriesAddedSource } from 'lib/utils/eventUsageLogic'
-import { convertPropertyGroupToProperties } from 'lib/utils'
+import { convertPropertyGroupToProperties, uuid } from 'lib/utils'
 
 export type LocalFilter = ActionFilter & {
     order: number
+    uuid: string
 }
 export type BareEntity = Pick<Entity, 'id' | 'name'>
 
@@ -17,9 +18,10 @@ export function toLocalFilters(filters: Partial<FilterType>): LocalFilter[] {
         filter.properties && Array.isArray(filter.properties)
             ? {
                   ...filter,
+                  uuid: uuid(),
                   properties: convertPropertyGroupToProperties(filter.properties),
               }
-            : filter
+            : { ...filter, uuid: uuid() }
     )
 }
 
@@ -197,6 +199,7 @@ export const entityFilterLogic = kea<entityFilterLogicType>([
             const order = precedingEntity ? precedingEntity.order + 1 : 0
             const newFilter = {
                 id: null,
+                uuid: uuid(),
                 type: EntityTypes.EVENTS,
                 order: order,
                 ...props.addFilterDefaultOptions,
@@ -218,6 +221,7 @@ export const entityFilterLogic = kea<entityFilterLogicType>([
             }
             newFilters.splice(order, 0, {
                 ...filter,
+                uuid: uuid(),
                 custom_name: undefined,
                 order: order + 1,
             } as LocalFilter)