diff --git a/src/renderer/src/lib/pane-manager/pane-close-focus-successor.test.ts b/src/renderer/src/lib/pane-manager/pane-close-focus-successor.test.ts new file mode 100644 index 00000000000..669eb55ec49 --- /dev/null +++ b/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 + +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() + }) +}) diff --git a/src/renderer/src/lib/pane-manager/pane-close-focus-successor.ts b/src/renderer/src/lib/pane-manager/pane-close-focus-successor.ts new file mode 100644 index 00000000000..b4365adc5ac --- /dev/null +++ b/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 +): 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 +} diff --git a/src/renderer/src/lib/pane-manager/pane-split-close.ts b/src/renderer/src/lib/pane-manager/pane-split-close.ts index 6cf53fe9a2d..f4b8e80135c 100644 --- a/src/renderer/src/lib/pane-manager/pane-split-close.ts +++ b/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() diff --git a/src/renderer/src/lib/pane-manager/pane-tree-equalization.ts b/src/renderer/src/lib/pane-manager/pane-tree-equalization.ts index c826d7aed9e..e1f8e6e2075 100644 --- a/src/renderer/src/lib/pane-manager/pane-tree-equalization.ts +++ b/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' } diff --git a/tests/e2e/terminal-close-pane-focus.spec.ts b/tests/e2e/terminal-close-pane-focus.spec.ts new file mode 100644 index 00000000000..b95cfc58971 --- /dev/null +++ b/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 { + await page.evaluate( + () => + new Promise((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 { + 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) + }) +}) diff --git a/tests/e2e/terminal-layout-parity-declared-differences.ts b/tests/e2e/terminal-layout-parity-declared-differences.ts index e779d83f1cd..da0a4aad9c6 100644 --- a/tests/e2e/terminal-layout-parity-declared-differences.ts +++ b/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[] = []