bug: duplicate .close() call

packages/dockview-core/src/__tests__/__mocks__/mockWindow.ts

@@ -1,4 +1,4 @@
-import { fromPartial } from "@total-typescript/shoehorn";
+import { fromPartial } from '@total-typescript/shoehorn';
 
 export function setupMockWindow() {
     const listeners: Record<string, (() => void)[]> = {};
@@ -16,6 +16,14 @@ export function setupMockWindow() {
                 listener();
             }
         },
+        removeEventListener: (type: string, listener: () => void) => {
+            if (listeners[type]) {
+                const index = listeners[type].indexOf(listener);
+                if (index > -1) {
+                    listeners[type].splice(index, 1);
+                }
+            }
+        },
         dispatchEvent: (event: Event) => {
             const items = listeners[event.type];
             if (!items) {
@@ -24,7 +32,9 @@ export function setupMockWindow() {
             items.forEach((item) => item());
         },
         document: document,
-        close: jest.fn(),
+        close: () => {
+            listeners['beforeunload']?.forEach((f) => f());
+        },
         get innerWidth() {
             return width++;
         },

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

@@ -8,7 +8,7 @@ import { PanelUpdateEvent } from '../../panel/types';
 import { Orientation } from '../../splitview/splitview';
 import { CompositeDisposable } from '../../lifecycle';
 import { Emitter } from '../../events';
-import { IDockviewPanel } from '../../dockview/dockviewPanel';
+import { DockviewPanel, IDockviewPanel } from '../../dockview/dockviewPanel';
 import { DockviewGroupPanel } from '../../dockview/dockviewGroupPanel';
 import { fireEvent, queryByTestId } from '@testing-library/dom';
 import { getPanelData } from '../../dnd/dataTransfer';
@@ -116,8 +116,6 @@ describe('dockviewComponent', () => {
                 }
             },
         });
-
-        window.open = jest.fn();
     });
 
     test('update className', () => {
@@ -4886,6 +4884,150 @@ describe('dockviewComponent', () => {
             ]);
         });
 
