fix(app): keep the review pane steady when the side terminal toggles (#55879)

This commit is contained in:
Luke Parker
2026-10-16 09:34:29 +10:00
committed by GitHub
Unverified
parent 2e7ac8c8ea
commit 867a77a38f
7 changed files with 79 additions and 38 deletions
+52 -10
View File
@@ -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()!)
+14
View File
@@ -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;
}
}
@@ -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. */}
<div
class="session-review-v2-tabs-bar sticky top-0 shrink-0 flex items-center"
style={{ "--tabs-bar-height": props.stacked ? "51px" : "48px" }}
>
<div class="session-review-v2-tabs-bar sticky top-0 shrink-0 flex items-center">
<Tabs.List
ref={(el: HTMLDivElement) => {
tabList = el
+2 -13
View File
@@ -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(() => {
+9 -7
View File
@@ -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: {
<>
<div class="flex-1 min-h-0 flex flex-col gap-2 px-[var(--shell-inline-inset,8px)] pb-[var(--shell-bottom-inset,8px)] pt-[var(--shell-top-inset,8px)]">
<div ref={screen.panel.ref} class="relative flex-1 min-h-0 flex flex-col md:flex-row gap-2">
{/* Keep the control outside panel animations; a side dock's 52px header includes a 1px divider. */}
{/* Keep the control outside panel animations. */}
<Show when={isDesktop() && messagesReady() && session.identity.params.id}>
<div
class="absolute end-3 top-0 z-30 flex items-center"
classList={{ "h-[51px]": sideDockVisible(), "h-12": !sideDockVisible() }}
class="absolute end-3 top-0 z-30 h-12 flex items-center"
data-slot="session-review-toggle"
onPointerDown={hideTimelineScrollbar}
onClick={hideTimelineScrollbar}
@@ -487,13 +489,13 @@ function SessionScreenContent(props: {
</Show>
</div>
<div class="absolute start-0 bottom-0 flex flex-col" style={{ width: screen.side.contentWidth() }}>
{/* The gap moves with both regions, so together they always fill the column and the review's
bottom corners are never covered. */}
<div
data-slot="session-side-panel-gap"
classList={{
"relative z-0 shrink-0 overflow-visible bg-v2-background-bg-deep transition-[height] duration-[40ms] ease-[cubic-bezier(0.22,1,0.36,1)] motion-reduce:transition-none": true,
"delay-0": !screen.side.gap.closing(),
"delay-[200ms]": screen.side.gap.closing(),
"transition-none": !regionAnimating(),
"relative z-0 shrink-0 overflow-visible bg-v2-background-bg-deep transition-[height] duration-[240ms] ease-[cubic-bezier(0.22,1,0.36,1)] motion-reduce:transition-none": true,
"transition-none": screen.size.active() || !store.sideHeightMotion || !regionAnimating(),
}}
style={{ height: screen.side.gap.height() }}
onPointerDown={() => screen.size.start()}
@@ -266,7 +266,7 @@ export default function TerminalPanel(props: { model: TerminalModel; session: Mo
}}
>
<div class="flex flex-col h-full">
<div class="h-[52px] shrink-0 flex border-b border-border-weaker-base">
<div class="h-[49px] shrink-0 flex border-b border-border-weaker-base">
<Tabs
variant="panel"
value={terminal().active()}
-1
View File
@@ -271,7 +271,6 @@
#terminal-panel &[data-variant="normal"][data-orientation="horizontal"] {
[data-slot="tabs-list"] {
height: 52px;
padding-inline-end: 12px;
gap: 8px;
}