Skip to content

Commit affafe2

Browse files
committed
fix(sidebar): close folders a drag spring-opened, and surface reorder failures
1 parent c155a57 commit affafe2

2 files changed

Lines changed: 256 additions & 50 deletions

File tree

apps/sim/app/workspace/[workspaceId]/w/components/sidebar/hooks/use-drag-drop.test.tsx

Lines changed: 133 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ vi.mock('next/navigation', () => ({
99
useParams: () => ({ workspaceId: 'ws-1' }),
1010
}))
1111

12+
/** Kept out of the module graph so this suite does not pull emcn's CSS modules through postcss. */
13+
vi.mock('@sim/emcn', () => ({ toast: { error: vi.fn() } }))
14+
1215
vi.mock('@/hooks/queries/folders', () => ({
1316
useReorderFolders: () => ({ mutateAsync: vi.fn() }),
1417
}))
@@ -29,13 +32,23 @@ vi.mock('@/lib/folders/tree', () => ({
2932
getFolderPath: () => [],
3033
}))
3134

32-
const { mockUseFolderStore } = vi.hoisted(() => {
33-
const folderState = { setExpanded: () => {}, expandedFolders: new Set<string>() }
35+
const { mockUseFolderStore, mockSetExpanded, expandedFolders } = vi.hoisted(() => {
36+
const expanded = new Set<string>()
37+
const setExpanded = vi.fn((folderId: string, isExpanded: boolean) => {
38+
if (isExpanded) expanded.add(folderId)
39+
else expanded.delete(folderId)
40+
})
41+
const folderState = {
42+
setExpanded,
43+
expandedFolders: expanded,
44+
clearSelection: () => {},
45+
clearFolderSelection: () => {},
46+
}
3447
const store = Object.assign(
3548
(selector: (state: typeof folderState) => unknown) => selector(folderState),
3649
{ getState: () => folderState }
3750
)
38-
return { mockUseFolderStore: store }
51+
return { mockUseFolderStore: store, mockSetExpanded: setExpanded, expandedFolders: expanded }
3952
})
4053
vi.mock('@/stores/folders/store', () => ({ useFolderStore: mockUseFolderStore }))
4154

@@ -63,6 +76,32 @@ function fakeDragOverEvent(): unknown {
6376
}
6477
}
6578

79+
/**
80+
* A `dragover` on a folder row. `clientY` sits in the middle band of the 100px rect, which is what
81+
* `calculateFolderDropPosition` reads as "inside" — the position that arms the spring-open timer.
82+
*/
83+
function fakeFolderDragOverEvent(): unknown {
84+
const currentTarget = {
85+
getBoundingClientRect: () => ({ top: 0, bottom: 100, height: 100 }),
86+
}
87+
return {
88+
preventDefault: () => {},
89+
stopPropagation: () => {},
90+
clientY: 50,
91+
target: {},
92+
currentTarget,
93+
}
94+
}
95+
96+
/** A `drop` carrying no selection payload: enough to record the destination, then bail. */
97+
function fakeDropEvent(): unknown {
98+
return {
99+
preventDefault: () => {},
100+
stopPropagation: () => {},
101+
dataTransfer: { getData: () => '' },
102+
}
103+
}
104+
66105
let container: HTMLDivElement
67106
let root: Root
68107

@@ -93,6 +132,7 @@ describe('useDragDrop stranded-drag reset', () => {
93132
container.remove()
94133
vi.unstubAllGlobals()
95134
vi.clearAllMocks()
135+
expandedFolders.clear()
96136
})
97137

98138
it('clears isDragging on a window dragend when no drop fired', () => {
@@ -122,3 +162,93 @@ describe('useDragDrop stranded-drag reset', () => {
122162
expect(latest.isDragging).toBe(true)
123163
})
124164
})
165+
166+
/**
167+
* Hovering a collapsed folder mid-drag spring-opens it so you can drop inside. Every folder opened
168+
* that way that the drop did NOT land in has to close again, or dragging past a folder silently
169+
* leaves it open and the sidebar grows rows the user never asked to see.
170+
*/
171+
describe('useDragDrop spring-open revert', () => {
172+
beforeEach(() => {
173+
vi.useFakeTimers()
174+
vi.stubGlobal(
175+
'requestAnimationFrame',
176+
() => 0 as unknown as ReturnType<typeof requestAnimationFrame>
177+
)
178+
vi.stubGlobal('cancelAnimationFrame', () => {})
179+
container = document.createElement('div')
180+
document.body.appendChild(container)
181+
root = createRoot(container)
182+
act(() => {
183+
root.render(<Harness />)
184+
})
185+
})
186+
187+
afterEach(() => {
188+
act(() => {
189+
root.unmount()
190+
})
191+
container.remove()
192+
vi.unstubAllGlobals()
193+
vi.useRealTimers()
194+
vi.clearAllMocks()
195+
expandedFolders.clear()
196+
})
197+
198+
/** Drives a drag that lingers over `folder-1` long enough to spring it open. */
199+
function dragOverFolderUntilExpanded() {
200+
act(() => {
201+
latest.handleDragStart(null)
202+
})
203+
act(() => {
204+
latest
205+
.createFolderDragHandlers('folder-1', null)
206+
.onDragOver(fakeFolderDragOverEvent() as never)
207+
})
208+
act(() => {
209+
vi.advanceTimersByTime(500)
210+
})
211+
}
212+
213+
it('closes a folder it spring-opened when the drag ends without dropping into it', () => {
214+
dragOverFolderUntilExpanded()
215+
expect(mockSetExpanded).toHaveBeenCalledWith('folder-1', true)
216+
217+
// Esc-cancel / release outside: `dragend` fires with no drop recorded.
218+
act(() => {
219+
latest.handleDragEnd()
220+
})
221+
222+
expect(mockSetExpanded).toHaveBeenCalledWith('folder-1', false)
223+
expect(expandedFolders.has('folder-1')).toBe(false)
224+
})
225+
226+
it('leaves a folder open when the drop landed inside it', () => {
227+
dragOverFolderUntilExpanded()
228+
mockSetExpanded.mockClear()
229+
230+
// Drop inside folder-1, then the drag ends as it always does.
231+
act(() => {
232+
void latest.createFolderDragHandlers('folder-1', null).onDrop(fakeDropEvent() as never)
233+
})
234+
act(() => {
235+
latest.handleDragEnd()
236+
})
237+
238+
expect(mockSetExpanded).not.toHaveBeenCalledWith('folder-1', false)
239+
expect(expandedFolders.has('folder-1')).toBe(true)
240+
})
241+
242+
it('never closes a folder the user had already opened themselves', () => {
243+
expandedFolders.add('folder-1')
244+
245+
dragOverFolderUntilExpanded()
246+
act(() => {
247+
latest.handleDragEnd()
248+
})
249+
250+
// Already-expanded folders are skipped by the spring-open effect, so nothing to revert.
251+
expect(mockSetExpanded).not.toHaveBeenCalledWith('folder-1', false)
252+
expect(expandedFolders.has('folder-1')).toBe(true)
253+
})
254+
})

0 commit comments

Comments
 (0)