+        test('popout / floating layouts', 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, 500);
+
+            let panel1 = dockview.addPanel({
+                id: 'panel_1',
+                component: 'default',
+            });
+
+            let panel2 = dockview.addPanel({
+                id: 'panel_2',
+                component: 'default',
+            });
+
+            let panel3 = dockview.addPanel({
+                id: 'panel_3',
+                component: 'default',
+            });
+
+            let panel4 = dockview.addPanel({
+                id: 'panel_4',
+                component: 'default',
+            });
+
+            expect(panel1.api.location.type).toBe('grid');
+            expect(panel2.api.location.type).toBe('grid');
+            expect(panel3.api.location.type).toBe('grid');
+            expect(panel4.api.location.type).toBe('grid');
+
+            dockview.addFloatingGroup(panel2);
+            dockview.addFloatingGroup(panel3);
+
+            expect(panel1.api.location.type).toBe('grid');
+            expect(panel2.api.location.type).toBe('floating');
+            expect(panel3.api.location.type).toBe('floating');
+            expect(panel4.api.location.type).toBe('grid');
+
+            await dockview.addPopoutGroup(panel2);
+            await dockview.addPopoutGroup(panel4);
+
+            expect(panel1.api.location.type).toBe('grid');
+            expect(panel2.api.location.type).toBe('popout');
+            expect(panel3.api.location.type).toBe('floating');
+            expect(panel4.api.location.type).toBe('popout');
+
+            const state = dockview.toJSON();
+            dockview.fromJSON(state);
+
+            /**
+             * exhaust task queue since popout group completion is async but not awaited in `fromJSON(...)`
+             */
+            await new Promise((resolve) => setTimeout(resolve, 0));
+
+            expect(dockview.panels.length).toBe(4);
+
+            panel1 = dockview.api.getPanel('panel_1') as DockviewPanel;
+            panel2 = dockview.api.getPanel('panel_2') as DockviewPanel;
+            panel3 = dockview.api.getPanel('panel_3') as DockviewPanel;
+            panel4 = dockview.api.getPanel('panel_4') as DockviewPanel;
+
+            expect(panel1.api.location.type).toBe('grid');
+            expect(panel2.api.location.type).toBe('popout');
+            expect(panel3.api.location.type).toBe('floating');
+            expect(panel4.api.location.type).toBe('popout');
+
+            dockview.clear();
+            expect(dockview.groups.length).toBe(0);
+            expect(dockview.panels.length).toBe(0);
+        });
+
+        test('close popout window object', async () => {
+            const container = document.createElement('div');
+
+            const mockWindow = setupMockWindow();
+            window.open = () => mockWindow;
+
+            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);
+
+            let panel1 = dockview.addPanel({
+                id: 'panel_1',
+                component: 'default',
+            });
+
+            let panel2 = dockview.addPanel({
+                id: 'panel_2',
+                component: 'default',
+                position: { referencePanel: panel1, direction: 'within' },
+            });
+
+            let panel3 = dockview.addPanel({
+                id: 'panel_3',
+                component: 'default',
+            });
+
+            dockview.addFloatingGroup(panel2);
+            await dockview.addPopoutGroup(panel2);
+
+            expect(panel1.group.api.location.type).toBe('grid');
+            expect(panel2.group.api.location.type).toBe('popout');
+            expect(panel3.group.api.location.type).toBe('grid');
+
+            mockWindow.close();
+
+            expect(panel1.group.api.location.type).toBe('grid');
+            expect(panel2.group.api.location.type).toBe('grid');
+            expect(panel3.group.api.location.type).toBe('grid');
+
+            dockview.clear();
+            expect(dockview.groups.length).toBe(0);
+            expect(dockview.panels.length).toBe(0);
+        });
+
         test('remove all panels from popout group', async () => {
             const container = document.createElement('div');
 

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

@@ -541,7 +541,6 @@ export class DockviewComponent
     addPopoutGroup(
         itemToPopout: DockviewPanel | DockviewGroupPanel,
         options?: {
-            skipRemoveGroup?: boolean;
             position?: Box;
             popoutUrl?: string;
             onDidOpen?: (event: { id: string; window: Window }) => void;
@@ -593,9 +592,12 @@ export class DockviewComponent
             }
         );
 
+        let windowExplicitlyClosed = false;
+
         const popoutWindowDisposable = new CompositeDisposable(
             _window,
             _window.onDidClose(() => {
+                windowExplicitlyClosed = true;
                 popoutWindowDisposable.dispose();
             })
         );
@@ -627,41 +629,51 @@ export class DockviewComponent
 
                 const referenceLocation = itemToPopout.api.location.type;
 
-                const group =
+                /**
-                    options?.overridePopoutGroup ??
+                 * The group that is being added doesn't already exist within the DOM, the most likely occurance
-                    this.createGroup({ id: groupId });
+                 * of this case is when being called from the `fromJSON(...)` method
+                 */
+                const isGroupAddedToDom =
+                    referenceGroup.element.parentElement !== null;
+
+                const group = !isGroupAddedToDom
+                    ? referenceGroup
+                    : options?.overridePopoutGroup ??
+                      this.createGroup({ id: groupId });
                 group.model.renderContainer = overlayRenderContainer;
                 group.layout(
                     _window.window!.innerWidth,
                     _window.window!.innerHeight
                 );
 
-                if (!options?.overridePopoutGroup) {
+                if (!this._groups.has(group.api.id)) {
                     this._onDidAddGroup.fire(group);
                 }
 
-                if (itemToPopout instanceof DockviewPanel) {
+                if (!options?.overridePopoutGroup && isGroupAddedToDom) {
-                    this.movingLock(() => {
+                    if (itemToPopout instanceof DockviewPanel) {
-                        const panel =
+                        this.movingLock(() => {
-                            referenceGroup.model.removePanel(itemToPopout);
+                            const panel =
-                        group.model.openPanel(panel);
+                                referenceGroup.model.removePanel(itemToPopout);
-                    });
+                            group.model.openPanel(panel);
-                } else {
+                        });
-                    this.movingLock(() =>
+                    } else {
-                        moveGroupWithoutDestroying({
+                        this.movingLock(() =>
-                            from: referenceGroup,
+                            moveGroupWithoutDestroying({
-                            to: group,
+                                from: referenceGroup,
-                        })
+                                to: group,
-                    );
+                            })
+                        );
 
-                    switch (referenceLocation) {
+                        switch (referenceLocation) {
-                        case 'grid':
+                            case 'grid':
-                            referenceGroup.api.setVisible(false);
+                                referenceGroup.api.setVisible(false);
-                            break;
+                                break;
-                        case 'floating':
+                            case 'floating':
-                        case 'popout':
+                            case 'popout':
-                            this.removeGroup(referenceGroup);
+                                this.removeGroup(referenceGroup);
-                            break;
+                                break;
+                        }
                     }
                 }
 
@@ -676,7 +688,10 @@ export class DockviewComponent
                     getWindow: () => _window.window!,
                 };
 
-                if (itemToPopout.api.location.type === 'grid') {
+                if (
+                    isGroupAddedToDom &&
+                    itemToPopout.api.location.type === 'grid'
+                ) {
                     itemToPopout.api.setVisible(false);
                 }
 
@@ -698,8 +713,12 @@ export class DockviewComponent
                 const value = {
                     window: _window,
                     popoutGroup: group,
-                    referenceGroup: this.getPanel(referenceGroup.id)
+                    referenceGroup: !isGroupAddedToDom
-                        ? referenceGroup.id
+                        ? undefined
+                        : referenceGroup
+                        ? this.getPanel(referenceGroup.id)
+                            ? referenceGroup.id
+                            : undefined
                         : undefined,
                     disposable: {
                         dispose: () => {
@@ -727,7 +746,10 @@ export class DockviewComponent
                     ),
                     overlayRenderContainer,
                     Disposable.from(() => {
-                        if (this.getPanel(referenceGroup.id)) {
+                        if (
+                            isGroupAddedToDom &&
+                            this.getPanel(referenceGroup.id)
+                        ) {
                             this.movingLock(() =>
                                 moveGroupWithoutDestroying({
                                     from: group,
@@ -745,14 +767,21 @@ export class DockviewComponent
                                 });
                             }
                         } else if (this.getPanel(group.id)) {
-                            const removedGroup = this.doRemoveGroup(group, {
+                            this.doRemoveGroup(group, {
                                 skipDispose: true,
                                 skipActive: true,
+                                skipPopoutReturn: true,
                             });
+
+                            const removedGroup = group;
+
                             removedGroup.model.renderContainer =
                                 this.overlayRenderContainer;
                             removedGroup.model.location = { type: 'grid' };
                             returnedGroup = removedGroup;
+
+                            this.doAddGroup(removedGroup, [0]);
+                            this.doSetGroupAndPanelActive(removedGroup);
                         }
                     })
                 );
@@ -1279,7 +1308,6 @@ export class DockviewComponent
                         ? this.getPanel(gridReferenceGroup)
                         : undefined) ?? group,
                     {
-                        skipRemoveGroup: true,
                         position: position ?? undefined,
                         overridePopoutGroup: gridReferenceGroup
                             ? group
@@ -1299,6 +1327,10 @@ export class DockviewComponent
                 }
             }
         } catch (err) {
+            console.error(
+                'dockview: failed to deserialize layout. Reverting changes',
+                err
+            );
             /**
              * Takes all the successfully created groups and remove all of their panels.
              */

packages/dockview-core/src/popoutWindow.ts

@@ -59,7 +59,6 @@ export class PopoutWindow extends CompositeDisposable {
             });
 
             this._window.disposable.dispose();
-            this._window.value.close();
             this._window = null;
 
             this._onDidClose.fire();

packages/docs/sandboxes/react/dockview/demo-dockview/src/gridActions.tsx

@@ -103,7 +103,8 @@ export const GridActions = (props: {
         if (state) {
             try {
                 props.api?.fromJSON(JSON.parse(state));
-            } catch {
+            } catch (err) {
+                console.error('failed to load state', err);
                 localStorage.removeItem('dv-demo-state');
             }
         }
@@ -121,8 +122,12 @@ export const GridActions = (props: {
 
     const onReset = () => {
         if (props.api) {
-            props.api.clear();
+            try {
-            defaultConfig(props.api);
+                props.api.clear();
+                defaultConfig(props.api);
+            } catch (err) {
+                localStorage.removeItem('dv-demo-state');
+            }
         }
     };