Simplify drag and drop integrations (#2126)

* Simplify drag and drop integrations

* Fix lnt issues

* Remove eslint for sonar

Author: sroy3

webview/src/experiments/components/table/MergeHeaderGroups.tsx

@@ -4,19 +4,15 @@ import { Experiment, Column } from 'dvc/src/experiments/webview/contract'
 import { HeaderGroup } from 'react-table'
 import { TableHeader } from './TableHeader'
 import styles from './styles.module.scss'
-import {
-  OnDragOver,
-  OnDragStart,
-  OnDrop
-} from '../../../shared/components/dragDrop/DragDropWorkbench'
+import { DragFunction } from '../../../shared/components/dragDrop/Draggable'
 
 export const MergedHeaderGroups: React.FC<{
   headerGroup: HeaderGroup<Experiment>
   columns: HeaderGroup<Experiment>[]
   orderedColumns: Column[]
-  onDragUpdate: OnDragOver
-  onDragStart: OnDragStart
-  onDragEnd: OnDrop
+  onDragUpdate: DragFunction
+  onDragStart: DragFunction
+  onDragEnd: DragFunction
   firstExpColumnCellId: string
   setExpColumnNeedsShadow: (needsShadow: boolean) => void
   root: HTMLElement | null
@@ -45,7 +41,7 @@ export const MergedHeaderGroups: React.FC<{
           orderedColumns={orderedColumns}
           column={column}
           columns={columns}
-          onDragOver={onDragUpdate}
+          onDragEnter={onDragUpdate}
           onDragStart={onDragStart}
           onDrop={onDragEnd}
           root={root}

webview/src/experiments/components/table/TableHead.tsx

@@ -1,6 +1,6 @@
 import { Experiment } from 'dvc/src/experiments/webview/contract'
 import cx from 'classnames'
-import React, { useRef } from 'react'
+import React, { DragEvent, useRef } from 'react'
 import { useSelector } from 'react-redux'
 import { HeaderGroup, TableInstance } from 'react-table'
 import { MessageFromWebviewType } from 'dvc/src/webview/contract'
@@ -12,10 +12,7 @@ import { useColumnOrder } from '../../hooks/useColumnOrder'
 import { ExperimentsState } from '../../store'
 import { sendMessage } from '../../../shared/vscode'
 import { leafColumnIds, reorderColumnIds } from '../../util/columns'
-import {
-  OnDragOver,
-  OnDragStart
-} from '../../../shared/components/dragDrop/DragDropWorkbench'
+import { DragFunction } from '../../../shared/components/dragDrop/Draggable'
 import { getSelectedForPlotsCount } from '../../util/rows'
 interface TableHeadProps {
   instance: TableInstance<Experiment>
@@ -46,8 +43,10 @@ export const TableHead = ({
   const fullColumnOrder = useRef<string[]>()
   const draggingIds = useRef<string[]>()
 
-  const onDragStart: OnDragStart = draggedId => {
-    const displacerHeader = allHeaders.find(header => header.id === draggedId)
+  const onDragStart: DragFunction = ({ currentTarget }) => {
+    const displacerHeader = allHeaders.find(
+      header => header.id === currentTarget.id
+    )
     if (displacerHeader) {
       draggingIds.current = leafColumnIds(displacerHeader)
       fullColumnOrder.current = allColumns.map(({ id }) => id)
@@ -65,10 +64,10 @@ export const TableHead = ({
     displacedHeader && cb(displacedHeader)
   }
 
-  const onDragUpdate: OnDragOver = (_, draggedOverId: string) => {
+  const onDragUpdate = (e: DragEvent<HTMLElement>) => {
     const displacer = draggingIds.current
     displacer &&
-      findDisplacedHeader(draggedOverId, displacedHeader => {
+      findDisplacedHeader(e.currentTarget.id, displacedHeader => {
         const displaced = leafColumnIds(displacedHeader)
         if (!displaced.some(id => displacer.includes(id))) {
           fullColumnOrder.current &&

webview/src/experiments/components/table/TableHeader.tsx

@@ -16,10 +16,8 @@ import { ContextMenu } from '../../../shared/components/contextMenu/ContextMenu'
 import { ExperimentsState } from '../../store'
 import {
   Draggable,
-  OnDragOver,
-  OnDragStart,
-  OnDrop
-} from '../../../shared/components/dragDrop/DragDropWorkbench'
+  DragFunction
+} from '../../../shared/components/dragDrop/Draggable'
 import { MessagesMenu } from '../../../shared/components/messagesMenu/MessagesMenu'
 import { MessagesMenuOptionProps } from '../../../shared/components/messagesMenu/MessagesMenuOption'
 import { IconMenu } from '../../../shared/components/iconMenu/IconMenu'
@@ -40,10 +38,10 @@ const possibleOrders = {
 export const ColumnDragHandle: React.FC<{
   disabled: boolean
   column: HeaderGroup<Experiment>
-  onDragOver: OnDragOver
-  onDragStart: OnDragStart
-  onDrop: OnDrop
-}> = ({ disabled, column, onDragOver, onDragStart, onDrop }) => {
+  onDragEnter: DragFunction
+  onDragStart: DragFunction
+  onDrop: DragFunction
+}> = ({ disabled, column, onDragEnter, onDragStart, onDrop }) => {
   const DropTarget = <span>{column?.name}</span>
 
   return (
@@ -61,7 +59,7 @@ export const ColumnDragHandle: React.FC<{
         disabled={disabled}
         group={'experiment-table'}
         dropTarget={DropTarget}
-        onDragOver={onDragOver}
+        onDragEnter={onDragEnter}
         onDragStart={onDragStart}
         onDrop={onDrop}
       >
@@ -153,9 +151,9 @@ const TableHeaderCellContents: React.FC<{
   hasFilter: boolean
   isDraggable: boolean
   menuSuppressed: boolean
-  onDragOver: OnDragOver
-  onDragStart: OnDragStart
-  onDrop: OnDrop
+  onDragEnter: DragFunction
+  onDragStart: DragFunction
+  onDrop: DragFunction
   canResize: boolean
   setMenuSuppressed: (menuSuppressed: boolean) => void
   resizerHeight: string
@@ -166,7 +164,7 @@ const TableHeaderCellContents: React.FC<{
   hasFilter,
   isDraggable,
   menuSuppressed,
-  onDragOver,
+  onDragEnter,
   onDragStart,
   onDrop,
   canResize,
@@ -181,7 +179,7 @@ const TableHeaderCellContents: React.FC<{
       <ColumnDragHandle
         column={column}
         disabled={!isDraggable || menuSuppressed}
-        onDragOver={onDragOver}
+        onDragEnter={onDragEnter}
         onDragStart={onDragStart}
         onDrop={onDrop}
       />
@@ -207,9 +205,9 @@ const TableHeaderCell: React.FC<{
   sortEnabled: boolean
   menuDisabled?: boolean
   menuContent?: React.ReactNode
-  onDragOver: OnDragOver
-  onDragStart: OnDragStart
-  onDrop: OnDrop
+  onDragEnter: DragFunction
+  onDragStart: DragFunction
+  onDrop: DragFunction
   firstExpColumnCellId: string
   setExpColumnNeedsShadow: (needsShadow: boolean) => void
   root: HTMLElement | null
@@ -222,7 +220,7 @@ const TableHeaderCell: React.FC<{
   sortEnabled,
   menuContent,
   menuDisabled,
-  onDragOver,
+  onDragEnter,
   onDragStart,
   onDrop,
   root,
@@ -250,7 +248,7 @@ const TableHeaderCell: React.FC<{
       hasFilter={hasFilter}
       isDraggable={isDraggable}
       menuSuppressed={menuSuppressed}
-      onDragOver={onDragOver}
+      onDragEnter={onDragEnter}
       onDragStart={onDragStart}
       onDrop={onDrop}
       canResize={canResize}
@@ -293,9 +291,9 @@ interface TableHeaderProps {
   column: HeaderGroup<Experiment>
   columns: HeaderGroup<Experiment>[]
   orderedColumns: Column[]
-  onDragOver: OnDragOver
-  onDragStart: OnDragStart
-  onDrop: OnDrop
+  onDragEnter: DragFunction
+  onDragStart: DragFunction
+  onDrop: DragFunction
   firstExpColumnCellId: string
   setExpColumnNeedsShadow: (needsShadow: boolean) => void
   root: HTMLElement | null
@@ -305,7 +303,7 @@ export const TableHeader: React.FC<TableHeaderProps> = ({
   column,
   columns,
   orderedColumns,
-  onDragOver,
+  onDragEnter,
   onDragStart,
   onDrop,
   root,
@@ -359,7 +357,7 @@ export const TableHeader: React.FC<TableHeaderProps> = ({
       sortOrder={sortOrder}
       sortEnabled={isSortable}
       hasFilter={hasFilter}
-      onDragOver={onDragOver}
+      onDragEnter={onDragEnter}
       onDragStart={onDragStart}
       onDrop={onDrop}
       menuDisabled={!isSortable && column.group !== ColumnType.PARAMS}

webview/src/shared/components/dragDrop/DragDropContainer.tsx

@@ -4,13 +4,13 @@ import React, {
   useEffect,
   useState,
   useRef,
-  DragEventHandler,
   CSSProperties
 } from 'react'
 import { useDispatch, useSelector } from 'react-redux'
 import { DragEnterDirection, getDragEnterDirection } from './util'
 import { changeRef } from './dragDropSlice'
 import styles from './styles.module.scss'
+import { DropTarget } from './DropTarget'
 import { getIDIndex, getIDWithoutIndex } from '../../../util/ids'
 import { Any } from '../../../util/objects'
 import { PlotsState } from '../../../plots/store'
@@ -38,25 +38,6 @@ export type OnDrop = (
   groupId: string,
   position: number
 ) => void
-
-export const makeTarget = (
-  dropTarget: JSX.Element,
-  handleDragOver: DragEventHandler<HTMLElement>,
-  handleOnDrop: DragEventHandler<HTMLElement>,
-  id: string,
-  className?: string
-) => (
-  <div
-    data-testid="drop-target"
-    key="drop-target"
-    onDragOver={handleDragOver}
-    onDrop={handleOnDrop}
-    id={`${id}__drop`}
-    className={className}
-  >
-    {dropTarget}
-  </div>
-)
 interface DragDropContainerProps {
   order: string[]
   setOrder: (order: string[]) => void
@@ -253,20 +234,26 @@ export const DragDropContainer: React.FC<DragDropContainerProps> = ({
     />
   )
 
-  const createItemWithDropTarget = (id: string, item: JSX.Element) => {
-    const isEnteringRight = direction === DragEnterDirection.RIGHT
-    const targetClassName = shouldShowOnDrag
+  const getDropTargetClassNames = (isEnteringRight: boolean) =>
+    shouldShowOnDrag
       ? cx(styles.dropTargetWhenShowingOnDrag, {
           [styles.dropTargetWhenShowingOnDragLeft]: !isEnteringRight,
           [styles.dropTargetWhenShowingOnDragRight]: isEnteringRight
         })
       : undefined
-    const target = makeTarget(
-      dropTarget,
-      handleDragOver,
-      handleOnDrop,
-      id,
-      targetClassName
+
+  const createItemWithDropTarget = (id: string, item: JSX.Element) => {
+    const isEnteringRight = direction === DragEnterDirection.RIGHT
+    const target = (
+      <DropTarget
+        key="drop-target"
+        onDragOver={handleDragOver}
+        onDrop={handleOnDrop}
+        id={id}
+        className={getDropTargetClassNames(isEnteringRight)}
+      >
+        {dropTarget}
+      </DropTarget>
     )
     const itemWithTag = shouldShowOnDrag ? (
       <div key="item" {...item.props} />

webview/src/shared/components/dragDrop/DragDropWorkbench.tsx renamed to webview/src/shared/components/dragDrop/Draggable.tsx

@@ -1,22 +1,20 @@
 import React, { DragEvent } from 'react'
 import { useDispatch, useSelector } from 'react-redux'
-import { makeTarget } from './DragDropContainer'
 import { setGroup } from './dragDropSlice'
+import { DropTarget } from './DropTarget'
 import { ExperimentsState } from '../../../experiments/store'
 
-export type OnDrop = (draggedId: string, draggedOverId: string) => void
-export type OnDragStart = (draggedId: string) => void
-export type OnDragOver = (draggedId: string, draggedOverId: string) => void
+export type DragFunction = (e: DragEvent<HTMLElement>) => void
 
 export interface DraggableProps {
   id: string
   group: string
   disabled: boolean
-  dropTarget: JSX.Element
   children: JSX.Element
-  onDrop?: OnDrop
-  onDragStart?: OnDragStart
-  onDragOver?: OnDragOver
+  dropTarget: JSX.Element
+  onDrop: DragFunction
+  onDragStart: DragFunction
+  onDragEnter: DragFunction
 }
 
 export const Draggable: React.FC<DraggableProps> = ({
@@ -26,16 +24,13 @@ export const Draggable: React.FC<DraggableProps> = ({
   disabled,
   dropTarget,
   onDrop,
-  onDragOver,
+  onDragEnter,
   onDragStart
-  // eslint-disable-next-line sonarjs/cognitive-complexity
 }) => {
-  const groupStates = useSelector(
-    (state: ExperimentsState) => state.dragAndDrop.groups
+  const groupState = useSelector(
+    (state: ExperimentsState) => state.dragAndDrop.groups[group] || {}
   )
   const dispatch = useDispatch()
-
-  const groupState = groupStates[group] || {}
   const { draggedOverId, draggedId } = groupState
 
   const modifyGroup = (id: string) => {
@@ -54,15 +49,10 @@ export const Draggable: React.FC<DraggableProps> = ({
     const { id } = e.currentTarget
     e.dataTransfer.effectAllowed = 'move'
     e.dataTransfer.dropEffect = 'move'
+    e.dataTransfer.setData('itemId', id)
+    e.dataTransfer.setData('group', group)
     modifyGroup(id)
-    onDragStart?.(id)
-  }
-
-  const handleOnDrop = () => {
-    !disabled &&
-      draggedId &&
-      draggedOverId &&
-      onDrop?.(draggedId, draggedOverId)
+    onDragStart(e)
   }
 
   const handleDragEnter = (e: DragEvent<HTMLElement>) => {
@@ -71,7 +61,7 @@ export const Draggable: React.FC<DraggableProps> = ({
 
       if (id !== draggedId && id !== draggedOverId) {
         modifyGroup(id)
-        onDragOver?.(draggedId, id)
+        onDragEnter(e)
       }
     }
   }
@@ -93,21 +83,29 @@ export const Draggable: React.FC<DraggableProps> = ({
     )
   }
 
-  const item = (
+  if (dropTarget && id === draggedOverId) {
+    return (
+      <DropTarget
+        onDragOver={handleDragOver}
+        onDrop={onDrop}
+        id={id}
+        key="drop-target"
+      >
+        {dropTarget}
+      </DropTarget>
+    )
+  }
+
+  return (
     <children.type
       {...children.props}
       id={id}
       onDragStart={handleDragStart}
       onDragEnd={handleDragEnd}
       onDragOver={handleDragOver}
       onDragEnter={handleDragEnter}
-      onDrop={handleOnDrop}
+      onDrop={onDrop}
       draggable={!disabled}
     />
   )
-  if (id === draggedOverId) {
-    return makeTarget(dropTarget, handleDragOver, handleOnDrop, id)
-  }
-
-  return item
 }