Keep sidebar position when deleting active worktree (#16040)

This commit is contained in:
Neil
2026-08-24 18:14:12 -07:00
committed by GitHub
parent 31562c5b27
commit c83499fc8c
7 changed files with 582 additions and 7 deletions
@@ -97,7 +97,7 @@ describe('prepareActiveWorktreeFocusAfterDelete', () => {
simulateDelete('wt-del', true)
commit()
expect(activateAndRevealWorktree).toHaveBeenCalledWith('wt-b')
expect(activateAndRevealWorktree).toHaveBeenCalledWith('wt-b', { revealInSidebar: false })
})
it('falls back to the base/primary worktree when no other workspace remains', () => {
@@ -108,7 +108,7 @@ describe('prepareActiveWorktreeFocusAfterDelete', () => {
simulateDelete('wt-del', true)
commit()
expect(activateAndRevealWorktree).toHaveBeenCalledWith('main')
expect(activateAndRevealWorktree).toHaveBeenCalledWith('main', { revealInSidebar: false })
})
it('does not re-focus a sibling hosted on a torn-down runtime-owned SSH target', () => {
@@ -140,7 +140,9 @@ describe('prepareActiveWorktreeFocusAfterDelete', () => {
simulateDelete('wt-del', true)
commit()
expect(activateAndRevealWorktree).toHaveBeenCalledWith('main-1')
expect(activateAndRevealWorktree).toHaveBeenCalledWith('main-1', {
revealInSidebar: false
})
})
it('does not steal focus when the deleted worktree was not the active one', () => {
@@ -229,7 +231,7 @@ describe('prepareActiveWorktreeFocusAfterDelete', () => {
mocks.state.deleteStateByWorktreeId = { 'wt-a': { isDeleting: true } }
commit()
expect(activateAndRevealWorktree).toHaveBeenCalledWith('main')
expect(activateAndRevealWorktree).toHaveBeenCalledWith('main', { revealInSidebar: false })
})
it('does not steal focus when a non-worktree workspace is active', () => {
@@ -86,7 +86,8 @@ function focusNextWorktreeAfterActiveDelete(
}
const nextWorktreeId = pickNextWorktreeIdAfterDelete(state, repoId, deletedWorktreeId)
if (nextWorktreeId) {
activateAndRevealWorktree(nextWorktreeId)
// Keep successor focus from replacing the deleted row's spatial context.
activateAndRevealWorktree(nextWorktreeId, { revealInSidebar: false })
}
}
@@ -14,6 +14,7 @@ import {
import { getRenderRowKey } from '../listing/render-row'
import type { RenderRow } from '../listing/render-row'
import type { WorktreeListVirtualizer } from './use-virtualizer'
import { useVirtualRowRemovalAnimation } from './use-row-removal-animation'
const recordKeyCountCache = new WeakMap<Record<string, unknown>, number>()
@@ -123,5 +124,12 @@ export function useVirtualRowMeasurementSync(args: {
virtualizer
})
useVirtualRowRemovalAnimation({
renderRows,
rekeyedRowKeys: lineageRowRekeys,
scrollRef,
virtualItems
})
return { virtualItems, measureVirtualRowElement }
}
@@ -0,0 +1,99 @@
import { describe, expect, it } from 'vitest'
import {
buildVirtualRowRemovalMotions,
type VirtualRowLayoutSnapshot
} from './use-row-removal-animation'
function snapshot(args: {
identities: string[]
scrollTop: number
starts: [string, number][]
}): VirtualRowLayoutSnapshot {
return {
rowIdentityKeys: new Set(args.identities),
scrollTop: args.scrollTop,
startsByKey: new Map(args.starts)
}
}
describe('buildVirtualRowRemovalMotions', () => {
it('moves surviving rows from their pre-delete viewport positions', () => {
const motions = buildVirtualRowRemovalMotions({
previous: snapshot({
identities: ['wt:a', 'wt:b', 'wt:c'],
scrollTop: 100,
starts: [
['wt:a', 100],
['wt:b', 220],
['wt:c', 340]
]
}),
current: snapshot({
identities: ['wt:a', 'wt:c'],
scrollTop: 100,
starts: [
['wt:a', 100],
['wt:c', 220]
]
}),
rekeyedRowKeys: new Map()
})
expect(motions).toEqual([{ key: 'wt:c', deltaY: 120 }])
})
it('does not double-move rows when anchor restoration offsets a deletion above the viewport', () => {
const motions = buildVirtualRowRemovalMotions({
previous: snapshot({
identities: ['wt:deleted', 'wt:a'],
scrollTop: 240,
starts: [['wt:a', 240]]
}),
current: snapshot({
identities: ['wt:a'],
scrollTop: 120,
starts: [['wt:a', 120]]
}),
rekeyedRowKeys: new Map()
})
expect(motions).toEqual([])
})
it('follows a surviving row through a lineage rekey', () => {
const motions = buildVirtualRowRemovalMotions({
previous: snapshot({
identities: ['wt:parent', 'wt:child'],
scrollTop: 0,
starts: [['lineage-group:parent', 100]]
}),
current: snapshot({
identities: ['wt:parent'],
scrollTop: 0,
starts: [['wt:parent', 100]]
}),
rekeyedRowKeys: new Map([['lineage-group:parent', 'wt:parent']])
})
expect(motions).toEqual([])
})
it('ignores additions and measurement-only movement', () => {
const previous = snapshot({
identities: ['wt:a'],
scrollTop: 0,
starts: [['wt:a', 100]]
})
expect(
buildVirtualRowRemovalMotions({
previous,
current: snapshot({
identities: ['wt:a', 'wt:b'],
scrollTop: 0,
starts: [['wt:a', 140]]
}),
rekeyedRowKeys: new Map()
})
).toEqual([])
})
})
@@ -0,0 +1,121 @@
import { useLayoutEffect, useMemo, useRef } from 'react'
import type React from 'react'
import type { VirtualItem } from '@tanstack/react-virtual'
import type { RenderRow } from '../listing/render-row'
import { getRenderRowKey } from '../listing/render-row'
export const WORKTREE_ROW_REMOVAL_ANIMATION_MS = 180
export type VirtualRowLayoutSnapshot = {
rowIdentityKeys: ReadonlySet<string>
scrollTop: number
startsByKey: ReadonlyMap<string, number>
}
export type VirtualRowRemovalMotion = {
deltaY: number
key: string
}
export function getSidebarRowIdentityKeys(rows: readonly RenderRow[]): ReadonlySet<string> {
const keys = new Set<string>()
for (const row of rows) {
if (row.type === 'lineage-group') {
row.rows.forEach((member) => keys.add(`wt:${member.rowKey}`))
} else {
keys.add(getRenderRowKey(row))
}
}
return keys
}
export function buildVirtualRowRemovalMotions(args: {
previous: VirtualRowLayoutSnapshot | null
current: VirtualRowLayoutSnapshot
rekeyedRowKeys: ReadonlyMap<string, string>
}): VirtualRowRemovalMotion[] {
const { previous, current, rekeyedRowKeys } = args
if (previous === null || previous.rowIdentityKeys === current.rowIdentityKeys) {
return []
}
let removedRow = false
for (const key of previous.rowIdentityKeys) {
if (!current.rowIdentityKeys.has(key)) {
removedRow = true
break
}
}
if (!removedRow) {
return []
}
const previousKeyByCurrentKey = new Map<string, string>()
rekeyedRowKeys.forEach((currentKey, previousKey) => {
previousKeyByCurrentKey.set(currentKey, previousKey)
})
const motions: VirtualRowRemovalMotion[] = []
current.startsByKey.forEach((currentStart, key) => {
const previousKey = previous.startsByKey.has(key) ? key : previousKeyByCurrentKey.get(key)
const previousStart = previousKey ? previous.startsByKey.get(previousKey) : undefined
if (previousStart === undefined) {
return
}
const deltaY = previousStart - previous.scrollTop - (currentStart - current.scrollTop)
if (Math.abs(deltaY) > 0.5) {
motions.push({ deltaY, key })
}
})
return motions
}
export function useVirtualRowRemovalAnimation(args: {
renderRows: readonly RenderRow[]
rekeyedRowKeys: ReadonlyMap<string, string>
scrollRef: React.RefObject<HTMLDivElement | null>
virtualItems: readonly VirtualItem[]
}): void {
const { renderRows, rekeyedRowKeys, scrollRef, virtualItems } = args
const previousSnapshotRef = useRef<VirtualRowLayoutSnapshot | null>(null)
const rowIdentityKeys = useMemo(() => getSidebarRowIdentityKeys(renderRows), [renderRows])
useLayoutEffect(() => {
const scrollElement = scrollRef.current
if (!scrollElement) {
return
}
const current: VirtualRowLayoutSnapshot = {
rowIdentityKeys,
scrollTop: scrollElement.scrollTop,
startsByKey: new Map(virtualItems.map((item) => [String(item.key), item.start]))
}
const motions = buildVirtualRowRemovalMotions({
previous: previousSnapshotRef.current,
current,
rekeyedRowKeys
})
previousSnapshotRef.current = current
if (motions.length === 0 || window.matchMedia('(prefers-reduced-motion: reduce)').matches) {
return
}
const elementsByKey = new Map(
Array.from(
scrollElement.querySelectorAll<HTMLElement>('[data-worktree-virtual-row-key]')
).map((element) => [element.dataset.worktreeVirtualRowKey ?? '', element])
)
for (const motion of motions) {
const element = elementsByKey.get(motion.key)
if (!element || element.hasAttribute('data-worktree-sticky-header-active')) {
continue
}
const content = element.firstElementChild
if (!(content instanceof HTMLElement)) {
continue
}
content.animate([{ translate: `0 ${motion.deltaY}px` }, { translate: '0 0' }], {
duration: WORKTREE_ROW_REMOVAL_ANIMATION_MS,
easing: 'cubic-bezier(0.16, 1, 0.3, 1)'
})
}
}, [rekeyedRowKeys, rowIdentityKeys, scrollRef, virtualItems])
}
+8 -2
View File
@@ -47,6 +47,8 @@ type OrcaTestFixtures = {
// Why: most E2E specs need a ready project before assertions start. Golden
// first-run specs opt out so they can prove the zero-project onboarding path.
seedTestRepo: boolean
// Synthetic-list specs need only the primary checkout; switching specs keep the two-row default.
minimumSeededWorktreeCount: number
// Why: spec-scoped launch env. Mutating process.env at spec module scope
// leaks into other specs when a worker reloads files without replaying the
// first spec's afterAll; per-test launch env cannot leak.
@@ -273,13 +275,17 @@ export const test = base.extend<OrcaTestFixtures, OrcaWorkerFixtures>({
// Default: dismiss the onboarding overlay so it doesn't intercept clicks.
dismissOnboarding: [true, { option: true }],
seedTestRepo: [true, { option: true }],
minimumSeededWorktreeCount: [2, { option: true }],
launchEnv: [{}, { option: true }],
orcaAppExtraEnv: [{}, { option: true }],
orcaAppExtraArgs: [[], { option: true }],
// Test-scoped: grab the first BrowserWindow, add the test repo, and wait
// until the session is fully ready with a worktree active.
sharedPage: async ({ electronApp, seedTestRepo, testRepoPath }, provideFixture) => {
sharedPage: async (
{ electronApp, minimumSeededWorktreeCount, seedTestRepo, testRepoPath },
provideFixture
) => {
// Why: the Electron app may take a while to create the first window,
// especially on cold start with no prior dev userData. Isolated per-test
// profiles make late-suite launches slower, so use the full test budget.
@@ -379,7 +385,7 @@ export const test = base.extend<OrcaTestFixtures, OrcaWorkerFixtures>({
message: 'seeded e2e worktrees did not load'
}
)
.toBeGreaterThanOrEqual(2)
.toBeGreaterThanOrEqual(minimumSeededWorktreeCount)
// Wait for workspaceSessionReady to become true
await page.waitForFunction(
@@ -0,0 +1,338 @@
import type { Page } from '@stablyai/playwright-test'
import { expect, test } from './helpers/orca-app'
import { waitForSessionReady } from './helpers/store'
const TARGET_INDEX = 24
const SYNTHETIC_COUNT = 40
const VISUAL_PROOF_PAUSE_MS = 1_200
const POST_REMOVAL_SAMPLE_FRAMES = 20
const MAX_REMOVAL_WAIT_FRAMES = 300
test.use({ minimumSeededWorktreeCount: 1 })
type RowRemovalFrame = {
animationCount: number
belowTop: number | null
scrollTop: number
targetExists: boolean
}
async function pauseForVisualProof(page: Page): Promise<void> {
if (process.env.ORCA_E2E_RECORD_VIDEO === '1') {
await page.waitForTimeout(VISUAL_PROOF_PAUSE_MS)
}
}
async function seedActiveDeletionRows(page: Page): Promise<{
belowId: string
successorId: string
targetId: string
}> {
return page.evaluate(
({ count, targetIndex }) => {
const store = window.__store
if (!store) {
throw new Error('window.__store is not available')
}
const state = store.getState()
const repo = state.repos[0]
const source = repo
? state.worktreesByRepo[repo.id]?.find((worktree) => worktree.isMainWorktree)
: null
if (!repo || !source || !state.settings) {
throw new Error('Expected a seeded e2e worktree and hydrated settings')
}
const now = Date.now()
const worktrees = Array.from({ length: count }, (_, index) => {
const suffix = String(index).padStart(2, '0')
return {
...source,
id: `${repo.id}::active-delete-${suffix}`,
instanceId: `active-delete-${suffix}`,
path: source.path,
displayName: `Active delete row ${suffix}`,
branch: `active-delete-${suffix}`,
isMainWorktree: false,
isPinned: false,
isUnread: false,
sortOrder: count - index,
manualOrder: count - index,
lastActivityAt: now - index,
parentWorktreeId: null,
childWorktreeIds: [],
lineage: null
}
})
const target = worktrees[targetIndex]
const below = worktrees[targetIndex + 1]
const successor = worktrees[0]
if (!target || !below || !successor) {
throw new Error('Synthetic worktree fixture is too small')
}
const targetId = target.id
const belowId = below.id
const successorId = successor.id
store.setState({
activeRepoId: repo.id,
activeView: 'terminal',
activeWorktreeId: targetId,
activeWorkspaceKey: `worktree:${targetId}`,
filterRepoIds: [],
groupBy: 'none',
hideDefaultBranchWorkspace: false,
lastVisitedAtByWorktreeId: { [successorId]: now + 1_000 },
pendingRevealSidebarRow: null,
pendingRevealWorktree: null,
repos: state.repos.map((candidate) =>
candidate.id === repo.id
? {
...candidate,
hookSettings: {
mode: candidate.hookSettings?.mode ?? 'auto',
...candidate.hookSettings,
scripts: {
archive: candidate.hookSettings?.scripts.archive ?? '',
setup: 'true'
}
}
}
: candidate
),
settings: { ...state.settings, skipDeleteWorktreeConfirm: true },
setupScriptPromptDismissedRepoIds: [`generation-v1:local\0${repo.id}`],
showActiveOnly: false,
showSleepingWorkspaces: true,
sidebarOpen: true,
sortBy: 'manual',
worktreesByRepo: { ...state.worktreesByRepo, [repo.id]: worktrees },
removeWorktree: async (target) => {
const id = typeof target === 'string' ? target : target.id
store.setState((current) => ({
activeWorktreeId: current.activeWorktreeId === id ? null : current.activeWorktreeId,
activeWorkspaceKey: current.activeWorktreeId === id ? null : current.activeWorkspaceKey,
worktreesByRepo: {
...current.worktreesByRepo,
[repo.id]: (current.worktreesByRepo[repo.id] ?? []).filter(
(worktree) => worktree.id !== id
)
}
}))
return { ok: true }
}
})
return { belowId, successorId, targetId }
},
{ count: SYNTHETIC_COUNT, targetIndex: TARGET_INDEX }
)
}
async function prepareScrolledActiveRow(page: Page, targetId: string): Promise<void> {
const target = page.locator(
`[data-worktree-sidebar] [data-worktree-id=${JSON.stringify(targetId)}]`
)
const scroller = page.locator('[data-worktree-sidebar]')
await expect
.poll(async () => {
if ((await target.count()) > 0) {
return true
}
await scroller.evaluate((element) => {
element.scrollTop = Math.min(
element.scrollHeight,
element.scrollTop + Math.max(100, element.clientHeight / 2)
)
element.dispatchEvent(new Event('scroll', { bubbles: true }))
})
return false
})
.toBe(true)
await target.evaluate((element) => element.scrollIntoView({ block: 'center' }))
await target.evaluate((element) => {
const scroller = element.closest<HTMLElement>('[data-worktree-sidebar]')
if (!scroller) {
throw new Error('Worktree sidebar is unavailable')
}
const targetOffset = element.getBoundingClientRect().top - scroller.getBoundingClientRect().top
scroller.scrollTop += targetOffset - 160
scroller.dispatchEvent(new Event('scroll', { bubbles: true }))
})
await expect(target).toBeVisible()
await expect(target).toHaveAttribute('aria-current', 'page')
}
async function startRowRemovalSampling(
page: Page,
targetId: string,
belowId: string
): Promise<void> {
await page.evaluate(
({ belowId, maxRemovalWaitFrames, postRemovalSampleFrames, targetId }) => {
const sample = async (): Promise<RowRemovalFrame[]> => {
const readFrame = (): RowRemovalFrame => {
const scroller = document.querySelector<HTMLElement>('[data-worktree-sidebar]')
const below = document.querySelector<HTMLElement>(
`[data-worktree-sidebar] [data-worktree-id=${JSON.stringify(belowId)}]`
)
const targetExists = Boolean(
document.querySelector(
`[data-worktree-sidebar] [data-worktree-id=${JSON.stringify(targetId)}]`
)
)
return {
animationCount:
below?.closest('[data-worktree-virtual-row]')?.firstElementChild?.getAnimations()
.length ?? 0,
belowTop: below?.getBoundingClientRect().top ?? null,
scrollTop: scroller?.scrollTop ?? 0,
targetExists
}
}
const frames: RowRemovalFrame[] = [readFrame()]
let framesAfterRemoval = 0
for (let index = 0; index < maxRemovalWaitFrames + postRemovalSampleFrames; index += 1) {
await new Promise<void>((resolve) => requestAnimationFrame(() => resolve()))
const frame = readFrame()
frames.push(frame)
framesAfterRemoval = frame.targetExists ? 0 : framesAfterRemoval + 1
if (framesAfterRemoval >= postRemovalSampleFrames) {
break
}
}
return frames
}
Reflect.set(window, '__activeDeleteRowRemovalFrames', sample())
},
{
belowId,
maxRemovalWaitFrames: MAX_REMOVAL_WAIT_FRAMES,
postRemovalSampleFrames: POST_REMOVAL_SAMPLE_FRAMES,
targetId
}
)
}
async function finishRowRemovalSampling(page: Page): Promise<RowRemovalFrame[]> {
return page.evaluate(async () => {
const pending = Reflect.get(window, '__activeDeleteRowRemovalFrames')
if (!(pending instanceof Promise)) {
throw new Error('Row removal sampling was not started')
}
return pending
})
}
test('deleting the active scrolled worktree preserves position and closes the row gap', async ({
orcaPage
}) => {
await waitForSessionReady(orcaPage)
await orcaPage.setViewportSize({ width: 1_200, height: 800 })
const { belowId, successorId, targetId } = await seedActiveDeletionRows(orcaPage)
await prepareScrolledActiveRow(orcaPage, targetId)
const target = orcaPage.locator(
`[data-worktree-sidebar] [data-worktree-id=${JSON.stringify(targetId)}]`
)
const below = orcaPage.locator(
`[data-worktree-sidebar] [data-worktree-id=${JSON.stringify(belowId)}]`
)
await pauseForVisualProof(orcaPage)
await target.evaluate((element) => {
const scope = element.querySelector<HTMLElement>(
'[data-worktree-context-menu-scope="worktree"]'
)
if (!scope) {
throw new Error('Worktree context-menu scope is unavailable')
}
scope.dispatchEvent(
new MouseEvent('contextmenu', {
bubbles: true,
button: 2,
cancelable: true,
clientX: scope.getBoundingClientRect().left + 10,
clientY: scope.getBoundingClientRect().top + 10
})
)
})
const deleteItem = orcaPage.getByRole('menuitem', { name: 'Delete', exact: true })
await expect(deleteItem).toBeVisible()
await expect(deleteItem).toBeInViewport()
await pauseForVisualProof(orcaPage)
await startRowRemovalSampling(orcaPage, targetId, belowId)
await deleteItem.click()
await expect(target).toHaveCount(0)
await expect(below).toBeVisible()
await expect
.poll(() => orcaPage.evaluate(() => window.__store?.getState().activeWorktreeId ?? null))
.toBe(successorId)
const frames = await finishRowRemovalSampling(orcaPage)
await pauseForVisualProof(orcaPage)
const mountedTops = frames.flatMap((frame) => (frame.belowTop === null ? [] : [frame.belowTop]))
const firstRemovedFrame = frames.findIndex((frame) => !frame.targetExists)
const scrollTopBeforeDelete = frames[0]?.scrollTop
if (scrollTopBeforeDelete === undefined) {
throw new Error('Row removal sampler recorded no pre-delete frame')
}
expect(firstRemovedFrame).toBeGreaterThan(0)
expect(frames.slice(firstRemovedFrame).every((frame) => !frame.targetExists)).toBe(true)
expect(Math.max(...frames.map((frame) => frame.animationCount))).toBeGreaterThan(0)
expect(Math.max(...mountedTops) - Math.min(...mountedTops)).toBeGreaterThan(30)
expect(Math.max(...frames.map((frame) => frame.scrollTop))).toBeLessThanOrEqual(
scrollTopBeforeDelete + 1
)
expect(Math.min(...frames.map((frame) => frame.scrollTop))).toBeGreaterThanOrEqual(
scrollTopBeforeDelete - 1
)
await expect(
orcaPage.locator(`[data-worktree-sidebar] [data-worktree-id=${JSON.stringify(successorId)}]`)
).toHaveCount(0)
})
test('reduced motion removes the active row without animating its neighbor', async ({
orcaPage
}) => {
await orcaPage.emulateMedia({ reducedMotion: 'reduce' })
await waitForSessionReady(orcaPage)
const { belowId, targetId } = await seedActiveDeletionRows(orcaPage)
await prepareScrolledActiveRow(orcaPage, targetId)
const animationCount = await orcaPage.evaluate(
async ({ belowId, targetId }) => {
const store = window.__store
if (!store) {
throw new Error('window.__store is not available')
}
const state = store.getState()
const repo = state.repos[0]
if (!repo) {
throw new Error('Expected a seeded e2e repo')
}
const repoId = repo.id
const worktrees = state.worktreesByRepo[repoId]
if (!worktrees) {
throw new Error('Expected seeded e2e worktrees')
}
store.setState({
activeWorktreeId: null,
activeWorkspaceKey: null,
worktreesByRepo: {
...state.worktreesByRepo,
[repoId]: worktrees.filter((worktree) => worktree.id !== targetId)
}
})
await new Promise<void>((resolve) => requestAnimationFrame(() => resolve()))
const below = document.querySelector<HTMLElement>(
`[data-worktree-sidebar] [data-worktree-id=${JSON.stringify(belowId)}]`
)
return (
below?.closest('[data-worktree-virtual-row]')?.firstElementChild?.getAnimations().length ??
0
)
},
{ belowId, targetId }
)
expect(animationCount).toBe(0)
})