Skip to content

Commit

Permalink
bug: duplicate container HTML Element
Browse files Browse the repository at this point in the history
  • Loading branch information
@mathuo
mathuo committed Jan 10, 2025
1 parent 872ec7c commit 1a3c6ea
Show file tree
Hide file tree
Showing 13 changed files with 91 additions and 106 deletions.
15 changes: 15 additions & 0 deletions packages/dockview-core/src/__tests__/gridview/baseComponentGridview.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,21 @@ class ClassUnderTest extends BaseGrid<TestPanel> {
}

describe('baseComponentGridview', () => {
test('that the container is not removed when grid is disposed', () => {
const root = document.createElement('div');
const container = document.createElement('div');
root.appendChild(container);

const cut = new ClassUnderTest(container, {
orientation: Orientation.HORIZONTAL,
proportionalLayout: true,
});

cut.dispose();

expect(container.parentElement).toBe(root);
});

test('that .layout(...) force flag works', () => {
const cut = new ClassUnderTest(document.createElement('div'), {
orientation: Orientation.HORIZONTAL,
Expand Down
59 changes: 23 additions & 36 deletions packages/dockview-core/src/__tests__/paneview/paneviewComponent.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,27 @@ describe('componentPaneview', () => {
container.className = 'container';
});

test('that the container is not removed when grid is disposed', () => {
const root = document.createElement('div');
const container = document.createElement('div');
root.appendChild(container);

const paneview = new PaneviewComponent(container, {
createComponent: (options) => {
switch (options.name) {
case 'default':
return new TestPanel(options.id, options.name);
default:
throw new Error('unsupported');
}
},
});

paneview.dispose();

expect(container.parentElement).toBe(root);
});

test('vertical panels', () => {
const disposables = new CompositeDisposable();

Expand Down Expand Up @@ -293,40 +314,6 @@ describe('componentPaneview', () => {
disposable.dispose();
});

test('dispose of paneviewComponent', () => {
expect(container.childNodes.length).toBe(0);

const paneview = new PaneviewComponent(container, {
createComponent: (options) => {
switch (options.name) {
case 'default':
return new TestPanel(options.id, options.name);
default:
throw new Error('unsupported');
}
},
});

paneview.layout(1000, 1000);

paneview.addPanel({
id: 'panel1',
component: 'default',
title: 'Panel 1',
});
paneview.addPanel({
id: 'panel2',
component: 'default',
title: 'Panel 2',
});

expect(container.childNodes.length).toBeGreaterThan(0);

paneview.dispose();

expect(container.childNodes.length).toBe(0);
});

test('panel is disposed of when component is disposed', () => {
const paneview = new PaneviewComponent(container, {
createComponent: (options) => {
Expand Down Expand Up @@ -606,10 +593,10 @@ describe('componentPaneview', () => {
className: 'test-a test-b',
});

expect(paneview.element.className).toBe('container test-a test-b');
expect(paneview.element.className).toBe('test-a test-b');

paneview.updateOptions({ className: 'test-b test-c' });

expect(paneview.element.className).toBe('container test-b test-c');
expect(paneview.element.className).toBe('test-b test-c');
});
});
59 changes: 24 additions & 35 deletions packages/dockview-core/src/__tests__/splitview/splitviewComponent.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,28 @@ describe('componentSplitview', () => {
container.className = 'container';
});

test('that the container is not removed when grid is disposed', () => {
const root = document.createElement('div');
const container = document.createElement('div');
root.appendChild(container);

const splitview = new SplitviewComponent(container, {
orientation: Orientation.VERTICAL,
createComponent: (options) => {
switch (options.name) {
case 'default':
return new TestPanel(options.id, options.name);
default:
throw new Error('unsupported');
}
},
});

splitview.dispose();

expect(container.parentElement).toBe(root);
});

test('event leakage', () => {
Emitter.setLeakageMonitorEnabled(true);

Expand Down Expand Up @@ -451,39 +473,6 @@ describe('componentSplitview', () => {
disposable.dispose();
});

test('dispose of splitviewComponent', () => {
expect(container.childNodes.length).toBe(0);

const splitview = new SplitviewComponent(container, {
orientation: Orientation.HORIZONTAL,
createComponent: (options) => {
switch (options.name) {
case 'default':
return new TestPanel(options.id, options.name);
default:
throw new Error('unsupported');
}
},
});

splitview.layout(1000, 1000);

splitview.addPanel({
id: 'panel1',
component: 'default',
});
splitview.addPanel({
id: 'panel2',
component: 'default',
});

expect(container.childNodes.length).toBeGreaterThan(0);

splitview.dispose();

expect(container.childNodes.length).toBe(0);
});

test('panel is disposed of when component is disposed', () => {
const splitview = new SplitviewComponent(container, {
orientation: Orientation.HORIZONTAL,
Expand Down Expand Up @@ -736,10 +725,10 @@ describe('componentSplitview', () => {
className: 'test-a test-b',
});

expect(splitview.element.className).toBe('container test-a test-b');
expect(splitview.element.className).toBe('test-a test-b');

splitview.updateOptions({ className: 'test-b test-c' });

expect(splitview.element.className).toBe('container test-b test-c');
expect(splitview.element.className).toBe('test-b test-c');
});
});
4 changes: 2 additions & 2 deletions packages/dockview-core/src/dockview/dockviewComponent.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -368,8 +368,8 @@ export class DockviewComponent
return this._floatingGroups;
}

constructor(parentElement: HTMLElement, options: DockviewComponentOptions) {
super(parentElement, {
constructor(container: HTMLElement, options: DockviewComponentOptions) {
super(container, {
proportionalLayout: true,
orientation: Orientation.HORIZONTAL,
styles: options.hideBorders
Expand Down
7 changes: 5 additions & 2 deletions packages/dockview-core/src/gridview/baseComponentGridview.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,14 +155,17 @@ export abstract class BaseGrid<T extends IGridPanelView>
this.gridview.locked = value;
}

constructor(parentElement: HTMLElement, options: BaseGridOptions) {
super(parentElement, options.disableAutoResizing);
constructor(container: HTMLElement, options: BaseGridOptions) {
super(document.createElement('div'), options.disableAutoResizing);
this.element.style.height = '100%';
this.element.style.width = '100%';

this._classNames = new Classnames(this.element);
this._classNames.setClassNames(options.className ?? '');

// the container is owned by the third-party, do not modify/delete it
container.appendChild(this.element);

this.gridview = new Gridview(
!!options.proportionalLayout,
options.styles,
Expand Down
4 changes: 2 additions & 2 deletions packages/dockview-core/src/gridview/gridviewComponent.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@ export class GridviewComponent
this._deserializer = value;
}

constructor(parentElement: HTMLElement, options: GridviewComponentOptions) {
super(parentElement, {
constructor(container: HTMLElement, options: GridviewComponentOptions) {
super(container, {
proportionalLayout: options.proportionalLayout ?? true,
orientation: options.orientation,
styles: options.hideBorders
Expand Down
9 changes: 7 additions & 2 deletions packages/dockview-core/src/paneview/paneviewComponent.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,10 @@ export class PaneviewComponent extends Resizable implements IPaneviewComponent {
return this._options;
}

constructor(parentElement: HTMLElement, options: PaneviewComponentOptions) {
super(parentElement, options.disableAutoResizing);
constructor(container: HTMLElement, options: PaneviewComponentOptions) {
super(document.createElement('div'), options.disableAutoResizing);
this.element.style.height = '100%';
this.element.style.width = '100%';

this.addDisposables(
this._onDidLayoutChange,
Expand All @@ -210,6 +212,9 @@ export class PaneviewComponent extends Resizable implements IPaneviewComponent {
this._classNames = new Classnames(this.element);
this._classNames.setClassNames(options.className ?? '');

// the container is owned by the third-party, do not modify/delete it
container.appendChild(this.element);

this._options = options;

this.paneview = new Paneview(this.element, {
Expand Down
4 changes: 2 additions & 2 deletions packages/dockview-core/src/splitview/splitview.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export interface ISplitviewStyles {
}

export interface SplitViewOptions {
orientation: Orientation;
orientation?: Orientation;
descriptor?: ISplitViewDescriptor;
proportionalLayout?: boolean;
styles?: ISplitviewStyles;
Expand Down Expand Up @@ -225,7 +225,7 @@ export class Splitview {
private readonly container: HTMLElement,
options: SplitViewOptions
) {
this._orientation = options.orientation;
this._orientation = options.orientation ?? Orientation.VERTICAL;
this.element = this.createContainer();

this.margin = options.margin ?? 0;
Expand Down
12 changes: 7 additions & 5 deletions packages/dockview-core/src/splitview/splitviewComponent.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,15 +155,17 @@ export class SplitviewComponent
: this.splitview.orthogonalSize;
}

constructor(
parentElement: HTMLElement,
options: SplitviewComponentOptions
) {
super(parentElement, options.disableAutoResizing);
constructor(container: HTMLElement, options: SplitviewComponentOptions) {
super(document.createElement('div'), options.disableAutoResizing);
this.element.style.height = '100%';
this.element.style.width = '100%';

this._classNames = new Classnames(this.element);
this._classNames.setClassNames(options.className ?? '');

// the container is owned by the third-party, do not modify/delete it
container.appendChild(this.element);

this._options = options;

this.splitview = new Splitview(this.element, options);
Expand Down
6 changes: 1 addition & 5 deletions packages/dockview/src/dockview/dockview.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -318,11 +318,7 @@ export const DockviewReact = React.forwardRef(
}, [props.prefixHeaderActionsComponent]);

return (
<div
className={props.className}
style={{ height: '100%', width: '100%' }}
ref={domRef}
>
<div style={{ height: '100%', width: '100%' }} ref={domRef}>
{portals}
</div>
);
Expand Down
6 changes: 1 addition & 5 deletions packages/dockview/src/gridview/gridview.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,11 +126,7 @@ export const GridviewReact = React.forwardRef(
}, [props.components]);

return (
<div
className={props.className}
style={{ height: '100%', width: '100%' }}
ref={domRef}
>
<div style={{ height: '100%', width: '100%' }} ref={domRef}>
{portals}
</div>
);
Expand Down
6 changes: 1 addition & 5 deletions packages/dockview/src/paneview/paneview.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,11 +176,7 @@ export const PaneviewReact = React.forwardRef(
}, [props.onDidDrop]);

return (
<div
className={props.className}
style={{ height: '100%', width: '100%' }}
ref={domRef}
>
<div style={{ height: '100%', width: '100%' }} ref={domRef}>
{portals}
</div>
);
Expand Down
6 changes: 1 addition & 5 deletions packages/dockview/src/splitview/splitview.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,11 +126,7 @@ export const SplitviewReact = React.forwardRef(
}, [props.components]);

return (
<div
className={props.className}
style={{ height: '100%', width: '100%' }}
ref={domRef}
>
<div style={{ height: '100%', width: '100%' }} ref={domRef}>
{portals}
</div>
);
Expand Down

0 comments on commit 1a3c6ea

Please sign in to comment.