From ba280fc7ba972cdcc906f6fa79f19d353a978bc7 Mon Sep 17 00:00:00 2001 From: Alison Sabuwala Date: Tue, 18 Aug 2026 14:53:59 -0400 Subject: [PATCH] fix: make sidebar drop indicator and insertion agree across nested folder boundaries --- ...d.workspace.$workspaceId.debug.reorder.tsx | 14 +- .../project-navigation-sidebar.tsx | 1 + .../use-sidebar-drag-and-drop.test.ts | 525 ++++++++++++++ .../use-sidebar-drag-and-drop.tsx | 676 +++++++++++++++--- 4 files changed, 1120 insertions(+), 96 deletions(-) create mode 100644 packages/insomnia/src/ui/components/sidebar/project-navigation-sidebar/use-sidebar-drag-and-drop.test.ts diff --git a/packages/insomnia/src/routes/organization.$organizationId.project.$projectId.workspace.$workspaceId.debug.reorder.tsx b/packages/insomnia/src/routes/organization.$organizationId.project.$projectId.workspace.$workspaceId.debug.reorder.tsx index cc56e3c5c39..d9596aa2585 100644 --- a/packages/insomnia/src/routes/organization.$organizationId.project.$projectId.workspace.$workspaceId.debug.reorder.tsx +++ b/packages/insomnia/src/routes/organization.$organizationId.project.$projectId.workspace.$workspaceId.debug.reorder.tsx @@ -17,7 +17,7 @@ const getCollectionItem = async (id: string) => { }; export async function clientAction({ request }: Route.ClientActionArgs) { - const { id, targetId, dropPosition, metaSortKey } = await request.json(); + const { id, targetId, dropPosition, metaSortKey, parentId: requestedParentId } = await request.json(); invariant(typeof id === 'string', 'ID is required'); invariant(typeof targetId === 'string', 'Target ID is required'); @@ -53,7 +53,15 @@ export async function clientAction({ request }: Route.ClientActionArgs) { const item = await getCollectionItem(id); const targetItem = await getCollectionItem(targetId); - const parentId = dropPosition === 'after' && isRequestGroup(targetItem) ? targetItem._id : targetItem.parentId; + // The sidebar resolves its own destination and sends it outright, from the + // same value its drop indicator renders. Other callers (debug.tsx onReorder) + // omit it and get the old "after a folder means into it" inference. + const parentId = + typeof requestedParentId === 'string' + ? requestedParentId + : dropPosition === 'after' && isRequestGroup(targetItem) + ? targetItem._id + : targetItem.parentId; await (isRequestGroup(item) ? services.requestGroup.update(item, { parentId, metaSortKey }) @@ -79,6 +87,8 @@ export const useDebugReorderActionFetcher = createFetcherSubmitHook( targetId: string; dropPosition: string; metaSortKey: number; + // Destination parent, when the caller resolved it itself. + parentId?: string; } | { type: 'move-workspace'; diff --git a/packages/insomnia/src/ui/components/sidebar/project-navigation-sidebar/project-navigation-sidebar.tsx b/packages/insomnia/src/ui/components/sidebar/project-navigation-sidebar/project-navigation-sidebar.tsx index 5ad17687d45..f3b38dfe9cb 100644 --- a/packages/insomnia/src/ui/components/sidebar/project-navigation-sidebar/project-navigation-sidebar.tsx +++ b/packages/insomnia/src/ui/components/sidebar/project-navigation-sidebar/project-navigation-sidebar.tsx @@ -881,6 +881,7 @@ const ProjectNavigationSidebarInner = ( virtualizer, onWorkspaceReorder: handleLocalWorkspaceReorder, expandedProjectAndWorkspaceIds, + depthOffset: treeDepthOffset, }); const { selectedItemId, routeInfo } = useProjectNavigationSidebarNavigation({ setActiveTab, diff --git a/packages/insomnia/src/ui/components/sidebar/project-navigation-sidebar/use-sidebar-drag-and-drop.test.ts b/packages/insomnia/src/ui/components/sidebar/project-navigation-sidebar/use-sidebar-drag-and-drop.test.ts new file mode 100644 index 00000000000..0831124d915 --- /dev/null +++ b/packages/insomnia/src/ui/components/sidebar/project-navigation-sidebar/use-sidebar-drag-and-drop.test.ts @@ -0,0 +1,525 @@ +import { describe, expect, it } from 'vitest'; + +import type { CollectionChildFlatItem, EmptyNodeFlatItem, FlatItem, ProjectWithGitRepository } from './types'; +import type { BoundaryTargets, DropContext } from './use-sidebar-drag-and-drop'; +import { + canDrop, + getIndentLevel, + levelFromX, + resolveDrop, + SidebarDropTargetDelegate, +} from './use-sidebar-drag-and-drop'; + +// Fixtures mirror the sidebar's flat list: `level` is 0 for a direct child of a +// workspace, and `ancestors` is the root-first folder chain, so +// `ancestors.length === level`. Code under test indexes `ancestors` by level. + +const project = { _id: 'proj_1', remoteId: null, name: 'Local' } as unknown as ProjectWithGitRepository; +const workspaceDoc = { _id: 'wrk_1', scope: 'collection', name: 'Collection' }; + +const workspaceItem = { + kind: 'workspace', + doc: workspaceDoc, + project, + collapsed: false, + hidden: false, + organizationId: 'org_1', +} as unknown as FlatItem; + +interface ChildOptions { + id: string; + level: number; + parentId: string; + ancestors: string[]; + folder?: boolean; + metaSortKey?: number; + name?: string; +} + +function child({ id, level, parentId, ancestors, folder = false, metaSortKey = 0, name }: ChildOptions) { + return { + kind: 'collectionChild', + doc: { + _id: id, + type: folder ? 'RequestGroup' : 'Request', + name: name ?? id, + parentId, + metaSortKey, + }, + project, + workspace: workspaceDoc, + ancestors, + level, + pinned: false, + collapsed: false, + hidden: false, + organizationId: 'org_1', + } as unknown as CollectionChildFlatItem; +} + +function emptyFolderNode(id: string, level: number, requestGroupId: string) { + return { + kind: 'emptyFolder', + doc: { _id: id, name: 'Folder is empty' }, + project, + workspace: workspaceDoc, + requestGroup: { _id: requestGroupId }, + level, + hidden: false, + organizationId: 'org_1', + } as unknown as EmptyNodeFlatItem; +} + +// Rebuilds the lookups the hook derives from its flat list. +function buildIndex(items: FlatItem[]) { + const visibles = items.filter(item => !item.hidden); + const flatItemsById = new Map( + visibles.map((item, index) => [item.doc._id, [item, visibles[index - 1], visibles[index + 1]] as const]), + ); + const lookup = (id: string) => flatItemsById.get(id)?.[0] ?? null; + // Mirrors the hook: a childless expanded folder still shows an interior. + const hasVisibleChildren = (folderId: string) => + visibles.some(candidate => { + if (candidate.hidden) { + return false; + } + if (candidate.kind === 'emptyFolder') { + return candidate.requestGroup?._id === folderId; + } + return candidate.kind === 'collectionChild' && candidate.doc.parentId === folderId; + }); + return { flatItemsById, lookup, hasVisibleChildren }; +} + +type DropPosition = 'before' | 'after' | 'on'; + +function boundaryAt(items: FlatItem[], dropPosition: DropPosition, key: string) { + const { flatItemsById, lookup, hasVisibleChildren } = buildIndex(items); + const entry = flatItemsById.get(key); + if (!entry) { + throw new Error(`fixture has no visible item with id ${key}`); + } + const [targetItem, prevItem, nextItem] = entry; + const boundary: BoundaryTargets = { + dropPosition, + targetItem, + realTargetItem: dropPosition === 'before' ? prevItem : targetItem, + nextItem, + }; + return { boundary, flatItemsById, lookup, hasVisibleChildren }; +} + +function resolveAt(items: FlatItem[], dropPosition: DropPosition, key: string, preferredLevel: number) { + const { boundary, flatItemsById, lookup, hasVisibleChildren } = boundaryAt(items, dropPosition, key); + const ctx: DropContext = { hasVisibleChildren, lookup, flatItemsById, preferredLevel }; + return { boundary, ...resolveDrop(boundary, ctx) }; +} + +// Produces every ambiguous gap in one tree. +// +// folder A level 0 +// request A1 level 1 +// subfolder B level 1 +// request B1 level 2 +// request C level 0 +const A = child({ id: 'A', level: 0, parentId: 'wrk_1', ancestors: [], folder: true, name: 'Alpha' }); +const A1 = child({ id: 'A1', level: 1, parentId: 'A', ancestors: ['A'], metaSortKey: 100 }); +const B = child({ id: 'B', level: 1, parentId: 'A', ancestors: ['A'], folder: true, metaSortKey: 200, name: 'Bravo' }); +const B1 = child({ id: 'B1', level: 2, parentId: 'B', ancestors: ['A', 'B'], metaSortKey: 300 }); +const C = child({ id: 'C', level: 0, parentId: 'wrk_1', ancestors: [], metaSortKey: 400 }); +const nestedTree: FlatItem[] = [workspaceItem, A, A1, B, B1, C]; + +// folder A level 0 +// subfolder E level 1 (empty, expanded) +// "Folder empty" level 1 (placeholder, not a real item) +// request C level 0 +const E = child({ id: 'E', level: 1, parentId: 'A', ancestors: ['A'], folder: true, metaSortKey: 200, name: 'Echo' }); +const emptyPlaceholder = emptyFolderNode('empty_E', 1, 'E'); +const emptyTree: FlatItem[] = [workspaceItem, A, E, emptyPlaceholder, C]; + +describe('levelFromX', () => { + // 1rem is 13px by default here, not the css default of 16. + it.each([ + { level: 0, x: 39 }, + { level: 1, x: 52 }, + { level: 2, x: 65 }, + { level: 3, x: 78 }, + ])('maps a row indent guide to its own level at 13px per rem (level $level)', ({ level, x }) => { + expect(levelFromX(x, 0, 13)).toBe(level); + }); + + it.each([ + { level: 0, x: 48 }, + { level: 1, x: 64 }, + { level: 2, x: 80 }, + ])('maps a row indent guide to its own level at 16px per rem (level $level)', ({ level, x }) => { + expect(levelFromX(x, 0, 16)).toBe(level); + }); + + it('accounts for depthOffset, which strips ancestor levels in focus mode', () => { + // Focus mode renders level 0 at 1rem = 13px. + expect(levelFromX(13, 2, 13)).toBe(0); + expect(levelFromX(39, 2, 13)).toBe(2); + }); + + it('rounds, giving each level a half-step lead-in before its guide', () => { + // Level 1's guide is 52px; the lead-in makes deeper levels easier to hit. + expect(levelFromX(46, 0, 13)).toBe(1); + expect(levelFromX(45, 0, 13)).toBe(0); + }); + + it('returns negative levels left of the shallowest indent', () => { + // Callers clamp; this does not. + expect(levelFromX(0, 0, 13)).toBe(-3); + expect(levelFromX(0, 2, 13)).toBe(-1); + }); +}); + +describe('getIndentLevel', () => { + it('reads the level off a collection child', () => { + expect(getIndentLevel(B1)).toBe(2); + }); + + it('treats a missing item as top level', () => { + expect(getIndentLevel(null)).toBe(0); + }); + + it('reports pinned requests as top level, matching how they render', () => { + const pinned = { ...(B1 as CollectionChildFlatItem), kind: 'pinnedRequest' } as unknown as FlatItem; + expect(getIndentLevel(pinned)).toBe(0); + }); + + it('falls back to top level for a node carrying no level', () => { + expect(getIndentLevel(emptyFolderNode('e', undefined as unknown as number, 'E'))).toBe(0); + }); +}); + +describe('resolveDrop — folder trailing edge', () => { + it('lands inside the folder when the cursor asks to go deeper', () => { + expect(resolveAt(nestedTree, 'after', 'A', 1).normalized).toMatchObject({ dropPosition: 'on', targetItem: A }); + }); + + it('stays beside the folder when the cursor stays at its level', () => { + const { normalized } = resolveAt(nestedTree, 'after', 'A', 0); + expect(normalized).toMatchObject({ dropPosition: 'after', targetItem: A }); + }); + + it('does not merge into a folder with no visible children', () => { + const { normalized } = resolveAt([workspaceItem, A, E, C], 'after', 'E', 5); + expect(normalized).toMatchObject({ dropPosition: 'after', targetItem: E }); + }); +}); + +describe('resolveDrop — first child leading edge', () => { + // The form react-aria actually renders for the gap above. + it('keeps the child target when the cursor asks for the child level', () => { + const { normalized } = resolveAt(nestedTree, 'before', 'A1', 1); + expect(normalized).toMatchObject({ dropPosition: 'before', targetItem: A1 }); + }); + + it('redirects to the parent folder when the cursor goes shallower', () => { + const { normalized } = resolveAt(nestedTree, 'before', 'A1', 0); + expect(normalized).toMatchObject({ dropPosition: 'after', targetItem: A }); + }); + + it('never redirects shallower than the folder that owns the gap', () => { + // The gap spans "inside A" to "beside A" only. + const { normalized } = resolveAt(nestedTree, 'before', 'A1', -5); + expect(normalized).toMatchObject({ dropPosition: 'after', targetItem: A }); + }); +}); + +describe('resolveDrop — subtree tail', () => { + it.each([ + { level: 2, expected: 'B1', description: 'deepest — stays inside the subfolder' }, + { level: 1, expected: 'B', description: 'intermediate — inside the containing folder' }, + { level: 0, expected: 'A', description: 'shallowest — beside the containing folder' }, + ])('walks the ancestor chain to level $level ($description)', ({ level, expected }) => { + const { normalized } = resolveAt(nestedTree, 'after', 'B1', level); + expect(normalized.targetItem.doc._id).toBe(expected); + }); + + it('clamps a cursor left of the shallow row rather than escaping the gap', () => { + expect(resolveAt(nestedTree, 'after', 'B1', -4).normalized.targetItem.doc._id).toBe('A'); + }); + + it('clamps a cursor past the deep row onto that row', () => { + expect(resolveAt(nestedTree, 'after', 'B1', 99).normalized.targetItem.doc._id).toBe('B1'); + }); +}); + +describe('resolveDrop — empty folder placeholder', () => { + it('keeps the placeholder form when landing inside the empty folder', () => { + const { normalized } = resolveAt(emptyTree, 'before', 'empty_E', 2); + expect(normalized).toMatchObject({ dropPosition: 'before', targetItem: emptyPlaceholder }); + }); + + it('rewrites to the folder trailing edge when landing beside the folder', () => { + // Also fixes ordering: the placeholder form carries a first-child sort key. + const { normalized } = resolveAt(emptyTree, 'before', 'empty_E', 1); + expect(normalized).toMatchObject({ dropPosition: 'after', targetItem: E }); + }); +}); + +describe('resolveDrop — destination parent', () => { + it('parents on the workspace root at the top of a collection', () => { + expect(resolveAt(nestedTree, 'before', 'A', 0).parentId).toBe('wrk_1'); + }); + + it('parents inside a folder for an "on" drop', () => { + expect(resolveAt(nestedTree, 'after', 'A', 1).parentId).toBe('A'); + }); + + it('parents on the workspace for a top-level sibling insert', () => { + expect(resolveAt(nestedTree, 'before', 'C', 0).parentId).toBe('wrk_1'); + }); + + it('moves a nested request out to the collection top level', () => { + // Requests may live directly under a collection, with no folder at all. + expect(resolveAt(nestedTree, 'after', 'B1', 0).parentId).toBe('wrk_1'); + expect(resolveAt(nestedTree, 'after', 'B1', 0).folderName).toBeNull(); + }); + + it('parents inside the empty folder when the cursor stays deep', () => { + expect(resolveAt(emptyTree, 'before', 'empty_E', 2).parentId).toBe('E'); + }); + + it('parents outside the empty folder when the cursor goes shallow', () => { + expect(resolveAt(emptyTree, 'before', 'empty_E', 1).parentId).toBe('A'); + }); +}); + +describe('both raw forms of one gap agree', () => { + // The invariant the module exists to uphold: react-stately may hand the commit + // path one form and the render path the other. + it.each([-2, -1, 0, 1, 2, 3, 10])('subtree tail resolves identically at level %i', level => { + const viaAfter = resolveAt(nestedTree, 'after', 'B1', level); + const viaBefore = resolveAt(nestedTree, 'before', 'C', level); + expect(viaAfter.parentId).toBe(viaBefore.parentId); + expect(viaAfter.folderName).toBe(viaBefore.folderName); + }); + + it.each([-2, -1, 0, 1, 2, 3, 10])('folder trailing edge resolves identically at level %i', level => { + const viaAfter = resolveAt(nestedTree, 'after', 'A', level); + const viaBefore = resolveAt(nestedTree, 'before', 'A1', level); + expect(viaAfter.parentId).toBe(viaBefore.parentId); + }); + + it.each([-2, -1, 0, 1, 2, 3, 10])('empty folder boundary resolves identically at level %i', level => { + const viaAfter = resolveAt(emptyTree, 'after', 'E', level); + const viaBefore = resolveAt(emptyTree, 'before', 'empty_E', level); + expect(viaAfter.parentId).toBe(viaBefore.parentId); + }); +}); + +describe('resolveDrop — destination folder name', () => { + it('names the folder a nested insert actually lands in', () => { + expect(resolveAt(nestedTree, 'after', 'B1', 2).folderName).toBe('Bravo'); + }); + + it('names the containing folder once the cursor steps out one level', () => { + expect(resolveAt(nestedTree, 'after', 'B1', 1).folderName).toBe('Alpha'); + }); + + it('names nothing at top level', () => { + expect(resolveAt(nestedTree, 'after', 'B1', 0).folderName).toBeNull(); + }); + + it('stays silent on a folder trailing edge that did not merge inside', () => { + expect(resolveAt(nestedTree, 'after', 'A', 0).folderName).toBeNull(); + }); + + it('names the folder when the trailing edge did merge inside', () => { + expect(resolveAt(nestedTree, 'after', 'A', 1).folderName).toBe('Alpha'); + }); +}); + +describe('canDrop', () => { + const at = (items: FlatItem[], dropPosition: DropPosition, key: string) => + boundaryAt(items, dropPosition, key).boundary; + // canDrop judges containment against the resolved destination. + const dest = (items: FlatItem[], parentId: string | null) => ({ + parentId, + parentItem: parentId ? (buildIndex(items).flatItemsById.get(parentId)?.[0] ?? null) : null, + }); + + it('rejects a drop that lands exactly where the item already is', () => { + expect(canDrop(B1, at(nestedTree, 'after', 'B1'), dest(nestedTree, 'B'))).toBe(false); + }); + + it('allows a drop beside the item when the level changes its parent', () => { + // Same boundary, cursor moved left: a real move out of the subfolder. + expect(canDrop(B1, at(nestedTree, 'after', 'B1'), dest(nestedTree, 'A'))).toBe(true); + }); + + it('rejects moving a folder into its own descendant', () => { + expect(canDrop(B, at(nestedTree, 'on', 'B1'), dest(nestedTree, 'B1'))).toBe(false); + }); + + it('allows moving a request into an unrelated folder', () => { + expect(canDrop(A1, at(nestedTree, 'on', 'B'), dest(nestedTree, 'B'))).toBe(true); + }); + + it('rejects an "on" drop onto a request rather than a folder', () => { + expect(canDrop(A1, at(nestedTree, 'on', 'C'), dest(nestedTree, 'wrk_1'))).toBe(false); + }); + + it('allows inserting after an empty-folder placeholder', () => { + // "after" the placeholder is the gap after the folder it stands in for. + expect(canDrop(C, at(emptyTree, 'after', 'empty_E'), dest(emptyTree, 'A'))).toBe(true); + }); + + // Self-parenting is unrecoverable: the folder and its subtree vanish. + it('never lets a folder become its own parent (leading edge of its first child)', () => { + const resolved = resolveAt(nestedTree, 'before', 'A1', 1); + expect(resolved.parentId).toBe('A'); + expect(canDrop(A, at(nestedTree, 'before', 'A1'), dest(nestedTree, resolved.parentId))).toBe(false); + }); + + it('never lets a folder become its own parent (its own trailing edge)', () => { + expect(canDrop(A, at(nestedTree, 'on', 'A'), dest(nestedTree, 'A'))).toBe(false); + }); + + it('never lets a folder land inside its own descendant', () => { + expect(canDrop(A, at(nestedTree, 'after', 'B1'), dest(nestedTree, 'B'))).toBe(false); + }); + + it('lets a folder escape via the gap below its own subtree', () => { + // The anchor row (B1) is inside B, but the destination is not. + expect(canDrop(B, at(nestedTree, 'after', 'B1'), dest(nestedTree, 'wrk_1'))).toBe(true); + }); + + it('allows moving a nested request out to the collection top level', () => { + // Previous row is a collection child, not the workspace header. + expect(canDrop(B1, at(nestedTree, 'before', 'C'), dest(nestedTree, 'wrk_1'))).toBe(true); + }); +}); + +describe('SidebarDropTargetDelegate', () => { + // Rows 20px tall. `y` arrives already in this space, so scrollOffset must not + // be added — react-aria measures against the GridList inside the scroller. + const rows = [ + { key: 'A', index: 0, start: 0, end: 20, size: 20, lane: 0 }, + { key: 'A1', index: 1, start: 20, end: 40, size: 20, lane: 0 }, + { key: 'B', index: 2, start: 40, end: 60, size: 20, lane: 0 }, + ]; + + const delegateWith = (scrollOffset: number, rendered = rows) => { + const virtualizer = { getVirtualItems: () => rendered, scrollOffset } as never; + const delegate = new SidebarDropTargetDelegate(virtualizer); + delegate.configure(0, () => {}); + return delegate; + }; + + const acceptBeforeAfter = (target: { type: string; dropPosition?: string }) => + target.type === 'item' && target.dropPosition !== 'on'; + + it('hit-tests against y directly when unscrolled', () => { + expect(delegateWith(0).getDropTargetFromPoint(40, 45, acceptBeforeAfter)).toMatchObject({ key: 'B' }); + }); + + it('ignores scrollOffset — y is already in content space', () => { + // y is near the top on purpose: double-counting lands past every row. + for (const scrollOffset of [0, 100, 5000]) { + expect(delegateWith(scrollOffset).getDropTargetFromPoint(40, 5, acceptBeforeAfter)).toMatchObject({ key: 'A' }); + } + }); + + it('resolves the row under the cursor rather than the last rendered row', () => { + const scrolled = [ + { key: 'X', index: 10, start: 200, end: 220, size: 20, lane: 0 }, + { key: 'Y', index: 11, start: 220, end: 240, size: 20, lane: 0 }, + ]; + expect(delegateWith(200, scrolled).getDropTargetFromPoint(40, 205, acceptBeforeAfter)).toMatchObject({ key: 'X' }); + }); + + it('falls back to root when nothing is rendered', () => { + expect(delegateWith(0, []).getDropTargetFromPoint(40, 45, acceptBeforeAfter)).toEqual({ type: 'root' }); + }); +}); + +describe('dropping at the very top of a collection', () => { + // The only boundary anchored on the collection header, so it takes its own + // path through canDrop — including on cloud-synced projects. + const topGap = (remoteId: string | null) => { + const cloudProject = { _id: 'proj_1', remoteId, name: 'P' } as unknown as ProjectWithGitRepository; + const withProject = (item: FlatItem) => ({ ...item, project: cloudProject }) as FlatItem; + const items = [workspaceItem, A, C].map(withProject); + const [wsRow, folderRow, requestRow] = items; + const { flatItemsById, lookup, hasVisibleChildren } = buildIndex(items); + const boundary: BoundaryTargets = { + dropPosition: 'before', + targetItem: folderRow, + realTargetItem: wsRow, + nextItem: requestRow, + }; + const ctx: DropContext = { hasVisibleChildren, lookup, flatItemsById, preferredLevel: 0 }; + return { dragged: requestRow, boundary, resolved: resolveDrop(boundary, ctx) }; + }; + + it.each([ + { kind: 'local', remoteId: null }, + { kind: 'cloud-synced', remoteId: 'remote_1' }, + ])('parents on the collection itself ($kind project)', ({ remoteId }) => { + expect(topGap(remoteId).resolved.parentId).toBe('wrk_1'); + }); + + it.each([ + { kind: 'local', remoteId: null }, + { kind: 'cloud-synced', remoteId: 'remote_1' }, + ])('allows the drop ($kind project)', ({ remoteId }) => { + const { dragged, boundary, resolved } = topGap(remoteId); + expect(canDrop(dragged, boundary, resolved)).toBe(true); + }); +}); + +describe('the region below an empty folder placeholder', () => { + // folder A level 0 + // subfolder E level 1 (expanded, empty) + // "Folder empty" (placeholder) + // subfolder S level 1 <- want to drop above this + // request R level 1 + const S = child({ id: 'S', level: 1, parentId: 'A', ancestors: ['A'], folder: true, metaSortKey: 300, name: 'Sierra' }); + const R = child({ id: 'R', level: 1, parentId: 'A', ancestors: ['A'], metaSortKey: 400 }); + const tree: FlatItem[] = [workspaceItem, A, E, emptyPlaceholder, S, R]; + + const resolveHere = (dropPosition: DropPosition, key: string, preferredLevel: number) => { + const { boundary, flatItemsById, lookup, hasVisibleChildren } = boundaryAt(tree, dropPosition, key); + const ctx: DropContext = { hasVisibleChildren, lookup, flatItemsById, preferredLevel }; + const resolved = resolveDrop(boundary, ctx); + return { ...resolved, valid: canDrop(R, boundary, resolved) }; + }; + + it.each([0, 1])('accepts a drop below the placeholder, shallow cursor (L%i)', level => { + // Both raw forms of that one gap. + for (const [dropPosition, key] of [['after', 'empty_E'], ['before', 'S']] as const) { + const resolved = resolveHere(dropPosition, key, level); + expect({ dropPosition, ...resolved }).toMatchObject({ parentId: 'A', valid: true }); + } + }); + + it('means one thing regardless of cursor depth', () => { + // Not level-aware: the opposite raw form cannot express "inside". + for (const level of [0, 1, 2, 5]) { + expect(resolveHere('after', 'empty_E', level)).toMatchObject({ parentId: 'A', valid: true }); + } + }); + + it('agrees across both raw forms at every level', () => { + for (const level of [-1, 0, 1, 2, 3]) { + expect(resolveHere('after', 'empty_E', level).parentId).toBe(resolveHere('before', 'S', level).parentId); + } + }); + + it('still refuses to insert beside an empty COLLECTION placeholder', () => { + const emptyCollection = { + ...emptyFolderNode('empty_ws', 0, 'wrk_1'), + kind: 'emptyCollection', + requestGroup: undefined, + } as unknown as FlatItem; + const items: FlatItem[] = [workspaceItem, emptyCollection]; + const { boundary, flatItemsById, lookup, hasVisibleChildren } = boundaryAt(items, 'after', 'empty_ws'); + const ctx: DropContext = { hasVisibleChildren, lookup, flatItemsById, preferredLevel: 0 }; + expect(canDrop(R, boundary, resolveDrop(boundary, ctx))).toBe(false); + }); +}); diff --git a/packages/insomnia/src/ui/components/sidebar/project-navigation-sidebar/use-sidebar-drag-and-drop.tsx b/packages/insomnia/src/ui/components/sidebar/project-navigation-sidebar/use-sidebar-drag-and-drop.tsx index 028b19d2e18..a3ba644b365 100644 --- a/packages/insomnia/src/ui/components/sidebar/project-navigation-sidebar/use-sidebar-drag-and-drop.tsx +++ b/packages/insomnia/src/ui/components/sidebar/project-navigation-sidebar/use-sidebar-drag-and-drop.tsx @@ -1,10 +1,11 @@ import type { Virtualizer } from '@tanstack/react-virtual'; import { models, type WorkspaceScope } from 'insomnia-data'; -import { useCallback, useMemo, useRef } from 'react'; -import type { DragAndDropHooks, ItemDropTarget } from 'react-aria-components'; +import { useCallback, useMemo, useRef, useState } from 'react'; +import type { DragAndDropHooks, DropTarget, ItemDropTarget } from 'react-aria-components'; import { DropIndicator, useDragAndDrop } from 'react-aria-components'; import { useDebugReorderActionFetcher } from '~/routes/organization.$organizationId.project.$projectId.workspace.$workspaceId.debug.reorder'; +import { Icon } from '~/ui/components/icon'; import type { CollectionChildFlatItem, EmptyNodeFlatItem, FlatItem } from './types'; @@ -31,27 +32,282 @@ function isEmptyNode(item: FlatItem): item is EmptyNodeFlatItem { return emptyNodeKinds.includes(item.kind); } -function canDrop( +export function getIndentLevel(item: FlatItem | null): number { + if (!item || item.kind === 'pinnedRequest') { + return 0; + } + return 'level' in item && typeof item.level === 'number' ? item.level : 0; +} + +type HasVisibleChildren = (folderId: string) => boolean; +type FlatItemById = (id: string) => FlatItem | null; +// Every visible row mapped to itself plus its previous and next visible rows. +type FlatItemsById = Map; +type DropPositionKind = ItemDropTarget['dropPosition']; + +function isCollectionChild(item: FlatItem | null | undefined): item is CollectionChildFlatItem { + return item?.kind === 'collectionChild'; +} + +function isFolderItem(item: FlatItem | null | undefined): item is CollectionChildFlatItem { + return isCollectionChild(item) && models.requestGroup.isRequestGroup(item.doc); +} + +// realTargetItem is the row the drop attaches to: the preceding row for a +// "before" drop, the target itself otherwise. +export interface BoundaryTargets { + dropPosition: DropPositionKind; + targetItem: FlatItem; + realTargetItem?: FlatItem | null; + nextItem?: FlatItem | null; +} + +export interface DropContext { + hasVisibleChildren: HasVisibleChildren; + lookup: FlatItemById; + flatItemsById: FlatItemsById; + // Nesting level the cursor's x currently selects (see levelFromX). + preferredLevel: number; +} + +interface NormalizedBoundary { + dropPosition: DropPositionKind; + targetItem: FlatItem; + redirected: boolean; +} + +// Not 16: the app sets html font-size from the interface fontSize setting +// (13px default) in use-settings-side-effects.ts, so it has to be measured. +const FALLBACK_PX_PER_REM = 16; +function measurePxPerRem(): number { + const rootFontSize = Number.parseFloat(getComputedStyle(document.documentElement).fontSize); + return Number.isFinite(rootFontSize) && rootFontSize > 0 ? rootFontSize : FALLBACK_PX_PER_REM; +} + +// Mirrors the rows' paddingLeft (request-node.tsx). levelFromX and +// indentRemForLevel must stay exact inverses of each other. +const ROOT_INDENT_REM = 3; +const MIN_INDENT_REM = 1; + +export function levelFromX(x: number, depthOffset: number, pxPerRem: number): number { + return Math.round(x / pxPerRem) - ROOT_INDENT_REM + depthOffset; +} + +function indentRemForLevel(level: number, depthOffset: number): number { + return Math.max(level + ROOT_INDENT_REM - depthOffset, MIN_INDENT_REM); +} + +// Deeper and shallower side of a multi-depth gap, or null for a same-level +// insert. Both raw forms of one gap map to the same pair. +function resolveGapSides( + { dropPosition, targetItem, realTargetItem, nextItem }: BoundaryTargets, +): { deep: CollectionChildFlatItem; shallow: CollectionChildFlatItem } | null { + const otherItem = dropPosition === 'before' ? realTargetItem : nextItem; + if (!isCollectionChild(targetItem) || !isCollectionChild(otherItem)) { + return null; + } + const targetLevel = getIndentLevel(targetItem); + const otherLevel = getIndentLevel(otherItem); + if (targetLevel === otherLevel) { + return null; + } + // A folder's own trailing edge; the merge-inside branch handles it. + if (dropPosition === 'after' && targetLevel < otherLevel) { + return null; + } + return targetLevel > otherLevel + ? { deep: targetItem, shallow: otherItem } + : { deep: otherItem, shallow: targetItem }; +} + +// react-stately treats `after X` and `before nextKey(X)` as one insertion point +// (getOppositeTarget in useDroppableCollectionState). In a tree they must differ +// — beside a folder vs. inside it — so both raw forms are normalized here, and +// every caller resolves through this. Doing it in the delegate instead breaks +// rendering: react-aria only tests candidates tied to specific rows. +// +// A gap can span several depths; preferredLevel picks one by indexing the deeper +// row's `ancestors` (root-first, so index === level). +export function normalizeBoundaryTarget( + boundary: BoundaryTargets, + { hasVisibleChildren, preferredLevel, lookup }: DropContext, +): NormalizedBoundary { + const { dropPosition, targetItem, realTargetItem } = boundary; + // Same gap as "before" its first child: land inside only if the cursor asks + // to go deeper than the folder itself. + if ( + dropPosition === 'after' && + isFolderItem(targetItem) && + hasVisibleChildren(targetItem.doc._id) && + preferredLevel > getIndentLevel(targetItem) + ) { + return { dropPosition: 'on', targetItem, redirected: false }; + } + // A "Folder is empty" placeholder stands in for the folder's interior, so the + // gap above it is the folder's trailing edge. Rewriting also fixes ordering: + // the placeholder form carries a first-child sort key. + if ( + dropPosition === 'before' && + isEmptyNode(targetItem) && + isFolderItem(realTargetItem) && + preferredLevel <= getIndentLevel(realTargetItem) + ) { + return { dropPosition: 'after', targetItem: realTargetItem, redirected: true }; + } + // The placeholder's lower edge: the gap after the folder's whole interior. + // Not level-aware on purpose — the opposite raw form can't express "inside", + // so offering it here would make the two forms disagree. + if (dropPosition === 'after' && isEmptyNode(targetItem)) { + const owningFolder = targetItem.requestGroup ? lookup(targetItem.requestGroup._id) : null; + if (isFolderItem(owningFolder)) { + return { dropPosition: 'after', targetItem: owningFolder, redirected: true }; + } + } + + // Clamp the requested level to the range the gap spans, then walk the deeper + // row's ancestor chain. Three raw forms share this one walk by design. + const gap = resolveGapSides(boundary); + if (gap) { + const deepLevel = getIndentLevel(gap.deep); + const level = Math.min(Math.max(preferredLevel, getIndentLevel(gap.shallow)), deepLevel); + const ancestorId = level < deepLevel ? gap.deep.ancestors?.[level] : undefined; + const ancestor = ancestorId ? lookup(ancestorId) : null; + if (isCollectionChild(ancestor)) { + return { dropPosition: 'after', targetItem: ancestor, redirected: ancestor !== targetItem }; + } + // Deepest landing. If the deep row is the target the caller's form already + // says so; otherwise re-anchor onto it. + return gap.deep === targetItem + ? { dropPosition, targetItem, redirected: false } + : { dropPosition: 'after', targetItem: gap.deep, redirected: true }; + } + + return { dropPosition, targetItem, redirected: false }; +} + +// The dragged item's new parent, sent to the reorder route outright so there is +// one implementation of "where does this land". +function destinationParentIdFor( + { targetItem, dropPosition }: NormalizedBoundary, + { realTargetItem }: BoundaryTargets, +): string | null { + // At the top of a collection the previous row is the workspace header itself. + if (realTargetItem?.kind === 'workspace' && models.workspace.isCollection(realTargetItem.doc)) { + return realTargetItem.doc._id; + } + + if (dropPosition === 'on' && isFolderItem(targetItem)) { + return targetItem.doc._id; + } + + // Only the "inside" case reaches here; normalizeBoundaryTarget rewrites the + // shallower one. + if (isEmptyNode(targetItem) && isFolderItem(realTargetItem)) { + return realTargetItem.doc._id; + } + + if (isCollectionChild(targetItem) && 'parentId' in targetItem.doc) { + return targetItem.doc.parentId; + } + + return null; +} + +// Folder a drop lands inside, or null at top level. Worth naming because the +// folder adjacent to a boundary is often not the destination. +function destinationFolderNameFor(normalized: NormalizedBoundary, parentItem: FlatItem | null): string | null { + // Hovering a folder's own row without merging to 'on': naming it would read + // as "into it" when the drop goes beside it. A redirect is unambiguous. + if (normalized.dropPosition === 'after' && !normalized.redirected && isFolderItem(normalized.targetItem)) { + return null; + } + return isFolderItem(parentItem) ? parentItem.doc.name || 'Untitled folder' : null; +} + +export interface ResolvedDrop { + normalized: NormalizedBoundary; + parentId: string | null; + // The destination as a row, when visible — canDrop needs its ancestry. + parentItem: FlatItem | null; + folderName: string | null; + landingLevel: number; +} + +// One normalization pass, shared by the indicator and the commit so they cannot +// disagree about where a drop lands. +export function resolveDrop(boundary: BoundaryTargets, ctx: DropContext): ResolvedDrop { + const normalized = normalizeBoundaryTarget(boundary, ctx); + const parentId = destinationParentIdFor(normalized, boundary); + const parentItem = parentId ? (ctx.flatItemsById.get(parentId)?.[0] ?? null) : null; + return { + normalized, + parentId, + parentItem, + folderName: destinationFolderNameFor(normalized, parentItem), + landingLevel: getIndentLevel(normalized.targetItem), + }; +} + +export function canDrop( dragItem: FlatItem, - dropItem: FlatItem, - { dropPosition }: ItemDropTarget, - dropPrevItem: FlatItem | null, - dropNextItem: FlatItem | null, + boundary: BoundaryTargets, + // Level selection can redirect the landing far from the row the boundary sits + // on, so containment below is tested against this, not the anchor row. + destination: Pick, expandedProjectAndWorkspaceIds?: string[], ) { - const realDropItem = dropPosition === 'before' ? dropPrevItem : dropItem; + const { dropPosition, targetItem: dropItem, realTargetItem: realDropItem, nextItem } = boundary; + const { parentId: destinationParentId, parentItem: destinationParentItem } = destination; + if (dropPosition === 'on') { + // Dropping directly "on" a row means "move inside this folder" — only valid + // when dragging a request/request group onto another folder. + if ( + dragItem.doc._id === dropItem.doc._id || + dragItem.kind !== 'collectionChild' || + !isAllowDragItem(dragItem) || + !isFolderItem(dropItem) + ) { + return false; + } + // moving a folder into itself or one of its own descendants is not allowed + return !(models.requestGroup.isRequestGroup(dragItem.doc) && dropItem.ancestors?.includes(dragItem.doc._id)); + } + // The item following realDropItem in the list - const itemAfterRealDrop = dropPosition === 'before' ? dropItem : dropNextItem; - // drag and drop items are same. - if ( - !realDropItem || - dragItem.doc._id === dropItem.doc._id || - dragItem.doc._id === realDropItem.doc._id || - !isAllowDropTarget(realDropItem) - ) { + const itemAfterRealDrop = dropPosition === 'before' ? dropItem : nextItem; + if (!realDropItem || !isAllowDropTarget(realDropItem)) { return false; } + // Nothing may land inside itself, and no folder inside its own descendants. + // Self-parenting is unrecoverable: roots come from `parentId === workspaceId`, + // so the folder and its subtree would vanish from the sidebar. + if (isCollectionChild(dragItem)) { + if (destinationParentId === dragItem.doc._id) { + return false; + } + if ( + models.requestGroup.isRequestGroup(dragItem.doc) && + isCollectionChild(destinationParentItem) && + destinationParentItem.ancestors?.includes(dragItem.doc._id) + ) { + return false; + } + } + + // Beside the dragged item is normally a no-op, but level selection can make it + // a real move out of the parent — so only reject it if the parent is unchanged. + const isBesideDragItem = dragItem.doc._id === dropItem.doc._id || dragItem.doc._id === realDropItem.doc._id; + if (isBesideDragItem) { + const changesParent = + dragItem.kind === 'collectionChild' && + destinationParentId != null && + destinationParentId !== dragItem.doc.parentId; + if (!changesParent) { + return false; + } + } + if (!isAllowDragItem(dragItem)) { return false; } @@ -122,17 +378,187 @@ function canDrop( models.workspace.isCollection(realDropItem.doc) && (models.requestGroup.isRequestGroup(dragItem.doc) || models.request.isRequest(dragItem.doc)) ) { - // not same collection and none are in cloud + // Repositioning inside its own collection is not a cross-collection move, so + // the cloud rule below does not apply. + if (isCollectionChild(dragItem) && dragItem.workspace._id === realDropItem.doc._id) { + return true; + } + // Moving BETWEEN collections cannot involve cloud sync on either side. const dropInCloud = models.project.isRemoteProject(realDropItem.project); return !dragInCloud && !dropInCloud; } - // move other things into workspace is not allowed, or move after empty node is not allowed - if (realDropItem.kind === 'workspace' || isEmptyNode(realDropItem)) { + // An empty PROJECT or COLLECTION placeholder has no sibling list to join. An + // empty FOLDER's does, since it stands in for that folder's interior. + if (realDropItem.kind === 'workspace' || (isEmptyNode(realDropItem) && realDropItem.kind !== 'emptyFolder')) { return false; } - return !(models.requestGroup.isRequestGroup(dragItem.doc) && realDropItem.ancestors?.includes(dragItem.doc._id)); + return true; +} + +// Widens react-aria's 5px before/after band around an "on"-capable row. +const EDGE_PX = 10; + +interface VirtualRow { + key: string | number | bigint; + start: number; + end: number; + size: number; +} + +// Hit-tests from the virtualizer's row geometry, with no DOM measurement. Knows +// only row positions; normalizeBoundaryTarget interprets the result. +export class SidebarDropTargetDelegate { + private depthOffset = 0; + private onHoverLevelChange: ((level: number) => void) | null = null; + private pxPerRem = FALLBACK_PX_PER_REM; + + constructor(private virtualizer: Virtualizer) {} + + // onHoverLevelChange is setState-backed: React bails out on an unchanged + // value, so calling it every dragover only re-renders when the level moves. + configure(depthOffset: number, onHoverLevelChange: (level: number) => void) { + this.depthOffset = depthOffset; + this.onHoverLevelChange = onHoverLevelChange; + } + + // Once per drag: it can't change mid-drag, and reading it per pointer hit + // forces a style recalculation in the drag hot path. + sampleRootFontSize() { + this.pxPerRem = measurePxPerRem(); + } + + private classify( + contentY: number, + virtualItems: VirtualRow[], + isValidDropTarget: (target: DropTarget) => boolean, + ): DropTarget { + let matchIndex = virtualItems.findIndex(v => contentY >= v.start && contentY < v.end); + if (matchIndex === -1) { + matchIndex = contentY < virtualItems[0].start ? 0 : virtualItems.length - 1; + } + const match = virtualItems[matchIndex]; + + // Virtual item keys can technically be bigint; our own keys are always + // doc ids (strings), and react-aria's Key type doesn't include bigint. + const key = String(match.key); + const onTarget: ItemDropTarget = { type: 'item', key, dropPosition: 'on' }; + const { start, end, size } = match; + + if (isValidDropTarget(onTarget)) { + const edge = Math.min(EDGE_PX, size / 3); + if (contentY <= start + edge) { + const before: ItemDropTarget = { ...onTarget, dropPosition: 'before' }; + if (isValidDropTarget(before)) { + return before; + } + } + if (contentY >= end - edge) { + const after: ItemDropTarget = { ...onTarget, dropPosition: 'after' }; + if (isValidDropTarget(after)) { + return after; + } + } + return onTarget; + } + + const mid = start + size / 2; + if (contentY <= mid) { + const before: ItemDropTarget = { ...onTarget, dropPosition: 'before' }; + if (isValidDropTarget(before)) { + return before; + } + } + const after: ItemDropTarget = { ...onTarget, dropPosition: 'after' }; + if (isValidDropTarget(after)) { + return after; + } + return onTarget; + } + + getDropTargetFromPoint(x: number, y: number, isValidDropTarget: (target: DropTarget) => boolean): DropTarget { + this.onHoverLevelChange?.(levelFromX(x, this.depthOffset, this.pxPerRem)); + + const virtualItems = this.virtualizer.getVirtualItems(); + if (virtualItems.length === 0) { + return { type: 'root' }; + } + + // Do NOT add scrollOffset: react-aria measures y against the GridList, which + // is inside the scroller at full content height, so the offset cancels out. + return this.classify(y, virtualItems, isValidDropTarget); + } +} + +interface SidebarDropIndicatorProps { + target: ItemDropTarget; + rowStart: number; + rowEnd: number; + rowHeight: number; + isValid: boolean; + // Indent matching the level the item will land at. + indentRem: number; + folderName: string | null; +} + +function SidebarDropIndicator({ + target, + rowStart, + rowEnd, + rowHeight, + isValid, + indentRem, + folderName, +}: SidebarDropIndicatorProps) { + const outlineClass = isValid ? 'outline-(--color-surprise)' : 'outline-(--color-danger)'; + + // "on" means inside: outline the whole row rather than draw a line. + if (target.dropPosition === 'on') { + return ( + + + + ); + } + + return ( + + {folderName ? ( + + + {folderName} + + ) : ( + + )} + + ); } interface UseSidebarDragAndDropOptions { @@ -148,6 +574,8 @@ interface UseSidebarDragAndDropOptions { dropPosition: 'before' | 'after', ) => void; expandedProjectAndWorkspaceIds?: string[]; + // Ancestor indent levels stripped from row rendering (see request-node.tsx). + depthOffset?: number; } export const useSidebarDragAndDrop = ({ @@ -156,6 +584,7 @@ export const useSidebarDragAndDrop = ({ virtualizer, onWorkspaceReorder, expandedProjectAndWorkspaceIds, + depthOffset = 0, }: UseSidebarDragAndDropOptions): DragAndDropHooks => { const reorderFetcher = useDebugReorderActionFetcher(); @@ -166,6 +595,32 @@ export const useSidebarDragAndDrop = ({ visibles.map((item, index) => [item.doc._id, [item, visibles[index - 1], visibles[index + 1]]] as const), ); }, [flatItems]); + + const dropTargetDelegateRef = useRef(null); + if (!dropTargetDelegateRef.current) { + dropTargetDelegateRef.current = new SidebarDropTargetDelegate(virtualizer); + } + + // Nesting level the cursor's x selects, for gaps spanning several depths. + const [hoverLevel, setHoverLevel] = useState(0); + dropTargetDelegateRef.current.configure(depthOffset, setHoverLevel); + + // Whether the folder currently shows an interior to drop into — which an + // expanded but childless folder does, via its placeholder row. + const hasVisibleChildren = useCallback( + folderId => + flatItems.some(candidate => { + if (candidate.hidden) { + return false; + } + if (candidate.kind === 'emptyFolder') { + return candidate.requestGroup?._id === folderId; + } + return candidate.kind === 'collectionChild' && candidate.doc.parentId === folderId; + }), + [flatItems], + ); + const draggingCollectionItemIdRef = useRef(null); const getCollectionItemByKey = useCallback( @@ -179,27 +634,54 @@ export const useSidebarDragAndDrop = ({ [flatItemsById], ); + const dropContext: DropContext = { + hasVisibleChildren, + lookup: getCollectionItemByKey, + flatItemsById, + preferredLevel: hoverLevel, + }; + const collectionDragAndDrop = useDragAndDrop({ getItems: keys => [...keys].map(key => ({ 'text/plain': key.toString() })), + // Without this, the browser allows copy/move/link and picks the cursor icon itself, + // which shows up as a flickering "+" (copy) cursor instead of a stable move cursor. + getAllowedDropOperations: () => ['move'], + // Our own delegate replaces react-aria's default hit-testing (which uses + // a hardcoded, easy-to-jitter-across 5px edge band) with wider zones + // computed from the virtualizer's own row geometry. + dropTargetDelegate: dropTargetDelegateRef.current, onDragStart(event) { const [draggedKey] = event.keys; draggingCollectionItemIdRef.current = draggedKey?.toString() || null; + dropTargetDelegateRef.current?.sampleRootFontSize(); }, onDragEnd() { draggingCollectionItemIdRef.current = null; + // Or the next drag opens against this one's last cursor column. + setHoverLevel(0); }, getDropOperation(target, _types) { - if (target.type !== 'item' || target.dropPosition === 'on') { + if (target.type !== 'item') { + return 'cancel'; + } + if (target.dropPosition === 'on') { + const dropItem = getCollectionItemByKey(target.key); + // Only allow the "on" drop position (move inside) for folders. This lets + // react-aria carve out real before/after edge zones for every other row, + // instead of the whole row being one big "move inside" target. + if (isFolderItem(dropItem)) { + return 'move'; + } return 'cancel'; } return 'move'; }, onMove(event) { - const { type, dropPosition: _dropPosition, key } = event.target; + const { type, dropPosition: rawDropPosition, key } = event.target; if (type !== 'item') { return; } - let dropPosition = _dropPosition; + let dropPosition = rawDropPosition; const isBefore = dropPosition === 'before'; const droppedKey = key.toString(); @@ -207,24 +689,29 @@ export const useSidebarDragAndDrop = ({ const draggedItem = getCollectionItemByKey(draggedKey) as AllowDragItem | null; const targetItem = getCollectionItemByKey(droppedKey) as AllowDropTarget | null; const realTargetItem = isBefore ? flatItemsById.get(droppedKey)?.[1] : targetItem; + const nextTargetItem = flatItemsById.get(droppedKey)?.[2]; + // Resolved from the RAW target, the same way the indicator resolves it. + const boundary: BoundaryTargets | null = targetItem && { + dropPosition: rawDropPosition, + targetItem, + realTargetItem, + nextItem: nextTargetItem, + }; + const resolved = boundary ? resolveDrop(boundary, dropContext) : null; + const destinationParentId = resolved?.parentId ?? null; if ( !draggedItem || !targetItem || - !canDrop( - draggedItem, - targetItem, - event.target, - flatItemsById.get(droppedKey)?.[1] || null, - flatItemsById.get(droppedKey)?.[2] || null, - expandedProjectAndWorkspaceIds, - ) + !boundary || + !resolved || + !canDrop(draggedItem, boundary, resolved, expandedProjectAndWorkspaceIds) ) { return; } // move workspace to another project or reorder within same project if (draggedItem.kind === 'workspace') { - if (_dropPosition === 'on') { + if (rawDropPosition === 'on') { return; } // Dropping after the last child of a collection @@ -277,7 +764,7 @@ export const useSidebarDragAndDrop = ({ targetProjectId, draggedItem.doc._id, targetItem.doc._id, - _dropPosition, + rawDropPosition, ); } } else { @@ -289,7 +776,7 @@ export const useSidebarDragAndDrop = ({ draggedItem.project._id, draggedItem.doc._id, targetItem.doc._id, - _dropPosition, + rawDropPosition, ); } } @@ -298,7 +785,7 @@ export const useSidebarDragAndDrop = ({ // move request or request group into collection if (realTargetItem?.kind === 'workspace' && models.workspace.isCollection(realTargetItem!.doc)) { - const siblingItem = flatItems.find( + const firstChildOfCollection = flatItems.find( (item): item is CollectionChildFlatItem => item.kind === 'collectionChild' && item.doc.parentId === realTargetItem!.doc._id, ); @@ -310,55 +797,57 @@ export const useSidebarDragAndDrop = ({ targetId: realTargetItem!.doc._id, id: draggedItem.doc._id, dropPosition: 'after', // collection only accepts move into, so treat all drops as "after" - metaSortKey: siblingItem?.doc.metaSortKey != null ? siblingItem.doc.metaSortKey - 100 : -1 * Date.now(), + metaSortKey: firstChildOfCollection?.doc.metaSortKey != null ? firstChildOfCollection.doc.metaSortKey - 100 : -1 * Date.now(), }, }); return; } + const normalized = normalizeBoundaryTarget(boundary, dropContext); + dropPosition = normalized.dropPosition; + // normalizeBoundaryTarget only ever swaps in a 'collectionChild'. + const normalizedTargetItem = normalized.targetItem as AllowDropTarget; + const id = draggedItem.doc._id; - let targetId = targetItem.doc._id; - const targetIsEmptyNode = isEmptyNode(targetItem); + let targetId = normalizedTargetItem.doc._id; + const targetIsEmptyNode = isEmptyNode(normalizedTargetItem); const workspaceCollectionItems = flatItems.filter( (item): item is CollectionChildFlatItem => item.kind === 'collectionChild' && item.workspace._id === draggedItem.workspace._id, ); let metaSortKey = 0; - const isMovingItemInsideFolder = - !targetIsEmptyNode && models.requestGroup.isRequestGroup(targetItem.doc) && dropPosition === 'after'; - - const isMovingOnEmptyNode = - realTargetItem && - 'type' in realTargetItem.doc && - models.requestGroup.isRequestGroup(realTargetItem.doc) && - targetIsEmptyNode; + const isMovingItemInsideFolder = dropPosition === 'on' && isFolderItem(normalizedTargetItem); + const isMovingIntoEmptyFolder = targetIsEmptyNode && isFolderItem(realTargetItem); if (isMovingItemInsideFolder) { - // The reorder route interprets "after folder" as moving into that folder. + // Sort ahead of the folder's current first child so it lands at the top. const children = workspaceCollectionItems.filter(item => item.doc.parentId === targetId); metaSortKey = children.length > 0 ? children[0].doc.metaSortKey - 100 : -1 * Date.now(); - } else if (isMovingOnEmptyNode) { + } else if (isMovingIntoEmptyFolder) { + // The placeholder has no doc of its own to sort against, so target the + // folder that owns it; parentId below puts the item inside that folder. targetId = realTargetItem.doc._id; - dropPosition = 'after'; metaSortKey = -1 * Date.now(); } else { // move before or after another request in same or different collection const siblingItems = workspaceCollectionItems.filter( - item => 'parentId' in targetItem.doc && item.doc.parentId === targetItem.doc.parentId, + item => 'parentId' in normalizedTargetItem.doc && item.doc.parentId === normalizedTargetItem.doc.parentId, ); const targetIndex = siblingItems.findIndex(item => item.doc._id === targetId); - if ('metaSortKey' in targetItem.doc && targetItem.doc.metaSortKey != null) { + if ('metaSortKey' in normalizedTargetItem.doc && normalizedTargetItem.doc.metaSortKey != null) { if (dropPosition === 'after') { const afterItem = siblingItems[targetIndex + 1]; metaSortKey = afterItem - ? targetItem.doc.metaSortKey - (targetItem.doc.metaSortKey - afterItem.doc.metaSortKey) / 2 - : targetItem.doc.metaSortKey + 100; + ? normalizedTargetItem.doc.metaSortKey - + (normalizedTargetItem.doc.metaSortKey - afterItem.doc.metaSortKey) / 2 + : normalizedTargetItem.doc.metaSortKey + 100; } else { const beforeItem = siblingItems[targetIndex - 1]; metaSortKey = beforeItem - ? targetItem.doc.metaSortKey - (targetItem.doc.metaSortKey - beforeItem.doc.metaSortKey) / 2 - : targetItem.doc.metaSortKey - 100; + ? normalizedTargetItem.doc.metaSortKey - + (normalizedTargetItem.doc.metaSortKey - beforeItem.doc.metaSortKey) / 2 + : normalizedTargetItem.doc.metaSortKey - 100; } } } @@ -376,53 +865,52 @@ export const useSidebarDragAndDrop = ({ id, dropPosition, metaSortKey, + parentId: destinationParentId ?? undefined, }, }); }, renderDropIndicator(target) { - if (target.type === 'item') { - const item = virtualizer.getVirtualItems().find(virtualItem => virtualItem.key === target.key); - if (item) { - const draggedItem = getCollectionItemByKey(draggingCollectionItemIdRef.current); - const targetItem = getCollectionItemByKey(target.key); - if ( - draggedItem == null || - targetItem == null || - !canDrop( - draggedItem as FlatItem, - targetItem as FlatItem, - target, - flatItemsById.get(target.key.toString())?.[1] || null, - flatItemsById.get(target.key.toString())?.[2] || null, - expandedProjectAndWorkspaceIds, - ) - ) { - return ( - - ); - } - return ( - - ); - } + const row = + target.type === 'item' + ? virtualizer.getVirtualItems().find(virtualItem => virtualItem.key === target.key) + : undefined; + if (target.type !== 'item' || !row) { + return ( + + ); } + const entry = flatItemsById.get(target.key.toString()); + const draggedItem = getCollectionItemByKey(draggingCollectionItemIdRef.current); + const targetItem = getCollectionItemByKey(target.key); + const boundary: BoundaryTargets | null = targetItem && { + dropPosition: target.dropPosition, + targetItem, + realTargetItem: target.dropPosition === 'before' ? entry?.[1] : targetItem, + nextItem: entry?.[2], + }; + + // One pass drives validity, indent and the named destination. + const resolved = boundary ? resolveDrop(boundary, dropContext) : null; + const isValid = + draggedItem != null && + boundary != null && + resolved != null && + canDrop(draggedItem, boundary, resolved, expandedProjectAndWorkspaceIds); + return ( - ); },