diff --git a/cucumber-report.json b/cucumber-report.json new file mode 100644 index 00000000..07a29a79 --- /dev/null +++ b/cucumber-report.json @@ -0,0 +1,345 @@ +[ + { + "description": " As a user with multiple workspaces\n I want to organize them into groups\n So that I can manage them more efficiently", + "elements": [ + { + "description": "", + "id": "workspace-grouping;dragging-a-workspace-from-a-collapsed-group", + "keyword": "Scenario", + "line": 86, + "name": "Dragging a workspace from a collapsed group", + "steps": [ + { + "arguments": [], + "keyword": "Given ", + "line": 8, + "name": "I cleanup test wiki so it could create a new one on start", + "result": { + "status": "undefined", + "duration": 0 + } + }, + { + "arguments": [], + "keyword": "When ", + "line": 9, + "name": "I launch the TidGi application", + "result": { + "status": "undefined", + "duration": 0 + } + }, + { + "arguments": [], + "keyword": "And ", + "line": 10, + "name": "I wait for the page to load completely", + "result": { + "status": "undefined", + "duration": 0 + } + }, + { + "arguments": [], + "keyword": "And ", + "line": 11, + "name": "the browser view should be loaded and visible", + "result": { + "status": "undefined", + "duration": 0 + } + }, + { + "arguments": [], + "keyword": "When ", + "line": 87, + "name": "I create a new wiki workspace with name \"Collapsed Group Alpha\"", + "result": { + "status": "undefined", + "duration": 0 + } + }, + { + "arguments": [], + "keyword": "And ", + "line": 88, + "name": "I create a new wiki workspace with name \"Collapsed Group Beta\"", + "result": { + "status": "undefined", + "duration": 0 + } + }, + { + "arguments": [], + "keyword": "And ", + "line": 89, + "name": "I create a new wiki workspace with name \"Collapsed Group Gamma\"", + "result": { + "status": "undefined", + "duration": 0 + } + }, + { + "arguments": [ + { + "rows": [ + { + "cells": [ + "Collapsed Group Alpha" + ] + }, + { + "cells": [ + "Collapsed Group Beta" + ] + } + ] + } + ], + "keyword": "Given ", + "line": 90, + "name": "workspace group \"Collapsed Test Group\" contains workspaces:", + "result": { + "status": "undefined", + "duration": 0 + } + }, + { + "arguments": [], + "keyword": "When ", + "line": 93, + "name": "I collapse workspace group \"Collapsed Test Group\"", + "result": { + "status": "undefined", + "duration": 0 + } + }, + { + "arguments": [], + "keyword": "And ", + "line": 94, + "name": "I expand workspace group \"Collapsed Test Group\"", + "result": { + "status": "undefined", + "duration": 0 + } + }, + { + "arguments": [], + "keyword": "And ", + "line": 95, + "name": "I drag workspace \"Collapsed Group Alpha\" onto workspace \"Collapsed Group Gamma\"", + "result": { + "status": "undefined", + "duration": 0 + } + }, + { + "arguments": [], + "keyword": "Then ", + "line": 96, + "name": "workspaces \"Collapsed Group Alpha\" and \"Collapsed Group Gamma\" should share a group", + "result": { + "status": "undefined", + "duration": 0 + } + }, + { + "arguments": [], + "keyword": "And ", + "line": 97, + "name": "workspace \"Collapsed Group Beta\" should be in a group", + "result": { + "status": "undefined", + "duration": 0 + } + } + ], + "tags": [ + { + "name": "@workspace-group", + "line": 1 + } + ], + "type": "scenario" + }, + { + "description": "", + "id": "workspace-grouping;reordering-group-headers", + "keyword": "Scenario", + "line": 129, + "name": "Reordering group headers", + "steps": [ + { + "arguments": [], + "keyword": "Given ", + "line": 8, + "name": "I cleanup test wiki so it could create a new one on start", + "result": { + "status": "undefined", + "duration": 0 + } + }, + { + "arguments": [], + "keyword": "When ", + "line": 9, + "name": "I launch the TidGi application", + "result": { + "status": "undefined", + "duration": 0 + } + }, + { + "arguments": [], + "keyword": "And ", + "line": 10, + "name": "I wait for the page to load completely", + "result": { + "status": "undefined", + "duration": 0 + } + }, + { + "arguments": [], + "keyword": "And ", + "line": 11, + "name": "the browser view should be loaded and visible", + "result": { + "status": "undefined", + "duration": 0 + } + }, + { + "arguments": [], + "keyword": "When ", + "line": 130, + "name": "I create a new wiki workspace with name \"Group Order Alpha\"", + "result": { + "status": "undefined", + "duration": 0 + } + }, + { + "arguments": [], + "keyword": "And ", + "line": 131, + "name": "I create a new wiki workspace with name \"Group Order Beta\"", + "result": { + "status": "undefined", + "duration": 0 + } + }, + { + "arguments": [], + "keyword": "And ", + "line": 132, + "name": "I create a new wiki workspace with name \"Group Order Gamma\"", + "result": { + "status": "undefined", + "duration": 0 + } + }, + { + "arguments": [], + "keyword": "And ", + "line": 133, + "name": "I create a new wiki workspace with name \"Group Order Delta\"", + "result": { + "status": "undefined", + "duration": 0 + } + }, + { + "arguments": [ + { + "rows": [ + { + "cells": [ + "Group Order Alpha" + ] + }, + { + "cells": [ + "Group Order Beta" + ] + } + ] + } + ], + "keyword": "Given ", + "line": 134, + "name": "workspace group \"Group Order A\" contains workspaces:", + "result": { + "status": "undefined", + "duration": 0 + } + }, + { + "arguments": [ + { + "rows": [ + { + "cells": [ + "Group Order Gamma" + ] + }, + { + "cells": [ + "Group Order Delta" + ] + } + ] + } + ], + "keyword": "Given ", + "line": 137, + "name": "workspace group \"Group Order B\" contains workspaces:", + "result": { + "status": "undefined", + "duration": 0 + } + }, + { + "arguments": [], + "keyword": "When ", + "line": 140, + "name": "I drag group header \"Group Order B\" onto group header \"Group Order A\"", + "result": { + "status": "undefined", + "duration": 0 + } + }, + { + "arguments": [], + "keyword": "Then ", + "line": 141, + "name": "group \"Group Order B\" should appear before group \"Group Order A\"", + "result": { + "status": "undefined", + "duration": 0 + } + } + ], + "tags": [ + { + "name": "@workspace-group", + "line": 1 + } + ], + "type": "scenario" + } + ], + "id": "workspace-grouping", + "line": 2, + "keyword": "Feature", + "name": "Workspace Grouping", + "tags": [ + { + "name": "@workspace-group", + "line": 1 + } + ], + "uri": "features\\workspaceGroup.feature" + } +] \ No newline at end of file diff --git a/features/stepDefinitions/application.ts b/features/stepDefinitions/application.ts index 6c16da5f..e2d2d814 100644 --- a/features/stepDefinitions/application.ts +++ b/features/stepDefinitions/application.ts @@ -75,6 +75,7 @@ export class ApplicationWorld { savedWorkspaceId: string | undefined; // For storing workspace ID between steps scenarioName: string = 'default'; // Scenario name from Cucumber pickle scenarioSlug: string = 'default'; // Sanitized scenario name for file paths + scenarioTags: string[] = []; providerConfig: import('@services/externalAPI/interface').AIProviderConfig | undefined; // Scenario-specific AI provider config launchEnvOverrides: Record = {}; diff --git a/features/stepDefinitions/cleanup.ts b/features/stepDefinitions/cleanup.ts index d2f8c116..733142e8 100644 --- a/features/stepDefinitions/cleanup.ts +++ b/features/stepDefinitions/cleanup.ts @@ -11,6 +11,7 @@ Before(async function(this: ApplicationWorld, { pickle }) { // Initialize scenario-specific paths this.scenarioName = pickle.name; this.scenarioSlug = makeSlugPath(pickle.name, 60); + this.scenarioTags = pickle.tags.map((tag) => tag.name); const scenarioRoot = path.resolve(process.cwd(), 'test-artifacts', this.scenarioSlug); const logsDirectory = path.resolve(scenarioRoot, 'userData-test', 'logs'); diff --git a/features/stepDefinitions/wiki.ts b/features/stepDefinitions/wiki.ts index e2e5a73e..ce56dc18 100644 --- a/features/stepDefinitions/wiki.ts +++ b/features/stepDefinitions/wiki.ts @@ -182,7 +182,7 @@ When('I cleanup test wiki so it could create a new one on start', async function try { await backOff( async () => { - fs.writeJsonSync(getSettingsPath(this), { ...settings, workspaces: filtered }, { spaces: 2 }); + fs.writeJsonSync(getSettingsPath(this), { ...settings, workspaces: filtered, workspaceGroups: {} }, { spaces: 2 }); }, { numOfAttempts: 3, @@ -971,6 +971,8 @@ When('I create a new wiki workspace with name {string}', async function(this: Ap throw new Error('Application is not available'); } + const isWorkspaceGroupScenario = this.scenarioTags.includes('@workspace-group'); + // Construct the full wiki path const wikiPath = path.join(getWikiTestRootPath(this), workspaceName); @@ -986,21 +988,24 @@ When('I create a new wiki workspace with name {string}', async function(this: Ap }, }); - // Initialize fresh git repository for the new wiki using dugite - try { - // Initialize git repository with master branch - await gitExec(['init', '-b', 'master'], wikiPath); + // Workspace-group scenarios only validate grouping and drag behavior. + // Skipping git bootstrap avoids repeated add/commit overhead across dozens of test workspaces. + if (!isWorkspaceGroupScenario) { + try { + // Initialize git repository with master branch + await gitExec(['init', '-b', 'master'], wikiPath); - // Configure git user - await gitExec(['config', 'user.email', 'test@tidgi.test'], wikiPath); - await gitExec(['config', 'user.name', 'TidGi Test'], wikiPath); + // Configure git user + await gitExec(['config', 'user.email', 'test@tidgi.test'], wikiPath); + await gitExec(['config', 'user.name', 'TidGi Test'], wikiPath); - // Add all files and create initial commit - await gitExec(['add', '.'], wikiPath); - await gitExec(['commit', '-m', 'Initial commit'], wikiPath); - } catch (error) { - // Git initialization is not critical for the test, continue anyway - console.log('Git initialization skipped:', (error as Error).message); + // Add all files and create initial commit + await gitExec(['add', '.'], wikiPath); + await gitExec(['commit', '-m', 'Initial commit'], wikiPath); + } catch (error) { + // Git initialization is not critical for the test, continue anyway + console.log('Git initialization skipped:', (error as Error).message); + } } // Now create workspace configuration @@ -1026,10 +1031,30 @@ When('I create a new wiki workspace with name {string}', async function(this: Ap `); }, { wikiName: workspaceName, wikiFullPath: wikiPath }); - // Wait for workspace to appear in UI - await this.app.evaluate(async () => { - await new Promise(resolve => setTimeout(resolve, 1000)); - }); + await backOff( + async () => { + const workspaces = await this.app!.evaluate(async ({ BrowserWindow }, name: string) => { + const windows = BrowserWindow.getAllWindows(); + const mainWindow = windows.find(win => !win.isDestroyed() && win.webContents && win.webContents.getURL().includes('index.html')); + + if (!mainWindow) { + throw new Error('Main window not found'); + } + + return await mainWindow.webContents.executeJavaScript(` + (async () => { + const all = await window.service.workspace.getWorkspacesAsList(); + return all.filter(workspace => !workspace.pageType).map(workspace => workspace.name); + })(); + `) as Promise; + }, workspaceName); + + if (!workspaces.includes(workspaceName)) { + throw new Error(`Workspace ${workspaceName} not visible yet`); + } + }, + BACKOFF_OPTIONS, + ); }); /** diff --git a/features/stepDefinitions/workspaceGroup.ts b/features/stepDefinitions/workspaceGroup.ts index 198d9fde..27dfdd9b 100644 --- a/features/stepDefinitions/workspaceGroup.ts +++ b/features/stepDefinitions/workspaceGroup.ts @@ -19,30 +19,21 @@ interface ITestWorkspace { pageType?: string | null; } -async function executeInMainWindow(world: ApplicationWorld, script: string): Promise { - if (!world.app) { - throw new Error('App not initialized'); +async function executeInMainWindow(world: ApplicationWorld, pageFunction: (...arguments_: any[]) => any, argument?: any): Promise { + if (!world.currentWindow) { + throw new Error('Current window not set'); } - - return await world.app.evaluate(async ({ webContents }, code) => { - const mainWindow = webContents.getAllWebContents().find(wc => wc.getURL().includes('index.html')); - if (!mainWindow) { - throw new Error('Main window not found'); - } - - return await mainWindow.executeJavaScript(code) as T; - }, script); + return await world.currentWindow.evaluate(pageFunction, argument); } async function getAllWikiWorkspaces(world: ApplicationWorld): Promise { return await executeInMainWindow( world, - ` - (async () => { + async () => { const all = await window.service.workspace.getWorkspacesAsList(); - return all.filter(workspace => !workspace.pageType); - })(); - `, + return all.filter(workspace => !workspace.pageType) as ITestWorkspace[]; + }, + undefined, ); } @@ -61,18 +52,16 @@ async function getWorkspaceByName(world: ApplicationWorld, workspaceName: string async function getGroups(world: ApplicationWorld): Promise { return await executeInMainWindow( world, - ` - window.service.workspace.getGroupsAsList() - `, + async () => window.service.workspace.getGroupsAsList(), + undefined, ); } async function getGroupById(world: ApplicationWorld, groupId: string): Promise { return await executeInMainWindow( world, - ` - window.service.workspace.getGroup(${JSON.stringify(groupId)}) - `, + async (id) => window.service.workspace.getGroup(id), + groupId, ); } @@ -87,9 +76,10 @@ async function createGroup(world: ApplicationWorld, groupName: string): Promise< await executeInMainWindow( world, - ` - window.service.workspace.setGroup(${JSON.stringify(newGroup.id)}, ${JSON.stringify(newGroup)}) - `, + async (group: IWorkspaceGroup) => { + await window.service.workspace.setGroup(group.id, group); + }, + newGroup, ); return newGroup; @@ -105,6 +95,16 @@ async function waitForWorkspaceGroupId(world: ApplicationWorld, workspaceName: s }, BACKOFF_OPTIONS); } +async function moveWorkspaceToGroup(world: ApplicationWorld, workspaceId: string, groupId: string | null, autoDisband = true): Promise { + await executeInMainWindow( + world, + async ({ workspaceId: id, groupId: gid, autoDisband: disband }: { workspaceId: string; groupId: string | null; autoDisband: boolean }) => { + await window.service.workspace.moveWorkspaceToGroup(id, gid, disband); + }, + { workspaceId, groupId, autoDisband }, + ); +} + async function waitForGroupVisibility(world: ApplicationWorld, groupId: string): Promise { await backOff(async () => { if (!world.currentWindow) { @@ -122,13 +122,33 @@ async function dragLocatorToCoordinates( world: ApplicationWorld, sourceSelector: string, resolveTargetCoordinates: () => Promise<{ targetX: number; targetY: number }>, + scrollTargetSelector?: string, ): Promise { if (!world.currentWindow) { throw new Error('Current window not set'); } + // Capture renderer-side errors (e.g. React crashes) that would otherwise be silent. + // React errors caught by ErrorBoundary go to console.error, not pageerror. + const pageErrors: string[] = []; + const consoleErrors: string[] = []; + const onPageError = (error: Error) => { + pageErrors.push(error.message); + console.error('[Renderer pageerror]', error.message); + }; + const onConsole = (message: import('playwright').ConsoleMessage) => { + if (message.type() === 'error') { + const text = message.text(); + consoleErrors.push(text); + console.error('[Renderer console.error]', text); + } + }; + world.currentWindow.on('pageerror', onPageError); + world.currentWindow.on('console', onConsole); + const sourceLocator = world.currentWindow.locator(sourceSelector); await sourceLocator.waitFor({ state: 'visible' }); + await sourceLocator.scrollIntoViewIfNeeded(); const sourceBox = await sourceLocator.boundingBox(); if (!sourceBox) { @@ -137,16 +157,90 @@ async function dragLocatorToCoordinates( const startX = sourceBox.x + sourceBox.width / 2; const startY = sourceBox.y + sourceBox.height / 2; + + // Pre-compute target coordinates before starting the drag. + // Once dnd-kit activates, CSS transitions on SortableGroupHeader can make + // Playwright's boundingBox() stall until they settle (or time out). + const initialTargetCoordinates = await resolveTargetCoordinates(); + await world.currentWindow.mouse.move(startX, startY); await world.currentWindow.mouse.down(); + // Small initial movement to satisfy dnd-kit PointerSensor activationConstraint (distance: 8) await world.currentWindow.mouse.move(startX + 12, startY + 12, { steps: 6 }); - const initialTargetCoordinates = await resolveTargetCoordinates(); - await world.currentWindow.mouse.move(initialTargetCoordinates.targetX, initialTargetCoordinates.targetY, { steps: 20 }); - await world.currentWindow.waitForTimeout(40); + // Wait for dnd-kit to start the drag and for SortableContext to shift items + await world.currentWindow.waitForTimeout(200); + + if (scrollTargetSelector) { + // Use synthetic pointer events to teleport the drag directly onto the target. + // This avoids coordinate drift that occurs with Playwright's mouse.move() over long distances. + await world.currentWindow.mouse.move(startX, startY); + await world.currentWindow.mouse.down(); + await world.currentWindow.mouse.move(startX + 12, startY + 12, { steps: 6 }); + await world.currentWindow.waitForTimeout(200); + + const targetBox = await world.currentWindow.evaluate((selector: string) => { + const element = document.querySelector(selector); + if (!element) return null; + const rect = element.getBoundingClientRect(); + return { x: rect.left + rect.width / 2, y: rect.top + rect.height / 2 }; + }, scrollTargetSelector); + + if (!targetBox) { + throw new Error(`Could not find target element ${scrollTargetSelector} for synthetic drag`); + } + + // Dispatch pointermove directly at the target center to update dnd-kit's drag position + await world.currentWindow.evaluate(({ x, y }: { x: number; y: number }) => { + window.dispatchEvent( + new PointerEvent('pointermove', { + bubbles: true, + clientX: x, + clientY: y, + }), + ); + }, targetBox); + + await world.currentWindow.waitForTimeout(400); + await world.currentWindow.mouse.up(); + return; + } + + // Move to target with a smooth path, then re-track once in case the target shifted + await world.currentWindow.mouse.move(initialTargetCoordinates.targetX, initialTargetCoordinates.targetY, { steps: 12 }); + await world.currentWindow.waitForTimeout(200); const settledTargetCoordinates = await resolveTargetCoordinates(); - await world.currentWindow.mouse.move(settledTargetCoordinates.targetX, settledTargetCoordinates.targetY, { steps: 10 }); - await world.currentWindow.waitForTimeout(80); + await world.currentWindow.mouse.move(settledTargetCoordinates.targetX, settledTargetCoordinates.targetY, { steps: 5 }); + await world.currentWindow.waitForTimeout(600); + // Final live re-track: read the target's current position and teleport the + // mouse there immediately. This compensates for CSS transitions applied by + // dnd-kit's SortableContext which can shift the target after we last + // measured it. + const liveTargetCoordinates = await resolveTargetCoordinates(); + await world.currentWindow.mouse.move(liveTargetCoordinates.targetX, liveTargetCoordinates.targetY, { steps: 1 }); + // Dispatch a synthetic pointermove at the live target coordinates. + // dnd-kit reads clientX/clientY from pointer events; Playwright's discrete + // mouse.move steps can leave the internal pointer position behind if the + // target element has shifted due to SortableContext layout changes. + await world.currentWindow.evaluate(({ x, y }: { x: number; y: number }) => { + window.dispatchEvent( + new PointerEvent('pointermove', { + bubbles: true, + clientX: x, + clientY: y, + }), + ); + }, { x: liveTargetCoordinates.targetX, y: liveTargetCoordinates.targetY }); + await world.currentWindow.waitForTimeout(100); await world.currentWindow.mouse.up(); + + world.currentWindow.off('pageerror', onPageError); + world.currentWindow.off('console', onConsole); + if (pageErrors.length > 0 || consoleErrors.length > 0) { + throw new Error( + `Renderer crashed during drag with ${pageErrors.length} page error(s) and ${consoleErrors.length} console error(s): ` + + [...pageErrors, ...consoleErrors].join('; '), + ); + } } async function dragLocatorAndHoldAtCoordinates( @@ -160,6 +254,7 @@ async function dragLocatorAndHoldAtCoordinates( const sourceLocator = world.currentWindow.locator(sourceSelector); await sourceLocator.waitFor({ state: 'visible' }); + await sourceLocator.scrollIntoViewIfNeeded(); const sourceBox = await sourceLocator.boundingBox(); if (!sourceBox) { @@ -180,18 +275,47 @@ async function dragLocatorAndHoldAtCoordinates( } async function getLocatorCenter( + world: ApplicationWorld, targetSelector: string, - locator: { boundingBox: () => Promise<{ x: number; y: number; width: number; height: number } | null> }, + _locator: { boundingBox: () => Promise<{ x: number; y: number; width: number; height: number } | null> }, ): Promise<{ targetX: number; targetY: number }> { - const targetBox = await locator.boundingBox(); - if (!targetBox) { - throw new Error(`Could not read bounding box for ${targetSelector}`); + // Use Playwright locator.evaluate with retries. + // React may still be re-rendering after group creation, so the element can + // appear slightly after the parent workspace item. + if (!world.currentWindow) { + throw new Error('Current window not set'); } - return { - targetX: targetBox.x + targetBox.width / 2, - targetY: targetBox.y + targetBox.height / 2, - }; + for (let attempt = 0; attempt < 6; attempt++) { + try { + const rect = await world.currentWindow.locator(targetSelector).evaluate( + (element: Element) => { + const r = element.getBoundingClientRect(); + return { x: r.left, y: r.top, width: r.width, height: r.height }; + }, + undefined, + { timeout: 1500 }, + ); + return { + targetX: rect.x + rect.width / 2, + targetY: rect.y + rect.height / 2, + }; + } catch { + if (attempt === 5) { + // Diagnostic: list all workspace/group testids currently in the DOM + const testIds = await world.currentWindow.evaluate(() => { + const elements = document.querySelectorAll('[data-testid]'); + return Array.from(elements).map(element => element.getAttribute('data-testid')).filter(Boolean); + }); + throw new Error( + `Could not read bounding box for ${targetSelector}. Current DOM testids: ${testIds.join(', ')}`, + ); + } + await new Promise(resolve => setTimeout(resolve, 300)); + } + } + + throw new Error(`Could not read bounding box for ${targetSelector}`); } Given('workspace group {string} contains workspaces:', async function(this: ApplicationWorld, groupName: string, dataTable: DataTable) { @@ -200,16 +324,34 @@ Given('workspace group {string} contains workspaces:', async function(this: Appl for (const workspaceName of rows) { const workspace = await getWorkspaceByName(this, workspaceName); - await executeInMainWindow( - this, - ` - window.service.workspace.moveWorkspaceToGroup(${JSON.stringify(workspace.id)}, ${JSON.stringify(group.id)}) - `, - ); + await moveWorkspaceToGroup(this, workspace.id, group.id); await waitForWorkspaceGroupId(this, workspaceName, group.id); } await waitForGroupVisibility(this, group.id); + + // Wait for every workspace in the group to actually appear in the DOM + // so that subsequent drag steps can locate their drop zones. + for (const workspaceName of rows) { + const workspace = await getWorkspaceByName(this, workspaceName); + await backOff(async () => { + if (!this.currentWindow) { + throw new Error('Current window not set'); + } + const itemCount = await this.currentWindow.locator(`[data-testid="workspace-item-${workspace.id}"]`).count(); + if (itemCount === 0) { + throw new Error(`Workspace item "${workspaceName}" not yet rendered in DOM`); + } + const dropZoneCount = await this.currentWindow.locator(`[data-testid="workspace-drop-zone-${workspace.id}-top"]`).count(); + if (dropZoneCount === 0) { + throw new Error(`Workspace drop zone "${workspaceName}" not yet rendered in DOM`); + } + }, BACKOFF_OPTIONS); + } + + // Allow any deferred async side-effects (e.g. tidgi.config.json writes) + // to finish so that React state stabilises before the drag step starts. + await this.currentWindow?.waitForTimeout(3000); }); When('I drag workspace {string} onto workspace {string}', async function(this: ApplicationWorld, sourceWorkspaceName: string, targetWorkspaceName: string) { @@ -222,9 +364,12 @@ When('I drag workspace {string} onto workspace {string}', async function(this: A const targetSelector = `[data-testid="workspace-drop-zone-${targetWorkspace.id}-center"]`; const targetLocator = this.currentWindow.locator(targetSelector); await targetLocator.waitFor({ state: 'visible' }); - await dragLocatorToCoordinates(this, `[data-testid="workspace-item-${sourceWorkspace.id}"]`, async () => { - return getLocatorCenter(targetSelector, targetLocator); - }); + + await dragLocatorToCoordinates( + this, + `[data-testid="workspace-item-${sourceWorkspace.id}"]`, + async () => getLocatorCenter(this, targetSelector, targetLocator), + ); }); When('I hover workspace {string} over workspace {string}', async function(this: ApplicationWorld, sourceWorkspaceName: string, targetWorkspaceName: string) { @@ -238,7 +383,7 @@ When('I hover workspace {string} over workspace {string}', async function(this: const targetLocator = this.currentWindow.locator(targetSelector); await targetLocator.waitFor({ state: 'visible' }); await dragLocatorAndHoldAtCoordinates(this, `[data-testid="workspace-item-${sourceWorkspace.id}"]`, async () => { - return getLocatorCenter(targetSelector, targetLocator); + return getLocatorCenter(this, targetSelector, targetLocator); }); }); @@ -261,7 +406,7 @@ When('I drag workspace {string} to the top zone of workspace {string}', async fu const targetLocator = this.currentWindow.locator(targetSelector); await targetLocator.waitFor({ state: 'visible' }); await dragLocatorToCoordinates(this, `[data-testid="workspace-item-${sourceWorkspace.id}"]`, async () => { - return getLocatorCenter(targetSelector, targetLocator); + return getLocatorCenter(this, targetSelector, targetLocator); }); }); @@ -276,7 +421,7 @@ When('I drag workspace {string} to the bottom zone of workspace {string}', async const targetLocator = this.currentWindow.locator(targetSelector); await targetLocator.waitFor({ state: 'visible' }); await dragLocatorToCoordinates(this, `[data-testid="workspace-item-${sourceWorkspace.id}"]`, async () => { - return getLocatorCenter(targetSelector, targetLocator); + return getLocatorCenter(this, targetSelector, targetLocator); }); }); @@ -292,48 +437,18 @@ When('I drag workspace {string} onto the header of its current group', async fun const sourceSelector = `[data-testid="workspace-item-${workspace.id}"]`; const groupHeaderSelector = `[data-testid="workspace-group-${workspace.groupId}"]`; - const sourceLocator = this.currentWindow.locator(sourceSelector); - const groupHeaderLocator = this.currentWindow.locator(groupHeaderSelector); - await sourceLocator.waitFor({ state: 'visible' }); - await groupHeaderLocator.waitFor({ state: 'visible' }); - const sourceBox = await sourceLocator.boundingBox(); - if (!sourceBox) { - throw new Error(`Could not read bounding box for ${sourceSelector}`); + if (!this.currentWindow) { + throw new Error('Current window not set'); } + const targetLocator = this.currentWindow.locator(groupHeaderSelector); + await targetLocator.waitFor({ state: 'visible' }); - const startX = sourceBox.x + sourceBox.width / 2; - const startY = sourceBox.y + sourceBox.height / 2; - await this.currentWindow.mouse.move(startX, startY); - await this.currentWindow.mouse.down(); - await this.currentWindow.mouse.move(startX + 12, startY + 12, { steps: 6 }); - - const liveTargetCoordinates = await this.currentWindow.evaluate((selector: string) => { - const element = document.querySelector(selector); - if (!(element instanceof HTMLElement)) { - return null; - } - - const rect = element.getBoundingClientRect(); - return { - targetX: rect.x + rect.width / 2, - targetY: rect.y + rect.height / 2, - rectTop: rect.top, - rectBottom: rect.bottom, - rectLeft: rect.left, - rectRight: rect.right, - }; - }, groupHeaderSelector); - - if (!liveTargetCoordinates) { - throw new Error(`Could not read bounding box for ${groupHeaderSelector}`); - } - - // Teleport directly to the target to avoid intermediate mousemove events - // that can trigger React re-renders and shift the DOM before we arrive. - await this.currentWindow.mouse.move(liveTargetCoordinates.targetX, liveTargetCoordinates.targetY); - await this.currentWindow.waitForTimeout(100); - await this.currentWindow.mouse.up(); + await dragLocatorToCoordinates( + this, + sourceSelector, + async () => getLocatorCenter(this, groupHeaderSelector, targetLocator), + ); }); When('I remove workspace {string} from its group without auto-disband', async function(this: ApplicationWorld, workspaceName: string) { @@ -342,12 +457,7 @@ When('I remove workspace {string} from its group without auto-disband', async fu throw new Error(`Workspace "${workspaceName}" is not currently grouped`); } - await executeInMainWindow( - this, - ` - window.service.workspace.moveWorkspaceToGroup(${JSON.stringify(workspace.id)}, null, false) - `, - ); + await moveWorkspaceToGroup(this, workspace.id, null, false); }); Then('workspaces {string} and {string} should share a group', async function(this: ApplicationWorld, firstWorkspaceName: string, secondWorkspaceName: string) { @@ -479,12 +589,14 @@ When('I collapse workspace group {string}', async function(this: ApplicationWorl await executeInMainWindow( this, - ` - window.service.workspace.setGroup(${JSON.stringify(group.id)}, { ...${JSON.stringify(group)}, collapsed: true }) - `, + async (g: IWorkspaceGroup) => { + await window.service.workspace.setGroup(g.id, { ...g, collapsed: true }); + }, + group, ); - await this.currentWindow?.waitForTimeout(200); + // Wait for Collapse unmountOnExit to fully remove children from DOM + await this.currentWindow?.waitForTimeout(400); }); When('I expand workspace group {string}', async function(this: ApplicationWorld, groupName: string) { @@ -496,12 +608,17 @@ When('I expand workspace group {string}', async function(this: ApplicationWorld, await executeInMainWindow( this, - ` - window.service.workspace.setGroup(${JSON.stringify(group.id)}, { ...${JSON.stringify(group)}, collapsed: false }) - `, + async (g: IWorkspaceGroup) => { + await window.service.workspace.setGroup(g.id, { ...g, collapsed: false }); + }, + group, ); - await this.currentWindow?.waitForTimeout(200); + // Wait for the MUI Collapse animation to finish so that + // overflow:hidden no longer clips pointer events on child elements. + // timeout='auto' can take 300-500ms for small lists; 2000ms ensures completion + // even on slower CI runners. + await this.currentWindow?.waitForTimeout(2000); }); When('I drag group header {string} onto group header {string}', async function(this: ApplicationWorld, sourceGroupName: string, targetGroupName: string) { @@ -526,7 +643,7 @@ When('I drag group header {string} onto group header {string}', async function(t await targetLocator.waitFor({ state: 'visible' }); await dragLocatorToCoordinates(this, sourceSelector, async () => { - return getLocatorCenter(targetSelector, targetLocator); + return getLocatorCenter(this, targetSelector, targetLocator); }); }); diff --git a/src/pages/Main/WorkspaceIconAndSelector/SortableWorkspaceSelectorButton.tsx b/src/pages/Main/WorkspaceIconAndSelector/SortableWorkspaceSelectorButton.tsx index a89bad98..ad27c430 100644 --- a/src/pages/Main/WorkspaceIconAndSelector/SortableWorkspaceSelectorButton.tsx +++ b/src/pages/Main/WorkspaceIconAndSelector/SortableWorkspaceSelectorButton.tsx @@ -46,13 +46,19 @@ export function SortableWorkspaceSelectorButton({ index, workspace, showSidebarT const hibernated = isWiki ? workspace.hibernated : false; const transparentBackground = isWiki ? workspace.transparentBackground : false; + // Only pass groupId in data to keep the reference stable when workspaces$ + // emits new objects with identical groupId values. Passing the whole + // workspace object caused dnd-kit useSortable to re-register on every + // emission, triggering an infinite render loop. + const sortableData = useMemo(() => ({ type: 'workspace' as const, groupId: workspace.groupId }), [workspace.groupId]); const { attributes, listeners, setNodeRef, isDragging } = useSortable({ id, - data: { type: 'workspace', workspace }, + data: sortableData, }); const isDragOverTarget = dragContext.overId === id; const dragIntent = isDragOverTarget ? dragContext.intent : null; + const isAnyDragActive = dragContext.activeId !== null; const style = { transform: 'translate3d(0, 0, 0)', @@ -90,6 +96,10 @@ export function SortableWorkspaceSelectorButton({ index, workspace, showSidebarT }, [isMiniWindow, preference?.tidgiMiniWindowFixedWorkspaceId, id, active]); const onWorkspaceClick = useCallback(async () => { + if (isAnyDragActive) { + return; + } + workspaceClickedLoadingSetter(true); try { // Special "add" workspace always opens add workspace window @@ -121,7 +131,7 @@ export function SortableWorkspaceSelectorButton({ index, workspace, showSidebarT } finally { workspaceClickedLoadingSetter(false); } - }, [id, setLocation, workspace, isMiniWindow]); + }, [id, isAnyDragActive, isMiniWindow, setLocation, workspace]); const onWorkspaceContextMenu = useCallback( async (event: MouseEvent) => { event.preventDefault(); diff --git a/src/pages/Main/WorkspaceIconAndSelector/SortableWorkspaceSelectorList.tsx b/src/pages/Main/WorkspaceIconAndSelector/SortableWorkspaceSelectorList.tsx index 458ce304..10ba7aef 100644 --- a/src/pages/Main/WorkspaceIconAndSelector/SortableWorkspaceSelectorList.tsx +++ b/src/pages/Main/WorkspaceIconAndSelector/SortableWorkspaceSelectorList.tsx @@ -136,38 +136,6 @@ function getGroupInitial(name: string): string { return first.toUpperCase(); } -function getWorkspaceZoneIntent({ - activeRect, - canGroup, - overRect, - pointerY, -}: { - activeRect: { height: number; top: number } | null | undefined; - canGroup: boolean; - overRect: { height: number; top: number }; - pointerY: number | null | undefined; -}): Exclude { - const fallbackY = activeRect ? activeRect.top + activeRect.height / 2 : overRect.top + overRect.height / 2; - const resolvedPointerY = pointerY ?? fallbackY; - const relativeY = Math.min(Math.max(resolvedPointerY - overRect.top, 0), overRect.height); - const beforeBoundary = overRect.height / 4; - const afterBoundary = overRect.height - beforeBoundary; - - if (relativeY <= beforeBoundary) { - return 'reorder-before'; - } - - if (relativeY >= afterBoundary) { - return 'reorder-after'; - } - - if (canGroup) { - return 'group'; - } - - return relativeY < overRect.height / 2 ? 'reorder-before' : 'reorder-after'; -} - function getReorderTargetIndex({ listLength, oldIndex, @@ -190,14 +158,17 @@ function getReorderTargetIndex({ function SortableGroupHeader({ group, onToggleCollapse }: SortableGroupHeaderProps): React.JSX.Element { const { t } = useTranslation(); + // Keep data reference stable; only groupId is needed by collision detection. + const sortableData = useMemo(() => ({ type: 'group' as const, groupId: group.id }), [group.id]); const { attributes, listeners, setNodeRef, transform, transition, isDragging } = useSortable({ id: `group-${group.id}`, - data: { type: 'group', group }, + data: sortableData, }); const dragContext = useDragContext(); const isDragOverTarget = dragContext.overId === `group-${group.id}`; const dragIntent = isDragOverTarget ? dragContext.intent : null; + const isAnyDragActive = dragContext.activeId !== null; const style = { transform: CSS.Transform.toString(transform), @@ -229,6 +200,10 @@ function SortableGroupHeader({ group, onToggleCollapse }: SortableGroupHeaderPro $isDragging={isDragging} $dragIntent={dragIntent} onClick={() => { + if (isAnyDragActive) { + return; + } + onToggleCollapse(group.id); }} onContextMenu={handleContextMenu} @@ -274,6 +249,8 @@ export function SortableWorkspaceSelectorList({ workspacesList, showSideBarText, const pendingReorderReference = useRef(false); const dragStateReference = useRef(initialDragState); + const lastResolvedDragStateReference = useRef(initialDragState); + const dragStateTimeoutReference = useRef | null>(null); // Drag preview and drop behavior must resolve from the same projected state. const [dragState, setDragState] = useState(initialDragState); @@ -344,9 +321,12 @@ export function SortableWorkspaceSelectorList({ workspacesList, showSideBarText, const isDraggingWorkspace = !activeId.startsWith('group-'); if (isDraggingWorkspace && collisions.length > 0) { - const activeWorkspace = (arguments_.active.data.current as { workspace?: IWorkspaceWithMetadata } | undefined)?.workspace; - const ownGroupHeaderId = activeWorkspace?.groupId ? `group-${activeWorkspace.groupId}` : null; + const activeGroupId = (arguments_.active.data.current as { groupId?: string | null } | undefined)?.groupId; + const ownGroupHeaderId = activeGroupId ? `group-${activeGroupId}` : null; + const workspaceCollisions = collisions.filter((collision) => !String(collision.id).startsWith('group-')); + // When the pointer overlaps its own group header, that header must outrank + // nearby workspaces so the drop result matches the ungroup affordance. if (ownGroupHeaderId) { const ownGroupHeaderCollision = collisions.find((collision) => String(collision.id) === ownGroupHeaderId); @@ -356,18 +336,26 @@ export function SortableWorkspaceSelectorList({ workspacesList, showSideBarText, ...collisions.filter((collision) => String(collision.id) !== ownGroupHeaderId), ]; } + + // Pointer is not over own header; exclude group headers so the drop + // lands on a workspace instead. + return workspaceCollisions.length > 0 ? workspaceCollisions : collisions; } - if (activeWorkspace?.groupId) { - return collisions; - } - - const workspaceCollisions = collisions.filter((collision) => !String(collision.id).startsWith('group-')); + // Ungrouped workspace drag: filter out group headers entirely. if (workspaceCollisions.length > 0) { return workspaceCollisions; } } + if (!isDraggingWorkspace && collisions.length > 0) { + const groupCollisions = collisions.filter((collision) => String(collision.id).startsWith('group-')); + + if (groupCollisions.length > 0) { + return groupCollisions; + } + } + return collisions; }, []); @@ -436,15 +424,33 @@ export function SortableWorkspaceSelectorList({ workspacesList, showSideBarText, } }, [applyDragState, workspacesList, groups]); + // Keep items stable during drag by deriving from canonical order only. + // Visual reordering is handled by displayedWorkspaces/displayedGroups; + // SortableContext items should not change during drag to avoid dnd-kit + // re-registration loops. See https://github.com/clauderic/dnd-kit/issues/900 const allDraggableIds = useMemo(() => { const ids: string[] = []; - ungroupedWorkspaces.forEach(w => ids.push(w.id)); - displayedGroups.forEach(group => { + const grouped: Record = {}; + + canonicalWorkspaces.forEach(workspace => { + if (workspace.groupId) { + if (!grouped[workspace.groupId]) { + grouped[workspace.groupId] = []; + } + grouped[workspace.groupId].push(workspace); + } else { + ids.push(workspace.id); + } + }); + + canonicalGroups.forEach(group => { ids.push(`group-${group.id}`); - (groupedWorkspaces[group.id] || []).forEach(w => ids.push(w.id)); + if (!group.collapsed) { + (grouped[group.id] || []).forEach(w => ids.push(w.id)); + } }); return ids; - }, [ungroupedWorkspaces, displayedGroups, groupedWorkspaces]); + }, [canonicalWorkspaces, canonicalGroups]); const handleToggleCollapse = useCallback(async (groupId: string) => { const group = groups?.find(g => g.id === groupId); @@ -478,9 +484,40 @@ export function SortableWorkspaceSelectorList({ workspacesList, showSideBarText, return arrayMove(canonicalWorkspaces, oldIndex, targetIndex).map(workspace => workspace.id); }, [canonicalWorkspaces]); + const computeGroupProjection = useCallback((activeGroupId: string, overGroupId: string, intent: TDragIntent): string[] | null => { + if (intent !== 'reorder-before' && intent !== 'reorder-after') { + return null; + } + + const oldIndex = canonicalGroups.findIndex(group => group.id === activeGroupId); + const overIndex = canonicalGroups.findIndex(group => group.id === overGroupId); + + if (oldIndex === -1 || overIndex === -1) { + return null; + } + + const targetIndex = getReorderTargetIndex({ + listLength: canonicalGroups.length, + oldIndex, + overIndex, + placement: intent === 'reorder-after' ? 'after' : 'before', + }); + + return arrayMove(canonicalGroups, oldIndex, targetIndex).map(group => group.id); + }, [canonicalGroups]); + + const clearDragStateTimeout = useCallback(() => { + if (dragStateTimeoutReference.current !== null) { + clearTimeout(dragStateTimeoutReference.current); + dragStateTimeoutReference.current = null; + } + }, []); + const resetDragState = useCallback(() => { + clearDragStateTimeout(); + lastResolvedDragStateReference.current = initialDragState; applyDragState(initialDragState); - }, [applyDragState]); + }, [applyDragState, clearDragStateTimeout]); const reorderWorkspaces = useCallback(async (activeId: string, overId: string, placement: 'before' | 'after' = 'before') => { const oldIndex = canonicalWorkspaces.findIndex(w => w.id === activeId); @@ -508,16 +545,25 @@ export function SortableWorkspaceSelectorList({ workspacesList, showSideBarText, await window.service.workspace.setWorkspaces(newWorkspaces); }, [canonicalWorkspaces]); + const createGroupWithWorkspaces = useCallback(async (workspaceIds: string[]) => { + const newGroupId = `group-${Date.now()}`; + const newGroup: IWorkspaceGroup = { + id: newGroupId, + name: t('WorkspaceGroup.DefaultGroupName', { number: canonicalGroups.length + 1 }), + collapsed: false, + order: canonicalGroups.length, + }; + + await window.service.workspace.setGroup(newGroupId, newGroup); + + for (const workspaceId of workspaceIds) { + await window.service.workspace.moveWorkspaceToGroup(workspaceId, newGroupId); + } + }, [canonicalGroups.length, t]); + const deriveDragState = useCallback((event: Pick): IDragState => { const { active, over } = event; const activeId = String(active.id); - const translatedRect = active.rect.current.translated; - const initialRect = active.rect.current.initial; - const pointerY = initialRect - ? initialRect.top + initialRect.height / 2 + event.delta.y - : translatedRect - ? translatedRect.top + translatedRect.height / 2 - : undefined; const overData = over?.data.current as { type?: string } | undefined; const effectiveOverId = over ? String(over.id) : null; const effectiveOverType = overData?.type; @@ -555,14 +601,28 @@ export function SortableWorkspaceSelectorList({ workspacesList, showSideBarText, } const isSameGroup = activeWorkspace?.groupId && overWorkspace?.groupId && activeWorkspace.groupId === overWorkspace.groupId; - const intent = overRect.height > 0 - ? getWorkspaceZoneIntent({ - activeRect, - canGroup: !isSameGroup && isGroupableWorkspace(activeWorkspace) && isGroupableWorkspace(overWorkspace), - overRect, - pointerY, - }) - : 'reorder-before'; + const canGroup = !isSameGroup && isGroupableWorkspace(activeWorkspace) && isGroupableWorkspace(overWorkspace); + // Use the active item's translated rect centre as the reference point. + // Both activeRect and overRect are measured by dnd-kit at the same moment, + // so their relative positions are stable even when SortableContext shifts + // items during the drag. + const activeCenterY = activeRect + ? activeRect.top + activeRect.height / 2 + : overRect.top + overRect.height / 2; + const relativeY = Math.min(Math.max(activeCenterY - overRect.top, 0), overRect.height); + const beforeBoundary = overRect.height / 4; + const afterBoundary = overRect.height - beforeBoundary; + let intent: TDragIntent; + + if (relativeY <= beforeBoundary) { + intent = 'reorder-before'; + } else if (relativeY >= afterBoundary) { + intent = 'reorder-after'; + } else if (canGroup) { + intent = 'group'; + } else { + intent = relativeY < overRect.height / 2 ? 'reorder-before' : 'reorder-after'; + } return { intent, @@ -579,17 +639,13 @@ export function SortableWorkspaceSelectorList({ workspacesList, showSideBarText, const overId = effectiveOverId; const activeGroupId = activeId.replace('group-', ''); const overGroupId = overId.replace('group-', ''); - const oldIndex = canonicalGroups.findIndex(group => group.id === activeGroupId); - const overIndex = canonicalGroups.findIndex(group => group.id === overGroupId); return { intent: 'reorder-before', overId, activeId, projectedWorkspaceOrder: null, - projectedGroupOrder: oldIndex === -1 || overIndex === -1 - ? null - : arrayMove(canonicalGroups, oldIndex, overIndex).map(group => group.id), + projectedGroupOrder: computeGroupProjection(activeGroupId, overGroupId, 'reorder-before'), }; } @@ -618,10 +674,39 @@ export function SortableWorkspaceSelectorList({ workspacesList, showSideBarText, projectedWorkspaceOrder: null, projectedGroupOrder: null, }; - }, [canonicalGroups, canonicalWorkspaces, computeWorkspaceProjection]); + }, [canonicalWorkspaces, computeGroupProjection, computeWorkspaceProjection]); const updateDragStateFromEvent = useCallback((event: DragMoveEvent | DragOverEvent) => { - applyDragState(deriveDragState(event)); + const nextDragState = deriveDragState(event); + + // Only cache group/ungroup intents that are not sensitive to minor pointer drift. + // Reorder intents (before/after) depend on exact pointer position within the target rect, + // so caching them can cause handleDragEnd to use a stale intent when the pointer + // briefly crossed a boundary during smooth mouse movement. + if ( + nextDragState.activeId !== null && + nextDragState.overId !== null && + (nextDragState.intent === 'group' || nextDragState.intent === 'ungroup') + ) { + lastResolvedDragStateReference.current = nextDragState; + } + + // Do NOT update dragStateReference.current here. + // applyDragState updates it when setDragState actually fires, so the equality + // check inside applyDragState works correctly. If we updated the ref early, + // the debounced applyDragState would see the same state and skip the render, + // breaking visual feedback (drag intent) during drag. + + // Debounce the React state update to prevent "Maximum update depth exceeded" + // when rapid onDragMove/onDragOver events fire in quick succession. + // See https://github.com/clauderic/dnd-kit/issues/900 + if (dragStateTimeoutReference.current !== null) { + clearTimeout(dragStateTimeoutReference.current); + } + dragStateTimeoutReference.current = setTimeout(() => { + dragStateTimeoutReference.current = null; + applyDragState(nextDragState); + }, 0); }, [applyDragState, deriveDragState]); const handleDragMove = useCallback((event: DragMoveEvent) => { @@ -633,10 +718,12 @@ export function SortableWorkspaceSelectorList({ workspacesList, showSideBarText, }, [updateDragStateFromEvent]); const handleDragStart = useCallback((event: DragStartEvent) => { + clearDragStateTimeout(); + lastResolvedDragStateReference.current = initialDragState; applyDragState(previous => ({ ...previous, activeId: String(event.active.id) })); - }, [applyDragState]); + }, [applyDragState, clearDragStateTimeout]); - const handleDragCancel = useCallback((_event: DragCancelEvent) => { + const handleDragCancel = useCallback(async (_event: DragCancelEvent) => { resetDragState(); }, [resetDragState]); @@ -644,18 +731,31 @@ export function SortableWorkspaceSelectorList({ workspacesList, showSideBarText, const { active } = event; const activeId = String(active.id); const previewDragState = dragStateReference.current; + const lastResolvedDragState = lastResolvedDragStateReference.current; const shouldUsePreviewDragState = previewDragState.activeId === activeId && ( previewDragState.overId !== null || previewDragState.intent !== null || previewDragState.projectedWorkspaceOrder !== null || previewDragState.projectedGroupOrder !== null ); - const currentDragState = shouldUsePreviewDragState ? previewDragState : deriveDragState(event); + const shouldUseLastResolvedDragState = lastResolvedDragState.activeId === activeId && ( + lastResolvedDragState.overId !== null || + lastResolvedDragState.intent !== null || + lastResolvedDragState.projectedWorkspaceOrder !== null || + lastResolvedDragState.projectedGroupOrder !== null + ); + const currentDragState = shouldUsePreviewDragState + ? previewDragState + : shouldUseLastResolvedDragState + ? lastResolvedDragState + : deriveDragState(event); dragStateReference.current = currentDragState; resetDragState(); const { intent: currentIntent, overId: currentOverId } = currentDragState; - if (!currentIntent || !currentOverId || activeId === currentOverId) return; + if (!currentIntent || !currentOverId || activeId === currentOverId) { + return; + } const overId = currentOverId; const resolvedOverType = overId.startsWith('group-') ? 'group' : 'workspace'; @@ -666,11 +766,20 @@ export function SortableWorkspaceSelectorList({ workspacesList, showSideBarText, const overGroupId = overId.replace('group-', ''); const oldIndex = canonicalGroups.findIndex(g => g.id === activeGroupId); - const newIndex = canonicalGroups.findIndex(g => g.id === overGroupId); + const overIndex = canonicalGroups.findIndex(g => g.id === overGroupId); - if (oldIndex === -1 || newIndex === -1) return; + if (oldIndex === -1 || overIndex === -1) return; - const reorderedGroups = arrayMove(canonicalGroups, oldIndex, newIndex); + const targetIndex = getReorderTargetIndex({ + listLength: canonicalGroups.length, + oldIndex, + overIndex, + placement: currentIntent === 'reorder-after' ? 'after' : 'before', + }); + + if (targetIndex === oldIndex) return; + + const reorderedGroups = arrayMove(canonicalGroups, oldIndex, targetIndex); pendingReorderReference.current = true; await Promise.all( @@ -713,9 +822,9 @@ export function SortableWorkspaceSelectorList({ workspacesList, showSideBarText, // Different contexts with 'group' intent if (currentIntent === 'group') { - // From grouped to ungrouped → remove from group + // From grouped to ungrouped → create a dedicated group with the hovered workspace if (activeWorkspace.groupId && !overWorkspace.groupId) { - await window.service.workspace.moveWorkspaceToGroup(activeId, null); + await createGroupWithWorkspaces([activeId, overId]); return; } @@ -733,17 +842,7 @@ export function SortableWorkspaceSelectorList({ workspacesList, showSideBarText, // Both ungrouped → create new group if (!activeWorkspace.groupId && !overWorkspace.groupId) { - const newGroupId = `group-${Date.now()}`; - const newGroup: IWorkspaceGroup = { - id: newGroupId, - name: t('WorkspaceGroup.DefaultGroupName', { number: canonicalGroups.length + 1 }), - collapsed: false, - order: canonicalGroups.length, - }; - - await window.service.workspace.setGroup(newGroupId, newGroup); - await window.service.workspace.moveWorkspaceToGroup(activeId, newGroupId); - await window.service.workspace.moveWorkspaceToGroup(overId, newGroupId); + await createGroupWithWorkspaces([activeId, overId]); return; } } @@ -751,7 +850,7 @@ export function SortableWorkspaceSelectorList({ workspacesList, showSideBarText, await reorderWorkspaces(activeId, overId, currentIntent === 'reorder-after' ? 'after' : 'before'); return; } - }, [canonicalGroups, canonicalWorkspaces, deriveDragState, reorderWorkspaces, resetDragState, t]); + }, [canonicalGroups, canonicalWorkspaces, createGroupWithWorkspaces, deriveDragState, reorderWorkspaces, resetDragState]); const activeWorkspace = dragState.activeId && !dragState.activeId.startsWith('group-') ? canonicalWorkspaces.find(w => w.id === dragState.activeId) @@ -792,7 +891,6 @@ export function SortableWorkspaceSelectorList({ workspacesList, showSideBarText, {/* Groups with their workspaces — flat structure in SortableContext */} {displayedGroups.map(group => { const workspacesInGroup = groupedWorkspaces[group.id] || []; - if (workspacesInGroup.length === 0) return null; return ( diff --git a/src/services/workspaces/index.ts b/src/services/workspaces/index.ts index 7f27e454..cc2835bb 100644 --- a/src/services/workspaces/index.ts +++ b/src/services/workspaces/index.ts @@ -51,6 +51,8 @@ export class Workspace implements IWorkspaceService { await registerMenu(); } + private previousWorkspacesWithMetadata: IWorkspacesWithMetadata | undefined; + public getWorkspacesWithMetadata(): IWorkspacesWithMetadata { return mapValues(this.getWorkspacesSync(), (workspace: IWorkspace, id): IWorkspaceWithMetadata => { // Only wiki workspaces can have metadata, dedicated workspaces are filtered out @@ -62,7 +64,14 @@ export class Workspace implements IWorkspaceService { } public updateWorkspaceSubject(): void { - this.workspaces$.next(this.getWorkspacesWithMetadata()); + const next = this.getWorkspacesWithMetadata(); + // Skip emission when nothing actually changed to break infinite render loops + // caused by unstable object references in renderer-side dnd-kit hooks. + if (this.previousWorkspacesWithMetadata !== undefined && isEqual(this.previousWorkspacesWithMetadata, next)) { + return; + } + this.previousWorkspacesWithMetadata = next; + this.workspaces$.next(next); // Also initialize groups observable this.getGroupsSync(); } @@ -532,21 +541,14 @@ export class Workspace implements IWorkspaceService { /** * Compute the order for a newly created wiki workspace so it appears at - * the TOP of the regular-workspace section (before page workspaces). - * Shifts all existing non-page workspaces down by 1 to make room. + * the BOTTOM of the regular-workspace section (after existing page workspaces). */ private async getNextInsertOrder(): Promise { const all = await this.getWorkspacesAsList(); const regularWorkspaces = all.filter(w => !w.pageType); if (regularWorkspaces.length === 0) return 0; - const minOrder = Math.min(...regularWorkspaces.map(w => w.order)); - // Shift every existing workspace's order up by 1 - for (const ws of all) { - if (ws.order >= minOrder) { - await this.set(ws.id, { ...ws, order: ws.order + 1 }); - } - } - return minOrder; + const maxOrder = Math.max(...regularWorkspaces.map(w => w.order)); + return maxOrder + 1; } public async create(newWorkspaceConfig: INewWikiWorkspaceConfig): Promise { @@ -740,6 +742,22 @@ export class Workspace implements IWorkspaceService { // Workspace group methods private groups: Record | undefined; public groups$ = new BehaviorSubject | undefined>(undefined); + private previousGroups: Record | undefined; + + private emitGroups(next: Record | undefined): void { + // Always emit when the reference is identical so that in-place mutations + // (e.g. groups[id] = group) are not swallowed. Only skip when the + // reference differs but the deep content is the same. + if (next !== undefined && this.previousGroups === next) { + this.groups$.next(next); + return; + } + if (this.previousGroups !== undefined && next !== undefined && isEqual(this.previousGroups, next)) { + return; + } + this.previousGroups = next; + this.groups$.next(next); + } private getGroupsSync(): Record { if (this.groups === undefined) { @@ -751,7 +769,7 @@ export class Workspace implements IWorkspaceService { this.groups = {}; } // Initialize the observable with current groups - this.groups$.next(this.groups); + this.emitGroups(this.groups); } return this.groups; } @@ -776,7 +794,7 @@ export class Workspace implements IWorkspaceService { const databaseService = container.get(serviceIdentifier.Database); databaseService.setSetting('workspaceGroups', groups); this.groups = groups; - this.groups$.next(groups); + this.emitGroups(groups); } public async removeGroup(id: string): Promise { @@ -785,7 +803,7 @@ export class Workspace implements IWorkspaceService { const databaseService = container.get(serviceIdentifier.Database); databaseService.setSetting('workspaceGroups', groups); this.groups = groups; - this.groups$.next(groups); + this.emitGroups(groups); // Move workspaces in this group to ungrouped const workspaces = this.getWorkspacesSync();