Skip to content

Commit a3ba60e

Browse files
authored
Fix drag leave of top and bottom sections (#2320)
* Add a defered drag leave effect to top and bottom sections * Add test
1 parent 7fa1198 commit a3ba60e

File tree

5 files changed

+121
-34
lines changed

5 files changed

+121
-34
lines changed

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

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,12 @@ import { SectionDescription } from './PlotsContainer'
4242
import { CheckpointPlotsById, plotDataStore } from './plotDataStore'
4343
import { plotsReducers } from '../store'
4444
import { vsCodeApi } from '../../shared/api'
45-
import { createBubbledEvent, dragAndDrop, dragEnter } from '../../test/dragDrop'
45+
import {
46+
createBubbledEvent,
47+
dragAndDrop,
48+
dragEnter,
49+
dragLeave
50+
} from '../../test/dragDrop'
4651
import { DragEnterDirection } from '../../shared/components/dragDrop/util'
4752
import { clearSelection, createWindowTextSelection } from '../../test/selection'
4853

@@ -1034,6 +1039,29 @@ describe('App', () => {
10341039
expect(topDropIcon).toBeInTheDocument()
10351040
})
10361041

1042+
it('should remove the drop zone when hovering out a new section', () => {
1043+
renderAppWithOptionalData({
1044+
template: complexTemplatePlotsFixture
1045+
})
1046+
1047+
const topSection = screen.getByTestId(NewSectionBlock.TOP)
1048+
const multiViewPlot = screen.getByTestId(
1049+
join('plot_other', 'multiview.tsv')
1050+
)
1051+
1052+
dragEnter(multiViewPlot, topSection.id, DragEnterDirection.LEFT)
1053+
1054+
let topDropIcon = screen.queryByTestId(`${NewSectionBlock.TOP}_drop-icon`)
1055+
1056+
expect(topDropIcon).toBeInTheDocument()
1057+
1058+
dragLeave(topSection)
1059+
1060+
topDropIcon = screen.queryByTestId(`${NewSectionBlock.TOP}_drop-icon`)
1061+
1062+
expect(topDropIcon).not.toBeInTheDocument()
1063+
})
1064+
10371065
it('should not show a drop target when moving an element from a whole different section (comparison to template)', () => {
10381066
renderAppWithOptionalData({
10391067
comparison: comparisonTableFixture,
@@ -1060,13 +1088,14 @@ describe('App', () => {
10601088

10611089
const topSection = screen.getByTestId(NewSectionBlock.TOP)
10621090

1063-
const dragOverEvent = createBubbledEvent('dragover', {
1064-
preventDefault: jest.fn()
1065-
})
1066-
1067-
topSection.dispatchEvent(dragOverEvent)
1091+
act(() => {
1092+
const dragOverEvent = createBubbledEvent('dragover', {
1093+
preventDefault: jest.fn()
1094+
})
10681095

1069-
expect(dragOverEvent.preventDefault).toHaveBeenCalled()
1096+
topSection.dispatchEvent(dragOverEvent)
1097+
expect(dragOverEvent.preventDefault).toHaveBeenCalled()
1098+
})
10701099
})
10711100

10721101
it('should show a drop target before a plot on drag enter from the left', () => {

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

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { getIDWithoutIndex } from '../../../util/ids'
77
import { PlotsState } from '../../store'
88
import { Icon } from '../../../shared/components/Icon'
99
import { GraphLine } from '../../../shared/components/icons'
10+
import { useDeferedDragLeave } from '../../../shared/hooks/useDeferedDragLeave'
1011

1112
interface AddedSectionProps {
1213
id: string
@@ -26,29 +27,37 @@ export const AddedSection: React.FC<AddedSectionProps> = ({
2627
acceptedGroups
2728
}) => {
2829
const { draggedRef } = useSelector((state: PlotsState) => state.dragAndDrop)
30+
const { immediateDragEnter, deferedDragLeave } = useDeferedDragLeave()
31+
32+
const isHovered = hoveredSection === id
33+
2934
const handleDragLeave = () => {
30-
setHoveredSection('')
35+
deferedDragLeave(() => setHoveredSection(''))
3136
}
3237

33-
const handleDragEnter = (e: DragEvent<HTMLElement>) => {
38+
const handleDragEnter = () => {
3439
const draggedGroup = getIDWithoutIndex(draggedRef?.group) || ''
3540
if (
3641
acceptedGroups.includes(draggedGroup) &&
3742
draggedGroup !== closestSection.group
3843
) {
39-
setHoveredSection(e.currentTarget.id)
44+
setHoveredSection(id)
45+
immediateDragEnter()
4046
}
4147
}
4248

43-
const isHovered = hoveredSection === id
49+
const handleDragOver = (e: DragEvent<HTMLElement>) => {
50+
e.preventDefault()
51+
immediateDragEnter()
52+
}
4453

4554
return (
4655
<div
4756
id={id}
4857
data-testid={id}
4958
onDragEnter={handleDragEnter}
50-
onDragExit={handleDragLeave}
51-
onDragOver={(e: DragEvent<HTMLElement>) => e.preventDefault()}
59+
onDragLeave={handleDragLeave}
60+
onDragOver={handleDragOver}
5261
onDrop={onDrop}
5362
draggable
5463
className={cx(

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

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import { Any } from '../../../util/objects'
1818
import { PlotsState } from '../../../plots/store'
1919
import { getStyleProperty } from '../../../util/styles'
2020
import { idToNode } from '../../../util/helpers'
21+
import { useDeferedDragLeave } from '../../hooks/useDeferedDragLeave'
2122

2223
const AFTER_DIRECTIONS = new Set([
2324
DragEnterDirection.RIGHT,
@@ -96,34 +97,35 @@ export const DragDropContainer: React.FC<DragDropContainerProps> = ({
9697
: DragEnterDirection.LEFT
9798

9899
const [draggedOverId, setDraggedOverId] = useState('')
99-
const [hoveringSomething, setHoveringSomething] = useState(false)
100-
const isHovering = useRef(false)
100+
const {
101+
hoveringSomething,
102+
immediateDragLeave,
103+
immediateDragEnter,
104+
deferedDragLeave
105+
} = useDeferedDragLeave()
101106
const [draggedId, setDraggedId] = useState('')
102107
const [direction, setDirection] = useState(defaultDragEnterDirection)
103108
const { draggedRef, draggedOverGroup } = useSelector(
104109
(state: PlotsState) => state.dragAndDrop
105110
)
106111
const draggedOverIdTimeout = useRef<number>(0)
107-
const hoveringTimeout = useRef(0)
108112
const dispatch = useDispatch()
109113

110114
const cleanup = useCallback(() => {
111-
setHoveringSomething(false)
112-
isHovering.current = false
115+
immediateDragLeave()
113116
setDraggedOverId('')
114117
setDraggedId('')
115118
setDirection(defaultDragEnterDirection)
116119
}, [
117-
setHoveringSomething,
118120
setDraggedOverId,
119121
setDirection,
120-
defaultDragEnterDirection
122+
defaultDragEnterDirection,
123+
immediateDragLeave
121124
])
122125

123126
useEffect(() => {
124127
return () => {
125128
clearTimeout(draggedOverIdTimeout.current)
126-
clearTimeout(hoveringTimeout.current)
127129
}
128130
}, [])
129131

@@ -214,13 +216,9 @@ export const DragDropContainer: React.FC<DragDropContainerProps> = ({
214216
}
215217
}
216218

217-
const setIsHovering = () => {
218-
setHoveringSomething(true)
219-
isHovering.current = true
220-
}
221-
222219
const handleDragEnter = (e: DragEvent<HTMLElement>) => {
223-
setIsHovering()
220+
immediateDragEnter()
221+
224222
if (isSameGroup(draggedRef?.group, group)) {
225223
const { id } = e.currentTarget
226224
if (
@@ -238,7 +236,7 @@ export const DragDropContainer: React.FC<DragDropContainerProps> = ({
238236
const handleDragOver = (e: DragEvent<HTMLElement>) => {
239237
e.preventDefault()
240238
if (draggedOverId && e.currentTarget.id.includes(draggedOverId)) {
241-
setIsHovering()
239+
immediateDragEnter()
242240
}
243241
if (isSameGroup(draggedRef?.group, group)) {
244242
const { id } = e.currentTarget
@@ -254,12 +252,7 @@ export const DragDropContainer: React.FC<DragDropContainerProps> = ({
254252
}
255253

256254
const handleDragLeave = () => {
257-
isHovering.current = false
258-
hoveringTimeout.current = window.setTimeout(() => {
259-
if (!isHovering.current) {
260-
setHoveringSomething(false)
261-
}
262-
}, 500)
255+
deferedDragLeave()
263256
}
264257

265258
const buildItem = (id: string, draggable: JSX.Element) => (
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
import { useCallback, useEffect, useRef, useState } from 'react'
2+
3+
export const useDeferedDragLeave = () => {
4+
const [hoveringSomething, setHoveringSomething] = useState(false)
5+
const isHovering = useRef(false)
6+
const hoveringTimeout = useRef<number>(0)
7+
8+
useEffect(() => {
9+
return () => {
10+
clearTimeout(hoveringTimeout.current)
11+
}
12+
}, [])
13+
14+
const deferedDragLeave = useCallback(
15+
(callback?: () => void) => {
16+
isHovering.current = false
17+
hoveringTimeout.current = window.setTimeout(() => {
18+
if (!isHovering.current) {
19+
setHoveringSomething(false)
20+
callback?.()
21+
}
22+
}, 500)
23+
},
24+
[setHoveringSomething]
25+
)
26+
27+
const immediateDragLeave = useCallback(() => {
28+
setHoveringSomething(false)
29+
isHovering.current = false
30+
}, [setHoveringSomething])
31+
32+
const immediateDragEnter = useCallback(() => {
33+
setHoveringSomething(true)
34+
isHovering.current = true
35+
}, [setHoveringSomething])
36+
37+
return {
38+
deferedDragLeave,
39+
hoveringSomething,
40+
immediateDragEnter,
41+
immediateDragLeave
42+
}
43+
}

webview/src/test/dragDrop.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,19 @@ export const dragEnter = (
8282

8383
jest.useRealTimers()
8484
}
85+
86+
export const dragLeave = (draggedOver: HTMLElement) => {
87+
jest.useFakeTimers()
88+
act(() => {
89+
draggedOver.dispatchEvent(createBubbledEvent('dragleave'))
90+
})
91+
92+
act(() => {
93+
jest.advanceTimersByTime(501)
94+
})
95+
jest.useRealTimers()
96+
}
97+
8598
export const dragAndDrop = (
8699
startingNode: HTMLElement,
87100
endingNode: HTMLElement,

0 commit comments

Comments
 (0)