fix(app): preserve virtual review tree lifecycle

Hona · Hona · commit af14e43292f2 · 2026-07-05T20:54:03.000+10:00
diff --git a/packages/app/e2e/regression/review-terminal-stacked.spec.ts b/packages/app/e2e/regression/review-terminal-stacked.spec.ts
@@ -1,4 +1,4 @@
-import { expect, test } from "@playwright/test"
+import { expect, test, type Page } from "@playwright/test"
 import { mockOpenCodeServer } from "../utils/mock-server"
 import { expectSessionTitle } from "../utils/waits"
 
@@ -14,6 +14,7 @@ const branchDiffs = [
 ]
 
 test("keeps the review tree and terminal sized when both panels are open", async ({ page }) => {
+  test.setTimeout(120_000)
   await page.setViewportSize({ width: 1400, height: 900 })
   await mockOpenCodeServer(page, {
     directory,
@@ -91,17 +92,73 @@ test("keeps the review tree and terminal sized when both panels are open", async
   await expect(page.locator("#terminal-panel")).toBeVisible()
   // Terminal activation retries focus through 240ms; let that settle before opening the mode select.
   await page.waitForTimeout(300)
-  await page.getByRole("button", { name: "Git changes" }).click()
-  await page.getByRole("option", { name: "Branch changes" }).click()
+  await selectMode(page, "Git changes", "Branch changes")
   await expect(page.getByRole("tab", { name: "Review 2740" })).toBeVisible()
-  await expect(page.getByRole("button", { name: "action.yml" })).toBeVisible()
+  await expectTree(page, 2_745, "action.yml")
+  await expectStackGeometry(page)
 
+  await page.locator('#review-panel [data-component="file-tree-v2"]').evaluate((element) => {
+    const viewport = element.closest<HTMLElement>(".scroll-view__viewport")!
+    viewport.scrollTop = viewport.scrollHeight
+  })
+  await selectMode(page, "Branch changes", "Git changes")
+  await expectTree(page, 2, "git.ts")
+  await selectMode(page, "Git changes", "Branch changes")
+  await expectTree(page, 2_745, "action.yml")
+
+  const filter = page.getByRole("searchbox", { name: "Filter files" })
+  await filter.fill("generated-2738")
+  await expectTree(page, 1, "generated-2738.ts")
+  await filter.fill("")
+  await expectTree(page, 2_745, "action.yml")
+
+  await page.getByRole("button", { name: "Toggle file tree" }).click()
+  await expect(page.locator('[data-slot="session-review-v2-sidebar"]')).toHaveAttribute("aria-hidden", "true")
+  await expect(page.locator('#review-panel [data-component="file-tree-v2"]')).toHaveCount(1)
+  await page.getByRole("button", { name: "Toggle file tree" }).click()
+  await expectTree(page, 2_745, "action.yml")
+
+  await page.keyboard.press("Control+Backquote")
+  await expect(page.locator("#terminal-panel")).toHaveCount(0)
+  await expectTree(page, 2_745, "action.yml")
+  await page.keyboard.press("Control+Backquote")
+  await expect(page.locator("#terminal-panel")).toBeVisible()
+  await expectTree(page, 2_745, "action.yml")
+
+  await page.getByRole("button", { name: "Toggle review" }).click()
+  await expect(page.locator("#review-panel")).toHaveAttribute("aria-hidden", "true")
+  await expect(page.locator('#review-panel [data-component="file-tree-v2"]')).toHaveCount(1)
+  await page.getByRole("button", { name: "Toggle review" }).click()
+  await expectTree(page, 2_745, "action.yml")
+  await page.setViewportSize({ width: 1_000, height: 700 })
+  await expectTree(page, 2_745, "action.yml")
+  await expectStackGeometry(page)
+})
+
+async function selectMode(page: Page, current: string, next: string) {
+  await page.getByRole("button", { name: current }).click()
+  await page.getByRole("option", { name: next }).click()
+}
+
+async function expectTree(page: Page, total: number, file: string) {
   const tree = page.locator('#review-panel [data-component="file-tree-v2"]')
-  await expect(tree).toHaveAttribute("data-total-rows", "2745")
-  const renderedRows = await tree.locator('[data-slot="file-tree-v2-row"]').count()
-  expect(renderedRows).toBeGreaterThan(0)
-  expect(renderedRows).toBeLessThanOrEqual(60)
+  await expect(tree).toHaveAttribute("data-total-rows", String(total))
+  await expect
+    .poll(() => tree.evaluate((element) => element.querySelectorAll('[data-slot="file-tree-v2-row"]').length))
+    .toBeGreaterThan(0)
+  const state = await tree.evaluate((element) => ({
+    root: element.getBoundingClientRect().height,
+    viewport: element.closest<HTMLElement>(".scroll-view__viewport")!.getBoundingClientRect().height,
+    rows: element.querySelectorAll('[data-slot="file-tree-v2-row"]').length,
+  }))
+  expect(state.viewport).toBeGreaterThan(0)
+  expect(state.root).toBeGreaterThan(0)
+  expect(state.rows).toBeGreaterThan(0)
+  expect(state.rows).toBeLessThanOrEqual(60)
+  await expect(page.getByRole("button", { name: file })).toBeVisible()
+}
 
+async function expectStackGeometry(page: Page) {
   const geometry = await page.evaluate(() => {
     const review = document.querySelector<HTMLElement>("#review-panel")!
     const terminal = document.querySelector<HTMLElement>("#terminal-panel")!
@@ -116,7 +173,7 @@ test("keeps the review tree and terminal sized when both panels are open", async
   })
   expect(Math.abs(geometry.review - geometry.reviewParent)).toBeLessThanOrEqual(1)
   expect(Math.abs(geometry.terminal - geometry.terminalParent)).toBeLessThanOrEqual(1)
-})
+}
 
 function base64Encode(value: string) {
   return Buffer.from(value, "utf8").toString("base64").replace(/\+/g, "-").replace(/\//g, "_").replace(/=/g, "")
diff --git a/packages/app/src/components/file-tree-v2.tsx b/packages/app/src/components/file-tree-v2.tsx
@@ -124,6 +124,7 @@ export default function FileTreeV2(props: {
   allowed?: readonly string[]
   kinds?: ReadonlyMap<string, Kind>
   draggable?: boolean
+  scrollElement?: HTMLDivElement
   onFileClick?: (file: FileNode) => void
 }) {
   const file = useFile()
@@ -137,7 +138,7 @@ export default function FileTreeV2(props: {
     get count() {
       return rows().length
     },
-    getScrollElement: () => root()?.closest<HTMLDivElement>(".scroll-view__viewport") ?? null,
+    getScrollElement: () => props.scrollElement ?? root()?.closest<HTMLDivElement>(".scroll-view__viewport") ?? null,
     estimateSize: () => 28,
     gap: 2,
     overscan: 10,
diff --git a/packages/app/src/pages/session.tsx b/packages/app/src/pages/session.tsx
@@ -2169,8 +2169,13 @@ export default function Page() {
         <Show when={newSessionDesign()}>
           <Show when={isDesktop() ? desktopV2PanelLayout().visible : terminalOpen()}>
             <div class="min-w-0 h-full flex flex-1 flex-col">
-              <Show when={isDesktop() && (desktopV2ReviewOpen() || desktopFileTreeOpen())}>
-                <div class="min-h-0 flex-1">
+              <Show when={isDesktop()}>
+                <div
+                  classList={{
+                    "min-h-0 flex-1": desktopV2ReviewOpen() || desktopFileTreeOpen(),
+                    "size-0 shrink-0 overflow-hidden": !(desktopV2ReviewOpen() || desktopFileTreeOpen()),
+                  }}
+                >
                   <SessionSidePanel
                     canReview={canReview}
                     diffs={reviewDiffs}
diff --git a/packages/app/src/pages/session/session-side-panel.tsx b/packages/app/src/pages/session/session-side-panel.tsx
@@ -79,6 +79,7 @@ export function SessionSidePanel(props: {
       }),
   )
   const open = createMemo(() => reviewOpen() || fileOpen())
+  const rendered = createMemo<boolean>((previous) => previous || open(), false)
   const reviewTab = createMemo(() => isDesktop())
   const panelWidth = createMemo(() => {
     if (!open()) return "0px"
@@ -159,6 +160,10 @@ export function SessionSidePanel(props: {
   const openedTabs = tabState.openedTabs
   const activeTab = tabState.activeTab
   const activeFileTab = tabState.activeFileTab
+  const reviewContentRendered = createMemo<boolean>(
+    (previous) => previous || (reviewOpen() && activeTab() === "review"),
+    false,
+  )
 
   const fileTreeTab = () => layout.fileTree.tab()
 
@@ -236,7 +241,7 @@ export function SessionSidePanel(props: {
         }}
         style={{ width: panelWidth() }}
       >
-        <Show when={open()}>
+        <Show when={rendered()}>
           <div
             class="size-full flex"
             classList={{
@@ -336,14 +341,17 @@ export function SessionSidePanel(props: {
                       </Tabs.List>
                     </div>
 
-                    <Show when={reviewTab() && props.canReview() && reviewOpen() && activeTab() === "review"}>
+                    <Show when={reviewTab() && props.canReview() && reviewContentRendered()}>
                       <div
                         id={reviewTabPanelID}
                         role="tabpanel"
                         aria-labelledby={reviewTabID}
+                        aria-hidden={activeTab() !== "review"}
+                        inert={activeTab() !== "review"}
                         tabIndex={props.reviewHasFocusableContent() ? undefined : 0}
                         data-slot="tabs-content"
                         class="flex flex-col h-full overflow-hidden contain-strict"
+                        classList={{ hidden: activeTab() !== "review" }}
                       >
                         {props.reviewPanel()}
                       </div>
diff --git a/packages/app/src/pages/session/v2/review-panel-v2.tsx b/packages/app/src/pages/session/v2/review-panel-v2.tsx
@@ -160,6 +160,7 @@ function ReviewPanelV2Sidebar(props: {
 }) {
   const language = useLanguage()
   const [explicitHighlight, setExplicitHighlight] = createSignal<string | undefined>()
+  const [viewport, setViewport] = createSignal<HTMLDivElement>()
   const highlightedPath = createMemo(() => {
     if (!props.searching()) return undefined
     const files = props.filteredFiles()
@@ -189,6 +190,7 @@ function ReviewPanelV2Sidebar(props: {
       onWidthChange={props.state.resizeSidebar}
       minWidth={SESSION_REVIEW_V2_SIDEBAR_WIDTH_MIN}
       maxWidth={SESSION_REVIEW_V2_SIDEBAR_WIDTH_MAX}
+      viewportRef={setViewport}
     >
       <Show
         when={props.diffsReady()}
@@ -207,6 +209,7 @@ function ReviewPanelV2Sidebar(props: {
               kinds={props.kinds()}
               draggable={false}
               active={props.activeDiff()}
+              scrollElement={viewport()}
               onFileClick={(node) => props.onSelectFile(node.path)}
             />
           }
@@ -220,6 +223,7 @@ function ReviewPanelV2Sidebar(props: {
               kinds={props.kinds()}
               active={props.activeDiff()}
               highlighted={highlightedPath()}
+              scrollElement={viewport()}
               onFileClick={(path) => {
                 setExplicitHighlight(path)
                 props.onSelectFile(path)
diff --git a/packages/app/src/pages/session/v2/session-file-list-v2.tsx b/packages/app/src/pages/session/v2/session-file-list-v2.tsx
@@ -42,6 +42,7 @@ export function SessionFileListV2(props: {
   active?: string
   highlighted?: string
   kinds?: ReadonlyMap<string, Kind>
+  scrollElement?: HTMLDivElement
   onFileClick: (path: string) => void
 }) {
   const active = () => normalizePath(props.active ?? "")
@@ -53,7 +54,7 @@ export function SessionFileListV2(props: {
     get count() {
       return props.files.length
     },
-    getScrollElement: () => root()?.closest<HTMLDivElement>(".scroll-view__viewport") ?? null,
+    getScrollElement: () => props.scrollElement ?? root()?.closest<HTMLDivElement>(".scroll-view__viewport") ?? null,
     estimateSize: () => 28,
     gap: 2,
     overscan: 10,
diff --git a/packages/session-ui/src/v2/components/session-review-v2.tsx b/packages/session-ui/src/v2/components/session-review-v2.tsx
@@ -49,6 +49,7 @@ export type SessionReviewV2SidebarProps = {
   onWidthChange?: (width: number) => void
   minWidth?: number
   maxWidth?: number
+  viewportRef?: (element: HTMLDivElement) => void
   children?: JSX.Element
 }
 
@@ -75,44 +76,47 @@ export function SessionReviewV2Sidebar(props: SessionReviewV2SidebarProps) {
         inert={!props.open}
         style={{ width: props.open ? `${width()}px` : "0px" }}
       >
-        <Show when={props.open}>
-          <div data-slot="session-review-v2-sidebar-header">
-            <div data-slot="session-review-v2-sidebar-title">{props.title}</div>
-            {props.stats}
-          </div>
-          <div data-slot="session-review-v2-sidebar-filter">
-            <TextInputV2
-              type="search"
-              value={props.filter}
-              onInput={(event) => props.onFilterChange(event.currentTarget.value)}
-              onKeyDown={props.onFilterKeyDown}
-              showClearButton={props.filter.length > 0}
-              clearLabel={i18n.t("ui.list.clearFilter")}
-              onClearClick={() => props.onFilterChange("")}
-              placeholder={i18n.t("ui.sessionReviewV2.filterFiles")}
-              aria-label={i18n.t("ui.sessionReviewV2.filterFiles")}
-              leadingIcon={
-                <svg
-                  width="14"
-                  height="14"
-                  viewBox="0 0 14 14"
-                  fill="none"
-                  xmlns="http://www.w3.org/2000/svg"
-                  aria-hidden="true"
-                >
-                  <path
-                    d="M12.25 12.25L10.0625 10.0625M11.0833 6.41667C11.0833 8.994 8.994 11.0833 6.41667 11.0833C3.83934 11.0833 1.75 8.994 1.75 6.41667C1.75 3.83934 3.83934 1.75 6.41667 1.75C8.994 1.75 11.0833 3.83934 11.0833 6.41667Z"
-                    stroke="currentColor"
-                    stroke-linecap="square"
-                  />
-                </svg>
-              }
-            />
-          </div>
-          <ScrollView data-slot="session-review-v2-sidebar-tree" class="group/file-tree-v2" thumbVisibility="scroll">
-            {props.children}
-          </ScrollView>
-        </Show>
+        <div data-slot="session-review-v2-sidebar-header">
+          <div data-slot="session-review-v2-sidebar-title">{props.title}</div>
+          {props.stats}
+        </div>
+        <div data-slot="session-review-v2-sidebar-filter">
+          <TextInputV2
+            type="search"
+            value={props.filter}
+            onInput={(event) => props.onFilterChange(event.currentTarget.value)}
+            onKeyDown={props.onFilterKeyDown}
+            showClearButton={props.filter.length > 0}
+            clearLabel={i18n.t("ui.list.clearFilter")}
+            onClearClick={() => props.onFilterChange("")}
+            placeholder={i18n.t("ui.sessionReviewV2.filterFiles")}
+            aria-label={i18n.t("ui.sessionReviewV2.filterFiles")}
+            leadingIcon={
+              <svg
+                width="14"
+                height="14"
+                viewBox="0 0 14 14"
+                fill="none"
+                xmlns="http://www.w3.org/2000/svg"
+                aria-hidden="true"
+              >
+                <path
+                  d="M12.25 12.25L10.0625 10.0625M11.0833 6.41667C11.0833 8.994 8.994 11.0833 6.41667 11.0833C3.83934 11.0833 1.75 8.994 1.75 6.41667C1.75 3.83934 3.83934 1.75 6.41667 1.75C8.994 1.75 11.0833 3.83934 11.0833 6.41667Z"
+                  stroke="currentColor"
+                  stroke-linecap="square"
+                />
+              </svg>
+            }
+          />
+        </div>
+        <ScrollView
+          data-slot="session-review-v2-sidebar-tree"
+          class="group/file-tree-v2"
+          thumbVisibility="scroll"
+          viewportRef={props.viewportRef}
+        >
+          {props.children}
+        </ScrollView>
       </aside>
       <Show when={props.open && props.onWidthChange}>
         <div data-slot="session-review-v2-sidebar-resize" onPointerDown={() => setResizing(true)}>
