fix: correct positioning when dragging groups from popout to main window

Fixes issue #958 where groups dragged from popout windows would be restored
to their original ghost position instead of the actual drop target.

Changes:
- Detect cross-window moves from popout to grid locations
- Prevent automatic restoration to reference group during disposal
- Clean up hidden reference groups when moving to new positions
- Ensure proper positioning at actual drop target
- Add comprehensive tests for cross-window drag positioning

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
mathuo 2025-07-17 22:28:12 +01:00
parent 23a5d84020
commit 05196fd86a
No known key found for this signature in database
GPG Key ID: C6EEDEFD6CA07281
2 changed files with 189 additions and 8 deletions

View File

@ -421,6 +421,153 @@ describe('dockviewComponent', () => {
expect(query.length).toBe(3);
});
test('that moving a popout group to specific position works correctly', async () => {
window.open = () => setupMockWindow();
dockview = new DockviewComponent(container, {
createComponent(options) {
switch (options.name) {
case 'default':
return new PanelContentPartTest(
options.id,
options.name
);
default:
throw new Error(`unsupported`);
}
},
});
dockview.layout(600, 1000);
const panel1 = dockview.addPanel({
id: 'panel1',
component: 'default',
});
const panel2 = dockview.addPanel({
id: 'panel2',
component: 'default',
position: { direction: 'right' },
});
await dockview.addPopoutGroup(panel1.api.group);
expect(panel1.api.location.type).toBe('popout');
expect(dockview.groups.length).toBe(3); // panel2 + hidden reference + popout
// Move popout group to left of panel2
panel1.api.group.api.moveTo({
group: panel2.api.group,
position: 'left',
});
// Core assertions: should be back in grid and positioned correctly
expect(panel1.api.location.type).toBe('grid');
expect(dockview.groups.length).toBe(2); // Should clean up properly
expect(dockview.panels.length).toBe(2);
// Verify both panels are visible and accessible
expect(panel1.api.isVisible).toBe(true);
expect(panel2.api.isVisible).toBe(true);
});
test('that moving a popout group to different positions works', async () => {
window.open = () => setupMockWindow();
dockview = new DockviewComponent(container, {
createComponent(options) {
switch (options.name) {
case 'default':
return new PanelContentPartTest(
options.id,
options.name
);
default:
throw new Error(`unsupported`);
}
},
});
dockview.layout(600, 1000);
const panel1 = dockview.addPanel({
id: 'panel1',
component: 'default',
});
const panel2 = dockview.addPanel({
id: 'panel2',
component: 'default',
position: { direction: 'right' },
});
await dockview.addPopoutGroup(panel1.api.group);
expect(panel1.api.location.type).toBe('popout');
// Test moving to different positions
['top', 'bottom', 'left', 'right'].forEach(position => {
panel1.api.group.api.moveTo({
group: panel2.api.group,
position: position as any,
});
// Should be back in grid and work correctly regardless of position
expect(panel1.api.location.type).toBe('grid');
expect(panel1.api.isVisible).toBe(true);
expect(panel2.api.isVisible).toBe(true);
expect(dockview.groups.length).toBeGreaterThanOrEqual(2);
expect(dockview.panels.length).toBe(2);
});
});
test('that reference group cleanup works when moving popout to new position', async () => {
window.open = () => setupMockWindow();
dockview = new DockviewComponent(container, {
createComponent(options) {
switch (options.name) {
case 'default':
return new PanelContentPartTest(
options.id,
options.name
);
default:
throw new Error(`unsupported`);
}
},
});
dockview.layout(600, 1000);
const panel1 = dockview.addPanel({
id: 'panel1',
component: 'default',
});
const panel2 = dockview.addPanel({
id: 'panel2',
component: 'default',
position: { direction: 'right' },
});
// Store reference group ID before popout
const originalGroupId = panel1.group.id;
await dockview.addPopoutGroup(panel1.api.group);
expect(panel1.api.location.type).toBe('popout');
expect(dockview.groups.length).toBe(3); // panel2 + hidden reference + popout
// Move to new position - should clean up reference group
panel1.api.group.api.moveTo({
group: panel2.api.group,
position: 'right',
});
expect(panel1.api.location.type).toBe('grid');
expect(dockview.groups.length).toBe(2); // Just panel2 + panel1 in new position
// Reference group should be cleaned up (no longer exist)
const referenceGroupStillExists = dockview.groups.some(g => g.id === originalGroupId);
expect(referenceGroupStillExists).toBe(false);
});
test('horizontal', () => {
dockview = new DockviewComponent(container, {
createComponent(options) {

View File

@ -2367,11 +2367,42 @@ export class DockviewComponent
if (!selectedPopoutGroup) {
throw new Error('failed to find popout group');
}
selectedPopoutGroup.disposable.dispose();
// For cross-window moves, we need to prevent the automatic restoration
// to the reference group that happens in the disposal cleanup
const isMovingToNewTarget = to.api.location.type === 'grid';
if (isMovingToNewTarget) {
// For cross-window moves to new positions, we need to clean up properly
// Remove from popout groups list to prevent automatic restoration
const index = this._popoutGroups.indexOf(selectedPopoutGroup);
if (index >= 0) {
this._popoutGroups.splice(index, 1);
}
// Clean up the reference group (ghost) if it exists and is hidden
if (selectedPopoutGroup.referenceGroup) {
const referenceGroup = this.getPanel(selectedPopoutGroup.referenceGroup);
if (referenceGroup && !referenceGroup.api.isVisible) {
this.doRemoveGroup(referenceGroup, { skipActive: true });
}
}
if (from.api.location.type !== 'popout') {
// Manually dispose the window without triggering restoration
selectedPopoutGroup.window.dispose();
// Update group's location and containers for main window
from.model.renderContainer = this.overlayRenderContainer;
from.model.dropTargetContainer = this.rootDropTargetContainer;
from.model.location = { type: 'grid' };
} else {
// Normal disposal that will restore to reference group
selectedPopoutGroup.disposable.dispose();
}
}
}
// For moves to grid locations (including cross-window moves from popout)
const referenceLocation = getGridLocation(to.element);
const dropLocation = getRelativeLocation(
this.gridview.orientation,
@ -2379,6 +2410,9 @@ export class DockviewComponent
target
);
// Add to grid for moves from non-popout sources, or for cross-window moves from popout
if (from.api.location.type !== 'popout' || to.api.location.type === 'grid') {
let size: number;
switch (this.gridview.orientation) {