From ac71a55294ac2a1a96f59db2cf8bc0641cdfa194 Mon Sep 17 00:00:00 2001 From: David Hill <1879069+iamdavidhill@users.noreply.github.com> Date: Thu, 3 Sep 2026 18:54:39 -0600 Subject: [PATCH] fix(app): keep right panel controls aligned (#46996) Co-authored-by: LukeParkerDev <10430890+Hona@users.noreply.github.com> --- .../component-tests/open-in-border.spec.ts | 18 ++ .../regression/review-toggle-position.spec.ts | 189 ++++++++++++++++++ .../session-header-controls.spec.ts | 2 +- .../src/session/files/session-side-panel.tsx | 7 +- .../session/header/session-header-actions.tsx | 22 ++ .../app/src/session/header/session-header.tsx | 18 +- packages/app/src/session/screen.tsx | 15 +- packages/app/src/session/terminal/panel.tsx | 91 +++++---- .../src/actions/split-button/split-button.css | 1 + .../split-button/split-button.stories.tsx | 14 ++ 10 files changed, 319 insertions(+), 58 deletions(-) create mode 100644 packages/app/component-tests/open-in-border.spec.ts create mode 100644 packages/app/e2e/regression/review-toggle-position.spec.ts diff --git a/packages/app/component-tests/open-in-border.spec.ts b/packages/app/component-tests/open-in-border.spec.ts new file mode 100644 index 00000000000..41f7551fd08 --- /dev/null +++ b/packages/app/component-tests/open-in-border.spec.ts @@ -0,0 +1,18 @@ +import { expect, story } from "../../storybook/playwright/story" + +for (const theme of ["light", "dark"]) { + story(`keeps the Open in border visible without hovering (${theme})`, async ({ mount, page }, testInfo) => { + const component = await mount("ui-split-button--open-in", { globals: { theme } }) + const control = component.locator('[data-component="split-button-v2"]') + await page.mouse.move(0, 0) + await expect(control).toBeVisible() + await expect(control).not.toHaveCSS("box-shadow", "none") + const border = await control.evaluate((element) => getComputedStyle(element).boxShadow) + + await component.getByRole("button", { name: "Open options" }).hover() + await expect(control).toHaveCSS("box-shadow", border) + await page.mouse.move(0, 0) + await expect(control).toHaveCSS("box-shadow", border) + await control.screenshot({ path: testInfo.outputPath(`open-in-${theme}.png`) }) + }) +} diff --git a/packages/app/e2e/regression/review-toggle-position.spec.ts b/packages/app/e2e/regression/review-toggle-position.spec.ts new file mode 100644 index 00000000000..16380077fca --- /dev/null +++ b/packages/app/e2e/regression/review-toggle-position.spec.ts @@ -0,0 +1,189 @@ +import { base64Encode } from "@opencode-ai/util/encode" +import { expect, test, type Locator } from "@playwright/test" +import { mockOpenCodeServer } from "../utils/mock-server" +import { expectSessionTitle } from "../utils/waits" + +const directory = "C:/OpenCode/ReviewTogglePosition" +const sessionID = "ses_review_toggle_position" +const server = `http://${process.env.PLAYWRIGHT_SERVER_HOST ?? "127.0.0.1"}:${process.env.PLAYWRIGHT_SERVER_PORT ?? "4096"}` + +test.beforeEach(async ({ page }) => { + await mockOpenCodeServer(page, { + directory, + project: { + id: "proj_review_toggle_position", + worktree: directory, + vcs: "git", + name: "review-toggle-position", + time: { created: 1700000000000, updated: 1700000000000 }, + sandboxes: [], + }, + provider: { all: [], connected: [], default: {} }, + sessions: [ + { + id: sessionID, + slug: "review-toggle-position", + projectID: "proj_review_toggle_position", + directory, + title: "Review toggle position", + version: "dev", + time: { created: 1700000000000, updated: 1700000000000 }, + }, + ], + pageMessages: () => ({ items: [] }), + }) +}) + +for (const width of [1000, 1440]) { + for (const direction of ["ltr", "rtl"] as const) { + test(`keeps the review toggle at the outer header edge (${width}px, ${direction})`, async ({ page }) => { + await page.setViewportSize({ width, height: 900 }) + await page.goto(`/server/${base64Encode(server)}/session/${sessionID}`) + await expectSessionTitle(page, "Review toggle position") + await page.locator("html").evaluate((element, dir) => element.setAttribute("dir", dir), direction) + + const toggle = page.getByRole("button", { name: "Toggle review", exact: true }) + const header = page.locator("[data-session-title]") + const panel = page.locator("#review-panel") + await expect(toggle).toHaveAttribute("aria-expanded", "false") + const closed = await toggle.boundingBox() + if (!closed) throw new Error("Review toggle bounds are unavailable") + const headerBox = await header.boundingBox() + if (!headerBox) throw new Error("Session header bounds are unavailable") + expect(closed.y).toBeGreaterThanOrEqual(headerBox.y) + expect(closed.y + closed.height).toBeLessThanOrEqual(headerBox.y + headerBox.height) + + await toggle.click() + await expect(toggle).toHaveAttribute("aria-expanded", "true") + await expect(panel).toHaveAttribute("aria-hidden", "false") + await expect(toggle).toHaveCount(1) + await expect.poll(() => toggle.boundingBox()).toEqual(closed) + await expect + .poll(async () => { + const box = await panel.boundingBox() + if (!box) return false + return ( + closed.x >= box.x && + closed.x + closed.width <= box.x + box.width && + closed.y >= box.y && + closed.y + closed.height <= box.y + 52 + ) + }) + .toBe(true) + + await expect + .poll(async () => { + const box = await panel.locator('[data-slot="session-side-panel-actions"]').boundingBox() + return box ? box.y + box.height / 2 : undefined + }) + .toBe(closed.y + closed.height / 2) + + await toggle.press("Enter") + await expect(toggle).toHaveAttribute("aria-expanded", "false") + await expect(toggle).toBeFocused() + await expect(toggle).toHaveCount(1) + await expect.poll(() => toggle.boundingBox()).toEqual(closed) + }) + + test(`keeps terminal controls clear of the review toggle (${width}px, ${direction})`, async ({ page }) => { + await page.setViewportSize({ width, height: 900 }) + const ptys: { id: string; title: string }[] = [] + const removed: string[] = [] + await page.route("**/api/pty**", async (route) => { + const path = new URL(route.request().url()).pathname + const location = { directory, project: { id: "proj_review_toggle_position", directory } } + if (route.request().method() === "DELETE") { + removed.push(path.split("/").at(-1)!) + return route.fulfill({ status: 204 }) + } + if (path.endsWith("/connect-token")) { + return route.fulfill({ json: { location, data: { ticket: "e2e-ticket", expires_in: 60 } } }) + } + if (path === "/api/pty" && route.request().method() === "POST") { + const pty = { id: `pty_review_${ptys.length + 1}`, title: `Terminal ${ptys.length + 1}` } + ptys.push(pty) + return route.fulfill({ json: { location, data: pty } }) + } + return route.fulfill({ json: { location, data: ptys.find((pty) => path.endsWith(pty.id)) ?? ptys } }) + }) + await page.routeWebSocket(/\/api\/pty\/pty_review_\d+\/connect/, () => undefined) + await page.goto(`/server/${base64Encode(server)}/session/${sessionID}`) + await expectSessionTitle(page, "Review toggle position") + await page.locator("html").evaluate((element, dir) => element.setAttribute("dir", dir), direction) + + const toggle = page.getByRole("button", { name: "Toggle review", exact: true }) + await expect(toggle).toHaveAttribute("aria-expanded", "false") + await page.keyboard.press("Control+Backquote") + const terminal = page.getByRole("region", { name: "Terminal", exact: true }) + await expect(terminal.getByRole("tab", { name: "Terminal 1", exact: true })).toHaveAttribute( + "aria-selected", + "true", + ) + for (const number of [2, 3, 4]) { + await terminal.getByRole("button", { name: "New terminal", exact: true }).click() + await expect(terminal.getByRole("tab", { name: `Terminal ${number}`, exact: true })).toHaveAttribute( + "aria-selected", + "true", + ) + } + + await expect + .poll(async () => { + const tabs = await terminal.getByRole("tablist").boundingBox() + const button = await toggle.boundingBox() + if (!tabs || !button) return false + return direction === "rtl" ? tabs.x >= button.x + button.width : tabs.x + tabs.width <= button.x + }) + .toBe(true) + await expectTerminalControlsAligned(terminal, toggle) + const fourth = terminal.locator('[data-slot="tabs-trigger-wrapper"][data-value="pty_review_4"]') + await fourth.getByRole("button", { name: "Close terminal", exact: true }).click() + await expect(terminal.getByRole("tab")).toHaveText(["Terminal 1", "Terminal 2", "Terminal 3"]) + expect(removed).toEqual(["pty_review_4"]) + await expect(toggle).toHaveAttribute("aria-expanded", "false") + + await terminal.getByRole("button", { name: "New terminal", exact: true }).click() + await expect(terminal.getByRole("tab", { name: "Terminal 5", exact: true })).toHaveAttribute( + "aria-selected", + "true", + ) + await expect(toggle).toHaveAttribute("aria-expanded", "false") + const position = await toggle.boundingBox() + await toggle.click() + await expect(toggle).toHaveAttribute("aria-expanded", "true") + await expect(page.locator("#review-panel")).toHaveAttribute("aria-hidden", "false") + await expect.poll(() => toggle.boundingBox()).toEqual(position) + await expect + .poll(async () => { + const actions = await page.locator('[data-slot="session-side-panel-actions"]').boundingBox() + const button = await toggle.boundingBox() + if (!actions || !button) return undefined + return actions.y + actions.height / 2 - (button.y + button.height / 2) + }) + .toBe(0) + await toggle.press("Enter") + await expect(toggle).toHaveAttribute("aria-expanded", "false") + await expect(toggle).toBeFocused() + await expect.poll(() => toggle.boundingBox()).toEqual(position) + await expectTerminalControlsAligned(terminal, toggle) + }) + } +} + +async function expectTerminalControlsAligned(terminal: Locator, toggle: Locator) { + await expect + .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 + const path = svg.getBBox() + return new DOMPoint(path.x + path.width / 2, path.y + path.height / 2).matrixTransform(svg.getScreenCTM()!) + .y + }), + ), + ) + return centers[0]! - centers[1]! + }) + .toBeCloseTo(0, 1) +} diff --git a/packages/app/e2e/regression/session-header-controls.spec.ts b/packages/app/e2e/regression/session-header-controls.spec.ts index 224ba79d9c9..f6c8866bf89 100644 --- a/packages/app/e2e/regression/session-header-controls.spec.ts +++ b/packages/app/e2e/regression/session-header-controls.spec.ts @@ -24,7 +24,7 @@ for (const direction of ["ltr", "rtl"] as const) { const header = page.locator("[data-session-title]") const more = header.getByRole("button", { name: "More options", exact: true }) const project = header.getByRole("button", { name: fixture.project.name, exact: true }) - const review = header.getByRole("button", { name: "Toggle review", exact: true }) + const review = page.getByRole("button", { name: "Toggle review", exact: true }) const details = header.getByRole("button", { name: "Session details", exact: true }) await expect(header.getByRole("heading")).toHaveText(fixture.expected.targetTitle) await page.evaluate((direction) => document.documentElement.setAttribute("dir", direction), direction) diff --git a/packages/app/src/session/files/session-side-panel.tsx b/packages/app/src/session/files/session-side-panel.tsx index 5689dd03d32..6913690c902 100644 --- a/packages/app/src/session/files/session-side-panel.tsx +++ b/packages/app/src/session/files/session-side-panel.tsx @@ -427,11 +427,16 @@ export function SessionSidePanel(props: {
event.stopPropagation()} onClick={(event) => event.stopPropagation()} > + +
+
diff --git a/packages/app/src/session/header/session-header-actions.tsx b/packages/app/src/session/header/session-header-actions.tsx index 31116b84e92..9572f1a3fb3 100644 --- a/packages/app/src/session/header/session-header-actions.tsx +++ b/packages/app/src/session/header/session-header-actions.tsx @@ -3,6 +3,28 @@ import { Icon } from "@opencode-ai/ui/icon" import { IconButton } from "@opencode-ai/ui/icon-button" import { Keybind } from "@opencode-ai/ui/keybind" import { Tooltip } from "@opencode-ai/ui/tooltip" +import { useCommand } from "@/shell/commands/command" +import { reviewTooltipKeybind } from "@/shell/commands/tooltip-keybind" +import { useLanguage } from "@/runtime/i18n/language" +import { useSessionLayout } from "@/session/session-layout" + +export function SessionReviewToggle() { + const command = useCommand() + const language = useLanguage() + const { view } = useSessionLayout() + + return ( + view().reviewPanel.toggle(), + }} + /> + ) +} export type SessionHeaderActionsState = { reviewLabel: string diff --git a/packages/app/src/session/header/session-header.tsx b/packages/app/src/session/header/session-header.tsx index 35b3c35b8fa..9acc4845a20 100644 --- a/packages/app/src/session/header/session-header.tsx +++ b/packages/app/src/session/header/session-header.tsx @@ -1,31 +1,19 @@ -import { createMemo, Show } from "solid-js" +import { Show } from "solid-js" import { createMediaQuery } from "@solid-primitives/media" -import { useCommand } from "@/shell/commands/command" import { useLanguage } from "@/runtime/i18n/language" import { useSettings } from "@/settings/model" import { useSessionLayout } from "@/session/session-layout" -import { reviewTooltipKeybind } from "@/shell/commands/tooltip-keybind" import { StatusPopover } from "@/shell/status/status-popover" import { TitlebarRight } from "@/shell/titlebar/right-slot" import { Tooltip } from "@opencode-ai/ui/tooltip" -import { SessionHeaderActions, type SessionHeaderActionsState } from "./session-header-actions" export function SessionHeader() { - const command = useCommand() const language = useLanguage() const settings = useSettings() const { view } = useSessionLayout() const isDesktop = createMediaQuery("(min-width: 768px)") - const actions = createMemo(() => ({ - reviewLabel: language.t("command.review.toggle"), - reviewKeybind: reviewTooltipKeybind(command), - reviewVisible: isDesktop(), - reviewOpened: view().reviewPanel.opened(), - onReviewToggle: () => view().reviewPanel.toggle(), - })) - return ( <> @@ -35,7 +23,9 @@ export function SessionHeader() { - + +
+ ) } diff --git a/packages/app/src/session/screen.tsx b/packages/app/src/session/screen.tsx index f227eba5868..77991338ccb 100644 --- a/packages/app/src/session/screen.tsx +++ b/packages/app/src/session/screen.tsx @@ -28,6 +28,7 @@ import { SessionContextTab } from "./files/session-context-tab" import { createSessionTimelineInteraction } from "./timeline/interaction" import { ActiveSessionComposerRegion, createActiveSessionRegion } from "./composer/region" import { SessionIdentityHeader } from "./session-identity-header" +import { SessionReviewToggle } from "./header/session-header-actions" import { createAnimatedPresence } from "@/runtime/animated-presence" const SessionMobileFiles = lazy(async () => { @@ -274,10 +275,19 @@ export function SessionScreen(props: { session: SessionModel }) { <>
+ {/* Keep the control outside panel animations; the terminal's 52px header includes a 1px divider. */} + +
+ +
+
diff --git a/packages/app/src/session/terminal/panel.tsx b/packages/app/src/session/terminal/panel.tsx index 635e7d759c8..29b205eb8cd 100644 --- a/packages/app/src/session/terminal/panel.tsx +++ b/packages/app/src/session/terminal/panel.tsx @@ -45,6 +45,7 @@ export function TerminalPanel( contentHeight?: string embedded?: boolean animate?: boolean + reserveReviewToggle?: boolean } = {}, ) { const terminal = useTerminal() @@ -249,7 +250,10 @@ export function TerminalPanel( when={terminal.ready() || store.surfaces.length > 0} fallback={
-
+
{(title) => (
@@ -291,46 +295,53 @@ export function TerminalPanel( }} >
- terminal.open(id)} - class="!h-[52px] !flex-none" - > - { - const active = document.activeElement - if (event.target === active) return - if (active instanceof HTMLInputElement && event.currentTarget.contains(active)) active.blur() - }} +
+ terminal.open(id)} + class="!h-full min-w-0 !flex-1" > - - {(pty, index) => } - -
- - {language.t("command.terminal.new")} - 0}> - - - - } - placement="bottom" - class="flex items-center" - > - } - variant="ghost" - onClick={() => terminal.new({ focus: true })} - aria-label={language.t("command.terminal.new")} - /> - -
- -
+ { + const active = document.activeElement + if (event.target === active) return + if (active instanceof HTMLInputElement && event.currentTarget.contains(active)) active.blur() + }} + > + + {(pty, index) => } + +
+ + {language.t("command.terminal.new")} + 0}> + + + + } + placement="bottom" + class="flex items-center" + > + } + variant="ghost" + onClick={() => terminal.new({ focus: true })} + aria-label={language.t("command.terminal.new")} + /> + +
+
+ + {/* Reserve outside the scroll viewport so overflowing tabs cannot cover the toggle. */} + +
+ +
{(surface) => ( diff --git a/packages/ui/src/actions/split-button/split-button.css b/packages/ui/src/actions/split-button/split-button.css index 254cc3fa05e..181683d7c67 100644 --- a/packages/ui/src/actions/split-button/split-button.css +++ b/packages/ui/src/actions/split-button/split-button.css @@ -9,6 +9,7 @@ overflow: hidden; } +[data-component="split-button-v2"].session-review-v2-open-in-app, [data-component="split-button-v2"]:is(:hover, :has([data-component="split-button-v2-menu-trigger"][data-expanded])) { box-shadow: inset 0 0 0 1px var(--v2-border-border-muted); } diff --git a/packages/ui/src/actions/split-button/split-button.stories.tsx b/packages/ui/src/actions/split-button/split-button.stories.tsx index cb66e00c79d..8fc92704264 100644 --- a/packages/ui/src/actions/split-button/split-button.stories.tsx +++ b/packages/ui/src/actions/split-button/split-button.stories.tsx @@ -1,4 +1,5 @@ import { Icon } from "@opencode-ai/ui/icon" +import { AppIcon } from "@opencode-ai/ui/app-icon" import { SplitButton, SplitButtonAction, SplitButtonMenuTrigger } from "./split-button" export default { @@ -29,3 +30,16 @@ export const Disabled = { ), } + +export const OpenIn = { + render: () => ( + + + + + + + + + ), +}