From fd332e51feee915ca209adec142ef9b969b27066 Mon Sep 17 00:00:00 2001 From: Kian Bazarjani Date: Wed, 18 Feb 2026 19:05:46 -0500 Subject: [PATCH] fix(popup-menu): clear stale aim guard state on close and unmount --- .../popup-menu/components/popup/popup.tsx | 29 ++++++++ .../submenu-trigger/submenu-trigger.tsx | 61 +++++++++++++++++ .../popup-menu/hooks/use-aim-guard.tsx | 38 +++++++---- .../internal/popup-menu/popup-menu.test.tsx | 68 +++++++++++++++++++ 4 files changed, 181 insertions(+), 15 deletions(-) diff --git a/packages/react/src/internal/popup-menu/components/popup/popup.tsx b/packages/react/src/internal/popup-menu/components/popup/popup.tsx index 9ea1d387..f4f2c673 100644 --- a/packages/react/src/internal/popup-menu/components/popup/popup.tsx +++ b/packages/react/src/internal/popup-menu/components/popup/popup.tsx @@ -342,6 +342,35 @@ export const PopupMenuPopup = React.forwardRef< clearAimGuard, ]) + React.useEffect(() => { + if ( + submenuContext?.open !== false || + !aimGuardActiveRef.current || + guardedSubmenuSurfaceIdRef.current !== surfaceId + ) { + return + } + + clearAimGuard() + }, [ + submenuContext?.open, + aimGuardActiveRef, + guardedSubmenuSurfaceIdRef, + surfaceId, + clearAimGuard, + ]) + + React.useEffect(() => { + return () => { + if ( + aimGuardActiveRef.current && + guardedSubmenuSurfaceIdRef.current === surfaceId + ) { + clearAimGuard() + } + } + }, [aimGuardActiveRef, guardedSubmenuSurfaceIdRef, surfaceId, clearAimGuard]) + // Transfer focus ownership when pointer moves inside this submenu popup // We ignore events shortly after open to prevent focus transfer when // the popup appears under a stationary cursor diff --git a/packages/react/src/internal/popup-menu/components/submenu-trigger/submenu-trigger.tsx b/packages/react/src/internal/popup-menu/components/submenu-trigger/submenu-trigger.tsx index 1e676af3..8167dc2a 100644 --- a/packages/react/src/internal/popup-menu/components/submenu-trigger/submenu-trigger.tsx +++ b/packages/react/src/internal/popup-menu/components/submenu-trigger/submenu-trigger.tsx @@ -416,6 +416,67 @@ export const PopupMenuSubmenuTrigger = React.forwardRef< } }, [open, clearCloseTimer]) + React.useEffect(() => { + if ( + open || + !aimGuardActiveRef.current || + guardedTriggerIdRef.current !== item.id || + guardedDepthRef.current !== parentDepth + ) { + return + } + + clearAimGuard() + }, [ + open, + item.id, + parentDepth, + aimGuardActiveRef, + guardedTriggerIdRef, + guardedDepthRef, + clearAimGuard, + ]) + + React.useEffect(() => { + if ( + item.isVisible || + !aimGuardActiveRef.current || + guardedTriggerIdRef.current !== item.id || + guardedDepthRef.current !== parentDepth + ) { + return + } + + clearAimGuard() + }, [ + item.isVisible, + item.id, + parentDepth, + aimGuardActiveRef, + guardedTriggerIdRef, + guardedDepthRef, + clearAimGuard, + ]) + + React.useEffect(() => { + return () => { + if ( + aimGuardActiveRef.current && + guardedTriggerIdRef.current === item.id && + guardedDepthRef.current === parentDepth + ) { + clearAimGuard() + } + } + }, [ + item.id, + parentDepth, + aimGuardActiveRef, + guardedTriggerIdRef, + guardedDepthRef, + clearAimGuard, + ]) + React.useEffect(() => { if (!open) { return diff --git a/packages/react/src/internal/popup-menu/hooks/use-aim-guard.tsx b/packages/react/src/internal/popup-menu/hooks/use-aim-guard.tsx index c9bd3219..af463c6f 100644 --- a/packages/react/src/internal/popup-menu/hooks/use-aim-guard.tsx +++ b/packages/react/src/internal/popup-menu/hooks/use-aim-guard.tsx @@ -77,11 +77,7 @@ export function AimGuardProvider({ children }: AimGuardProviderProps) { const guardTimerRef = React.useRef(null) - const clearAimGuard = React.useCallback(() => { - if (guardTimerRef.current) { - window.clearTimeout(guardTimerRef.current) - guardTimerRef.current = null - } + const resetAimGuardState = React.useCallback(() => { aimGuardActiveRef.current = false guardedTriggerIdRef.current = null guardedDepthRef.current = null @@ -92,6 +88,14 @@ export function AimGuardProvider({ children }: AimGuardProviderProps) { setGuardedSubmenuSurfaceId(null) }, []) + const clearAimGuard = React.useCallback(() => { + if (guardTimerRef.current !== null) { + window.clearTimeout(guardTimerRef.current) + guardTimerRef.current = null + } + resetAimGuardState() + }, [resetAimGuardState]) + const activateAimGuard = React.useCallback( ( triggerId: string, @@ -107,22 +111,26 @@ export function AimGuardProvider({ children }: AimGuardProviderProps) { setGuardedDepth(depth) setGuardedSubmenuSurfaceId(submenuSurfaceId) setAimGuardActive(true) - if (guardTimerRef.current) window.clearTimeout(guardTimerRef.current) + if (guardTimerRef.current !== null) { + window.clearTimeout(guardTimerRef.current) + } guardTimerRef.current = window.setTimeout(() => { - aimGuardActiveRef.current = false - guardedTriggerIdRef.current = null - guardedDepthRef.current = null - guardedSubmenuSurfaceIdRef.current = null - setAimGuardActive(false) - setGuardedTriggerId(null) - setGuardedDepth(null) - setGuardedSubmenuSurfaceId(null) + resetAimGuardState() guardTimerRef.current = null }, timeoutMs) as unknown as number }, - [], + [resetAimGuardState], ) + React.useEffect(() => { + return () => { + if (guardTimerRef.current !== null) { + window.clearTimeout(guardTimerRef.current) + guardTimerRef.current = null + } + } + }, []) + const isGuardBlocking = React.useCallback( (rowId: string) => aimGuardActiveRef.current && guardedTriggerIdRef.current !== rowId, diff --git a/packages/react/src/internal/popup-menu/popup-menu.test.tsx b/packages/react/src/internal/popup-menu/popup-menu.test.tsx index b944dde5..90563abd 100644 --- a/packages/react/src/internal/popup-menu/popup-menu.test.tsx +++ b/packages/react/src/internal/popup-menu/popup-menu.test.tsx @@ -1122,6 +1122,74 @@ describe('PopupMenu', () => { }) }) + describe('aim guard lifecycle', () => { + it('clears aim guard when the root menu closes', async () => { + const user = userEvent.setup() + render() + + await user.click(screen.getByTestId('trigger')) + + await waitFor(() => { + expect(screen.getByTestId('popup-root')).toBeInTheDocument() + }) + + const submenuTrigger = screen.getByTestId('submenu-trigger-1') + await user.hover(submenuTrigger) + + await waitFor(() => { + expect(screen.getByTestId('popup-submenu-1')).toBeInTheDocument() + }) + + const submenuPopup = screen.getByTestId('popup-submenu-1') + const triggerRectSpy = vi + .spyOn(submenuTrigger, 'getBoundingClientRect') + .mockImplementation(() => + createRect({ top: 60, left: 80, width: 120, height: 30 }), + ) + const popupRectSpy = vi + .spyOn(submenuPopup, 'getBoundingClientRect') + .mockImplementation(() => + createRect({ top: 40, left: 240, width: 180, height: 160 }), + ) + + try { + fireEvent.pointerMove(window, { clientX: 120, clientY: 90 }) + fireEvent.pointerMove(window, { clientX: 150, clientY: 92 }) + fireEvent.pointerMove(window, { clientX: 180, clientY: 94 }) + + fireEvent.pointerLeave(submenuTrigger, { clientX: 190, clientY: 94 }) + fireEvent.pointerMove(window, { clientX: 190, clientY: 94 }) + + await user.click(screen.getByTestId('trigger')) + + await waitFor(() => { + expect(screen.queryByTestId('popup-root')).not.toBeInTheDocument() + }) + + await user.click(screen.getByTestId('trigger')) + + await waitFor(() => { + expect(screen.getByTestId('popup-root')).toBeInTheDocument() + }) + + const rootItem = screen.getByTestId('root-item-1') + + fireEvent.pointerMove(rootItem, { clientX: 96, clientY: 74 }) + fireEvent.pointerMove(rootItem, { clientX: 100, clientY: 78 }) + + await waitFor( + () => { + expect(rootItem).toHaveAttribute('data-highlighted', '') + }, + { timeout: 250 }, + ) + } finally { + triggerRectSpy.mockRestore() + popupRectSpy.mockRestore() + } + }) + }) + describe('search and filtering', () => { it('filters items by keyword search', async () => { const user = userEvent.setup()