From 9ae54bd779b7c4fd57828d1c89b8f96ca00180eb Mon Sep 17 00:00:00 2001 From: mathuo <6710312+mathuo@users.noreply.github.com> Date: Thu, 10 Mar 2022 22:06:55 +0000 Subject: [PATCH 1/2] chore: refactor to use existing code --- .../dockview/src/dnd/abstractDragHandler.ts | 15 ++-- packages/dockview/src/groupview/tab.ts | 84 ++++++++----------- packages/dockview/src/lifecycle.ts | 1 + 3 files changed, 45 insertions(+), 55 deletions(-) diff --git a/packages/dockview/src/dnd/abstractDragHandler.ts b/packages/dockview/src/dnd/abstractDragHandler.ts index 1e45bc6ac..a80bd74ba 100644 --- a/packages/dockview/src/dnd/abstractDragHandler.ts +++ b/packages/dockview/src/dnd/abstractDragHandler.ts @@ -1,14 +1,15 @@ +import { MutableDisposable } from '..'; import { getElementsByTagName } from '../dom'; import { addDisposableListener, Emitter } from '../events'; import { CompositeDisposable, IDisposable } from '../lifecycle'; export abstract class DragHandler extends CompositeDisposable { - private iframes: HTMLElement[] = []; + private readonly disposable = new MutableDisposable(); private readonly _onDragStart = new Emitter(); readonly onDragStart = this._onDragStart.event; - private disposable: IDisposable | undefined; + private iframes: HTMLElement[] = []; constructor(private readonly el: HTMLElement) { super(); @@ -32,8 +33,11 @@ export abstract class DragHandler extends CompositeDisposable { this.el.classList.add('dragged'); setTimeout(() => this.el.classList.remove('dragged'), 0); - this.disposable?.dispose(); - this.disposable = this.getData(); + this.disposable.value = this.getData(); + + if (event.dataTransfer) { + event.dataTransfer.effectAllowed = 'move'; + } }), addDisposableListener(this.el, 'dragend', (ev) => { for (const iframe of this.iframes) { @@ -41,8 +45,7 @@ export abstract class DragHandler extends CompositeDisposable { } this.iframes = []; - this.disposable?.dispose(); - this.disposable = undefined; + this.disposable.dispose(); }) ); } diff --git a/packages/dockview/src/groupview/tab.ts b/packages/dockview/src/groupview/tab.ts index ae90b1a48..34a82a67e 100644 --- a/packages/dockview/src/groupview/tab.ts +++ b/packages/dockview/src/groupview/tab.ts @@ -1,17 +1,18 @@ import { addDisposableListener, Emitter, Event } from '../events'; -import { CompositeDisposable } from '../lifecycle'; +import { CompositeDisposable, IDisposable } from '../lifecycle'; import { getPanelData, LocalSelectionTransfer, PanelTransfer, } from '../dnd/dataTransfer'; -import { getElementsByTagName, toggleClass } from '../dom'; +import { toggleClass } from '../dom'; import { IDockviewComponent } from '../dockview/dockviewComponent'; import { ITabRenderer } from './types'; import { IGroupPanel } from './groupPanel'; import { GroupviewPanel } from './groupviewPanel'; import { DroptargetEvent, Droptarget } from '../dnd/droptarget'; import { DockviewDropTargets } from './dnd'; +import { DragHandler } from '../dnd/abstractDragHandler'; export enum MouseEventKind { CLICK = 'CLICK', @@ -19,15 +20,15 @@ export enum MouseEventKind { } export interface LayoutMouseEvent { - kind: MouseEventKind; - event: MouseEvent; - panel?: IGroupPanel; - tab?: boolean; + readonly kind: MouseEventKind; + readonly event: MouseEvent; + readonly panel?: IGroupPanel; + readonly tab?: boolean; } export interface ITab { - panelId: string; - element: HTMLElement; + readonly panelId: string; + readonly element: HTMLElement; setContent: (element: ITabRenderer) => void; onChanged: Event; onDrop: Event; @@ -35,8 +36,8 @@ export interface ITab { } export class Tab extends CompositeDisposable implements ITab { - private _element: HTMLElement; - private droptarget: Droptarget; + private readonly _element: HTMLElement; + private readonly droptarget: Droptarget; private content?: ITabRenderer; private readonly _onChanged = new Emitter(); @@ -45,19 +46,14 @@ export class Tab extends CompositeDisposable implements ITab { private readonly _onDropped = new Emitter(); readonly onDrop: Event = this._onDropped.event; - private readonly panelTransfer = - LocalSelectionTransfer.getInstance(); - public get element() { return this._element; } - private iframes: HTMLElement[] = []; - constructor( - public panelId: string, - private readonly accessor: IDockviewComponent, - private group: GroupviewPanel + public readonly panelId: string, + accessor: IDockviewComponent, + private readonly group: GroupviewPanel ) { super(); @@ -69,42 +65,32 @@ export class Tab extends CompositeDisposable implements ITab { this._element.draggable = true; this.addDisposables( - addDisposableListener(this._element, 'dragstart', (event) => { - this.iframes = [ - ...getElementsByTagName('iframe'), - ...getElementsByTagName('webview'), - ]; + new (class Handler extends DragHandler { + private readonly panelTransfer = + LocalSelectionTransfer.getInstance(); - for (const iframe of this.iframes) { - iframe.style.pointerEvents = 'none'; + getData(): IDisposable { + this.panelTransfer.setData( + [new PanelTransfer(accessor.id, group.id, panelId)], + PanelTransfer.prototype + ); + + return { + dispose: () => { + this.panelTransfer.clearData( + PanelTransfer.prototype + ); + }, + }; } - this.element.classList.add('dragged'); - setTimeout(() => this.element.classList.remove('dragged'), 0); - - this.panelTransfer.setData( - [ - new PanelTransfer( - this.accessor.id, - this.group.id, - this.panelId - ), - ], - PanelTransfer.prototype - ); - - if (event.dataTransfer) { - event.dataTransfer.effectAllowed = 'move'; + public dispose(): void { + // } - }), - addDisposableListener(this._element, 'dragend', (ev) => { - for (const iframe of this.iframes) { - iframe.style.pointerEvents = 'auto'; - } - this.iframes = []; + })(this._element) + ); - this.panelTransfer.clearData(PanelTransfer.prototype); - }), + this.addDisposables( addDisposableListener(this._element, 'mousedown', (event) => { if (event.defaultPrevented) { return; diff --git a/packages/dockview/src/lifecycle.ts b/packages/dockview/src/lifecycle.ts index 2e6d04540..0486cedff 100644 --- a/packages/dockview/src/lifecycle.ts +++ b/packages/dockview/src/lifecycle.ts @@ -48,6 +48,7 @@ export class MutableDisposable implements IDisposable { public dispose() { if (this._disposable) { this._disposable.dispose(); + this._disposable = Disposable.NONE; } } } From b42d8f985245530dadc7201962985f1552cdbb2d Mon Sep 17 00:00:00 2001 From: mathuo <6710312+mathuo@users.noreply.github.com> Date: Thu, 10 Mar 2022 22:37:42 +0000 Subject: [PATCH 2/2] feat: dispose of event listeners --- packages/dockview/src/api/gridviewPanelApi.ts | 6 ++++++ packages/dockview/src/api/groupPanelApi.ts | 10 ++++++---- packages/dockview/src/api/panelApi.ts | 2 ++ packages/dockview/src/api/paneviewPanelApi.ts | 6 ++++++ packages/dockview/src/api/splitviewPanelApi.ts | 12 ++++++------ packages/dockview/src/dnd/abstractDragHandler.ts | 1 + packages/dockview/src/dnd/droptarget.ts | 2 +- .../dockview/src/dockview/dockviewComponent.ts | 15 ++++++--------- packages/dockview/src/gridview/branchNode.ts | 1 + packages/dockview/src/gridview/gridviewPanel.ts | 5 +---- packages/dockview/src/gridview/leafNode.ts | 1 + packages/dockview/src/groupview/panel/content.ts | 2 ++ packages/dockview/src/paneview/paneview.ts | 1 + .../dockview/src/paneview/paneviewComponent.ts | 7 +++++++ .../src/react/dockview/reactContentPart.ts | 2 ++ .../dockview/src/splitview/splitviewComponent.ts | 7 ++++++- packages/dockview/src/splitview/splitviewPanel.ts | 1 + 17 files changed, 56 insertions(+), 25 deletions(-) diff --git a/packages/dockview/src/api/gridviewPanelApi.ts b/packages/dockview/src/api/gridviewPanelApi.ts index 8523a0c4f..34495c64e 100644 --- a/packages/dockview/src/api/gridviewPanelApi.ts +++ b/packages/dockview/src/api/gridviewPanelApi.ts @@ -50,6 +50,12 @@ export class GridviewPanelApiImpl constructor(id: string) { super(id); + + this.addDisposables( + this._onDidConstraintsChangeInternal, + this._onDidConstraintsChange, + this._onDidSizeChange + ); } public setConstraints(value: GridConstraintChangeEvent) { diff --git a/packages/dockview/src/api/groupPanelApi.ts b/packages/dockview/src/api/groupPanelApi.ts index 2dc4a1765..1690cd02d 100644 --- a/packages/dockview/src/api/groupPanelApi.ts +++ b/packages/dockview/src/api/groupPanelApi.ts @@ -69,6 +69,12 @@ export class DockviewPanelApiImpl constructor(private panel: IGroupPanel, group: GroupviewPanel | undefined) { super(panel.id); this._group = group; + + this.addDisposables( + this._onDidTitleChange, + this._titleChanged, + this._suppressClosableChanged + ); } public setTitle(title: string) { @@ -85,8 +91,4 @@ export class DockviewPanelApiImpl public interceptOnCloseAction(interceptor: () => Promise) { this._interceptor = interceptor; } - - public dispose() { - super.dispose(); - } } diff --git a/packages/dockview/src/api/panelApi.ts b/packages/dockview/src/api/panelApi.ts index 5609febe9..88e657531 100644 --- a/packages/dockview/src/api/panelApi.ts +++ b/packages/dockview/src/api/panelApi.ts @@ -126,6 +126,8 @@ export class PanelApiImpl extends CompositeDisposable implements PanelApi { this._onDidVisibilityChange, this._onDidActiveChange, this._onFocusEvent, + this._onActiveChange, + this._onVisibilityChange, this.onDidFocusChange((event) => { this._isFocused = event.isFocused; }), diff --git a/packages/dockview/src/api/paneviewPanelApi.ts b/packages/dockview/src/api/paneviewPanelApi.ts index 4391edf66..c90a70db4 100644 --- a/packages/dockview/src/api/paneviewPanelApi.ts +++ b/packages/dockview/src/api/paneviewPanelApi.ts @@ -37,6 +37,12 @@ export class PaneviewPanelApiImpl constructor(id: string) { super(id); + + this.addDisposables( + this._onDidExpansionChange, + this._onMouseEnter, + this._onMouseLeave + ); } setExpanded(isExpanded: boolean): void { diff --git a/packages/dockview/src/api/splitviewPanelApi.ts b/packages/dockview/src/api/splitviewPanelApi.ts index a90f71639..cfe3cd103 100644 --- a/packages/dockview/src/api/splitviewPanelApi.ts +++ b/packages/dockview/src/api/splitviewPanelApi.ts @@ -47,6 +47,12 @@ export class SplitviewPanelApiImpl constructor(id: string) { super(id); + + this.addDisposables( + this._onDidConstraintsChangeInternal, + this._onDidConstraintsChange, + this._onDidSizeChange + ); } setConstraints(value: PanelConstraintChangeEvent2) { @@ -56,10 +62,4 @@ export class SplitviewPanelApiImpl setSize(event: PanelSizeEvent) { this._onDidSizeChange.fire(event); } - - dispose() { - super.dispose(); - this._onDidConstraintsChange.dispose(); - this._onDidSizeChange.dispose(); - } } diff --git a/packages/dockview/src/dnd/abstractDragHandler.ts b/packages/dockview/src/dnd/abstractDragHandler.ts index a80bd74ba..d21c8ad7d 100644 --- a/packages/dockview/src/dnd/abstractDragHandler.ts +++ b/packages/dockview/src/dnd/abstractDragHandler.ts @@ -20,6 +20,7 @@ export abstract class DragHandler extends CompositeDisposable { private configure() { this.addDisposables( + this._onDragStart, addDisposableListener(this.el, 'dragstart', (event) => { this.iframes = [ ...getElementsByTagName('iframe'), diff --git a/packages/dockview/src/dnd/droptarget.ts b/packages/dockview/src/dnd/droptarget.ts index 059bde19d..fc3ff4bba 100644 --- a/packages/dockview/src/dnd/droptarget.ts +++ b/packages/dockview/src/dnd/droptarget.ts @@ -56,6 +56,7 @@ export class Droptarget extends CompositeDisposable { super(); this.addDisposables( + this._onDrop, new DragAndDropObserver(this.element, { onDragEnter: (e) => undefined, onDragOver: (e) => { @@ -177,7 +178,6 @@ export class Droptarget extends CompositeDisposable { } public dispose() { - this._onDrop.dispose(); this.removeDropTarget(); } diff --git a/packages/dockview/src/dockview/dockviewComponent.ts b/packages/dockview/src/dockview/dockviewComponent.ts index 0bc59136e..658bac7f8 100644 --- a/packages/dockview/src/dockview/dockviewComponent.ts +++ b/packages/dockview/src/dockview/dockviewComponent.ts @@ -186,6 +186,12 @@ export class DockviewComponent styles: options.styles, }); + this.addDisposables( + this._onTabInteractionEvent, + this._onTabContextMenu, + this._onDidDrop + ); + this._options = options; if (!this.options.components) { @@ -719,15 +725,6 @@ export class DockviewComponent return view; } - dispose(): void { - super.dispose(); - - this._onGridEvent.dispose(); - this._onDidDrop.dispose(); - this._onTabContextMenu.dispose(); - this._onTabInteractionEvent.dispose(); - } - private _addPanel(options: AddPanelOptions): IGroupPanel { const view = new DefaultGroupPanelView({ content: this.createContentComponent(options.id, options.component), diff --git a/packages/dockview/src/gridview/branchNode.ts b/packages/dockview/src/gridview/branchNode.ts index 159561a28..d915f90b3 100644 --- a/packages/dockview/src/gridview/branchNode.ts +++ b/packages/dockview/src/gridview/branchNode.ts @@ -155,6 +155,7 @@ export class BranchNode extends CompositeDisposable implements IView { } this.addDisposables( + this._onDidChange, this.splitview.onDidSashEnd(() => { this._onDidChange.fire(undefined); }) diff --git a/packages/dockview/src/gridview/gridviewPanel.ts b/packages/dockview/src/gridview/gridviewPanel.ts index 29474976b..8d49c8cd9 100644 --- a/packages/dockview/src/gridview/gridviewPanel.ts +++ b/packages/dockview/src/gridview/gridviewPanel.ts @@ -129,6 +129,7 @@ export abstract class GridviewPanel super(id, component, api); this.addDisposables( + this._onDidChange, this.api.onVisibilityChange((event) => { const { isVisible } = event; const { containerApi } = this._params as GridviewInitParameters; @@ -230,10 +231,6 @@ export abstract class GridviewPanel priority: this.priority, }; } - - dispose() { - super.dispose(); - } } export interface GridPanelViewState extends BasePanelViewState { diff --git a/packages/dockview/src/gridview/leafNode.ts b/packages/dockview/src/gridview/leafNode.ts index 7bed7c10b..5da71ad7f 100644 --- a/packages/dockview/src/gridview/leafNode.ts +++ b/packages/dockview/src/gridview/leafNode.ts @@ -128,6 +128,7 @@ export class LeafNode implements IView { } public dispose() { + this._onDidChange.dispose(); this._disposable.dispose(); } } diff --git a/packages/dockview/src/groupview/panel/content.ts b/packages/dockview/src/groupview/panel/content.ts index 35418628f..21dd13248 100644 --- a/packages/dockview/src/groupview/panel/content.ts +++ b/packages/dockview/src/groupview/panel/content.ts @@ -42,6 +42,8 @@ export class ContentContainer this._element.className = 'content-container'; this._element.tabIndex = -1; + this.addDisposables(this._onDidFocus, this._onDidBlur); + // for hosted containers // 1) register a drop target on the host // 2) register window dragStart events to disable pointer events diff --git a/packages/dockview/src/paneview/paneview.ts b/packages/dockview/src/paneview/paneview.ts index b04ed9643..8e09033c4 100644 --- a/packages/dockview/src/paneview/paneview.ts +++ b/packages/dockview/src/paneview/paneview.ts @@ -95,6 +95,7 @@ export class Paneview extends CompositeDisposable implements IDisposable { }); this.addDisposables( + this._onDidChange, this.splitview.onDidSashEnd(() => { this._onDidChange.fire(undefined); }), diff --git a/packages/dockview/src/paneview/paneviewComponent.ts b/packages/dockview/src/paneview/paneviewComponent.ts index f2aff1b94..642b8a730 100644 --- a/packages/dockview/src/paneview/paneviewComponent.ts +++ b/packages/dockview/src/paneview/paneviewComponent.ts @@ -185,6 +185,13 @@ export class PaneviewComponent ) { super(); + this.addDisposables( + this._onDidLayoutChange, + this._onDidDrop, + this._onDidAddView, + this._onDidRemoveView + ); + this._options = options; if (!options.components) { diff --git a/packages/dockview/src/react/dockview/reactContentPart.ts b/packages/dockview/src/react/dockview/reactContentPart.ts index aa6db0abd..b1a73699c 100644 --- a/packages/dockview/src/react/dockview/reactContentPart.ts +++ b/packages/dockview/src/react/dockview/reactContentPart.ts @@ -124,6 +124,8 @@ export class ReactPanelContentPart implements IContentRenderer { } public dispose() { + this._onDidFocus.dispose(); + this._onDidBlur.dispose(); this.part?.dispose(); // this.hostedContainer?.dispose(); this.actionsPart?.dispose(); diff --git a/packages/dockview/src/splitview/splitviewComponent.ts b/packages/dockview/src/splitview/splitviewComponent.ts index 96bf62041..fd3c34733 100644 --- a/packages/dockview/src/splitview/splitviewComponent.ts +++ b/packages/dockview/src/splitview/splitviewComponent.ts @@ -168,7 +168,12 @@ export class SplitviewComponent this.splitview = new Splitview(this.element, options); - this.addDisposables(this._disposable); + this.addDisposables( + this._disposable, + this._onDidAddView, + this._onDidRemoveView, + this._onDidLayoutChange + ); } updateOptions(options: Partial): void { diff --git a/packages/dockview/src/splitview/splitviewPanel.ts b/packages/dockview/src/splitview/splitviewPanel.ts index 60337a255..3b2a21886 100644 --- a/packages/dockview/src/splitview/splitviewPanel.ts +++ b/packages/dockview/src/splitview/splitviewPanel.ts @@ -82,6 +82,7 @@ export abstract class SplitviewPanel super(id, componentName, new SplitviewPanelApiImpl(id)); this.addDisposables( + this._onDidChange, this.api.onVisibilityChange((event) => { const { isVisible } = event; const { containerApi } = this