Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make drop zones larger when dragging in the same section #2206

Merged
merged 13 commits into from
Aug 18, 2022
66 changes: 64 additions & 2 deletions webview/src/plots/components/App.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -801,13 +801,75 @@ describe('App', () => {

dragEnter(
anotherSingleViewPlot,
'plots-section_template-single_0',
DragEnterDirection.RIGHT
'template-single_0',
DragEnterDirection.LEFT
)

expect(screen.getByTestId('plot_drop-target')).toBeInTheDocument()
})

it('should show a drop target at the end of the template plots section when moving a plot inside of one section but not over any other plot', () => {
renderAppWithOptionalData({
template: complexTemplatePlotsFixture
})

const aSingleViewPlot = screen.getByTestId(join('plot_other', 'plot.tsv'))

dragEnter(aSingleViewPlot, 'template-single_0', DragEnterDirection.LEFT)

expect(screen.getByTestId('plot_drop-target')).toBeInTheDocument()
})

it('should drop a plot at the end of the template plots section when moving a plot inside of one section but not over any other plot', () => {
renderAppWithOptionalData({
template: complexTemplatePlotsFixture
})

const aSingleViewPlot = screen.getByTestId(join('plot_other', 'plot.tsv'))
const topSection = screen.getByTestId('plots-section_template-single_0')

dragAndDrop(aSingleViewPlot, topSection)
const plots = within(topSection).getAllByTestId(/^plot_/)

expect(plots.map(plot => plot.id)).toStrictEqual([
join('logs', 'loss.tsv'),
join('other', 'plot.tsv')
])
})

it('should show a drop target at the end of the checkpoint plots when moving a plot inside the section but not over any other plot', () => {
renderAppWithOptionalData({
checkpoint: checkpointPlotsFixture
})

const plots = screen.getAllByTestId(/summary\.json/)

dragEnter(plots[0], 'checkpoint-plots', DragEnterDirection.LEFT)

expect(screen.getByTestId('plot_drop-target')).toBeInTheDocument()
})

it('should show a drop a plot at the end of the checkpoint plots when moving a plot inside the section but not over any other plot', () => {
renderAppWithOptionalData({
checkpoint: checkpointPlotsFixture
})

const plots = screen.getAllByTestId(/summary\.json/)

dragAndDrop(plots[0], screen.getByTestId('checkpoint-plots'))

const expectedOrder = [
'summary.json:accuracy',
'summary.json:val_loss',
'summary.json:val_accuracy',
'summary.json:loss'
]

expect(
screen.getAllByTestId(/summary\.json/).map(plot => plot.id)
).toStrictEqual(expectedOrder)
})

