From 2ad471123d99acb337ceec6a47ed91342e75ea0f Mon Sep 17 00:00:00 2001 From: _Kerman Date: Tue, 11 Aug 2026 15:57:43 +0800 Subject: [PATCH] fix(client): persist workspace drag order --- .../runtime/src/client/workspaces/manager.ts | 17 ++++-- .../runtime/tests/workspaces-service.spec.ts | 6 +++ .../src/client/WorkspaceBrowser.tsx | 54 +++++++++++++------ .../tests/workspace-browser.spec.tsx | 28 ++++++++++ 4 files changed, 84 insertions(+), 21 deletions(-) diff --git a/packages/client/runtime/src/client/workspaces/manager.ts b/packages/client/runtime/src/client/workspaces/manager.ts index 89f96295ef..6f2e95d351 100644 --- a/packages/client/runtime/src/client/workspaces/manager.ts +++ b/packages/client/runtime/src/client/workspaces/manager.ts @@ -173,10 +173,19 @@ export class WorkspaceManager { const frameGeneration = this.orderFrameGeneration const previousOrder = this.itemViews().map(workspace => workspace.workspaceId) this.installOrder(insertIdBefore(previousOrder, workspaceId, beforeWorkspaceId)) - const { result } = await this.api.workspace.insertBefore({ - workspaceId, - ...beforeWorkspaceId === undefined ? {} : { beforeWorkspaceId }, - }) + let result: RpcResult<{ workspaceIds: WorkspaceId[] }> + try { + ;({ result } = await this.api.workspace.insertBefore({ + workspaceId, + ...beforeWorkspaceId === undefined ? {} : { beforeWorkspaceId }, + })) + } catch (error) { + if (requestGeneration === this.orderRequestGeneration + && frameGeneration === this.orderFrameGeneration) { + this.installOrder(previousOrder) + } + throw error + } if (result.ok && requestGeneration === this.orderRequestGeneration && frameGeneration === this.orderFrameGeneration) { this.installOrder(result.value.workspaceIds) diff --git a/packages/client/runtime/tests/workspaces-service.spec.ts b/packages/client/runtime/tests/workspaces-service.spec.ts index 2c4ed551d8..db80e98337 100644 --- a/packages/client/runtime/tests/workspaces-service.spec.ts +++ b/packages/client/runtime/tests/workspaces-service.spec.ts @@ -107,6 +107,12 @@ describe('WorkspaceManager', () => { expect(manager.getSnapshot().items.map(item => item.workspaceId)).toEqual(['one', 'two', 'three']) await expect(rejected).resolves.toMatchObject({ ok: false }) expect(manager.getSnapshot().items.map(item => item.workspaceId)).toEqual(['one', 'three', 'two']) + + api.onWorkspaceInsertBefore = () => Promise.reject(new Error('transport down')) + const disconnected = manager.insertBefore(wid('three'), wid('one')) + expect(manager.getSnapshot().items.map(item => item.workspaceId)).toEqual(['three', 'one', 'two']) + await expect(disconnected).rejects.toThrow('transport down') + expect(manager.getSnapshot().items.map(item => item.workspaceId)).toEqual(['one', 'three', 'two']) }) it('replays removal over an in-flight baseline and ignores duplicate or late updates', async () => { diff --git a/packages/client/ui-workspace/src/client/WorkspaceBrowser.tsx b/packages/client/ui-workspace/src/client/WorkspaceBrowser.tsx index 637b89c162..eeb47e2cb1 100644 --- a/packages/client/ui-workspace/src/client/WorkspaceBrowser.tsx +++ b/packages/client/ui-workspace/src/client/WorkspaceBrowser.tsx @@ -200,9 +200,10 @@ function SessionTree({ const [drag, setDrag] = useState(null) const sessionDropCommitted = useRef(false) const [workspaceDrag, setWorkspaceDrag] = useState(null) - const sessionDragging = drag !== null + const workspaceDropCommitted = useRef(false) + const nativeDragActive = drag !== null || workspaceDrag !== null useEffect(() => { - if (!sessionDragging) return + if (!nativeDragActive) return // Row hover still owns the insertion marker. Accept the native drag at // document level so releasing outside the list is not rendered as a // rejected drop before dragend commits that last marker. @@ -217,7 +218,7 @@ function SessionTree({ document.removeEventListener('dragover', acceptDrag) document.removeEventListener('drop', acceptDrop) } - }, [sessionDragging]) + }, [nativeDragActive]) const currentGroup = current === undefined ? undefined : (workspaces.find(w => w.sessionIds.includes(current))?.workspaceId as string | undefined) @@ -310,6 +311,26 @@ function SessionTree({ console.warn('session reorder rejected:', reason) }) } + const commitWorkspaceDrag = ( + activeDrag: WorkspaceDragState, + over: NonNullable, + ): void => { + if (workspaceDropCommitted.current) return + workspaceDropCommitted.current = true + setWorkspaceDrag(null) + const rowIndex = workspaces.findIndex(workspace => workspace.workspaceId === over.id) + if (rowIndex === -1) return + const anchor = over.half === 'before' ? over.id : workspaces[rowIndex + 1]?.workspaceId + if (anchor === activeDrag.workspaceId) return + const sourceIndex = workspaces.findIndex(workspace => workspace.workspaceId === activeDrag.workspaceId) + const anchorIndex = anchor === undefined + ? workspaces.length + : workspaces.findIndex(workspace => workspace.workspaceId === anchor) + if (sourceIndex !== -1 && (anchorIndex === sourceIndex || anchorIndex === sourceIndex + 1)) return + insertWorkspaceBefore(activeDrag.workspaceId, anchor).catch((reason: unknown) => { + console.warn('workspace reorder rejected:', reason) + }) + } return (
@@ -323,8 +344,18 @@ function SessionTree({ ? workspaceDrag.over.half : null const workspaceDragProps = workspaceId === undefined ? undefined : { - start: () => { setWorkspaceDrag({ workspaceId, over: null }) }, - end: () => { setWorkspaceDrag(null) }, + start: () => { + workspaceDropCommitted.current = false + setWorkspaceDrag({ workspaceId, over: null }) + }, + end: () => { + if (workspaceDrag?.over !== null && workspaceDrag?.over !== undefined) { + commitWorkspaceDrag(workspaceDrag, workspaceDrag.over) + } else { + setWorkspaceDrag(null) + } + workspaceDropCommitted.current = false + }, } const hoverWorkspace = workspaceId === undefined ? undefined @@ -337,18 +368,7 @@ function SessionTree({ ? undefined : (half: 'before' | 'after') => { if (workspaceDrag === null) return - const rowIndex = workspaces.findIndex(workspace => workspace.workspaceId === workspaceId) - const anchor = half === 'before' ? workspaceId : workspaces[rowIndex + 1]?.workspaceId - setWorkspaceDrag(null) - if (anchor === workspaceDrag.workspaceId) return - const sourceIndex = workspaces.findIndex(workspace => workspace.workspaceId === workspaceDrag.workspaceId) - const anchorIndex = anchor === undefined - ? workspaces.length - : workspaces.findIndex(workspace => workspace.workspaceId === anchor) - if (sourceIndex !== -1 && (anchorIndex === sourceIndex || anchorIndex === sourceIndex + 1)) return - insertWorkspaceBefore(workspaceDrag.workspaceId, anchor).catch((reason: unknown) => { - console.warn('workspace reorder rejected:', reason) - }) + commitWorkspaceDrag(workspaceDrag, { id: workspaceId, half }) } return ( // Group section: header row + expanded top-level session rows. The diff --git a/packages/client/ui-workspace/tests/workspace-browser.spec.tsx b/packages/client/ui-workspace/tests/workspace-browser.spec.tsx index ca48733dda..414353f684 100644 --- a/packages/client/ui-workspace/tests/workspace-browser.spec.tsx +++ b/packages/client/ui-workspace/tests/workspace-browser.spec.tsx @@ -629,6 +629,34 @@ describe('WorkspaceBrowser', () => { expect(insertWorkspaceBefore).toHaveBeenCalledWith(wid('tail'), wid('beta')) }) + it('accepts a document-level drop and commits the last Workspace marker on drag end', () => { + const insertWorkspaceBefore = vi.fn(async () => {}) + mount({ + useWorkspaces: hook(workspaceState([ + workspace('alpha', []), + workspace('beta', []), + workspace('tail', []), + ])), + insertWorkspaceBefore, + }) + const source = screen.getByText('tail').closest('[role="treeitem"]') as HTMLElement + let target = screen.getByText('beta').closest('[role="treeitem"]')?.parentElement as HTMLElement + while (target.parentElement?.getAttribute('role') !== 'tree') { + target = target.parentElement as HTMLElement + } + target.getBoundingClientRect = () => ({ + top: 100, bottom: 134, left: 0, right: 200, width: 200, height: 34, x: 0, y: 100, toJSON: () => ({}), + }) + fireEvent.dragStart(source, { dataTransfer: dragData() }) + fireDrag(target, 'dragOver', 105) + const outsideDrop = createEvent.drop(document.body) + Object.defineProperty(outsideDrop, 'dataTransfer', { value: dragData() }) + fireEvent(document.body, outsideDrop) + expect(outsideDrop.defaultPrevented).toBe(true) + fireEvent.dragEnd(source) + expect(insertWorkspaceBefore).toHaveBeenCalledWith(wid('tail'), wid('beta')) + }) + it('drag reorder reports the anchor to insertSessionBefore and skips no-op drops', () => { const insertSessionBefore = vi.fn(async () => {}) const sessions = sessionState([summary('one', 3), summary('two', 2), summary('three', 1)])