Skip to content

Commit

Permalink
bug: duplicate .close() call
Browse files Browse the repository at this point in the history
  • Loading branch information
@mathuo
mathuo committed Nov 2, 2024
1 parent ee7cf63 commit ff5fed7
Show file tree
Hide file tree
Showing 5 changed files with 148 additions and 37 deletions.
10 changes: 9 additions & 1 deletion packages/dockview-core/src/__tests__/__mocks__/mockWindow.ts
View file
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 Down
86 changes: 85 additions & 1 deletion packages/dockview-core/src/__tests__/dockview/dockviewComponent.spec.ts
View file
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 All @@ -21,6 +21,7 @@ import { DockviewApi } from '../../api/component.api';
import { DockviewDndOverlayEvent } from '../../dockview/options';
import { SizeEvent } from '../../api/gridviewPanelApi';
import { setupMockWindow } from '../__mocks__/mockWindow';
import { exhaustMicrotaskQueue } from '../__test_utils__/utils';

class PanelContentPartTest implements IContentRenderer {
element: HTMLElement = document.createElement('div');
Expand Down Expand Up @@ -4886,6 +4887,89 @@ describe('dockviewComponent', () => {
]);
});

test('popout / floating layouts', 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);

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');
});

test('remove all panels from popout group', async () => {
const container = document.createElement('div');

Expand Down
77 changes: 46 additions & 31 deletions packages/dockview-core/src/dockview/dockviewComponent.ts
View file
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 @@ -627,41 +626,50 @@ 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 +684,7 @@ 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 +706,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 +739,7 @@ export class DockviewComponent
),
overlayRenderContainer,
Disposable.from(() => {
if (this.getPanel(referenceGroup.id)) {
if (isGroupAddedToDom && this.getPanel(referenceGroup.id)) {
this.movingLock(() =>
moveGroupWithoutDestroying({
from: group,
Expand Down Expand Up @@ -1279,7 +1291,6 @@ export class DockviewComponent
? this.getPanel(gridReferenceGroup)
: undefined) ?? group,
{
skipRemoveGroup: true,
position: position ?? undefined,
overridePopoutGroup: gridReferenceGroup
? group
Expand All @@ -1299,6 +1310,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
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
11 changes: 8 additions & 3 deletions packages/docs/sandboxes/react/dockview/demo-dockview/src/gridActions.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
}
Expand All @@ -121,8 +122,12 @@ export const GridActions = (props: {

const onReset = () => {
if (props.api) {
props.api.clear();
defaultConfig(props.api);
try {
props.api.clear();
defaultConfig(props.api);
} catch (err) {
localStorage.removeItem('dv-demo-state');
}
}
};

Expand Down

0 comments on commit ff5fed7

Please sign in to comment.