diff --git a/src/info-panel/UIManager.ts b/src/info-panel/UIManager.ts index 64080d9..9958072 100644 --- a/src/info-panel/UIManager.ts +++ b/src/info-panel/UIManager.ts @@ -45,6 +45,7 @@ export class UIManager { elements: InfoPanelElements; nodeSelector: NodeSelector; currentDropdown: HTMLDivElement | null = null; + private currentDropdownCleanup: (() => void) | null = null; onNodeSelected: ((nodeId: number) => void) | null = null; // Track active widget editors for cleanup private activeEditors: Map = new Map(); @@ -1464,7 +1465,7 @@ export class UIManager { item.addEventListener('click', (e) => { e.stopPropagation(); this.hideDropdown(); - cleanup(); + // cleanup() is called by hideDropdown // Set selected node in state this.stateManager.setSelectedNode(node.id); @@ -1491,13 +1492,20 @@ export class UIManager { if (this.elements.panel) { this.elements.panel.removeEventListener('mousedown', panelCloseHandler, true); } + // Clear the global reference + if (this.currentDropdownCleanup === cleanup) { + this.currentDropdownCleanup = null; + } }; + // Store cleanup globally so hideDropdown can call it + this.currentDropdownCleanup = cleanup; + // Close on click outside - use mousedown with capture to ensure it fires before other handlers const closeHandler = (e: MouseEvent) => { if (!dropdown.contains(e.target as Node) && !anchorElement.contains(e.target as Node)) { this.hideDropdown(); - cleanup(); + // Cleanup is now handled by hideDropdown calling this.currentDropdownCleanup } }; @@ -1505,7 +1513,7 @@ export class UIManager { const panelCloseHandler = (e: MouseEvent) => { if (!dropdown.contains(e.target as Node) && !anchorElement.contains(e.target as Node)) { this.hideDropdown(); - cleanup(); + // Cleanup is now handled by hideDropdown calling this.currentDropdownCleanup } }; @@ -1544,7 +1552,7 @@ export class UIManager { e.preventDefault(); e.stopPropagation(); this.hideDropdown(); - cleanup(); + // Cleanup is now handled by hideDropdown return; } @@ -1591,6 +1599,12 @@ export class UIManager { * Hide the current dropdown. */ hideDropdown(): void { + // Run cleanup if it exists + if (this.currentDropdownCleanup) { + this.currentDropdownCleanup(); + this.currentDropdownCleanup = null; + } + if (this.currentDropdown && this.currentDropdown.parentNode) { this.currentDropdown.parentNode.removeChild(this.currentDropdown); this.currentDropdown = null; diff --git a/tests/unit/UIManager_Dropdown.test.ts b/tests/unit/UIManager_Dropdown.test.ts index e9c8818..ac822f5 100644 --- a/tests/unit/UIManager_Dropdown.test.ts +++ b/tests/unit/UIManager_Dropdown.test.ts @@ -126,17 +126,41 @@ describe('UIManager Dropdown Accessibility', () => { expect(document.activeElement).toBe(otherInput); // Arrow Down - should NOT change selection if focus is elsewhere - // We need to spy on preventDefault to verify it wasn't called, ensuring event propagates const event = new KeyboardEvent('keydown', { key: 'ArrowDown', bubbles: true, cancelable: true }); const preventDefaultSpy = vi.spyOn(event, 'preventDefault'); document.dispatchEvent(event); - // If the fix is implemented, preventDefault should NOT be called - // And the selection should NOT change expect(preventDefaultSpy).not.toHaveBeenCalled(); expect(items[1].getAttribute('aria-selected')).toBe('false'); // Should stay on first item vi.useRealTimers(); }); + + it('should cleanup previous event listeners when opening a new dropdown via anchor click', () => { + vi.useFakeTimers(); + const removeEventListenerSpy = vi.spyOn(document, 'removeEventListener'); + + const nodes = [{ id: 1, title: 'Node 1', type: 'Type A' }]; + const anchor = document.createElement('div'); + document.body.appendChild(anchor); + + // Open first time + (uiManager as any).createDropdown(nodes, anchor, 'title'); + vi.advanceTimersByTime(100); + + // Reset spy to track calls for the next action + removeEventListenerSpy.mockClear(); + + // Simulate re-opening (which calls hideDropdown then createDropdown) + // This simulates clicking the anchor again, or clicking another anchor which triggers hideDropdown first + (uiManager as any).showTitleDropdown(anchor); + + // Verify that cleanup for the FIRST dropdown occurred + // We expect removeEventListener to be called for the listeners attached by the first call + expect(removeEventListenerSpy).toHaveBeenCalledWith('keydown', expect.any(Function)); + expect(removeEventListenerSpy).toHaveBeenCalledWith('mousedown', expect.any(Function), true); + + vi.useRealTimers(); + }); });