From 8275cbe5080575b5d3bca99e955412abacbfbb3d Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 30 May 2026 11:49:16 -0700 Subject: [PATCH] Cancel floating workspace focus frames from root cleanup (#3457) * Cancel floating workspace focus frames from root cleanup * test: cover floating panel ref cleanup Co-authored-by: Orca --------- Co-authored-by: Jinwoo-H Co-authored-by: Orca --- src/renderer/src/App.tsx | 11 ++- .../FloatingTerminalPanel.test.tsx | 68 +++++++++++++++++-- .../FloatingTerminalPanel.tsx | 16 ++++- 3 files changed, 85 insertions(+), 10 deletions(-) diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index f775133e582..a42a8db82dd 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -369,7 +369,15 @@ function App(): React.JSX.Element { floatingTerminalReturnFocusFrameRef.current = null }, []) - useEffect(() => cancelFloatingTerminalReturnFocusFrame, [cancelFloatingTerminalReturnFocusFrame]) + const setAppRootNode = useCallback( + (node: HTMLDivElement | null): void => { + // Why: return-focus frames are only valid while the App root is mounted. + if (!node) { + cancelFloatingTerminalReturnFocusFrame() + } + }, + [cancelFloatingTerminalReturnFocusFrame] + ) const rememberFloatingTerminalReturnFocus = useCallback((): void => { const active = document.activeElement @@ -1562,6 +1570,7 @@ function App(): React.JSX.Element { return (
{ await Promise.resolve() await Promise.resolve() @@ -583,7 +591,7 @@ describe('FloatingTerminalPanel close behavior', () => { const element = await renderPanel(true) const panel = findByProp(element, 'data-floating-terminal-panel') const panelElement = { focus: vi.fn() } - ;(panel.props.ref as { current: unknown }).current = panelElement + attachRef(panel.props.ref, panelElement) runEffects() @@ -702,7 +710,7 @@ describe('FloatingTerminalPanel close behavior', () => { } Object.setPrototypeOf(activeElement, HTMLElement.prototype) Object.setPrototypeOf(target, HTMLElement.prototype) - ;(panel.props.ref as { current: unknown }).current = panelElement + attachRef(panel.props.ref, panelElement) vi.stubGlobal('document', { activeElement, addEventListener: vi.fn(), @@ -753,7 +761,7 @@ describe('FloatingTerminalPanel close behavior', () => { const target = { classList: { contains: vi.fn().mockReturnValue(true) }, closest: vi.fn() } Object.setPrototypeOf(activeElement, HTMLElement.prototype) Object.setPrototypeOf(target, HTMLElement.prototype) - ;(panel.props.ref as { current: unknown }).current = panelElement + attachRef(panel.props.ref, panelElement) vi.stubGlobal('document', { activeElement, addEventListener: vi.fn(), @@ -787,6 +795,54 @@ describe('FloatingTerminalPanel close behavior', () => { expect(panelElement.focus).toHaveBeenCalledWith({ preventScroll: true }) }) + it('cancels pending shortcut focus when the panel root unmounts', async () => { + setFloatingTabs([makeTab({ id: 'tab-1' })]) + const cancelAnimationFrame = vi.fn() + vi.stubGlobal('cancelAnimationFrame', cancelAnimationFrame) + vi.mocked(window.requestAnimationFrame).mockReturnValue(42) + const element = await renderPanel(true) + const panel = findByProp(element, 'data-floating-terminal-panel') + const panelElement = { contains: vi.fn().mockReturnValue(true), focus: vi.fn() } + const activeElement = { closest: vi.fn().mockReturnValue(panelElement) } + const target = { classList: { contains: vi.fn().mockReturnValue(true) }, closest: vi.fn() } + Object.setPrototypeOf(activeElement, HTMLElement.prototype) + Object.setPrototypeOf(target, HTMLElement.prototype) + attachRef(panel.props.ref, panelElement) + vi.stubGlobal('document', { + activeElement, + addEventListener: vi.fn(), + removeEventListener: vi.fn() + }) + runEffects() + const keydownListener = vi + .mocked(window.addEventListener) + .mock.calls.find(([type]) => type === 'keydown')?.[1] as + | ((event: unknown) => void) + | undefined + if (!keydownListener) { + throw new Error('keydown listener not registered') + } + + keydownListener({ + altKey: false, + ctrlKey: false, + defaultPrevented: false, + key: 'w', + metaKey: true, + preventDefault: vi.fn(), + repeat: false, + shiftKey: false, + stopImmediatePropagation: vi.fn(), + stopPropagation: vi.fn(), + target + }) + attachRef(panel.props.ref, null) + + expect(mocks.closeTab).toHaveBeenCalledWith('tab-1') + expect(cancelAnimationFrame).toHaveBeenCalledWith(42) + expect(panelElement.focus).not.toHaveBeenCalled() + }) + it('does not steal focus from the next floating tab after Cmd+W closes one of many tabs', async () => { setFloatingTabs([makeTab({ id: 'tab-1' }), makeTab({ id: 'tab-2' })]) const element = await renderPanel(true) @@ -796,7 +852,7 @@ describe('FloatingTerminalPanel close behavior', () => { const target = { classList: { contains: vi.fn().mockReturnValue(true) }, closest: vi.fn() } Object.setPrototypeOf(activeElement, HTMLElement.prototype) Object.setPrototypeOf(target, HTMLElement.prototype) - ;(panel.props.ref as { current: unknown }).current = panelElement + attachRef(panel.props.ref, panelElement) vi.stubGlobal('document', { activeElement, addEventListener: vi.fn(), @@ -840,7 +896,7 @@ describe('FloatingTerminalPanel close behavior', () => { const titlebarTarget = { closest: vi.fn().mockReturnValue(null) } Object.setPrototypeOf(activeElement, HTMLElement.prototype) Object.setPrototypeOf(titlebarTarget, HTMLElement.prototype) - ;(panel.props.ref as { current: unknown }).current = panelElement + attachRef(panel.props.ref, panelElement) vi.stubGlobal('document', { activeElement }) ;(titlebar.props.onPointerDown as (event: unknown) => void)({ @@ -865,7 +921,7 @@ describe('FloatingTerminalPanel close behavior', () => { const titlebarTarget = { closest: vi.fn().mockReturnValue(null) } Object.setPrototypeOf(activeElement, HTMLElement.prototype) Object.setPrototypeOf(titlebarTarget, HTMLElement.prototype) - ;(panel.props.ref as { current: unknown }).current = panelElement + attachRef(panel.props.ref, panelElement) vi.stubGlobal('document', { activeElement }) ;(titlebar.props.onPointerDown as (event: unknown) => void)({ diff --git a/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx b/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx index 65439c96520..c469ee09455 100644 --- a/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx +++ b/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx @@ -139,7 +139,7 @@ export function FloatingTerminalPanel({ const normalizedInitialBoundsRef = useRef(false) const pendingEditorCloseQueueRef = useRef([]) const saveDialogFileIdRef = useRef(null) - const panelRef = useRef(null) + const panelRef = useRef(null) const shortcutFocusFrameRef = useRef(null) const shortcutFocusTimeoutRef = useRef(null) const mountedRef = useMountedRef() @@ -725,7 +725,17 @@ export function FloatingTerminalPanel({ } }, []) - useEffect(() => cancelShortcutFocusFrame, [cancelShortcutFocusFrame]) + const setPanelNode = useCallback( + (node: HTMLDivElement | null): void => { + // Why: the deferred shortcut focus targets this panel and must stop + // when the panel root leaves the DOM. + if (!node) { + cancelShortcutFocusFrame() + } + panelRef.current = node + }, + [cancelShortcutFocusFrame] + ) const focusPanelForShortcutsAfterClose = useCallback(() => { if (typeof window === 'undefined') { @@ -1067,7 +1077,7 @@ export function FloatingTerminalPanel({ return (