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 <help@stably.ai>

---------

Co-authored-by: Jinwoo-H <jinwoo0825@gmail.com>
Co-authored-by: Orca <help@stably.ai>

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 (
     <div
+      ref={setAppRootNode}
       className="flex flex-col h-screen w-screen overflow-hidden"
       style={
         {

src/renderer/src/components/floating-terminal/FloatingTerminalPanel.test.tsx

@@ -471,6 +471,14 @@ function runEffects(): void {
   }
 }
 
+function attachRef(ref: unknown, value: unknown): void {
+  if (typeof ref === 'function') {
+    ref(value)
+    return
+  }
+  ;(ref as { current: unknown }).current = value
+}
+
 async function flushAsyncWork(): Promise<void> {
   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)({

src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx

@@ -139,7 +139,7 @@ export function FloatingTerminalPanel({
   const normalizedInitialBoundsRef = useRef(false)
   const pendingEditorCloseQueueRef = useRef<string[]>([])
   const saveDialogFileIdRef = useRef<string | null>(null)
-  const panelRef = useRef<HTMLDivElement>(null)
+  const panelRef = useRef<HTMLDivElement | null>(null)
   const shortcutFocusFrameRef = useRef<number | null>(null)
   const shortcutFocusTimeoutRef = useRef<number | null>(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 (
     <div
-      ref={panelRef}
+      ref={setPanelNode}
       data-floating-terminal-panel
       aria-hidden={!open}
       tabIndex={-1}