Dragging feedback to other section

webview/src/plots/components/App.test.tsx

@@ -758,7 +758,7 @@ describe('App', () => {
     ])
   })
 
-  it('should move a template plot from one type in another section of the same type', async () => {
+  it('should move a template plot from one type in another section of the same type and show a drop targets', async () => {
     renderAppWithData({
       sectionCollapsed: DEFAULT_SECTION_COLLAPSED,
       template: complexTemplatePlotsFixture
@@ -777,6 +777,14 @@ describe('App', () => {
       join('plot_other', 'plot.tsv')
     )
 
+    dragEnter(
+      anotherSingleViewPlot,
+      movedSingleViewPlot,
+      DragEnterDirection.LEFT
+    )
+
+    expect(screen.getAllByTestId('drop-target').length).toBe(2) // One in the old section and on in the newest one
+
     dragAndDrop(anotherSingleViewPlot, movedSingleViewPlot)
 
     const sections = screen.getAllByTestId(/^plots-section_/)

webview/src/plots/components/checkpointPlots/CheckpointPlots.tsx

@@ -3,11 +3,14 @@ import {
   CheckpointPlotData,
   CheckpointPlotsColors
 } from 'dvc/src/plots/webview/contract'
-import React, { useEffect, useState } from 'react'
+import React, { useEffect, useRef, useState } from 'react'
 import { Plot } from './Plot'
 import styles from '../styles.module.scss'
 import { EmptyState } from '../../../shared/components/emptyState/EmptyState'
-import { DragDropContainer } from '../../../shared/components/dragDrop/DragDropContainer'
+import {
+  DragDropContainer,
+  DraggedInfo
+} from '../../../shared/components/dragDrop/DragDropContainer'
 import { performOrderedUpdate } from '../../../util/objects'
 import { withScale } from '../../../util/styles'
 import { GripIcon } from '../../../shared/components/dragDrop/GripIcon'
@@ -24,6 +27,7 @@ export const CheckpointPlots: React.FC<CheckpointPlotsProps> = ({
   colors
 }) => {
   const [order, setOrder] = useState(plots.map(plot => plot.title))
+  const draggedRef = useRef<DraggedInfo>()
 
   useEffect(() => {
     setOrder(pastOrder => performOrderedUpdate(pastOrder, plots, 'title'))
@@ -68,6 +72,7 @@ export const CheckpointPlots: React.FC<CheckpointPlotsProps> = ({
         disabledDropIds={[]}
         items={items as JSX.Element[]}
         group="live-plots"
+        draggedRef={draggedRef}
         dropTarget={{
           element: <DropTarget />,
           wrapperTag: 'div'

webview/src/plots/components/comparisonTable/ComparisonTableHead.tsx

@@ -1,10 +1,13 @@
-import React from 'react'
+import React, { useRef } from 'react'
 import { ComparisonRevision } from 'dvc/src/plots/webview/contract'
 import cx from 'classnames'
 import styles from './styles.module.scss'
 import { DropTarget } from './DropTarget'
 import { ComparisonTableHeader } from './ComparisonTableHeader'
-import { DragDropContainer } from '../../../shared/components/dragDrop/DragDropContainer'
+import {
+  DragDropContainer,
+  DraggedInfo
+} from '../../../shared/components/dragDrop/DragDropContainer'
 
 export type ComparisonTableColumn = ComparisonRevision
 
@@ -21,6 +24,8 @@ export const ComparisonTableHead: React.FC<ComparisonTableHeadProps> = ({
   setColumnsOrder,
   setPinnedColumn
 }) => {
+  const draggedRef = useRef<DraggedInfo>()
+
   const items = columns.map(({ revision, displayColor }) => {
     const isPinned = revision === pinnedColumn
     return (
@@ -51,6 +56,7 @@ export const ComparisonTableHead: React.FC<ComparisonTableHeadProps> = ({
           disabledDropIds={[pinnedColumn]}
           items={items}
           group="comparison"
+          draggedRef={draggedRef}
           dropTarget={{
             element: <DropTarget />,
             wrapperTag: 'th'

webview/src/plots/components/templatePlots/TemplatePlots.tsx

@@ -81,19 +81,20 @@ export const TemplatePlots: React.FC<TemplatePlotsProps> = ({ plots }) => {
 
     if (e.currentTarget.id === NewSectionBlock.TOP) {
       if (firstSection.group !== group) {
-        setTimeout(() => setSectionOrder([newSection, ...updatedSections]), 0)
+        setTimeout(() => setSectionOrder([newSection, ...updatedSections]), 1)
       }
       return
     }
     if (lastSection.group !== group) {
-      setTimeout(() => setSectionOrder([...updatedSections, newSection]), 0)
+      setTimeout(() => setSectionOrder([...updatedSections, newSection]), 1)
     }
   }
 
   const handleDropInSection = (
     draggedId: string,
     draggedGroup: string,
-    groupId: string
+    groupId: string,
+    position: number
   ) => {
     if (draggedGroup === groupId) {
       return
@@ -108,7 +109,8 @@ export const TemplatePlots: React.FC<TemplatePlotsProps> = ({ plots }) => {
       oldGroupId,
       draggedId,
       newGroupId,
-      entry
+      entry,
+      position
     )
 
     setSectionOrder(updatedSections)

webview/src/plots/components/templatePlots/TemplatePlotsGrid.tsx

@@ -7,7 +7,8 @@ import styles from '../styles.module.scss'
 import { config } from '../constants'
 import {
   DragDropContainer,
-  DraggedInfo
+  DraggedInfo,
+  OnDrop
 } from '../../../shared/components/dragDrop/DragDropContainer'
 import { GripIcon } from '../../../shared/components/dragDrop/GripIcon'
 import { withScale } from '../../../util/styles'
@@ -17,12 +18,8 @@ interface TemplatePlotsGridProps {
   entries: TemplatePlotEntry[]
   groupId: string
   groupIndex: number
-  onDropInSection: (
-    draggedId: string,
-    draggedGroup: string,
-    groupId: string
-  ) => void
-  draggedRef?: MutableRefObject<DraggedInfo | undefined>
+  onDropInSection: OnDrop
+  draggedRef: MutableRefObject<DraggedInfo | undefined>
   multiView: boolean
   setSectionEntries: (groupIndex: number, entries: TemplatePlotEntry[]) => void
 }

webview/src/plots/components/templatePlots/util.ts

@@ -13,24 +13,34 @@ const remove = (section: TemplatePlotSection, entryId: string) => {
     : null
 }
 
-const add = (section: TemplatePlotSection, entry: TemplatePlotEntry) => {
-  section.entries.push(entry)
-  return { entries: section.entries, group: section.group }
+const add = (
+  section: TemplatePlotSection,
+  entry: TemplatePlotEntry,
+  position?: number
+) => {
+  const entries = [...section.entries]
+  entries.splice(
+    position !== undefined ? position : entries.length - 1,
+    0,
+    entry
+  )
+  return { entries, group: section.group }
 }
 
 export const removeFromPreviousAndAddToNewSection = (
   sections: TemplatePlotSection[],
   oldSectionIndex: number,
   entryId: string,
   newGroupIndex?: number,
-  entry?: TemplatePlotEntry
+  entry?: TemplatePlotEntry,
+  position?: number
 ) =>
   sections
     .map((section, i) => {
       if (i === oldSectionIndex) {
         return remove(section, entryId)
       } else if (i === newGroupIndex && entry) {
-        return add(section, entry)
+        return add(section, entry, position)
       }
       return section
     })

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

@@ -14,34 +14,32 @@ export type DraggedInfo = {
   group: string
 }
 
-const orderIdxTune = (
-  hasDropTargetOrIsNew: boolean,
-  direction: DragEnterDirection,
-  isAfter: boolean
-) => {
-  if (!hasDropTargetOrIsNew) {
-    return 0
-  }
-
+const orderIdxTune = (direction: DragEnterDirection, isAfter: boolean) => {
   if (direction === DragEnterDirection.RIGHT) {
     return isAfter ? 0 : 1
   }
 
   return isAfter ? -1 : 0
 }
 
-const isSameGroup = (group1: string, group2: string) =>
+const isSameGroup = (group1?: string, group2?: string) =>
   getIDWithoutIndex(group1) === getIDWithoutIndex(group2)
 
+export type OnDrop = (
+  draggedId: string,
+  draggedGroup: string,
+  groupId: string,
+  position: number
+) => void
 interface DragDropContainerProps {
   order: string[]
   setOrder: (order: string[]) => void
   disabledDropIds?: string[]
   items: JSX.Element[] // Every item must have a id prop for drag and drop to work
   group: string
-  onDrop?: (draggedId: string, draggedGroup: string, groupId: string) => void
-  draggedRef?: MutableRefObject<DraggedInfo | undefined>
-  dropTarget?: {
+  onDrop?: OnDrop
+  draggedRef: MutableRefObject<DraggedInfo | undefined>
+  dropTarget: {
     element: JSX.Element
     wrapperTag: 'div' | 'th'
   }
@@ -62,15 +60,19 @@ export const DragDropContainer: React.FC<DragDropContainerProps> = ({
   const [direction, setDirection] = useState(DragEnterDirection.RIGHT)
   const draggedOverIdTimeout = useRef<number>(0)
 
+  const cleanup = () => {
+    setDraggedOverId('')
+    setDraggedId('')
+    setDirection(DragEnterDirection.RIGHT)
+  }
+
   useEffect(() => {
     return () => clearTimeout(draggedOverIdTimeout.current)
   }, [])
 
-  const setDraggedRef = (draggedInfo?: DraggedInfo) => {
-    if (draggedRef) {
-      draggedRef.current = draggedInfo
-    }
-  }
+  useEffect(() => {
+    cleanup()
+  }, [items])
 
   const handleDragStart = (e: DragEvent<HTMLElement>) => {
     const { id } = e.currentTarget
@@ -89,23 +91,17 @@ export const DragDropContainer: React.FC<DragDropContainerProps> = ({
     e.dataTransfer.setData('group', group)
     e.dataTransfer.effectAllowed = 'move'
     e.dataTransfer.dropEffect = 'move'
-    setDraggedRef({
+    draggedRef.current = {
       group,
       itemId: id,
       itemIndex
-    })
+    }
     draggedOverIdTimeout.current = window.setTimeout(() => {
       setDraggedId(id)
       setDraggedOverId(order[toIdx])
     }, 0)
   }
 
-  const handleDragEnd = () => {
-    setDraggedOverId('')
-    setDraggedId('')
-    setDirection(DragEnterDirection.RIGHT)
-  }
-
   const applyDrop = (
     e: DragEvent<HTMLElement>,
     droppedIndex: number,
@@ -119,9 +115,9 @@ export const DragDropContainer: React.FC<DragDropContainerProps> = ({
     newOrder.splice(droppedIndex, 0, dragged)
 
     setOrder(newOrder)
-    setDraggedRef(undefined)
+    draggedRef.current = undefined
 
-    onDrop?.(oldDraggedId, e.dataTransfer.getData('group'), group)
+    onDrop?.(oldDraggedId, e.dataTransfer.getData('group'), group, droppedIndex)
   }
 
   const handleOnDrop = (e: DragEvent<HTMLElement>) => {
@@ -136,11 +132,7 @@ export const DragDropContainer: React.FC<DragDropContainerProps> = ({
       : getIDIndex(e.dataTransfer.getData('itemIndex'))
 
     const droppedIndex = order.indexOf(e.currentTarget.id.split('__')[0])
-    const orderIdxChange = orderIdxTune(
-      !!dropTarget && !isNew,
-      direction,
-      droppedIndex > draggedIndex
-    )
+    const orderIdxChange = orderIdxTune(direction, droppedIndex > draggedIndex)
     const orderIdxChanged = droppedIndex + orderIdxChange
     const isEnabled = !disabledDropIds.includes(order[orderIdxChanged])
 
@@ -150,7 +142,7 @@ export const DragDropContainer: React.FC<DragDropContainerProps> = ({
   }
 
   const handleDragEnter = (e: DragEvent<HTMLElement>) => {
-    if (draggedId) {
+    if (isSameGroup(draggedRef.current?.group, group)) {
       const { id } = e.currentTarget
       if (id !== draggedId && !id.includes('__drop')) {
         setDraggedOverId(id)
@@ -170,15 +162,12 @@ export const DragDropContainer: React.FC<DragDropContainerProps> = ({
       key={draggable.key}
       {...draggable.props}
       onDragStart={handleDragStart}
-      onDragEnd={handleDragEnd}
+      onDragEnd={cleanup}
       onDragOver={handleDragOver}
       onDragEnter={handleDragEnter}
       onDrop={handleOnDrop}
       draggable={!disabledDropIds.includes(id)}
-      style={
-        (id === draggedId && dropTarget && { display: 'none' }) ||
-        draggable.props.style
-      }
+      style={(id === draggedId && { display: 'none' }) || draggable.props.style}
     />
   )
 
@@ -188,7 +177,7 @@ export const DragDropContainer: React.FC<DragDropContainerProps> = ({
         const { id } = draggable.props
         const item = id && buildItem(id, draggable)
 
-        if (id === draggedOverId && dropTarget && direction) {
+        if (id === draggedOverId) {
           const target = (
             <dropTarget.wrapperTag
               data-testid="drop-target"