fix: move registerPanel after doAddGroup to prevent undefined API errors

This fixes a timing issue where registerPanel was called before doAddGroup,
causing "Cannot read properties of undefined (reading 'onDidFocusChange')"
errors when adding panels with position references.

The fix ensures the panel is fully integrated into the grid before
registering event handlers, making the order consistent with the fromJSON
method which also calls registerPanel after grid integration.

Changes:
- Move registerPanel call after doAddGroup in addPanel method
- Add test to verify the fix and prevent regressions
- Ensure API events are accessible immediately after panel creation

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
mathuo
2025-10-30 22:09:24 +00:00
parent 9e6eaf5c77
commit 7397dde9b9
2 changed files with 66 additions and 2 deletions

View File

@@ -2936,4 +2936,69 @@ describe('gridview', () => {
expect(panel1.api.isVisible).toBeTruthy();
expect(panel2.api.isVisible).toBeTruthy();
});
test('registerPanel is called after doAddGroup - panel api events work immediately', () => {
// This test verifies the fix for the timing issue where registerPanel
// was called before doAddGroup, causing "Cannot read properties of undefined" errors
const gridview = new GridviewComponent(container, {
proportionalLayout: false,
orientation: Orientation.VERTICAL,
createComponent: (options) => {
switch (options.name) {
case 'default':
return new TestGridview(options.id, options.name);
default:
throw new Error('unsupported');
}
},
});
gridview.layout(800, 400);
// Add first panel
const panel1 = gridview.addPanel({
id: 'panel_1',
component: 'default',
});
// Verify the panel API is immediately accessible and functional
expect(panel1.api).toBeDefined();
expect(panel1.api.onDidFocusChange).toBeDefined();
// Subscribe to focus events to verify event subscription works
let focusEventCount = 0;
const disposable = panel1.api.onDidFocusChange((event) => {
focusEventCount++;
});
// This should not throw an error - before the fix, this would throw:
// "Cannot read properties of undefined (reading 'onDidFocusChange')"
const panel2 = gridview.addPanel({
id: 'panel_2',
component: 'default',
position: { referencePanel: panel1.id, direction: 'right' },
});
// Verify both panels have working APIs
expect(panel1.api).toBeDefined();
expect(panel2.api).toBeDefined();
expect(panel1.api.onDidFocusChange).toBeDefined();
expect(panel2.api.onDidFocusChange).toBeDefined();
// Verify that the API is functional by checking properties
expect(panel1.api.isVisible).toBeTruthy();
expect(panel2.api.isVisible).toBeTruthy();
// Verify we can subscribe to events on the second panel too
const disposable2 = panel2.api.onDidFocusChange((event) => {
focusEventCount++;
});
// Clean up
disposable.dispose();
disposable2.dispose();
// The main test is that we got this far without errors
expect(true).toBeTruthy();
});
});

View File

@@ -366,9 +366,8 @@ export class GridviewComponent
isVisible: true,
});
this.registerPanel(view);
this.doAddGroup(view, relativeLocation, options.size);
this.registerPanel(view);
this.doSetGroupActive(view);
return view;