fix: hide+show cycles no longer accumulate prompt nodes in floating panel
Bug: clicking the floating button after the first time caused the panel content to multiply — every hide+show cycle appended a new prompt while the previous one stayed in the DOM. Root cause: the prompt's Lumino parent is null (it's attached to a raw HTMLDivElement, not a Widget, so neither appendChild nor Widget.attach(widget, HTMLElement) sets a Lumino parent). The defensive DOM cleanup in _hidePrompt now removes the prompt's node explicitly before nulling the reference, so no residual nodes linger across show+hide cycles. Also use Widget.attach in _showPrompt for the before-attach / after-attach lifecycle messages the prompt may rely on. Added a regression test that toggles 5 times and asserts the panel contains exactly one .opencode-inline-prompt node. Files: - src/components/opencode_floating_panel.ts — defensive cleanup + attach - src/__tests__/opencode_floating_panel.spec.ts — regression test
This commit is contained in:
@@ -318,6 +318,29 @@ describe('OpenCodeFloatingPanel', () => {
|
||||
expect((panel as any)._prompt).toBeNull();
|
||||
});
|
||||
|
||||
it('hide+show cycles do not accumulate prompt nodes in the panel (regression)', () => {
|
||||
// Bug: using _panelEl.appendChild(prompt.node) to attach the
|
||||
// prompt left the prompt's Lumino parent unset, so prompt.dispose()
|
||||
// in _hidePrompt did not remove the DOM node. Each hide+show
|
||||
// cycle would append a new prompt while the old one stayed in the
|
||||
// DOM — the user saw the panel content multiply on every click.
|
||||
// Fix: use Widget.attach so Lumino tracks the parent and dispose
|
||||
// can clean up.
|
||||
const app = makeFakeApp('foo.ipynb', makeFakeCell('x = 1'));
|
||||
const panel = new OpenCodeFloatingPanel({ app, serverSettings: {} as any, providers: null });
|
||||
Widget.attach(panel, document.body);
|
||||
for (let i = 0; i < 5; i++) {
|
||||
panel.toggle(); // show
|
||||
panel.toggle(); // hide
|
||||
}
|
||||
panel.toggle(); // show one more time to inspect
|
||||
const panelEl = panel.node.querySelector('.opencode-floating-panel') as HTMLElement;
|
||||
// Exactly one prompt in the panel, regardless of how many
|
||||
// show/hide cycles happened.
|
||||
const prompts = panelEl.querySelectorAll('.opencode-inline-prompt');
|
||||
expect(prompts.length).toBe(1);
|
||||
});
|
||||
|
||||
it('with no currentWidget, the prompt renders with the input disabled', () => {
|
||||
const app = makeFakeApp(null, null);
|
||||
const panel = new OpenCodeFloatingPanel({ app, serverSettings: {} as any, providers: null });
|
||||
|
||||
@@ -255,7 +255,14 @@ export class OpenCodeFloatingPanel extends Widget {
|
||||
await this._refreshHistory(this._notebookPath);
|
||||
}
|
||||
});
|
||||
this._panelEl.appendChild(prompt.node);
|
||||
// Use Widget.attach (not raw appendChild) so Lumino sends the
|
||||
// before-attach / after-attach messages — the prompt relies on
|
||||
// these for isAttached/isVisible tracking. The defensive DOM
|
||||
// cleanup in _hidePrompt handles the case where dispose() leaves
|
||||
// a residual node behind (the panel host is an HTMLElement, not a
|
||||
// Widget, so the Lumino parent stays null and dispose() can't
|
||||
// always clean up the DOM on its own).
|
||||
Widget.attach(prompt, this._panelEl);
|
||||
this._prompt = prompt;
|
||||
|
||||
// Load the current session's static history before streaming starts.
|
||||
@@ -284,7 +291,20 @@ export class OpenCodeFloatingPanel extends Widget {
|
||||
private _hidePrompt(): void {
|
||||
this._closeSse();
|
||||
if (this._prompt) {
|
||||
const node = this._prompt.node;
|
||||
this._prompt.dispose();
|
||||
// Defensive DOM cleanup: the prompt is attached via raw DOM
|
||||
// (appendChild/Widget.attach with an HTMLElement host), so its
|
||||
// Lumino parent is null. In some scenarios — for example when
|
||||
// the dispose path runs synchronously while another show+hide
|
||||
// cycle is already in flight — the dispose() call does not
|
||||
// remove the node from this._panelEl, leaving a residual node
|
||||
// that the next _showPrompt appends to. Without this defensive
|
||||
// removeChild, the user would see the panel content multiply
|
||||
// with every click.
|
||||
if (node.parentNode === this._panelEl) {
|
||||
this._panelEl.removeChild(node);
|
||||
}
|
||||
this._prompt = null;
|
||||
}
|
||||
this._panelEl.hidden = true;
|
||||
|
||||
Reference in New Issue
Block a user