From 1a3c6ea7db37b8b2d621c5fba6f2d5b7f6cc6ce9 Mon Sep 17 00:00:00 2001 From: mathuo <6710312+mathuo@users.noreply.github.com> Date: Fri, 10 Jan 2025 20:55:25 +0000 Subject: [PATCH 1/2] bug: duplicate container HTML Element --- .../gridview/baseComponentGridview.spec.ts | 15 +++++ .../paneview/paneviewComponent.spec.ts | 59 ++++++++----------- .../splitview/splitviewComponent.spec.ts | 59 ++++++++----------- .../src/dockview/dockviewComponent.ts | 4 +- .../src/gridview/baseComponentGridview.ts | 7 ++- .../src/gridview/gridviewComponent.ts | 4 +- .../src/paneview/paneviewComponent.ts | 9 ++- .../dockview-core/src/splitview/splitview.ts | 4 +- .../src/splitview/splitviewComponent.ts | 12 ++-- packages/dockview/src/dockview/dockview.tsx | 6 +- packages/dockview/src/gridview/gridview.tsx | 6 +- packages/dockview/src/paneview/paneview.tsx | 6 +- packages/dockview/src/splitview/splitview.tsx | 6 +- 13 files changed, 91 insertions(+), 106 deletions(-) diff --git a/packages/dockview-core/src/__tests__/gridview/baseComponentGridview.spec.ts b/packages/dockview-core/src/__tests__/gridview/baseComponentGridview.spec.ts index d8ed45209..3ab49faa7 100644 --- a/packages/dockview-core/src/__tests__/gridview/baseComponentGridview.spec.ts +++ b/packages/dockview-core/src/__tests__/gridview/baseComponentGridview.spec.ts @@ -105,6 +105,21 @@ class ClassUnderTest extends BaseGrid { } 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, diff --git a/packages/dockview-core/src/__tests__/paneview/paneviewComponent.spec.ts b/packages/dockview-core/src/__tests__/paneview/paneviewComponent.spec.ts index 77a58712b..8cfc7fc2e 100644 --- a/packages/dockview-core/src/__tests__/paneview/paneviewComponent.spec.ts +++ b/packages/dockview-core/src/__tests__/paneview/paneviewComponent.spec.ts @@ -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(); @@ -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) => { @@ -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'); }); }); diff --git a/packages/dockview-core/src/__tests__/splitview/splitviewComponent.spec.ts b/packages/dockview-core/src/__tests__/splitview/splitviewComponent.spec.ts index 6ab1415b2..90d0b3986 100644 --- a/packages/dockview-core/src/__tests__/splitview/splitviewComponent.spec.ts +++ b/packages/dockview-core/src/__tests__/splitview/splitviewComponent.spec.ts @@ -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); @@ -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, @@ -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'); }); }); diff --git a/packages/dockview-core/src/dockview/dockviewComponent.ts b/packages/dockview-core/src/dockview/dockviewComponent.ts index 63915751d..305b73536 100644 --- a/packages/dockview-core/src/dockview/dockviewComponent.ts +++ b/packages/dockview-core/src/dockview/dockviewComponent.ts @@ -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 diff --git a/packages/dockview-core/src/gridview/baseComponentGridview.ts b/packages/dockview-core/src/gridview/baseComponentGridview.ts index 537435310..93779f5a0 100644 --- a/packages/dockview-core/src/gridview/baseComponentGridview.ts +++ b/packages/dockview-core/src/gridview/baseComponentGridview.ts @@ -155,14 +155,17 @@ export abstract class BaseGrid 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, diff --git a/packages/dockview-core/src/gridview/gridviewComponent.ts b/packages/dockview-core/src/gridview/gridviewComponent.ts index 00084ce44..80df754bd 100644 --- a/packages/dockview-core/src/gridview/gridviewComponent.ts +++ b/packages/dockview-core/src/gridview/gridviewComponent.ts @@ -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 diff --git a/packages/dockview-core/src/paneview/paneviewComponent.ts b/packages/dockview-core/src/paneview/paneviewComponent.ts index 86c87a690..b47ca0181 100644 --- a/packages/dockview-core/src/paneview/paneviewComponent.ts +++ b/packages/dockview-core/src/paneview/paneviewComponent.ts @@ -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, @@ -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, { diff --git a/packages/dockview-core/src/splitview/splitview.ts b/packages/dockview-core/src/splitview/splitview.ts index fc5857d08..77fbb9cf5 100644 --- a/packages/dockview-core/src/splitview/splitview.ts +++ b/packages/dockview-core/src/splitview/splitview.ts @@ -32,7 +32,7 @@ export interface ISplitviewStyles { } export interface SplitViewOptions { - orientation: Orientation; + orientation?: Orientation; descriptor?: ISplitViewDescriptor; proportionalLayout?: boolean; styles?: ISplitviewStyles; @@ -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; diff --git a/packages/dockview-core/src/splitview/splitviewComponent.ts b/packages/dockview-core/src/splitview/splitviewComponent.ts index 5e988c422..a165ba4b3 100644 --- a/packages/dockview-core/src/splitview/splitviewComponent.ts +++ b/packages/dockview-core/src/splitview/splitviewComponent.ts @@ -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); diff --git a/packages/dockview/src/dockview/dockview.tsx b/packages/dockview/src/dockview/dockview.tsx index fefc605f1..2e4e2fc57 100644 --- a/packages/dockview/src/dockview/dockview.tsx +++ b/packages/dockview/src/dockview/dockview.tsx @@ -318,11 +318,7 @@ export const DockviewReact = React.forwardRef( }, [props.prefixHeaderActionsComponent]); return ( -
+
{portals}
); diff --git a/packages/dockview/src/gridview/gridview.tsx b/packages/dockview/src/gridview/gridview.tsx index 60b172651..9bc9dd60b 100644 --- a/packages/dockview/src/gridview/gridview.tsx +++ b/packages/dockview/src/gridview/gridview.tsx @@ -126,11 +126,7 @@ export const GridviewReact = React.forwardRef( }, [props.components]); return ( -
+
{portals}
); diff --git a/packages/dockview/src/paneview/paneview.tsx b/packages/dockview/src/paneview/paneview.tsx index 34ab300b9..e0d49d98d 100644 --- a/packages/dockview/src/paneview/paneview.tsx +++ b/packages/dockview/src/paneview/paneview.tsx @@ -176,11 +176,7 @@ export const PaneviewReact = React.forwardRef( }, [props.onDidDrop]); return ( -
+
{portals}
); diff --git a/packages/dockview/src/splitview/splitview.tsx b/packages/dockview/src/splitview/splitview.tsx index 63d2cf5d9..bb8593381 100644 --- a/packages/dockview/src/splitview/splitview.tsx +++ b/packages/dockview/src/splitview/splitview.tsx @@ -126,11 +126,7 @@ export const SplitviewReact = React.forwardRef( }, [props.components]); return ( -
+
{portals}
); From c122bfd310c21491fddcf3ebfd33ce2fabaa5610 Mon Sep 17 00:00:00 2001 From: mathuo <6710312+mathuo@users.noreply.github.com> Date: Fri, 10 Jan 2025 22:09:24 +0000 Subject: [PATCH 2/2] fixup demo wrt React.StrictMode --- .../react/dockview/demo-dockview/src/app.tsx | 143 ++++++++++-------- .../demo-dockview/src/groupActions.tsx | 2 - packages/docs/src/theme/Root.tsx | 6 +- 3 files changed, 85 insertions(+), 66 deletions(-) diff --git a/packages/docs/sandboxes/react/dockview/demo-dockview/src/app.tsx b/packages/docs/sandboxes/react/dockview/demo-dockview/src/app.tsx index a9f4ea97c..896e3968d 100644 --- a/packages/docs/sandboxes/react/dockview/demo-dockview/src/app.tsx +++ b/packages/docs/sandboxes/react/dockview/demo-dockview/src/app.tsx @@ -176,75 +176,92 @@ const DockviewDemo = (props: { theme?: string }) => { setPending([]); }, [pending]); - const onReady = (event: DockviewReadyEvent) => { - setApi(event.api); - - event.api.onDidAddPanel((event) => { - setPanels((_) => [..._, event.id]); - addLogLine(`Panel Added ${event.id}`); - }); - event.api.onDidActivePanelChange((event) => { - setActivePanel(event?.id); - addLogLine(`Panel Activated ${event?.id}`); - }); - event.api.onDidRemovePanel((event) => { - setPanels((_) => { - const next = [..._]; - next.splice( - next.findIndex((x) => x === event.id), - 1 - ); + React.useEffect(() => { + if (!api) { + return; + } - return next; - }); - addLogLine(`Panel Removed ${event.id}`); - }); - - event.api.onDidAddGroup((event) => { - setGroups((_) => [..._, event.id]); - addLogLine(`Group Added ${event.id}`); - }); - - event.api.onDidMovePanel((event) => { - addLogLine(`Panel Moved ${event.panel.id}`); - }); - - event.api.onDidMaximizedGroupChange((event) => { - addLogLine( - `Group Maximized Changed ${event.group.api.id} [${event.isMaximized}]` - ); - }); - - event.api.onDidRemoveGroup((event) => { - setGroups((_) => { - const next = [..._]; - next.splice( - next.findIndex((x) => x === event.id), - 1 + const disposables = [ + api.onDidAddPanel((event) => { + setPanels((_) => [..._, event.id]); + addLogLine(`Panel Added ${event.id}`); + }), + api.onDidActivePanelChange((event) => { + setActivePanel(event?.id); + addLogLine(`Panel Activated ${event?.id}`); + }), + api.onDidRemovePanel((event) => { + setPanels((_) => { + const next = [..._]; + next.splice( + next.findIndex((x) => x === event.id), + 1 + ); + + return next; + }); + addLogLine(`Panel Removed ${event.id}`); + }), + + api.onDidAddGroup((event) => { + setGroups((_) => [..._, event.id]); + addLogLine(`Group Added ${event.id}`); + }), + + api.onDidMovePanel((event) => { + addLogLine(`Panel Moved ${event.panel.id}`); + }), + + api.onDidMaximizedGroupChange((event) => { + addLogLine( + `Group Maximized Changed ${event.group.api.id} [${event.isMaximized}]` ); + }), + + api.onDidRemoveGroup((event) => { + setGroups((_) => { + const next = [..._]; + next.splice( + next.findIndex((x) => x === event.id), + 1 + ); + + return next; + }); + addLogLine(`Group Removed ${event.id}`); + }), + + api.onDidActiveGroupChange((event) => { + setActiveGroup(event?.id); + addLogLine(`Group Activated ${event?.id}`); + }), + ]; + + const loadLayout = () => { + const state = localStorage.getItem('dv-demo-state'); + + if (state) { + try { + api.fromJSON(JSON.parse(state)); + return; + } catch { + localStorage.removeItem('dv-demo-state'); + } + return; + } - return next; - }); - addLogLine(`Group Removed ${event.id}`); - }); + defaultConfig(api); + }; - event.api.onDidActiveGroupChange((event) => { - setActiveGroup(event?.id); - addLogLine(`Group Activated ${event?.id}`); - }); + loadLayout(); - const state = localStorage.getItem('dv-demo-state'); - if (state) { - try { - event.api.fromJSON(JSON.parse(state)); - return; - } catch { - localStorage.removeItem('dv-demo-state'); - } - return; - } + return () => { + disposables.forEach((disposable) => disposable.dispose()); + }; + }, [api]); - defaultConfig(event.api); + const onReady = (event: DockviewReadyEvent) => { + setApi(event.api); }; const [watermark, setWatermark] = React.useState(false); diff --git a/packages/docs/sandboxes/react/dockview/demo-dockview/src/groupActions.tsx b/packages/docs/sandboxes/react/dockview/demo-dockview/src/groupActions.tsx index 815621b72..0af2a66cc 100644 --- a/packages/docs/sandboxes/react/dockview/demo-dockview/src/groupActions.tsx +++ b/packages/docs/sandboxes/react/dockview/demo-dockview/src/groupActions.tsx @@ -88,7 +88,6 @@ const GroupAction = (props: { } onClick={() => { if (group) { - props.api.addFloatingGroup(group, { width: 400, height: 300, @@ -99,7 +98,6 @@ const GroupAction = (props: { right: 50, }, }); - } }} > diff --git a/packages/docs/src/theme/Root.tsx b/packages/docs/src/theme/Root.tsx index 516775b04..eb8c4705e 100644 --- a/packages/docs/src/theme/Root.tsx +++ b/packages/docs/src/theme/Root.tsx @@ -3,5 +3,9 @@ import { RecoilRoot } from 'recoil'; // Default implementation, that you can customize export default function Root({ children }) { - return {children}; + return ( + + {children} + + ); }