Merge pull request #843 from mathuo/842-api-init-bug

bug: setup onDidAcitvePanelChange subscription quicker
mathuo · Jan 28, 2025 · bb93c9e · bb93c9e
2 parents ca09ae5 + c6dcef5
commit bb93c9e
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 23 deletions.
diff --git a/packages/dockview-core/src/__tests__/dockview/dockviewGroupPanel.spec.ts b/packages/dockview-core/src/__tests__/dockview/dockviewGroupPanel.spec.ts
@@ -2,10 +2,12 @@ import { DockviewComponent } from '../../dockview/dockviewComponent';
 import { DockviewGroupPanel } from '../../dockview/dockviewGroupPanel';
 import { fromPartial } from '@total-typescript/shoehorn';
 import { GroupOptions } from '../../dockview/dockviewGroupPanelModel';
-import { DockviewPanel } from '../../dockview/dockviewPanel';
+import { DockviewPanel, IDockviewPanel } from '../../dockview/dockviewPanel';
 import { DockviewPanelModelMock } from '../__mocks__/mockDockviewPanelModel';
 import { IContentRenderer, ITabRenderer } from '../../dockview/types';
 import { OverlayRenderContainer } from '../../overlay/overlayRenderContainer';
+import { IDockviewPanelModel } from '../../dockview/dockviewPanelModel';
+import { ContentContainer } from '../../dockview/components/panel/content';
 
 describe('dockviewGroupPanel', () => {
     test('default minimum/maximium width/height', () => {
@@ -24,6 +26,50 @@ describe('dockviewGroupPanel', () => {
         expect(cut.maximumWidth).toBe(Number.MAX_SAFE_INTEGER);
     });
 
+    test('that onDidActivePanelChange is configured at inline', () => {
+        const accessor = fromPartial<DockviewComponent>({
+            onDidActivePanelChange: jest.fn(),
+            onDidAddPanel: jest.fn(),
+            onDidRemovePanel: jest.fn(),
+            options: {},
+            api: {},
+            renderer: 'always',
+            overlayRenderContainer: {
+                attach: jest.fn(),
+                detatch: jest.fn(),
+            },
+            doSetGroupActive: jest.fn(),
+        });
+        const options = fromPartial<GroupOptions>({});
+
+        const cut = new DockviewGroupPanel(accessor, 'test_id', options);
+
+        let counter = 0;
+
+        cut.api.onDidActivePanelChange((event) => {
+            counter++;
+        });
+
+        cut.model.openPanel(
+            fromPartial<IDockviewPanel>({
+                updateParentGroup: jest.fn(),
+                view: {
+                    tab: { element: document.createElement('div') },
+                    content: new ContentContainer(accessor, cut.model),
+                },
+                api: {
+                    renderer: 'onlyWhenVisible',
+                    onDidTitleChange: jest.fn(),
+                    onDidParametersChange: jest.fn(),
+                },
+                layout: jest.fn(),
+                runEvents: jest.fn(),
+            })
+        );
+
+        expect(counter).toBe(1);
+    });
+
     test('group constraints', () => {
         const accessor = fromPartial<DockviewComponent>({
             onDidActivePanelChange: jest.fn(),

diff --git a/packages/dockview-core/src/api/dockviewGroupPanelApi.ts b/packages/dockview-core/src/api/dockviewGroupPanelApi.ts
@@ -6,7 +6,6 @@ import {
     DockviewGroupLocation,
 } from '../dockview/dockviewGroupPanelModel';
 import { Emitter, Event } from '../events';
-import { MutableDisposable } from '../lifecycle';
 import { GridviewPanelApi, GridviewPanelApiImpl } from './gridviewPanelApi';
 
 export interface DockviewGroupMoveParams {
@@ -41,17 +40,14 @@ const NOT_INITIALIZED_MESSAGE =
     'dockview: DockviewGroupPanelApiImpl not initialized';
 
 export class DockviewGroupPanelApiImpl extends GridviewPanelApiImpl {
-    private readonly _mutableDisposable = new MutableDisposable();
-
     private _group: DockviewGroupPanel | undefined;
 
     readonly _onDidLocationChange =
         new Emitter<DockviewGroupPanelFloatingChangeEvent>();
     readonly onDidLocationChange: Event<DockviewGroupPanelFloatingChangeEvent> =
         this._onDidLocationChange.event;
 
-    private readonly _onDidActivePanelChange =
-        new Emitter<DockviewGroupChangeEvent>();
+    readonly _onDidActivePanelChange = new Emitter<DockviewGroupChangeEvent>();
     readonly onDidActivePanelChange = this._onDidActivePanelChange.event;
 
     get location(): DockviewGroupLocation {
@@ -66,8 +62,7 @@ export class DockviewGroupPanelApiImpl extends GridviewPanelApiImpl {
 
         this.addDisposables(
             this._onDidLocationChange,
-            this._onDidActivePanelChange,
-            this._mutableDisposable
+            this._onDidActivePanelChange
         );
     }
 
@@ -140,21 +135,6 @@ export class DockviewGroupPanelApiImpl extends GridviewPanelApiImpl {
     }
 
     initialize(group: DockviewGroupPanel): void {
-        /**
-         * TODO: Annoying initialization order caveat, find a better way to initialize and avoid needing null checks
-         *
-         * Due to the order on initialization we know that the model isn't defined until later in the same stack-frame of setup.
-         * By queuing a microtask we can ensure the setup is completed within the same stack-frame, but after everything else has
-         * finished ensuring the `model` is defined.
-         */
-
         this._group = group;
-
-        queueMicrotask(() => {
-            this._mutableDisposable.value =
-                this._group!.model.onDidActivePanelChange((event) => {
-                    this._onDidActivePanelChange.fire(event);
-                });
-        });
     }
 }
diff --git a/packages/dockview-core/src/dockview/dockviewGroupPanel.ts b/packages/dockview-core/src/dockview/dockviewGroupPanel.ts
@@ -124,6 +124,12 @@ export class DockviewGroupPanel
             options,
             this
         );
+
+        this.addDisposables(
+            this.model.onDidActivePanelChange((event) => {
+                this.api._onDidActivePanelChange.fire(event);
+            })
+        );
     }
 
     override focus(): void {