From 8d7eec315f200998d3a0c509260451c96f297b1e Mon Sep 17 00:00:00 2001 From: atul-upadhyay-7 Date: Fri, 19 Jun 2026 22:16:41 +0530 Subject: [PATCH] fix(core): stop emitting spurious blur for non-focused widgets on unregister When unregistering a widget that precedes the focused one, FocusManager emitted a blur event for the removed widget even though it was never focused. Downstream handlers looking up this widget by ID got undefined and crashed. Only emit blur when the actually focused widget is removed. For non-focused widgets, silently adjust the index since the focused widget hasn't changed. Closes #1660 --- packages/core/src/events/FocusManager.test.ts | 99 +++++++++++++++++++ packages/core/src/events/FocusManager.ts | 17 +--- 2 files changed, 104 insertions(+), 12 deletions(-) diff --git a/packages/core/src/events/FocusManager.test.ts b/packages/core/src/events/FocusManager.test.ts index 2b886333..b25a7767 100644 --- a/packages/core/src/events/FocusManager.test.ts +++ b/packages/core/src/events/FocusManager.test.ts @@ -111,6 +111,105 @@ describe('FocusManager', () => { expect(fm.isFocused('a')).toBe(false); expect(fm.isFocused('b')).toBe(true); }); + + describe('unregister', () => { + it('does not emit blur when unregistering a non-focused widget', () => { + const fm = new FocusManager(); + const blurHandler = vi.fn(); + fm.on('blur', blurHandler); + + fm.register(makeWidget('a')); + fm.register(makeWidget('b')); + fm.register(makeWidget('c')); + // 'a' is focused + + fm.unregister('b'); // 'b' is NOT focused + + expect(blurHandler).not.toHaveBeenCalled(); + expect(fm.currentId).toBe('a'); + }); + + it('emits blur when unregistering the focused widget', () => { + const fm = new FocusManager(); + const blurHandler = vi.fn(); + fm.on('blur', blurHandler); + + fm.register(makeWidget('a')); + fm.register(makeWidget('b')); + // 'a' is focused + + fm.unregister('a'); + + expect(blurHandler).toHaveBeenCalledWith( + expect.objectContaining({ targetId: 'a', type: 'blur' }) + ); + }); + + it('moves focus to next widget when focused widget is unregistered', () => { + const fm = new FocusManager(); + fm.register(makeWidget('a')); + fm.register(makeWidget('b')); + fm.register(makeWidget('c')); + // 'a' is focused + + fm.unregister('a'); + + expect(fm.currentId).toBe('b'); + }); + + it('sets currentId to null when last widget is unregistered', () => { + const fm = new FocusManager(); + fm.register(makeWidget('a')); + + fm.unregister('a'); + + expect(fm.currentId).toBeNull(); + }); + + it('adjusts index correctly when non-focused widget before focused is removed', () => { + const fm = new FocusManager(); + fm.register(makeWidget('a')); + fm.register(makeWidget('b')); + fm.register(makeWidget('c')); + fm.focusWidget('c'); + // focused index = 2 + + fm.unregister('a'); + // 'c' should still be focused, index adjusted from 2 to 1 + + expect(fm.currentId).toBe('c'); + }); + + it('unregistering a non-focused widget after focused one does not affect focus', () => { + const fm = new FocusManager(); + fm.register(makeWidget('a')); + fm.register(makeWidget('b')); + fm.register(makeWidget('c')); + // 'a' is focused + + fm.unregister('c'); + + expect(fm.currentId).toBe('a'); + }); + + it('does not emit any events when unregistering a non-focused widget', () => { + const fm = new FocusManager(); + const focusHandler = vi.fn(); + const blurHandler = vi.fn(); + fm.on('focus', focusHandler); + fm.on('blur', blurHandler); + + fm.register(makeWidget('a')); + fm.register(makeWidget('b')); + focusHandler.mockClear(); + blurHandler.mockClear(); + + fm.unregister('b'); + + expect(focusHandler).not.toHaveBeenCalled(); + expect(blurHandler).not.toHaveBeenCalled(); + }); + }); }); describe('FocusManager Spatial Navigation', () => { diff --git a/packages/core/src/events/FocusManager.ts b/packages/core/src/events/FocusManager.ts index d19f2a72..338a385a 100644 --- a/packages/core/src/events/FocusManager.ts +++ b/packages/core/src/events/FocusManager.ts @@ -122,8 +122,9 @@ export class FocusManager { this._focusables.splice(idx, 1); if (wasFocused) { + // The focused widget is being removed — emit blur for it, then + // move focus to the next available widget if one exists. this._events.emit('blur', { targetId: id, type: 'blur', epoch: this._epoch++ }); - // Try to focus the next widget if (this._focusables.length > 0) { this._currentIndex = Math.min(this._currentIndex, this._focusables.length - 1); this._events.emit('focus', { @@ -135,18 +136,10 @@ export class FocusManager { this._currentIndex = -1; } } else if (idx < this._currentIndex) { - // Silent focus shift: the widget that preceded the removed item - // now occupies the focused position. Emit blur + focus to notify - // downstream so the visual focus state stays in sync. + // A non-focused widget before the focused one was removed — + // just adjust the index. No blur/focus events because the + // focused widget hasn't changed. this._currentIndex--; - this._events.emit('blur', { targetId: id, type: 'blur', epoch: this._epoch++ }); - if (this._currentIndex >= 0 && this._currentIndex < this._focusables.length) { - this._events.emit('focus', { - targetId: this._focusables[this._currentIndex].id, - type: 'focus', - epoch: this._epoch++, - }); - } } }