From 867a77a38fa2d5d748e7e5e172fc6be6addabc87 Mon Sep 17 00:00:00 2001 From: Luke Parker <10430890+Hona@users.noreply.github.com> Date: Fri, 16 Oct 2026 09:34:29 +1000 Subject: [PATCH] fix(app): keep the review pane steady when the side terminal toggles (#55879) --- .../app/e2e/regression/side-panels.spec.ts | 62 ++++++++++++++++--- packages/app/src/index.css | 14 +++++ .../app/src/runtime/extension/side-region.tsx | 7 +-- packages/app/src/session/screen-layout.ts | 15 +---- packages/app/src/session/screen.tsx | 16 ++--- .../gui-extensions/src/terminal/panel.tsx | 2 +- packages/ui/src/navigation/tabs/tabs.css | 1 - 7 files changed, 79 insertions(+), 38 deletions(-) diff --git a/packages/app/e2e/regression/side-panels.spec.ts b/packages/app/e2e/regression/side-panels.spec.ts index a41039333b1..bd78a6aa0c4 100644 --- a/packages/app/e2e/regression/side-panels.spec.ts +++ b/packages/app/e2e/regression/side-panels.spec.ts @@ -109,7 +109,7 @@ test("terminal stacks under review by default and spans the bottom when configur ), ] - await openSession(page, { name: "SideStack", pty: {}, vcsDiff: diffs }) + const { session } = await openSession(page, { name: "SideStack", pty: {}, vcsDiff: diffs }) const toggle = page.getByRole("button", { name: "Toggle review" }) const reviewPanel = page.locator("#review-panel") const terminalPanel = page.locator("#terminal-panel") @@ -163,13 +163,30 @@ test("terminal stacks under review by default and spans the bottom when configur expect(Math.abs(closing.terminalBottom - stacked.terminalBottom)).toBeLessThanOrEqual(1) expect(closing.anchor).toBeLessThanOrEqual(8) expect(closing.terminalGap).toBeLessThanOrEqual(1) + expect(closing.reviewOverlap).toBeLessThanOrEqual(1) await toggle.click() await expect(reviewPanel).toBeVisible() await expectMarked(review) await expectTreeSentinel(page) - // Hiding the terminal mid-motion: both surfaces fill their regions and the clipped terminal keeps its size. + // Panels restored by a reload skip their entrance animation, and must still move together when the terminal hides. + await expect + .poll(() => + page.evaluate( + (key) => JSON.parse(localStorage.getItem("opencode.window.browser.dat:tabs.panes") ?? "{}")[key], + tabKey(session.id), + ), + ) + .toMatchObject({ review: true, terminal: true }) + await page.reload() + await expect(reviewPanel).toBeVisible() + await expect(terminal).toBeVisible() + await expectTreeSentinel(page) + await mark(terminal) + + // Hiding the terminal mid-motion: both surfaces fill their regions, the review never runs under the gap, and the + // clipped terminal keeps its size. const open = await stackGeometry(page) await holdTransitions(page) await page.keyboard.press("Control+Backquote") @@ -180,6 +197,7 @@ test("terminal stacks under review by default and spans the bottom when configur expect(hiding.terminalRegion).toBeLessThan(open.terminalRegion - 1) expect(hiding.terminalRegion).toBeGreaterThan(1) expect(hiding.reviewGap).toBeLessThanOrEqual(1) + expect(hiding.reviewOverlap).toBeLessThanOrEqual(1) expect(hiding.terminalGap).toBeLessThanOrEqual(1) expect(Math.abs(hiding.content.width - open.content.width)).toBeLessThanOrEqual(1) expect(Math.abs(hiding.content.height - open.content.height)).toBeLessThanOrEqual(1) @@ -293,6 +311,23 @@ for (const direction of ["ltr", "rtl"] as const) { return actions.y + actions.height / 2 - (button.y + button.height / 2) }) .toBe(0) + + // Toggling the terminal under the review leaves the review header and the toggle in place. + const header = async () => { + const box = await page.locator('[data-slot="session-side-panel-actions"]').boundingBox() + + return box && { y: box.y, height: box.height } + } + + const stacked = await header() + + for (const visible of [false, true]) { + await page.keyboard.press("Control+Backquote") + await expect(terminal).toBeVisible({ visible }) + await expect.poll(header).toEqual(stacked) + await expect.poll(() => toggle.boundingBox()).toEqual(position) + } + await toggle.press("Enter") await expect(toggle).toHaveAttribute("aria-expanded", "false") await expect(toggle).toBeFocused() @@ -321,6 +356,7 @@ async function expectTreeSentinel(page: Page) { // Pauses each CSS transition as it starts, so a real toggle can be measured mid-motion. function holdTransitions(page: Page) { return page.evaluate(() => { + // SAFETY: only these two helpers write this optional global, in the declared shape. const host = window as Window & { e2eHold?: { active: boolean } } if (!host.e2eHold) { @@ -341,7 +377,9 @@ function holdTransitions(page: Page) { function releaseTransitions(page: Page) { return page.evaluate(() => { - ;(window as Window & { e2eHold?: { active: boolean } }).e2eHold!.active = false + // SAFETY: `holdTransitions` installed this global, in this shape, before any release. + const host = window as Window & { e2eHold?: { active: boolean } } + host.e2eHold!.active = false document .getAnimations() .filter((item) => item instanceof CSSTransition && item.playState === "paused") @@ -359,7 +397,7 @@ async function stackGeometry(page: Page, progress?: number) { (item) => item instanceof CSSTransition && item.playState === "paused" && item.transitionProperty === "height", ) - .map((item) => ((item.effect as KeyframeEffect).target as Element).getAttribute("data-slot")), + .map((item) => (item.effect instanceof KeyframeEffect ? item.effect.target?.getAttribute("data-slot") : null)), ) if (progress !== undefined) { @@ -371,9 +409,10 @@ async function stackGeometry(page: Page, progress?: number) { .getAnimations() .filter((item): item is CSSTransition => item instanceof CSSTransition && item.playState === "paused") - const region = transitions.find( - (item) => ((item.effect as KeyframeEffect).target as Element).getAttribute("data-slot") === "session-side-region", - ) + const slot = (item: CSSTransition) => + item.effect instanceof KeyframeEffect ? item.effect.target?.getAttribute("data-slot") : null + + const region = transitions.find((item) => slot(item) === "session-side-region") if (progress !== undefined) { const time = Number(region!.effect!.getTiming().duration) * progress @@ -382,17 +421,20 @@ async function stackGeometry(page: Page, progress?: number) { const box = (selector: string) => document.querySelector(selector)?.getBoundingClientRect() const reviewRegion = box('[data-slot="session-side-region"]')! + const gap = box('[data-slot="session-side-panel-gap"]')! const terminalRegion = box('[data-slot="session-side-terminal-region"]')! const review = box("#review-panel") const terminal = box("#terminal-panel")! const content = box('[data-slot="terminal-panel-content"]')! return { - moving: transitions.map((item) => ((item.effect as KeyframeEffect).target as Element).getAttribute("data-slot")), + moving: transitions.map(slot), terminalRegion: terminalRegion.height, terminalBottom: terminal.bottom, anchor: Math.abs(terminal.top - content.top), reviewGap: review ? Math.abs(reviewRegion.height - review.height) : 0, + // The gap paints over the review region, so any overlap hides the review's bottom corners. + reviewOverlap: reviewRegion.bottom - gap.top, terminalGap: Math.abs(terminalRegion.height - terminal.height), content: { width: content.width, height: content.height }, } @@ -413,8 +455,8 @@ async function expectTerminalControlsAligned(terminal: Locator, toggle: Locator) .poll(async () => { const centers = await Promise.all( [terminal.getByRole("button", { name: "New terminal", exact: true }), toggle].map((button) => - button.locator("svg").evaluate((element) => { - const svg = element as SVGSVGElement + button.locator("svg").evaluate((svg) => { + if (!(svg instanceof SVGGraphicsElement)) return Number.NaN const path = svg.getBBox() return new DOMPoint(path.x + path.width / 2, path.y + path.height / 2).matrixTransform(svg.getScreenCTM()!) diff --git a/packages/app/src/index.css b/packages/app/src/index.css index defc7d1f6eb..ae897e01682 100644 --- a/packages/app/src/index.css +++ b/packages/app/src/index.css @@ -107,6 +107,11 @@ animation: side-region-presence-out 240ms cubic-bezier(0.22, 1, 0.36, 1) forwards; } + /* A collapsing frame's elevation outlives its height as a flat line over the panel beside it, so it fades out. */ + [data-slot="session-side-region-presence"][data-opened="false"] #review-panel { + animation: side-panel-frame-out 240ms cubic-bezier(0.22, 1, 0.36, 1) forwards; + } + [data-slot="terminal-panel-presence"][data-opened="true"] { animation: terminal-panel-presence-in 200ms cubic-bezier(0.22, 1, 0.36, 1); } @@ -137,6 +142,7 @@ [data-slot="session-side-panel-presence"], [data-slot="session-review-content"], [data-slot="session-side-region-presence"], + [data-slot="session-side-region-presence"] #review-panel, [data-component="terminal-panel"] { animation: none !important; } @@ -173,6 +179,7 @@ } } + /* Like `side-panel-frame-out`, the collapsing terminal frame drops its elevation. */ @keyframes side-terminal-panel-presence-out { from { opacity: 1; @@ -180,6 +187,13 @@ to { opacity: 0.999999; visibility: hidden; + box-shadow: none; + } + } + + @keyframes side-panel-frame-out { + to { + box-shadow: none; } } diff --git a/packages/app/src/runtime/extension/side-region.tsx b/packages/app/src/runtime/extension/side-region.tsx index 016308bdab8..7f4a0c2cf2f 100644 --- a/packages/app/src/runtime/extension/side-region.tsx +++ b/packages/app/src/runtime/extension/side-region.tsx @@ -159,12 +159,7 @@ export function SideRegion(props: { props.region.select(value) }} > - {/* Tabs and actions share the review toggle's row: the session header's 48px, or the 51px - above a side dock's divider. */} -
+
{ tabList = el diff --git a/packages/app/src/session/screen-layout.ts b/packages/app/src/session/screen-layout.ts index e276143b346..a721b163628 100644 --- a/packages/app/src/session/screen-layout.ts +++ b/packages/app/src/session/screen-layout.ts @@ -1,4 +1,4 @@ -import { createEffect, createMemo, type Accessor } from "solid-js" +import { createMemo, type Accessor } from "solid-js" import { createStore } from "solid-js/store" import { createResizeObserver } from "@solid-primitives/resize-observer" import { useLayout } from "@/shell/state/layout" @@ -74,14 +74,6 @@ export function createSessionScreenLayout( stacked: tabsOpen() && dockSideOpen(), })) - const [motion, setMotion] = createStore({ gap: panelLayout().stacked, closing: false }) - createEffect((previous) => { - const stacked = panelLayout().stacked - - if (previous !== stacked) setMotion({ gap: stacked, closing: !stacked }) - - return stacked - }, panelLayout().stacked) const sideRegionOpen = createMemo(() => tabsOpen() || fileTreeOpen()) const dockRegion = createMemo(() => @@ -124,10 +116,7 @@ export function createSessionScreenLayout( }, side: { contentWidth: sideContentWidth, - gap: { - closing: () => motion.closing, - height: createMemo(() => (motion.gap ? "8px" : "0px")), - }, + gap: { height: createMemo(() => (panelLayout().stacked ? "8px" : "0px")) }, layout: panelLayout, region: { height: createMemo(() => { diff --git a/packages/app/src/session/screen.tsx b/packages/app/src/session/screen.tsx index 8080d7afe82..fb5a261f0e8 100644 --- a/packages/app/src/session/screen.tsx +++ b/packages/app/src/session/screen.tsx @@ -172,6 +172,9 @@ function SessionScreenContent(props: { if (bottomDockVisible()) setStore("bottomDockCached", true) if (!sideVisible()) setStore("sideHeightMotion", false) + + // A side panel restored without its entrance animation never fires the animationend that enables height motion. + if (sideVisible() && !sidePresence.animate()) setStore("sideHeightMotion", true) }) createEffect(() => { if (!isDesktop() || screen.dock.bottom()) setStore("sideDockPresent", false) @@ -368,11 +371,10 @@ function SessionScreenContent(props: { <>
- {/* Keep the control outside panel animations; a side dock's 52px header includes a 1px divider. */} + {/* Keep the control outside panel animations. */}
+ {/* The gap moves with both regions, so together they always fill the column and the review's + bottom corners are never covered. */}
screen.size.start()} diff --git a/packages/gui-extensions/src/terminal/panel.tsx b/packages/gui-extensions/src/terminal/panel.tsx index a39906e7ba3..6e50565223a 100644 --- a/packages/gui-extensions/src/terminal/panel.tsx +++ b/packages/gui-extensions/src/terminal/panel.tsx @@ -266,7 +266,7 @@ export default function TerminalPanel(props: { model: TerminalModel; session: Mo }} >
-
+