Merge pull request #798 from mathuo/797-popout-group-flow-error

bug: fixup popout group flows

packages/dockview-core/src/__tests__/dockview/dockviewComponent.spec.ts

@@ -140,109 +140,114 @@ describe('dockviewComponent', () => {
         expect(dockview.element.className).toBe('test-b test-c');
     });
 
-    // describe('memory leakage', () => {
+    describe('memory leakage', () => {
-    //     beforeEach(() => {
+        beforeEach(() => {
-    //         window.open = () => fromPartial<Window>({
+            window.open = () => setupMockWindow();
-    //             addEventListener: jest.fn(),
+        });
-    //             close: jest.fn(),
-    //         });
-    //     });
 
-    //     test('event leakage', () => {
+        test('event leakage', async () => {
-    //         Emitter.setLeakageMonitorEnabled(true);
+            Emitter.setLeakageMonitorEnabled(true);
 
-    //         dockview = new DockviewComponent({
+            dockview = new DockviewComponent(container, {
-    //             parentElement: container,
+                createComponent(options) {
-    //             components: {
+                    switch (options.name) {
-    //                 default: PanelContentPartTest,
+                        case 'default':
-    //             },
+                            return new PanelContentPartTest(
-    //         });
+                                options.id,
+                                options.name
+                            );
+                        default:
+                            throw new Error(`unsupported`);
+                    }
+                },
+                className: 'test-a test-b',
+            });
 
-    //         dockview.layout(500, 1000);
+            dockview.layout(500, 1000);
 
-    //         const panel1 = dockview.addPanel({
+            const panel1 = dockview.addPanel({
-    //             id: 'panel1',
+                id: 'panel1',
-    //             component: 'default',
+                component: 'default',
-    //         });
+            });
 
-    //         const panel2 = dockview.addPanel({
+            const panel2 = dockview.addPanel({
-    //             id: 'panel2',
+                id: 'panel2',
-    //             component: 'default',
+                component: 'default',
-    //         });
+            });
 
-    //         dockview.removePanel(panel2);
+            dockview.removePanel(panel2);
 
-    //         const panel3 = dockview.addPanel({
+            const panel3 = dockview.addPanel({
-    //             id: 'panel3',
+                id: 'panel3',
-    //             component: 'default',
+                component: 'default',
-    //             position: {
+                position: {
-    //                 direction: 'right',
+                    direction: 'right',
-    //                 referencePanel: 'panel1',
+                    referencePanel: 'panel1',
-    //             },
+                },
-    //         });
+            });
 
-    //         const panel4 = dockview.addPanel({
+            const panel4 = dockview.addPanel({
-    //             id: 'panel4',
+                id: 'panel4',
-    //             component: 'default',
+                component: 'default',
-    //             position: {
+                position: {
-    //                 direction: 'above',
+                    direction: 'above',
-    //             },
+                },
-    //         });
+            });
 
-    //         dockview.moveGroupOrPanel(
+            panel4.api.group.api.moveTo({
-    //             panel4.group,
+                group: panel3.api.group,
-    //             panel3.group.id,
+                position: 'center',
-    //             panel3.id,
+            });
-    //             'center'
-    //         );
 
-    //         dockview.addPanel({
+            dockview.addPanel({
-    //             id: 'panel5',
+                id: 'panel5',
-    //             component: 'default',
+                component: 'default',
-    //             floating: true,
+                floating: true,
-    //         });
+            });
 
-    //         const panel6 = dockview.addPanel({
+            const panel6 = dockview.addPanel({
-    //             id: 'panel6',
+                id: 'panel6',
-    //             component: 'default',
+                component: 'default',
-    //             position: {
+                position: {
-    //                 referencePanel: 'panel5',
+                    referencePanel: 'panel5',
-    //                 direction: 'within',
+                    direction: 'within',
-    //             },
+                },
-    //         });
+            });
 
-    //         dockview.addFloatingGroup(panel4.api.group);
+            dockview.addFloatingGroup(panel4.api.group);
 
-    //         dockview.addPopoutGroup(panel6);
+            await dockview.addPopoutGroup(panel2);
 
-    //         dockview.moveGroupOrPanel(
+            panel1.api.group.api.moveTo({
-    //             panel1.group,
+                group: panel6.api.group,
-    //             panel6.group.id,
+                position: 'center',
-    //             panel6.id,
+            });
-    //             'center'
-    //         );
 
-    //         dockview.moveGroupOrPanel(
+            panel4.api.group.api.moveTo({
-    //             panel4.group,
+                group: panel6.api.group,
-    //             panel6.group.id,
+                position: 'center',
-    //             panel6.id,
+            });
-    //             'center'
-    //         );
 
-    //         dockview.dispose();
+            dockview.dispose();
 
-    //         if (Emitter.MEMORY_LEAK_WATCHER.size > 0) {
+            if (Emitter.MEMORY_LEAK_WATCHER.size > 0) {
-    //             for (const entry of Array.from(
+                console.warn(
-    //                 Emitter.MEMORY_LEAK_WATCHER.events
+                    `${Emitter.MEMORY_LEAK_WATCHER.size} undisposed resources`
-    //             )) {
+                );
-    //                 console.log('disposal', entry[1]);
-    //             }
-    //             throw new Error('not all listeners disposed');
-    //         }
 
-    //         Emitter.setLeakageMonitorEnabled(false);
+                for (const entry of Array.from(
-    //     });
+                    Emitter.MEMORY_LEAK_WATCHER.events
-    // });
+                )) {
+                    console.log('disposal', entry[1]);
+                }
+                throw new Error(
+                    `${Emitter.MEMORY_LEAK_WATCHER.size} undisposed resources`
+                );
+            }
+
+            Emitter.setLeakageMonitorEnabled(false);
+        });
+    });
 
     test('duplicate panel', () => {
         dockview.layout(500, 1000);
@@ -3707,16 +3712,16 @@ describe('dockviewComponent', () => {
                 floatingGroups: [
                     {
                         data: {
-                            views: ['panelB'],
+                            views: ['panelC'],
-                            activeView: 'panelB',
+                            activeView: 'panelC',
                             id: '3',
                         },
                         position: { left: 0, top: 0, height: 100, width: 100 },
                     },
                     {
                         data: {
-                            views: ['panelC'],
+                            views: ['panelD'],
-                            activeView: 'panelC',
+                            activeView: 'panelD',
                             id: '4',
                         },
                         position: { left: 0, top: 0, height: 100, width: 100 },
@@ -4867,6 +4872,155 @@ describe('dockviewComponent', () => {
             );
         });
 
+        test('basic', async () => {
+            jest.useRealTimers();
+
+            const container = document.createElement('div');
+
+            window.open = () => setupMockWindow();
+
+            const 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(1000, 1000);
+
+            dockview.fromJSON({
+                activeGroup: 'group-1',
+                grid: {
+                    root: {
+                        type: 'branch',
+                        data: [
+                            {
+                                type: 'leaf',
+                                data: {
+                                    views: ['panel1'],
+                                    id: 'group-1',
+                                    activeView: 'panel1',
+                                },
+                                size: 1000,
+                            },
+                        ],
+                        size: 1000,
+                    },
+                    height: 1000,
+                    width: 1000,
+                    orientation: Orientation.VERTICAL,
+                },
+                popoutGroups: [
+                    {
+                        data: {
+                            views: ['panel2'],
+                            id: 'group-2',
+                            activeView: 'panel2',
+                        },
+                        position: null,
+                    },
+                ],
+                panels: {
+                    panel1: {
+                        id: 'panel1',
+                        contentComponent: 'default',
+                        title: 'panel1',
+                    },
+                    panel2: {
+                        id: 'panel2',
+                        contentComponent: 'default',
+                        title: 'panel2',
+                    },
+                },
+            });
+
+            await new Promise((resolve) => setTimeout(resolve, 0));
+
+            const panel2 = dockview.api.getPanel('panel2');
+
+            const windowObject =
+                panel2?.api.location.type === 'popout'
+                    ? panel2?.api.location.getWindow()
+                    : undefined;
+
+            expect(windowObject).toBeTruthy();
+
+            windowObject!.close();
+        });
+
+        test('grid -> floating -> popout -> popout closed', async () => {
+            const container = document.createElement('div');
+
+            window.open = () => setupMockWindow();
+
+            const 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(1000, 500);
+
+            const panel1 = dockview.addPanel({
+                id: 'panel_1',
+                component: 'default',
+            });
+
+            const panel2 = dockview.addPanel({
+                id: 'panel_2',
+                component: 'default',
+            });
+
+            const panel3 = dockview.addPanel({
+                id: 'panel_3',
+                component: 'default',
+                position: { direction: 'right' },
+            });
+
+            expect(panel1.api.location.type).toBe('grid');
+            expect(panel2.api.location.type).toBe('grid');
+            expect(panel3.api.location.type).toBe('grid');
+
+            dockview.addFloatingGroup(panel2);
+
+            expect(panel1.api.location.type).toBe('grid');
+            expect(panel2.api.location.type).toBe('floating');
+            expect(panel3.api.location.type).toBe('grid');
+
+            await dockview.addPopoutGroup(panel2);
+
+            expect(panel1.api.location.type).toBe('grid');
+            expect(panel2.api.location.type).toBe('popout');
+            expect(panel3.api.location.type).toBe('grid');
+
+            const windowObject =
+                panel2.api.location.type === 'popout'
+                    ? panel2.api.location.getWindow()
+                    : undefined;
+            expect(windowObject).toBeTruthy();
+
+            windowObject!.close();
+
+            expect(panel1.api.location.type).toBe('grid');
+            expect(panel2.api.location.type).toBe('floating');
+            expect(panel3.api.location.type).toBe('grid');
+        });
+
         test('move popout group of 1 panel inside grid', async () => {
             const container = document.createElement('div');
 
@@ -5116,7 +5270,7 @@ describe('dockviewComponent', () => {
             mockWindow.close();
 
             expect(panel1.group.api.location.type).toBe('grid');
-            expect(panel2.group.api.location.type).toBe('grid');
+            expect(panel2.group.api.location.type).toBe('floating');
             expect(panel3.group.api.location.type).toBe('grid');
 
             dockview.clear();

packages/dockview-core/src/dockview/dockviewComponent.ts

@@ -73,6 +73,7 @@ import {
     OverlayRenderContainer,
 } from '../overlay/overlayRenderContainer';
 import { PopoutWindow } from '../popoutWindow';
+import { StrictEventsSequencing } from './strictEventsSequencing';
 
 const DEFAULT_ROOT_OVERLAY_MODEL: DroptargetOverlayModel = {
     activationSize: { type: 'pixels', value: 10 },
@@ -388,6 +389,10 @@ export class DockviewComponent
         toggleClass(this.gridview.element, 'dv-dockview', true);
         toggleClass(this.element, 'dv-debug', !!options.debug);
 
+        if (options.debug) {
+            this.addDisposables(new StrictEventsSequencing(this));
+        }
+
         this.addDisposables(
             this.overlayRenderContainer,
             this._onWillDragPanel,
@@ -404,6 +409,7 @@ export class DockviewComponent
             this._onDidRemoveGroup,
             this._onDidActiveGroupChange,
             this._onUnhandledDragOverEvent,
+            this._onDidMaximizedGroupChange,
             this.onDidViewVisibilityChangeMicroTaskQueue(() => {
                 this.updateWatermark();
             }),
@@ -571,6 +577,11 @@ export class DockviewComponent
         this.updateWatermark();
     }
 
+    override dispose(): void {
+        this.clear(); // explicitly clear the layout before cleaning up
+        super.dispose();
+    }
+
     override setVisible(panel: DockviewGroupPanel, visible: boolean): void {
         switch (panel.api.location.type) {
             case 'grid':
@@ -706,6 +717,8 @@ export class DockviewComponent
                     _window.window!.innerHeight
                 );
 
+                let floatingBox: AnchoredBox | undefined;
+
                 if (!options?.overridePopoutGroup && isGroupAddedToDom) {
                     if (itemToPopout instanceof DockviewPanel) {
                         this.movingLock(() => {
@@ -727,7 +740,16 @@ export class DockviewComponent
                                 break;
                             case 'floating':
                             case 'popout':
+                                floatingBox = this._floatingGroups
+                                    .find(
+                                        (value) =>
+                                            value.group.api.id ===
+                                            itemToPopout.api.id
+                                    )
+                                    ?.overlay.toJSON();
+
                                 this.removeGroup(referenceGroup);
+
                                 break;
                         }
                     }
@@ -825,20 +847,31 @@ export class DockviewComponent
                                 });
                             }
                         } else if (this.getPanel(group.id)) {
-                            this.doRemoveGroup(group, {
-                                skipDispose: true,
-                                skipActive: true,
-                                skipPopoutReturn: true,
-                            });
-
                             const removedGroup = group;
 
-                            removedGroup.model.renderContainer =
+                            if (floatingBox) {
-                                this.overlayRenderContainer;
+                                this.addFloatingGroup(removedGroup, {
-                            removedGroup.model.location = { type: 'grid' };
+                                    height: floatingBox.height,
-                            returnedGroup = removedGroup;
+                                    width: floatingBox.width,
+                                    position: floatingBox,
+                                });
+                            } else {
+                                this.doRemoveGroup(removedGroup, {
+                                    skipDispose: true,
+                                    skipActive: true,
+                                    skipPopoutReturn: true,
+                                });
 
-                            this.doAddGroup(removedGroup, [0]);
+                                removedGroup.model.renderContainer =
+                                    this.overlayRenderContainer;
+                                removedGroup.model.location = { type: 'grid' };
+                                returnedGroup = removedGroup;
+
+                                this.movingLock(() => {
+                                    // suppress group add events since the group already exists
+                                    this.doAddGroup(removedGroup, [0]);
+                                });
+                            }
                             this.doSetGroupAndPanelActive(removedGroup);
                         }
                     })
@@ -1290,6 +1323,7 @@ export class DockviewComponent
                     locked: !!locked,
                     hideHeader: !!hideHeader,
                 });
+                this._onDidAddGroup.fire(group);
 
                 const createdPanels: IDockviewPanel[] = [];
 
@@ -1306,8 +1340,6 @@ export class DockviewComponent
                     createdPanels.push(panel);
                 }
 
-                this._onDidAddGroup.fire(group);
-
                 for (let i = 0; i < views.length; i++) {
                     const panel = createdPanels[i];
 
@@ -1394,6 +1426,7 @@ export class DockviewComponent
                 'dockview: failed to deserialize layout. Reverting changes',
                 err
             );
+
             /**
              * Takes all the successfully created groups and remove all of their panels.
              */

packages/dockview-core/src/dockview/dockviewGroupPanelModel.ts

@@ -506,7 +506,9 @@ export class DockviewGroupPanelModel
             this._onDidAddPanel,
             this._onDidRemovePanel,
             this._onDidActivePanelChange,
-            this._onUnhandledDragOverEvent
+            this._onUnhandledDragOverEvent,
+            this._onDidPanelTitleChange,
+            this._onDidPanelParametersChange
         );
     }
 

packages/dockview-core/src/dockview/strictEventsSequencing.ts

@@ -0,0 +1,54 @@
+import { CompositeDisposable } from '../lifecycle';
+import { DockviewComponent } from './dockviewComponent';
+
+export class StrictEventsSequencing extends CompositeDisposable {
+    constructor(private readonly accessor: DockviewComponent) {
+        super();
+
+        this.init();
+    }
+
+    private init(): void {
+        const panels = new Set<string>();
+        const groups = new Set<string>();
+
+        this.addDisposables(
+            this.accessor.onDidAddPanel((panel) => {
+                if (panels.has(panel.api.id)) {
+                    throw new Error(
+                        `dockview: Invalid event sequence. [onDidAddPanel] called for panel ${panel.api.id} but panel already exists`
+                    );
+                } else {
+                    panels.add(panel.api.id);
+                }
+            }),
+            this.accessor.onDidRemovePanel((panel) => {
+                if (!panels.has(panel.api.id)) {
+                    throw new Error(
+                        `dockview: Invalid event sequence. [onDidRemovePanel] called for panel ${panel.api.id} but panel does not exists`
+                    );
+                } else {
+                    panels.delete(panel.api.id);
+                }
+            }),
+            this.accessor.onDidAddGroup((group) => {
+                if (groups.has(group.api.id)) {
+                    throw new Error(
+                        `dockview: Invalid event sequence. [onDidAddGroup] called for group ${group.api.id} but group already exists`
+                    );
+                } else {
+                    groups.add(group.api.id);
+                }
+            }),
+            this.accessor.onDidRemoveGroup((group) => {
+                if (!groups.has(group.api.id)) {
+                    throw new Error(
+                        `dockview: Invalid event sequence. [onDidRemoveGroup] called for group ${group.api.id} but group does not exists`
+                    );
+                } else {
+                    groups.delete(group.api.id);
+                }
+            })
+        );
+    }
+}

packages/dockview-core/src/gridview/baseComponentGridview.ts

@@ -205,6 +205,8 @@ export abstract class BaseGrid<T extends IGridPanelView>
             )(() => {
                 this._bufferOnDidLayoutChange.fire();
             }),
+            this._onDidMaximizedChange,
+            this._onDidViewVisibilityChangeMicroTaskQueue,
             this._bufferOnDidLayoutChange
         );
     }