Fix dragging between plot sections (#1594)

* Fix bug of placeholder when dragging comparison table header to new template plot section

* Add test

* Use a global draggedRef

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

@@ -816,6 +816,25 @@ describe('App', () => {
     expect(topDropIcon).toBeInTheDocument()
   })
 
+  it('should not show a drop target when moving an element from a whole different section (comparison to template)', () => {
+    renderAppWithData({
+      comparison: comparisonTableFixture,
+      sectionCollapsed: DEFAULT_SECTION_COLLAPSED,
+      template: complexTemplatePlotsFixture
+    })
+
+    const headers = screen.getAllByRole('columnheader')
+    const bottomSection = screen.getByTestId(NewSectionBlock.BOTTOM)
+
+    dragEnter(headers[1], bottomSection, DragEnterDirection.LEFT)
+
+    const bottomDropIcon = screen.queryByTestId(
+      `${NewSectionBlock.BOTTOM}_drop-icon`
+    )
+
+    expect(bottomDropIcon).not.toBeInTheDocument()
+  })
+
   it('should prevent default behaviour when dragging over a new section', () => {
     renderAppWithData({
       sectionCollapsed: DEFAULT_SECTION_COLLAPSED,

webview/src/plots/components/Plots.tsx

@@ -13,6 +13,7 @@ import { TemplatePlots } from './templatePlots/TemplatePlots'
 import { EmptyState } from '../../shared/components/emptyState/EmptyState'
 import { Modal } from '../../shared/components/modal/Modal'
 import { Theme } from '../../shared/components/theme/Theme'
+import { DragDropProvider } from '../../shared/components/dragDrop/DragDropContext'
 import { sendMessage } from '../../shared/vscode'
 import { PlotsWebviewState } from '../hooks/useAppReducer'
 
@@ -95,60 +96,62 @@ export const Plots = ({
 
   return (
     <Theme>
-      {templatePlots && (
-        <PlotsContainer
-          title={templatePlots.sectionName}
-          sectionKey={Section.TEMPLATE_PLOTS}
-          currentSize={templatePlots.size}
-          {...basicContainerProps}
-        >
-          <TemplatePlots
-            plots={templatePlots.plots}
-            onPlotClick={handlePlotClick}
-          />
-        </PlotsContainer>
-      )}
-      {comparisonTable && (
-        <PlotsContainer
-          title={comparisonTable.sectionName}
-          sectionKey={Section.COMPARISON_TABLE}
-          currentSize={comparisonTable.size}
-          {...basicContainerProps}
-        >
-          <ComparisonTable
-            plots={comparisonTable.plots}
-            revisions={comparisonTable.revisions}
-          />
-        </PlotsContainer>
-      )}
-      {checkpointPlots && (
-        <PlotsContainer
-          title={checkpointPlots.sectionName}
-          sectionKey={Section.CHECKPOINT_PLOTS}
-          menu={{
-            metrics,
-            selectedMetrics: selectedPlots,
-            setSelectedPlots: setSelectedMetrics
-          }}
-          currentSize={checkpointPlots.size}
-          {...basicContainerProps}
-        >
-          <CheckpointPlots
-            plots={checkpointPlots.plots.filter(plot =>
-              selectedPlots?.includes(plot.title)
-            )}
-            colors={checkpointPlots.colors}
-            onPlotClick={handlePlotClick}
-          />
-        </PlotsContainer>
-      )}
-      {zoomedInPlot && (
-        <Modal onClose={() => setZoomedInPlot(undefined)}>
-          <div className={styles.zoomedInPlot} data-testid="zoomed-in-plot">
-            {zoomedInPlot}
-          </div>
-        </Modal>
-      )}
+      <DragDropProvider>
+        {templatePlots && (
+          <PlotsContainer
+            title={templatePlots.sectionName}
+            sectionKey={Section.TEMPLATE_PLOTS}
+            currentSize={templatePlots.size}
+            {...basicContainerProps}
+          >
+            <TemplatePlots
+              plots={templatePlots.plots}
+              onPlotClick={handlePlotClick}
+            />
+          </PlotsContainer>
+        )}
+        {comparisonTable && (
+          <PlotsContainer
+            title={comparisonTable.sectionName}
+            sectionKey={Section.COMPARISON_TABLE}
+            currentSize={comparisonTable.size}
+            {...basicContainerProps}
+          >
+            <ComparisonTable
+              plots={comparisonTable.plots}
+              revisions={comparisonTable.revisions}
+            />
+          </PlotsContainer>
+        )}
+        {checkpointPlots && (
+          <PlotsContainer
+            title={checkpointPlots.sectionName}
+            sectionKey={Section.CHECKPOINT_PLOTS}
+            menu={{
+              metrics,
+              selectedMetrics: selectedPlots,
+              setSelectedPlots: setSelectedMetrics
+            }}
+            currentSize={checkpointPlots.size}
+            {...basicContainerProps}
+          >
+            <CheckpointPlots
+              plots={checkpointPlots.plots.filter(plot =>
+                selectedPlots?.includes(plot.title)
+              )}
+              colors={checkpointPlots.colors}
+              onPlotClick={handlePlotClick}
+            />
+          </PlotsContainer>
+        )}
+        {zoomedInPlot && (
+          <Modal onClose={() => setZoomedInPlot(undefined)}>
+            <div className={styles.zoomedInPlot} data-testid="zoomed-in-plot">
+              {zoomedInPlot}
+            </div>
+          </Modal>
+        )}
+      </DragDropProvider>
     </Theme>
   )
 }

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

@@ -1,23 +1,20 @@
+import React, { useEffect, useState } from 'react'
+import VegaLite from 'react-vega/lib/VegaLite'
 import {
   CheckpointPlotData,
   CheckpointPlotsColors
 } from 'dvc/src/plots/webview/contract'
 import { MessageFromWebviewType } from 'dvc/src/webview/contract'
-import React, { useEffect, useRef, useState } from 'react'
-import VegaLite from 'react-vega/lib/VegaLite'
 import { createSpec } from './util'
-import {
-  DragDropContainer,
-  DraggedInfo
-} from '../../../shared/components/dragDrop/DragDropContainer'
-import { GripIcon } from '../../../shared/components/dragDrop/GripIcon'
+import styles from '../styles.module.scss'
 import { EmptyState } from '../../../shared/components/emptyState/EmptyState'
-import { sendMessage } from '../../../shared/vscode'
+import { DragDropContainer } from '../../../shared/components/dragDrop/DragDropContainer'
 import { performOrderedUpdate } from '../../../util/objects'
 import { withScale } from '../../../util/styles'
+import { GripIcon } from '../../../shared/components/dragDrop/GripIcon'
+import { sendMessage } from '../../../shared/vscode'
 import { config } from '../constants'
 import { DropTarget } from '../DropTarget'
-import styles from '../styles.module.scss'
 import { ZoomablePlotProps } from '../templatePlots/util'
 
 interface CheckpointPlotsProps extends ZoomablePlotProps {
@@ -31,7 +28,6 @@ export const CheckpointPlots: React.FC<CheckpointPlotsProps> = ({
   onPlotClick
 }) => {
   const [order, setOrder] = useState(plots.map(plot => plot.title))
-  const draggedRef = useRef<DraggedInfo>()
 
   useEffect(() => {
     setOrder(pastOrder => performOrderedUpdate(pastOrder, plots, 'title'))
@@ -89,7 +85,6 @@ 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/ComparisonTable.test.tsx

@@ -14,6 +14,7 @@ import {
 } from '../../../test/dragDrop'
 import { vsCodeApi } from '../../../shared/api'
 import { DragEnterDirection } from '../../../shared/components/dragDrop/util'
+import { DragDropProvider } from '../../../shared/components/dragDrop/DragDropContext'
 
 const getHeaders = () => screen.getAllByRole('columnheader')
 
@@ -31,7 +32,11 @@ describe('ComparisonTable', () => {
   const basicProps: ComparisonTableProps = comparisonTableFixture
   const revisions = basicProps.revisions.map(({ revision }) => revision)
   const renderTable = (props = basicProps) =>
-    render(<ComparisonTable {...props} />)
+    render(
+      <DragDropProvider>
+        <ComparisonTable {...props} />
+      </DragDropProvider>
+    )
 
   it('should render a table', () => {
     renderTable()

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

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

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

@@ -1,18 +1,18 @@
-import React, { DragEvent, MutableRefObject } from 'react'
+import React, { DragEvent, useContext } from 'react'
 import cx from 'classnames'
 import { TemplatePlotSection } from 'dvc/src/plots/webview/contract'
 import styles from '../styles.module.scss'
 import { getIDWithoutIndex } from '../../../util/ids'
-import { DraggedInfo } from '../../../shared/components/dragDrop/DragDropContainer'
 import { AllIcons, Icon } from '../../../shared/components/Icon'
+import { DragDropContext } from '../../../shared/components/dragDrop/DragDropContext'
 
 interface AddedSectionProps {
   id: string
   hoveredSection: string
   setHoveredSection: (section: string) => void
   onDrop: (e: DragEvent<HTMLElement>) => void
   closestSection: TemplatePlotSection
-  draggedRef: MutableRefObject<DraggedInfo | undefined>
+  acceptedGroups: string[]
 }
 
 export const AddedSection: React.FC<AddedSectionProps> = ({
@@ -21,15 +21,19 @@ export const AddedSection: React.FC<AddedSectionProps> = ({
   hoveredSection,
   setHoveredSection,
   closestSection,
-  draggedRef
+  acceptedGroups
 }) => {
+  const { draggedRef } = useContext(DragDropContext)
   const handleDragLeave = () => {
     setHoveredSection('')
   }
 
   const handleDragEnter = (e: DragEvent<HTMLElement>) => {
-    const draggedGroup = getIDWithoutIndex(draggedRef.current?.group)
-    if (draggedGroup !== closestSection.group) {
+    const draggedGroup = getIDWithoutIndex(draggedRef?.group) || ''
+    if (
+      acceptedGroups.includes(draggedGroup) &&
+      draggedGroup !== closestSection.group
+    ) {
       setHoveredSection(e.currentTarget.id)
     }
   }

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

@@ -1,14 +1,13 @@
-import React, { DragEvent, useEffect, useRef, useState } from 'react'
 import {
   TemplatePlotEntry,
   TemplatePlotGroup,
   TemplatePlotSection
 } from 'dvc/src/plots/webview/contract'
+import React, { DragEvent, useState, useEffect } from 'react'
 import { MessageFromWebviewType } from 'dvc/src/webview/contract'
 import { AddedSection } from './AddedSection'
 import { TemplatePlotsGrid } from './TemplatePlotsGrid'
 import { removeFromPreviousAndAddToNewSection, ZoomablePlotProps } from './util'
-import { DraggedInfo } from '../../../shared/components/dragDrop/DragDropContainer'
 import { sendMessage } from '../../../shared/vscode'
 import { createIDWithIndex, getIDIndex } from '../../../util/ids'
 import styles from '../styles.module.scss'
@@ -28,7 +27,6 @@ export const TemplatePlots: React.FC<TemplatePlotsProps> = ({
 }) => {
   const [sections, setSections] = useState<TemplatePlotSection[]>([])
   const [hoveredSection, setHoveredSection] = useState('')
-  const draggedRef = useRef<DraggedInfo>()
 
   useEffect(() => {
     setSections(plots)
@@ -120,7 +118,7 @@ export const TemplatePlots: React.FC<TemplatePlotsProps> = ({
   }
 
   const newDropSection = {
-    draggedRef,
+    acceptedGroups: Object.values(TemplatePlotGroup),
     hoveredSection,
     onDrop: handleDropInNewSection,
     setHoveredSection
@@ -152,7 +150,6 @@ export const TemplatePlots: React.FC<TemplatePlotsProps> = ({
                 groupId={groupId}
                 groupIndex={i}
                 onDropInSection={handleDropInSection}
-                draggedRef={draggedRef}
                 multiView={section.group === TemplatePlotGroup.MULTI_VIEW}
                 setSectionEntries={setSectionEntries}
                 onPlotClick={onPlotClick}

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

@@ -1,12 +1,11 @@
 import cx from 'classnames'
 import { TemplatePlotEntry } from 'dvc/src/plots/webview/contract'
 import { reorderObjectList } from 'dvc/src/util/array'
-import React, { MutableRefObject, useEffect, useState } from 'react'
+import React, { useEffect, useState } from 'react'
 import { VegaLite, VisualizationSpec } from 'react-vega'
 import { ZoomablePlotProps } from './util'
 import {
   DragDropContainer,
-  DraggedInfo,
   OnDrop
 } from '../../../shared/components/dragDrop/DragDropContainer'
 import { GripIcon } from '../../../shared/components/dragDrop/GripIcon'
@@ -20,7 +19,6 @@ interface TemplatePlotsGridProps extends ZoomablePlotProps {
   groupId: string
   groupIndex: number
   onDropInSection: OnDrop
-  draggedRef: MutableRefObject<DraggedInfo | undefined>
   multiView: boolean
   setSectionEntries: (groupIndex: number, entries: TemplatePlotEntry[]) => void
 }
@@ -35,7 +33,6 @@ export const TemplatePlotsGrid: React.FC<TemplatePlotsGridProps> = ({
   groupId,
   groupIndex,
   onDropInSection,
-  draggedRef,
   multiView,
   setSectionEntries,
   onPlotClick
@@ -98,7 +95,6 @@ export const TemplatePlotsGrid: React.FC<TemplatePlotsGridProps> = ({
       items={items as JSX.Element[]}
       group={groupId}
       onDrop={onDropInSection}
-      draggedRef={draggedRef}
       dropTarget={{
         element: <DropTarget />,
         wrapperTag: 'div'