diff --git a/webview/src/plots/components/App.test.tsx b/webview/src/plots/components/App.test.tsx index 3a89cd4080..6bd10d9826 100644 --- a/webview/src/plots/components/App.test.tsx +++ b/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 two 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 one in the new one + dragAndDrop(anotherSingleViewPlot, movedSingleViewPlot) const sections = screen.getAllByTestId(/^plots-section_/) diff --git a/webview/src/plots/components/checkpointPlots/CheckpointPlots.tsx b/webview/src/plots/components/checkpointPlots/CheckpointPlots.tsx index 427aa8ee59..7f652f4523 100644 --- a/webview/src/plots/components/checkpointPlots/CheckpointPlots.tsx +++ b/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 = ({ colors }) => { const [order, setOrder] = useState(plots.map(plot => plot.title)) + const draggedRef = useRef() useEffect(() => { setOrder(pastOrder => performOrderedUpdate(pastOrder, plots, 'title')) @@ -68,6 +72,7 @@ export const CheckpointPlots: React.FC = ({ disabledDropIds={[]} items={items as JSX.Element[]} group="live-plots" + draggedRef={draggedRef} dropTarget={{ element: , wrapperTag: 'div' diff --git a/webview/src/plots/components/comparisonTable/ComparisonTableHead.tsx b/webview/src/plots/components/comparisonTable/ComparisonTableHead.tsx index f75b86f107..ea150c7fa3 100644 --- a/webview/src/plots/components/comparisonTable/ComparisonTableHead.tsx +++ b/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 = ({ setColumnsOrder, setPinnedColumn }) => { + const draggedRef = useRef() + const items = columns.map(({ revision, displayColor }) => { const isPinned = revision === pinnedColumn return ( @@ -51,6 +56,7 @@ export const ComparisonTableHead: React.FC = ({ disabledDropIds={[pinnedColumn]} items={items} group="comparison" + draggedRef={draggedRef} dropTarget={{ element: , wrapperTag: 'th' diff --git a/webview/src/plots/components/templatePlots/TemplatePlots.tsx b/webview/src/plots/components/templatePlots/TemplatePlots.tsx index 04fbe97a04..617b521d30 100644 --- a/webview/src/plots/components/templatePlots/TemplatePlots.tsx +++ b/webview/src/plots/components/templatePlots/TemplatePlots.tsx @@ -81,19 +81,20 @@ export const TemplatePlots: React.FC = ({ 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 = ({ plots }) => { oldGroupId, draggedId, newGroupId, - entry + entry, + position ) setSectionOrder(updatedSections) diff --git a/webview/src/plots/components/templatePlots/TemplatePlotsGrid.tsx b/webview/src/plots/components/templatePlots/TemplatePlotsGrid.tsx index 211435eadc..db772cf076 100644 --- a/webview/src/plots/components/templatePlots/TemplatePlotsGrid.tsx +++ b/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 + onDropInSection: OnDrop + draggedRef: MutableRefObject multiView: boolean setSectionEntries: (groupIndex: number, entries: TemplatePlotEntry[]) => void } diff --git a/webview/src/plots/components/templatePlots/util.ts b/webview/src/plots/components/templatePlots/util.ts index fff7c3a4a4..84a87cbfed 100644 --- a/webview/src/plots/components/templatePlots/util.ts +++ b/webview/src/plots/components/templatePlots/util.ts @@ -13,9 +13,18 @@ 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 = ( @@ -23,14 +32,15 @@ export const removeFromPreviousAndAddToNewSection = ( 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 }) diff --git a/webview/src/shared/components/dragDrop/DragDropContainer.tsx b/webview/src/shared/components/dragDrop/DragDropContainer.tsx index 2dcfd3339d..0b4c05fbac 100644 --- a/webview/src/shared/components/dragDrop/DragDropContainer.tsx +++ b/webview/src/shared/components/dragDrop/DragDropContainer.tsx @@ -14,15 +14,7 @@ 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 } @@ -30,18 +22,24 @@ const orderIdxTune = ( 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 - dropTarget?: { + onDrop?: OnDrop + draggedRef: MutableRefObject + dropTarget: { element: JSX.Element wrapperTag: 'div' | 'th' } @@ -62,15 +60,19 @@ export const DragDropContainer: React.FC = ({ const [direction, setDirection] = useState(DragEnterDirection.RIGHT) const draggedOverIdTimeout = useRef(0) + const cleanup = () => { + setDraggedOverId('') + setDraggedId('') + setDirection(DragEnterDirection.RIGHT) + } + useEffect(() => { return () => clearTimeout(draggedOverIdTimeout.current) }, []) - const setDraggedRef = (draggedInfo?: DraggedInfo) => { - if (draggedRef) { - draggedRef.current = draggedInfo - } - } + useEffect(() => { + cleanup() + }, [order]) const handleDragStart = (e: DragEvent) => { const { id } = e.currentTarget @@ -89,23 +91,17 @@ export const DragDropContainer: React.FC = ({ 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, droppedIndex: number, @@ -119,9 +115,9 @@ export const DragDropContainer: React.FC = ({ 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) => { @@ -136,11 +132,7 @@ export const DragDropContainer: React.FC = ({ : 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 = ({ } const handleDragEnter = (e: DragEvent) => { - 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 = ({ 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 = ({ const { id } = draggable.props const item = id && buildItem(id, draggable) - if (id === draggedOverId && dropTarget && direction) { + if (id === draggedOverId) { const target = (