Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: duplicate .close() call #748

Merged
merged 1 commit into from
Nov 3, 2024
Merged

bug: duplicate .close() call #748

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions packages/dockview-core/src/__tests__/__mocks__/mockWindow.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { fromPartial } from "@total-typescript/shoehorn";
import { fromPartial } from '@total-typescript/shoehorn';

export function setupMockWindow() {
const listeners: Record<string, (() => void)[]> = {};
Expand All @@ -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) {
Expand All @@ -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++;
},
Expand Down
148 changes: 145 additions & 3 deletions packages/dockview-core/src/__tests__/dockview/dockviewComponent.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -116,8 +116,6 @@ describe('dockviewComponent', () => {
}
},
});

window.open = jest.fn();
});

test('update className', () => {
Expand Down Expand Up @@ -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');

Expand Down
96 changes: 64 additions & 32 deletions packages/dockview-core/src/dockview/dockviewComponent.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -593,9 +592,12 @@ export class DockviewComponent
}
);

let windowExplicitlyClosed = false;

const popoutWindowDisposable = new CompositeDisposable(
_window,
_window.onDidClose(() => {
windowExplicitlyClosed = true;
popoutWindowDisposable.dispose();
})
);
Expand Down Expand Up @@ -627,41 +629,51 @@ export class DockviewComponent

const referenceLocation = itemToPopout.api.location.type;

const group =
options?.overridePopoutGroup ??
this.createGroup({ id: groupId });
/**
* The group that is being added doesn't already exist within the DOM, the most likely occurance
* 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) {
this.movingLock(() => {
const panel =
referenceGroup.model.removePanel(itemToPopout);
group.model.openPanel(panel);
});
} else {
this.movingLock(() =>
moveGroupWithoutDestroying({
from: referenceGroup,
to: group,
})
);
if (!options?.overridePopoutGroup && isGroupAddedToDom) {
if (itemToPopout instanceof DockviewPanel) {
this.movingLock(() => {
const panel =
referenceGroup.model.removePanel(itemToPopout);
group.model.openPanel(panel);
});
} else {
this.movingLock(() =>
moveGroupWithoutDestroying({
from: referenceGroup,
to: group,
})
);

switch (referenceLocation) {
case 'grid':
referenceGroup.api.setVisible(false);
break;
case 'floating':
case 'popout':
this.removeGroup(referenceGroup);
break;
switch (referenceLocation) {
case 'grid':
referenceGroup.api.setVisible(false);
break;
case 'floating':
case 'popout':
this.removeGroup(referenceGroup);
break;
}
}
}

Expand All @@ -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);
}

Expand All @@ -698,8 +713,12 @@ export class DockviewComponent
const value = {
window: _window,
popoutGroup: group,
referenceGroup: this.getPanel(referenceGroup.id)
? referenceGroup.id
referenceGroup: !isGroupAddedToDom
? undefined
: referenceGroup
? this.getPanel(referenceGroup.id)
? referenceGroup.id
: undefined
: undefined,
disposable: {
dispose: () => {
Expand Down Expand Up @@ -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,
Expand All @@ -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);
}
})
);
Expand Down Expand Up @@ -1279,7 +1308,6 @@ export class DockviewComponent
? this.getPanel(gridReferenceGroup)
: undefined) ?? group,
{
skipRemoveGroup: true,
position: position ?? undefined,
overridePopoutGroup: gridReferenceGroup
? group
Expand All @@ -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.
*/
Expand Down
1 change: 0 additions & 1 deletion packages/dockview-core/src/popoutWindow.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ export class PopoutWindow extends CompositeDisposable {
});

this._window.disposable.dispose();
this._window.value.close();
this._window = null;

this._onDidClose.fire();
Expand Down
Loading