Skip to content

Commit

Permalink
Merge pull request #825 from mathuo/818-container-with-theme-class-is…
Browse files Browse the repository at this point in the history
…-created-twice-1

bug: duplicate container HTML Element
  • Loading branch information
mathuo authored Jan 11, 2025
2 parents 680eaca + 3d23a2d commit 6678ae2
Show file tree
Hide file tree
Showing 16 changed files with 176 additions and 177 deletions.
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
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');
});
});
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
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
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
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
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
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
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
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
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
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
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
Loading

0 comments on commit 6678ae2

Please sign in to comment.