fix(terminal): focus the neighboring pane when a split pane closes (#32238)

* fix(terminal): focus the neighboring pane when a split pane closes

* test(terminal): cover focus after closing a split pane in the app

* fix(terminal): focus the neighboring pane when a split pane closes

* test(terminal): cover focus after closing a split pane in the app

* test(terminal): keep pane-close parity exclusions evidence based

---------

Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com>

src/renderer/src/lib/pane-manager/pane-close-focus-successor.test.ts

@@ -0,0 +1,136 @@
+// @vitest-environment happy-dom
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+import type { ManagedPane } from './pane-manager-types'
+
+// Why: happy-dom has no canvas, so xterm cannot open; focus routing never needs a rendered terminal.
+vi.mock('./pane-lifecycle', async (importOriginal) => {
+  const actual: object = await importOriginal()
+  return { ...actual, openTerminal: vi.fn() }
+})
+
+const { PaneManager } = await import('./pane-manager')
+
+type Manager = InstanceType<typeof PaneManager>
+
+function split(
+  manager: Manager,
+  source: ManagedPane,
+  direction: 'vertical' | 'horizontal'
+): ManagedPane {
+  const pane = manager.splitPane(source.id, direction)
+  if (!pane) {
+    throw new Error(`split of pane ${source.id} failed`)
+  }
+  return pane
+}
+
+/** a | b | c | d, each split taken from the newest pane. */
+function createFourColumns(manager: Manager): [ManagedPane, ManagedPane, ManagedPane, ManagedPane] {
+  const a = manager.createInitialPane()
+  const b = split(manager, a, 'vertical')
+  const c = split(manager, b, 'vertical')
+  const d = split(manager, c, 'vertical')
+  return [a, b, c, d]
+}
+
+describe('PaneManager focus after a pane leaves a split', () => {
+  let root: HTMLDivElement
+  let manager: Manager
+
+  beforeEach(() => {
+    vi.stubGlobal('requestAnimationFrame', () => 1)
+    vi.stubGlobal('cancelAnimationFrame', () => {})
+    root = document.createElement('div')
+    document.body.appendChild(root)
+    manager = new PaneManager(root, { linkOpenHint: () => '' })
+  })
+
+  afterEach(() => {
+    manager.destroy()
+    root.remove()
+    vi.unstubAllGlobals()
+  })
+
+  it('focuses the neighbour that takes over the space when the last of four panes closes', () => {
+    const [, , c, d] = createFourColumns(manager)
+    const focus = vi.spyOn(c.terminal, 'focus')
+
+    manager.closePane(d.id)
+
+    expect(manager.getActivePane()?.id).toBe(c.id)
+    expect(focus).toHaveBeenCalled()
+  })
+
+  it('focuses the adjacent pane of the subtree that takes over a closed middle pane', () => {
+    const [, b, c] = createFourColumns(manager)
+    manager.setActivePane(b.id)
+
+    manager.closePane(b.id)
+
+    expect(manager.getActivePane()?.id).toBe(c.id)
+  })
+
+  it('focuses the pane after a closed middle pane when that pane is its only sibling', () => {
+    const [, , c, d] = createFourColumns(manager)
+    manager.setActivePane(c.id)
+
+    manager.closePane(c.id)
+
+    expect(manager.getActivePane()?.id).toBe(d.id)
+  })
+
+  it('focuses the pane above when the lower half of a stacked split closes', () => {
+    const [, b] = createFourColumns(manager)
+    const below = split(manager, b, 'horizontal')
+
+    manager.closePane(below.id)
+
+    expect(manager.getActivePane()?.id).toBe(b.id)
+  })
+
+  it('walks into the edge of an orthogonal split that takes over the space', () => {
+    // [ a | b ] over [ c ], all left of d: closing d must land on b, which touches d's edge.
+    const a = manager.createInitialPane()
+    const d = split(manager, a, 'vertical')
+    split(manager, a, 'horizontal')
+    const b = split(manager, a, 'vertical')
+    manager.setActivePane(d.id)
+
+    manager.closePane(d.id)
+
+    expect(manager.getActivePane()?.id).toBe(b.id)
+  })
+
+  it('keeps the active pane when a different pane closes', () => {
+    const [, b, , d] = createFourColumns(manager)
+
+    manager.closePane(b.id)
+
+    expect(manager.getActivePane()?.id).toBe(d.id)
+  })
+
+  it('focuses the neighbour when the active pane is detached into another tab', () => {
+    const [, , c, d] = createFourColumns(manager)
+
+    expect(manager.detachPaneForExternalMove(d.id)).toBe(true)
+
+    expect(manager.getActivePane()?.id).toBe(c.id)
+  })
+
+  it('focuses the neighbour when the active pane is retired with its PTY kept', () => {
+    const [, , c, d] = createFourColumns(manager)
+    manager.setActivePane(c.id)
+
+    expect(manager.retirePanePreservingPty(c.id)).toBe(true)
+
+    expect(manager.getActivePane()?.id).toBe(d.id)
+  })
+
+  it('leaves no active pane when the only pane closes', () => {
+    const a = manager.createInitialPane()
+
+    manager.closePane(a.id)
+
+    expect(manager.getActivePane()).toBeNull()
+  })
+})

src/renderer/src/lib/pane-manager/pane-close-focus-successor.ts

@@ -0,0 +1,31 @@
+import type { ManagedPaneInternal } from './pane-manager-types'
+import { findPaneChildren, getSplitDirection } from './pane-tree-equalization'
+
+/**
+ * The pane to focus when `closedContainer` leaves its split: the sibling subtree takes over the
+ * freed space, so pick its leaf touching the closed pane's edge. Must run before the DOM removal.
+ */
+export function findClosedPaneSuccessor(
+  closedContainer: HTMLElement,
+  panes: ReadonlyMap<number, ManagedPaneInternal>
+): ManagedPaneInternal | null {
+  const parent = closedContainer.parentElement
+  if (!parent?.classList.contains('pane-split')) {
+    return null
+  }
+  const children = findPaneChildren(parent)
+  const closedIndex = children.indexOf(closedContainer)
+  if (closedIndex === -1) {
+    return null
+  }
+  const closedIsFirst = closedIndex === 0
+  const axis = getSplitDirection(parent)
+  let node: HTMLElement | undefined = closedIsFirst ? children[1] : children[closedIndex - 1]
+  while (node?.classList.contains('pane-split')) {
+    const nested = findPaneChildren(node)
+    // Why: an orthogonal split touches the closed edge with every child; reading order breaks the tie.
+    node = getSplitDirection(node) === axis && !closedIsFirst ? nested.at(-1) : nested[0]
+  }
+  const paneId = Number(node?.dataset.paneId)
+  return Number.isFinite(paneId) ? (panes.get(paneId) ?? null) : null
+}

src/renderer/src/lib/pane-manager/pane-split-close.ts

@@ -21,6 +21,7 @@ import { disposeWebgl } from './pane-webgl-renderer'
 import { clearPendingSplitScrollRestore, scheduleSplitScrollRestore } from './pane-split-scroll'
 import { reattachWebglIfNeeded } from './pane-webgl-reattach'
 import { toPublicPane } from './pane-public-view'
+import { findClosedPaneSuccessor } from './pane-close-focus-successor'
 import { releaseTerminalScrollIntentKey } from './terminal-scroll-intent-key-store'
 
 type MovedPaneSplitState = {
@@ -192,6 +193,8 @@ function teardownManagedPane(
     return
   }
   const closedLeafId = pane.leafId
+  const successor =
+    args.activePaneId === args.paneId ? findClosedPaneSuccessor(pane.container, args.panes) : null
   args.releasePaneIdentity(args.paneId)
   removePaneContainer(args, pane)
   if (reason === 'close') {
@@ -200,7 +203,7 @@ function teardownManagedPane(
     // must still be able to restore it.
     releaseTerminalScrollIntentKey(closedLeafId)
   }
-  const nextActivePaneId = activateReplacementPane(args)
+  const nextActivePaneId = activateReplacementPane(args, successor)
   applyPaneOpacity(args.panes.values(), nextActivePaneId, args.styleOptions)
   for (const p of args.panes.values()) {
     safeFit(p)
@@ -256,11 +259,15 @@ function removePaneContainer(args: CloseManagedPaneArgs, pane: ManagedPaneIntern
   }
 }
 
-function activateReplacementPane(args: CloseManagedPaneArgs): number | null {
+function activateReplacementPane(
+  args: CloseManagedPaneArgs,
+  successor: ManagedPaneInternal | null
+): number | null {
   if (args.activePaneId !== args.paneId) {
     return args.activePaneId
   }
-  const next = args.panes.values().next().value as ManagedPaneInternal | undefined
+  const next =
+    successor && args.panes.has(successor.id) ? successor : args.panes.values().next().value
   const nextActivePaneId = next?.id ?? null
   args.setActivePaneId(nextActivePaneId)
   next?.terminal.focus()

src/renderer/src/lib/pane-manager/pane-tree-equalization.ts

@@ -7,7 +7,7 @@ export function findPaneChildren(parent: HTMLElement): HTMLElement[] {
   )
 }
 
-function getSplitDirection(split: HTMLElement): 'vertical' | 'horizontal' {
+export function getSplitDirection(split: HTMLElement): 'vertical' | 'horizontal' {
   return split.classList.contains('is-horizontal') ? 'horizontal' : 'vertical'
 }
 

tests/e2e/terminal-close-pane-focus.spec.ts

@@ -0,0 +1,80 @@
+/**
+ * E2E test for keyboard focus after closing a split terminal pane.
+ *
+ * User Prompt:
+ * - after closing the focused pane, keep typing in the pane that takes over its space
+ */
+
+import type { Page } from '@stablyai/playwright-test'
+import { test, expect } from './helpers/orca-app'
+import {
+  focusActiveTerminalInput,
+  readTerminalPaneDomLeafOrder,
+  resolveActiveTabId,
+  waitForPaneCount,
+  waitForPaneIdentitySnapshot
+} from './helpers/terminal'
+import { closeActivePaneFromKeyboard } from './helpers/terminal-layout-journeys'
+import { registerTerminalPaneMountReadiness } from './helpers/terminal-pane-mount-readiness'
+import { waitForTerminalOutputForPtyId } from './artificial-opencode-pane-interactions'
+
+const SPLIT_RIGHT_CHORD = process.platform === 'darwin' ? 'Meta+d' : 'Control+Shift+d'
+
+async function waitThreeFrames(page: Page): Promise<void> {
+  await page.evaluate(
+    () =>
+      new Promise<void>((resolve) =>
+        requestAnimationFrame(() =>
+          requestAnimationFrame(() => requestAnimationFrame(() => resolve()))
+        )
+      )
+  )
+}
+
+/** The leaf whose xterm input holds DOM focus, or null when focus is anywhere else. */
+async function readFocusedTerminalLeafId(page: Page, tabId: string): Promise<string | null> {
+  return page.evaluate((tabId) => {
+    const active = document.activeElement
+    if (!active?.classList.contains('xterm-helper-textarea')) {
+      return null
+    }
+    const panes = window.__paneManagers?.get(tabId)?.getPanes() ?? []
+    return panes.find((pane) => pane.container.contains(active))?.leafId ?? null
+  }, tabId)
+}
+
+// Why: keep the suite serial so headful pane tests never open multiple Electron windows at once.
+test.describe.configure({ mode: 'serial' })
+test.describe('Terminal close pane focus', () => {
+  registerTerminalPaneMountReadiness()
+
+  test('closing the last of four split panes focuses the neighbor that fills its space', async ({
+    orcaPage
+  }) => {
+    await waitForPaneIdentitySnapshot(orcaPage, 1)
+    for (const paneCount of [2, 3, 4]) {
+      await focusActiveTerminalInput(orcaPage)
+      await orcaPage.keyboard.press(SPLIT_RIGHT_CHORD)
+      await waitForPaneCount(orcaPage, paneCount)
+    }
+    const snapshot = await waitForPaneIdentitySnapshot(orcaPage, 4)
+    const [, , neighborLeafId, closedLeafId] = await readTerminalPaneDomLeafOrder(orcaPage)
+    expect(snapshot.activeLeafId).toBe(closedLeafId)
+    const neighborPtyId = neighborLeafId ? snapshot.ptyIdsByLeafId[neighborLeafId] : undefined
+    if (!neighborLeafId || !neighborPtyId) {
+      throw new Error(`No bound neighbor pane in ${JSON.stringify(snapshot)}`)
+    }
+
+    await closeActivePaneFromKeyboard(orcaPage, 4)
+    await waitThreeFrames(orcaPage)
+
+    const tabId = await resolveActiveTabId(orcaPage)
+    if (!tabId) {
+      throw new Error('No active terminal tab after closing a pane')
+    }
+    expect(await readFocusedTerminalLeafId(orcaPage, tabId)).toBe(neighborLeafId)
+    const marker = `CLOSE_FOCUS_${Date.now()}`
+    await orcaPage.keyboard.type(`echo ${marker}\r`)
+    await waitForTerminalOutputForPtyId(orcaPage, neighborPtyId, marker, 10_000)
+  })
+})

tests/e2e/terminal-layout-parity-declared-differences.ts

@@ -7,7 +7,21 @@ import type {
  * Differences from main this branch is allowed, each tied to the named bug it fixes. Keep empty
  * on an inert PR. Paths are prefixes of the runner's report paths, e.g. `[0].persisted.local`.
  */
-export const TERMINAL_LAYOUT_PARITY_DECLARED_DIFFERENCES: readonly DeclaredParityDifference[] = []
+export const TERMINAL_LAYOUT_PARITY_DECLARED_DIFFERENCES: readonly DeclaredParityDifference[] = [
+  {
+    scenario: 'close-pane',
+    bugId: '#30326',
+    paths: [
+      '[0].renderer.mountedPanesByTabId.#1.leafOrder',
+      '[0].renderer.terminalLayoutsByTabId.#1.ptyIdsByLeafId',
+      '[0].renderer.terminalLayoutsByTabId.#1.root',
+      '[0].persisted.local.terminalLayoutsByTabId.#1.ptyIdsByLeafId',
+      '[0].persisted.local.terminalLayoutsByTabId.#1.root'
+    ],
+    reason:
+      'Closing the focused pane activates its sibling instead of the first pane; leaves are labelled from the active leaf, so the same two-pane layout relabels.'
+  }
+]
 
 /** Paths main itself does not reproduce; reported but not failed. Remove an entry once main is fixed. */
 export const TERMINAL_LAYOUT_PARITY_UNSTABLE_ON_MAIN: readonly UnstableOnMainPath[] = []