it('should show a drop zone when hovering a new section', () => {
renderAppWithOptionalData({
template: complexTemplatePlotsFixture
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useEffect, useState } from 'react'
import React, { DragEvent, useEffect, useState } from 'react'
import { useSelector } from 'react-redux'
import cx from 'classnames'
import { ColorScale } from 'dvc/src/plots/webview/contract'
Expand All @@ -17,6 +17,7 @@ import { VirtualizedGrid } from '../../../shared/components/virtualizedGrid/Virt
import { shouldUseVirtualizedGrid } from '../util'
import { useNbItemsPerRow } from '../../hooks/useNbItemsPerRow'
import { PlotsState } from '../../store'
import { changeOrderWithDraggedInfo } from '../../../util/array'

interface CheckpointPlotsProps {
plotsIds: string[]
Expand All @@ -29,7 +30,11 @@ export const CheckpointPlots: React.FC<CheckpointPlotsProps> = ({
}) => {
const [order, setOrder] = useState(plotsIds)
const { size, hasData } = useSelector((state: PlotsState) => state.checkpoint)
const [onSection, setOnSection] = useState(false)
const nbItemsPerRow = useNbItemsPerRow(size)
const draggedRef = useSelector(
(state: PlotsState) => state.dragAndDrop.draggedRef
)

useEffect(() => {
setOrder(pastOrder => performSimpleOrderedUpdate(pastOrder, plotsIds))
Expand All @@ -55,11 +60,26 @@ export const CheckpointPlots: React.FC<CheckpointPlotsProps> = ({

const useVirtualizedGrid = shouldUseVirtualizedGrid(items.length, size)

const handleDropAtTheEnd = () => {
setMetricOrder(changeOrderWithDraggedInfo(order, draggedRef))
}

const handleDragOver = (e: DragEvent) => {
e.preventDefault()
setOnSection(true)
}

return items.length > 0 ? (
<div
data-testid="checkpoint-plots"
id="checkpoint-plots"
className={cx(styles.singleViewPlotsGrid, {
[styles.noBigGrid]: !useVirtualizedGrid
})}
onDragEnter={() => setOnSection(true)}
onDragLeave={() => setOnSection(false)}
onDragOver={handleDragOver}
onDrop={handleDropAtTheEnd}
>
<DragDropContainer
order={order}
Expand All @@ -76,6 +96,7 @@ export const CheckpointPlots: React.FC<CheckpointPlotsProps> = ({
}
: undefined
}
parentDraggedOver={onSection}
/>
</div>
) : (
Expand Down
84 changes: 61 additions & 23 deletions webview/src/plots/components/templatePlots/TemplatePlots.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import React, { DragEvent, useState, useEffect, useRef } from 'react'
import cx from 'classnames'
import { useDispatch, useSelector } from 'react-redux'
import { MessageFromWebviewType } from 'dvc/src/webview/contract'
import { reorderObjectList } from 'dvc/src/util/array'
import { AddedSection } from './AddedSection'
import { TemplatePlotsGrid } from './TemplatePlotsGrid'
import { removeFromPreviousAndAddToNewSection } from './util'
Expand All @@ -19,6 +20,8 @@ import { PlotsState } from '../../store'
import { plotDataStore } from '../plotDataStore'
import { setDraggedOverGroup } from '../../../shared/components/dragDrop/dragDropSlice'
import { EmptyState } from '../../../shared/components/emptyState/EmptyState'
import { isSameGroup } from '../../../shared/components/dragDrop/DragDropContainer'
import { changeOrderWithDraggedInfo } from '../../../util/array'

export enum NewSectionBlock {
TOP = 'drop-section-top',
Expand Down Expand Up @@ -120,7 +123,7 @@ export const TemplatePlots: React.FC = () => {
draggedId: string,
draggedGroup: string,
groupId: string,
position: number
position?: number
) => {
if (draggedGroup === groupId) {
return
Expand Down Expand Up @@ -169,34 +172,69 @@ export const TemplatePlots: React.FC = () => {

const isMultiView = section.group === TemplatePlotGroup.MULTI_VIEW

const classes = cx({
const classes = cx(styles.sectionWrapper, {
[styles.multiViewPlotsGrid]: isMultiView,
[styles.singleViewPlotsGrid]: !isMultiView,
[styles.noBigGrid]: !useVirtualizedGrid
})

const handleDropAtTheEnd = () => {
handleEnteringSection('')
if (!draggedRef) {
return
}

if (draggedRef.group === groupId) {
const order = section.entries.map(s => s.id)
const updatedSections = [...sections]

const newOrder = changeOrderWithDraggedInfo(order, draggedRef)
updatedSections[i] = {
...sections[i],
entries: reorderObjectList<TemplatePlotEntry>(
newOrder,
section.entries,
'id'
)
}
setSections(updatedSections)
} else if (isSameGroup(draggedRef.group, groupId)) {
handleDropInSection(
draggedRef.itemId,
draggedRef.group,
groupId,
section.entries.length
)
}
}

const handleDragOver = (e: DragEvent) => {
e.preventDefault()
handleEnteringSection(groupId)
}

return (
section.entries.length > 0 && (
<div
key={groupId}
id={groupId}
data-testid={`plots-section_${groupId}`}
className={classes}
onDragEnter={() => handleEnteringSection(groupId)}
>
<TemplatePlotsGrid
entries={section.entries}
groupId={groupId}
groupIndex={i}
onDropInSection={handleDropInSection}
multiView={isMultiView}
setSectionEntries={setSectionEntries}
useVirtualizedGrid={useVirtualizedGrid}
nbItemsPerRow={nbItemsPerRow}
parentDraggedOver={draggedOverGroup === groupId}
/>
</div>
)
<div
key={groupId}
id={groupId}
data-testid={`plots-section_${groupId}`}
className={classes}
onDragEnter={() => handleEnteringSection(groupId)}
onDragOver={handleDragOver}
onDrop={handleDropAtTheEnd}
>
<TemplatePlotsGrid
entries={section.entries}
groupId={groupId}
groupIndex={i}
onDropInSection={handleDropInSection}
multiView={isMultiView}
setSectionEntries={setSectionEntries}
useVirtualizedGrid={useVirtualizedGrid}
nbItemsPerRow={nbItemsPerRow}
parentDraggedOver={draggedOverGroup === groupId}
/>
</div>
)
})}
<AddedSection
Expand Down
36 changes: 20 additions & 16 deletions webview/src/plots/components/templatePlots/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,10 @@ import {

const remove = (section: TemplatePlotSection, entryId: string) => {
const entries = section.entries.filter(({ id }) => id !== entryId)
return entries.length > 0
? {
entries,
group: section.group
}
: null
return {
entries,
group: section.group
}
}

const add = (
Expand All @@ -27,21 +25,27 @@ const add = (
return { entries, group: section.group }
}

const cleanup = (section: TemplatePlotSection) =>
section.entries.length > 0 ? section : null

export const removeFromPreviousAndAddToNewSection = (
sections: TemplatePlotSection[],
oldSectionIndex: number,
entryId: string,
newGroupIndex?: number,
entry?: TemplatePlotEntry,
position?: number
) =>
sections
.map((section, i) => {
if (i === oldSectionIndex) {
return remove(section, entryId)
} else if (i === newGroupIndex && entry) {
return add(section, entry, position)
}
return section
})
) => {
const newSections = sections.map((section, i) => {
if (i === oldSectionIndex) {
return remove(section, entryId)
} else if (i === newGroupIndex && entry) {
return add(section, entry, position)
}
return section
})

return newSections
.map(section => cleanup(section))
.filter(Boolean) as TemplatePlotSection[]
}
Loading