Skip to content

Commit 4fc0fb4

Browse files
authored
refactor(resources): converge Files onto the shared drag hook and batch bulk authorization (#6748)
* refactor(resources): converge Files onto the shared drag hook and batch bulk authorization Files kept a 280-line copy of the foldered-list drag logic because it also accepts OS file drops. The copies had already drifted, so the external drop becomes an option on the shared hook and the copy goes away. - Add `externalDrop` to `useFolderRowDragDrop`: folder rows highlight and spring open for an OS file drag exactly as for a move, while the body and breadcrumb decline so the page-level upload overlay owns those regions - Collapse the three drop-active booleans into one `ActiveDropTarget` union, so exactly one affordance is armed by construction rather than by hand-clearing - Keep drop-target writes identity-stable so `dragover` does not re-render the list on every event - Give each list its own drag MIME again, restoring the cross-surface isolation `drag-payload.ts` documents - Let a folder spring open more than once per drag, so a drag can walk back out through the breadcrumb and descend again; the guard against re-entering the folder already on screen moves to `useSpringNavigation`, the only layer that can state it - Resolve each bulk item against the workspace context the batch already holds, and memoize the effective-permission lookup for the batch, replacing two invariant queries per item - Fill the drop target at `--surface-active`: `--surface-4` is the button-base token and is lighter than hover in light mode, so the strongest row state read the faintest * fix(resources): dismiss the upload overlay on a folder drop and re-check permission per item The drag hook stops propagation on a drop it handles, so the page-level handler that cleared the upload overlay never ran and the chrome stayed up over the finished upload. Both consuming paths now share one dismissal. Drop the batch permission memo: each item in a bulk move or delete commits independently, so reusing one allow verdict let a revocation part-way through a batch go unseen by the remaining items. The workspace context is still resolved once per batch, which was the larger saving.
1 parent 77f520c commit 4fc0fb4

16 files changed

Lines changed: 531 additions & 478 deletions

File tree

apps/sim/app/workspace/[workspaceId]/components/folders/use-folder-row-drag-drop.ts

Lines changed: 138 additions & 42 deletions
Large diffs are not rendered by default.

apps/sim/app/workspace/[workspaceId]/components/folders/use-spring-loaded-folder.test.tsx

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -121,19 +121,25 @@ describe('useSpringLoadedFolder', () => {
121121
expect(onSpringOpen).not.toHaveBeenCalled()
122122
})
123123

124-
it('opens a folder at most once per drag', () => {
124+
it('opens a folder again when the drag comes back to it', () => {
125+
// Descend, walk back out through the breadcrumb, change your mind and descend again — one
126+
// gesture, and the second entry has to work. Re-entry costs another full delay, and
127+
// `useSpringNavigation` refuses the folder already on screen, so nothing oscillates.
125128
const onSpringOpen = vi.fn()
126129
const harness = renderSpringLoad(onSpringOpen)
127130

128131
act(() => harness.get().arm('folder-a'))
129132
rest()
130-
expect(onSpringOpen).toHaveBeenCalledTimes(1)
131-
132-
// Dragging back out and returning must not re-open it, which would loop at a boundary.
133-
act(() => harness.get().arm('folder-b'))
133+
act(() => harness.get().arm(null))
134+
rest()
134135
act(() => harness.get().arm('folder-a'))
135136
rest()
136-
expect(onSpringOpen).toHaveBeenCalledTimes(1)
137+
138+
expect(onSpringOpen.mock.calls).toEqual([
139+
['folder-a', { history: 'push' }],
140+
[null, { history: 'replace' }],
141+
['folder-a', { history: 'replace' }],
142+
])
137143
})
138144

139145
it('pushes the first spring-open of a drag and replaces the rest', () => {
@@ -197,17 +203,21 @@ describe('useSpringLoadedFolder', () => {
197203
expect(onSpringOpen).toHaveBeenCalledExactlyOnceWith(null, { history: 'push' })
198204
})
199205

200-
it('opens the root at most once per drag, like any other folder', () => {
206+
it('re-opens the root like any other folder, and only after a full rest', () => {
201207
const onSpringOpen = vi.fn()
202208
const harness = renderSpringLoad(onSpringOpen)
203209

204210
act(() => harness.get().arm(null))
205211
rest()
212+
213+
// Passing over another row cancels the countdown, so returning to the root has to wait out
214+
// the delay again rather than firing on whatever was left of the previous one.
206215
act(() => harness.get().arm('folder-a'))
207216
act(() => harness.get().arm(null))
217+
expect(onSpringOpen).toHaveBeenCalledTimes(1)
208218
rest()
209219

210-
expect(onSpringOpen).toHaveBeenCalledTimes(1)
220+
expect(onSpringOpen).toHaveBeenCalledTimes(2)
211221
})
212222

213223
it('never opens a folder after unmount', () => {

apps/sim/app/workspace/[workspaceId]/components/folders/use-spring-loaded-folder.ts

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ export interface SpringLoadedFolder {
4040
arm: (folderId: string | null) => void
4141
/** Cancels the pending open — the drag left the row, or the row stopped being a valid target. */
4242
disarm: () => void
43-
/** Cancels the pending open and forgets which folders already opened. Call when the drag ends. */
43+
/** Cancels the pending open and forgets that this drag opened anything. Call when the drag ends. */
4444
reset: () => void
4545
}
4646

@@ -51,9 +51,11 @@ export interface SpringLoadedFolder {
5151
* The dragged rows unmount when the list re-renders into the newly opened folder, which is why
5252
* the drag payload has to live in `dataTransfer` rather than only in the source row's state.
5353
*
54-
* A folder opens at most once per drag. Without that, dragging back out to a parent and
55-
* returning would re-open it on a loop, and a drag that rests near a boundary would flicker
56-
* between two levels.
54+
* A folder may open more than once in a single drag: walking back out through the breadcrumb and
55+
* descending again is a normal way to change your mind mid-gesture, and refusing the second entry
56+
* strands the drag one level up. Nothing oscillates, because every open costs another full
57+
* {@link SPRING_LOAD_DELAY_MS} of the drag holding still, and {@link useSpringNavigation} refuses
58+
* to arm the folder already on screen.
5759
*/
5860
export function useSpringLoadedFolder({
5961
onSpringOpen,
@@ -65,9 +67,8 @@ export function useSpringLoadedFolder({
6567
* nothing is armed — `null` is a real destination here, the workspace root.
6668
*/
6769
const armedFolderIdRef = useRef<string | null | undefined>(undefined)
68-
/** Folders already opened during this drag; each may only spring once. */
69-
const openedFolderIdsRef = useRef<Set<string | null> | null>(null)
70-
const openedFolderIds = (openedFolderIdsRef.current ??= new Set<string | null>())
70+
/** Whether this drag has already sprung a folder open, which decides push vs. replace. */
71+
const hasOpenedRef = useRef(false)
7172

7273
const onSpringOpenRef = useRef(onSpringOpen)
7374
onSpringOpenRef.current = onSpringOpen
@@ -91,27 +92,24 @@ export function useSpringLoadedFolder({
9192
* this would let the folder the drag just left open behind the cursor.
9293
*/
9394
clearTimer()
94-
if (openedFolderIds.has(folderId)) return
95-
9695
armedFolderIdRef.current = folderId
9796
timerRef.current = setTimeout(() => {
9897
timerRef.current = null
9998
armedFolderIdRef.current = undefined
100-
/** Read before the add: an empty set means nothing has opened in this drag yet. */
101-
const isFirstOpenOfDrag = openedFolderIds.size === 0
102-
openedFolderIds.add(folderId)
99+
const isFirstOpenOfDrag = !hasOpenedRef.current
100+
hasOpenedRef.current = true
103101
onSpringOpenRef.current(folderId, {
104102
history: isFirstOpenOfDrag ? 'push' : 'replace',
105103
})
106104
}, delayMs)
107105
},
108-
[clearTimer, delayMs, openedFolderIds]
106+
[clearTimer, delayMs]
109107
)
110108

111109
const reset = useCallback(() => {
112110
clearTimer()
113-
openedFolderIds.clear()
114-
}, [clearTimer, openedFolderIds])
111+
hasOpenedRef.current = false
112+
}, [clearTimer])
115113

116114
/**
117115
* Stable identity, not a fresh object per render. Consumers feed this handle into a

apps/sim/app/workspace/[workspaceId]/components/folders/use-spring-navigation.test.tsx

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,12 @@ function rest(harness: { rerender: () => void }) {
6868
harness.rerender()
6969
}
7070

71+
/** One spring-open: rest the drag on `folderId` until the timer fires and the list follows. */
72+
function descend(nav: ReturnType<typeof renderSpringNavigation>, folderId: string | null) {
73+
act(() => nav.get().arm(folderId))
74+
rest(nav)
75+
}
76+
7177
beforeEach(() => {
7278
vi.useFakeTimers()
7379
})
@@ -155,6 +161,130 @@ describe('useSpringNavigation', () => {
155161
expect(nav.navigate).not.toHaveBeenCalled()
156162
})
157163

164+
describe('walking a drag back out and in again', () => {
165+
it('re-enters a folder it already left through the breadcrumb', () => {
166+
// The whole point of the breadcrumb accepting a drag: descend, think better of it, walk
167+
// back up, then descend again — all inside one gesture without releasing the mouse.
168+
const nav = renderSpringNavigation(null)
169+
170+
act(() => nav.get().rememberOrigin())
171+
descend(nav, 'folder-a')
172+
expect(nav.currentFolderId()).toBe('folder-a')
173+
174+
descend(nav, null)
175+
expect(nav.currentFolderId()).toBeNull()
176+
177+
descend(nav, 'folder-a')
178+
expect(nav.currentFolderId()).toBe('folder-a')
179+
180+
expect(nav.navigate.mock.calls).toEqual([
181+
['folder-a', 'push'],
182+
[null, 'replace'],
183+
['folder-a', 'replace'],
184+
])
185+
})
186+
187+
it('never re-opens the folder already on screen', () => {
188+
// The crumb for the current folder is a legal drop target but not a navigation. Arming it
189+
// would re-enter the folder the drag is already standing in, on a loop.
190+
const nav = renderSpringNavigation(null)
191+
192+
act(() => nav.get().rememberOrigin())
193+
descend(nav, 'folder-a')
194+
195+
descend(nav, 'folder-a')
196+
197+
expect(nav.navigate).toHaveBeenCalledExactlyOnceWith('folder-a', 'push')
198+
})
199+
200+
it('cancels a pending open when the drag moves onto the current folder', () => {
201+
// Hovering a sibling folder starts its countdown; sliding onto the crumb of the folder
202+
// you are already in has to call that off, not let it fire from under the cursor.
203+
const nav = renderSpringNavigation('folder-a')
204+
205+
act(() => nav.get().rememberOrigin())
206+
act(() => nav.get().arm('folder-b'))
207+
act(() => {
208+
vi.advanceTimersByTime(SPRING_LOAD_DELAY_MS - 1)
209+
})
210+
descend(nav, 'folder-a')
211+
212+
expect(nav.navigate).not.toHaveBeenCalled()
213+
})
214+
215+
it('returns to the origin in one hop after a round trip that dropped nothing', () => {
216+
const nav = renderSpringNavigation('origin')
217+
218+
act(() => nav.get().rememberOrigin())
219+
descend(nav, 'folder-a')
220+
descend(nav, 'folder-b')
221+
descend(nav, 'folder-a')
222+
223+
nav.navigate.mockClear()
224+
act(() => nav.get().end())
225+
226+
expect(nav.navigate).toHaveBeenCalledExactlyOnceWith('origin', 'replace')
227+
expect(nav.currentFolderId()).toBe('origin')
228+
})
229+
230+
it('stays put when the round trip ends in a real drop', () => {
231+
const nav = renderSpringNavigation('origin')
232+
233+
act(() => nav.get().rememberOrigin())
234+
descend(nav, 'folder-a')
235+
descend(nav, null)
236+
descend(nav, 'folder-a')
237+
238+
nav.navigate.mockClear()
239+
act(() => {
240+
nav.get().markDropHandled()
241+
nav.get().end()
242+
})
243+
244+
expect(nav.navigate).not.toHaveBeenCalled()
245+
expect(nav.currentFolderId()).toBe('folder-a')
246+
})
247+
248+
it('walks back to the origin folder itself without then bouncing away from it', () => {
249+
// Ending a drag whose spring-opens happen to land back on the origin must not navigate
250+
// again — the guard is origin-vs-current, not "did anything open".
251+
const nav = renderSpringNavigation('origin')
252+
253+
act(() => nav.get().rememberOrigin())
254+
descend(nav, 'folder-a')
255+
descend(nav, 'origin')
256+
expect(nav.currentFolderId()).toBe('origin')
257+
258+
nav.navigate.mockClear()
259+
act(() => nav.get().end())
260+
261+
expect(nav.navigate).not.toHaveBeenCalled()
262+
})
263+
264+
it('starts the next drag from where the previous one left the user', () => {
265+
// A drag that ended on a new folder is the new origin. Reusing the old one would yank the
266+
// list back several folders on the next unrelated drag.
267+
const nav = renderSpringNavigation('origin')
268+
269+
act(() => nav.get().rememberOrigin())
270+
descend(nav, 'folder-a')
271+
act(() => {
272+
nav.get().markDropHandled()
273+
nav.get().end()
274+
})
275+
276+
nav.navigate.mockClear()
277+
act(() => nav.get().rememberOrigin())
278+
descend(nav, 'folder-b')
279+
act(() => nav.get().end())
280+
281+
expect(nav.navigate.mock.calls).toEqual([
282+
['folder-b', 'push'],
283+
['folder-a', 'replace'],
284+
])
285+
})
286+
})
287+
158288
it('does not carry drop state into the next drag', () => {
159289
const nav = renderSpringNavigation(null)
160290

apps/sim/app/workspace/[workspaceId]/components/folders/use-spring-navigation.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,7 @@ export interface SpringNavigation {
3838
* treated as part of the drag: unless a drop actually landed, ending the drag returns to where
3939
* it started. The workflow sidebar collapses its own spring-opened folders for the same reason.
4040
*
41-
* Shared by every foldered list. Files keeps its own drag configuration for OS file drops, but
42-
* this lifecycle is identical everywhere.
41+
* Shared by every foldered list, including a drag of OS files onto the Files page.
4342
*/
4443
export function useSpringNavigation({
4544
currentFolderId,
@@ -73,16 +72,25 @@ export function useSpringNavigation({
7372
* Seeds the origin for a drag that never reached {@link SpringNavigation.rememberOrigin} — a
7473
* drag of OS files starts outside the page, so there is no `dragstart` of ours to record it.
7574
* Without this the return lands on whatever folder the PREVIOUS drag began in.
75+
*
76+
* Refuses the folder already on screen. That target is not a navigation, and arming it is how
77+
* a drag resting on one spot would re-open the same folder over and over: the underlying timer
78+
* lets a folder spring more than once per drag so the user can descend, back out through the
79+
* breadcrumb, and descend again.
7680
*/
7781
const arm = useCallback(
7882
(folderId: string | null) => {
83+
if (folderId === currentFolderIdRef.current) {
84+
springLoad.disarm()
85+
return
86+
}
7987
if (!hasOriginRef.current) {
8088
originFolderIdRef.current = currentFolderIdRef.current
8189
hasOriginRef.current = true
8290
}
8391
springLoad.arm(folderId)
8492
},
85-
[springLoad.arm]
93+
[springLoad.arm, springLoad.disarm]
8694
)
8795

8896
const markDropHandled = useCallback(() => {

apps/sim/app/workspace/[workspaceId]/components/resource/resource.tsx

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -712,13 +712,7 @@ const DataRow = memo(function DataRow({
712712
onRowClick && 'cursor-pointer',
713713
isDraggable && 'cursor-grab active:cursor-grabbing',
714714
isRowActive && chipActiveSurfaceClass,
715-
/**
716-
* Neutral, matching the workflow sidebar's own drop-inside affordance
717-
* (`bg-[var(--text-subtle)] opacity-10` there, and `--text-subtle` for its reorder
718-
* line). A brand colour here would be the only place in the app that signals "release
719-
* here" with hue rather than weight. Drawn inside the row's own box
720-
* (`outline-offset-[-1px]`) so the ring never overlaps the rows above and below.
721-
*/
715+
/** See {@link chipDropTargetSurfaceClass} for why this is neutral and drawn inset. */
722716
isActiveDropTarget && chipDropTargetSurfaceClass,
723717
(isDragging || (isAnyDragActive && isSelected && !isActiveDropTarget)) && 'opacity-50'
724718
)}

0 commit comments

Comments
 (0)