From e86ec98711256cd400e62ccdc85b0ee896b04065 Mon Sep 17 00:00:00 2001 From: usrnk1 <7547651+usrnk1@users.noreply.github.com> Date: Mon, 5 Oct 2026 05:18:48 +0200 Subject: [PATCH] feat(desktop): move review controls into their panels (#51312) Co-authored-by: Luke Parker <10430890+Hona@users.noreply.github.com> --- packages/app/e2e/regression/review.spec.ts | 26 +++++++ .../app/src/runtime/extension/side-region.tsx | 8 -- packages/gui-extensions/src/file/browser.tsx | 18 ++++- packages/gui-extensions/src/file/context.ts | 8 ++ packages/gui-extensions/src/file/contract.ts | 14 ++++ packages/gui-extensions/src/file/index.ts | 4 +- .../gui-extensions/src/file/open-in-app.tsx | 20 ++--- packages/gui-extensions/src/file/renderer.tsx | 73 ++++++++++++++++--- packages/gui-extensions/src/review/index.ts | 7 +- packages/gui-extensions/src/review/panel.tsx | 17 +++++ packages/gui-extensions/src/review/parts.tsx | 36 +++++++-- packages/gui-extensions/src/sdk/points.ts | 2 - packages/gui-extensions/src/sdk/solid.ts | 2 +- .../v2/components/session-file-panel-v2.tsx | 11 ++- .../src/v2/components/session-review-v2.css | 37 +++++++++- .../src/v2/components/session-review-v2.tsx | 18 ++++- packages/ui/src/navigation/tabs/tabs.css | 51 +------------ 17 files changed, 252 insertions(+), 100 deletions(-) diff --git a/packages/app/e2e/regression/review.spec.ts b/packages/app/e2e/regression/review.spec.ts index c6a879e0f09..b2f35e1d186 100644 --- a/packages/app/e2e/regression/review.spec.ts +++ b/packages/app/e2e/regression/review.spec.ts @@ -833,6 +833,32 @@ test("a chosen mode the session stops offering shows as Git, and comes back when await expect(file).toHaveText("beta.ts") }) +test("names a single change source and keeps the review header rows at 48px", async ({ page }) => { + // Turn changes are offered only for Git, so another VCS offers one source. + await openSession(page, { name: "ReviewHeader", project: { vcs: "hg" }, vcsDiff: [fileDiff("src/alpha.ts")] }) + await page.getByRole("button", { name: "Toggle review", exact: true }).click() + const panel = page.locator("#review-panel") + const toggle = panel.getByRole("button", { name: "Toggle file tree" }) + const header = panel.locator('[data-slot="session-review-v2-sidebar-header"]') + const toolbar = panel.locator('[data-slot="session-review-v2-toolbar"]') + + await expect(header.getByText("Git changes", { exact: true })).toBeVisible() + await expect(panel.getByRole("button", { name: "Git changes" })).toHaveCount(0) + await expect(toggle).toHaveAttribute("aria-expanded", "true") + await expect(header).toHaveCSS("height", "48px") + await expect(toolbar).toHaveCSS("height", "48px") + + await toggle.click() + await expect(toggle).toHaveAttribute("aria-expanded", "false") + await expect(toolbar.getByText("Git changes", { exact: true })).toBeVisible() + await expect(toolbar).toHaveCSS("height", "48px") + + await toggle.click() + await expect(toggle).toHaveAttribute("aria-expanded", "true") + await expect(header).toHaveCSS("height", "48px") + await expect(toolbar).toHaveCSS("height", "48px") +}) + for (const direction of ["ltr", "rtl"] as const) { test(`review toggle stays at the header edge in ${direction}`, async ({ page }) => { await page.setViewportSize({ width: 1000, height: 900 }) diff --git a/packages/app/src/runtime/extension/side-region.tsx b/packages/app/src/runtime/extension/side-region.tsx index 309e89db73d..62d1ecb378e 100644 --- a/packages/app/src/runtime/extension/side-region.tsx +++ b/packages/app/src/runtime/extension/side-region.tsx @@ -14,7 +14,6 @@ import { Mark } from "@opencode/ui/logo" import { Keybind } from "@opencode/ui/keybind" import { Tooltip } from "@opencode/ui/tooltip" import { Menu } from "@opencode/ui/menu" -import { SessionReviewV2SidebarToggle } from "@opencode/session-ui/v2/session-review-v2" import { MenuItem, type MountedSession, @@ -160,13 +159,6 @@ export function SideRegion(props: { onCleanup(createTabStripScroll({ el, lead: props.region.lead })) }} > -
- -
{(key) => ( diff --git a/packages/gui-extensions/src/file/browser.tsx b/packages/gui-extensions/src/file/browser.tsx index 073b2816947..a4e427c6207 100644 --- a/packages/gui-extensions/src/file/browser.tsx +++ b/packages/gui-extensions/src/file/browser.tsx @@ -3,7 +3,7 @@ import { createStore } from "solid-js/store" import { createQuery, keepPreviousData } from "@tanstack/solid-query" import { Icon } from "@opencode/ui/icon" import { SessionFilePanelV2, SessionFilePanelV2Empty } from "@opencode/session-ui/v2/session-file-panel-v2" -import { SessionReviewV2Sidebar } from "@opencode/session-ui/v2/session-review-v2" +import { SessionReviewV2Sidebar, SessionReviewV2SidebarToggle } from "@opencode/session-ui/v2/session-review-v2" import { getFilename } from "@opencode/util/path" import type { ChangeKind } from "../review/contract" import { @@ -104,12 +104,24 @@ export function SessionFileBrowserTab(props: { // unmounts the whole panel on every file-tab switch and resets sidebar scroll. return ( {title()}} + sidebarCollapsed={!sidebarOpened()} + sidebarToggleGap={10} + sidebarToggle={ + props.mobile ? undefined : ( + + ) + } sidebar={ {title()}} + title={{title()}} filter={filter()} onFilterChange={setFilter} onFilterKeyDown={onFilterKeyDown} diff --git a/packages/gui-extensions/src/file/context.ts b/packages/gui-extensions/src/file/context.ts index 9cab5f69a3b..c25c5d959bb 100644 --- a/packages/gui-extensions/src/file/context.ts +++ b/packages/gui-extensions/src/file/context.ts @@ -6,6 +6,12 @@ import type { OpenApp } from "./apps" export type TreeTab = "changes" | "all" +/** An open-in-app request in flight: the app it opens, or undefined while none is. */ +export interface OpenRequest { + app(): OpenApp | undefined + set(app: OpenApp | undefined): void +} + /** Per-window file state that setup owns and its lazily loaded views share. */ export interface FileShared { readonly changes: Accessor> @@ -22,6 +28,8 @@ export interface FileShared { readonly installed: Map> /** The open-in-app choice. Desktop only. */ readonly app?: { current(): OpenApp; set(app: OpenApp): void } + /** The "Open in" buttons' request in flight, shared so the tab strip's and a panel header's agree across tab switches. */ + readonly request: OpenRequest /** The last selection of each file tab, readable before a session's file view state loads. */ readonly handoff: { get(session: string, path: string): LineRange | null | undefined diff --git a/packages/gui-extensions/src/file/contract.ts b/packages/gui-extensions/src/file/contract.ts index ef4c3bab211..74b52eef681 100644 --- a/packages/gui-extensions/src/file/contract.ts +++ b/packages/gui-extensions/src/file/contract.ts @@ -32,3 +32,17 @@ export interface FileTree { } export const FileTree = Contract.define("file.tree") + +export interface OpenInAppProps { + /** The session screen whose workspace opens. */ + readonly screen: SessionScreen + readonly session: MountedSession +} + +/** The desktop "Open in" button, for a panel header that shows it in place of the tab strip's. */ +export interface OpenInApp { + /** Renders nothing on the web or for a remote server. While it is mounted, the tab strip leaves out its own. */ + Button(props: OpenInAppProps): JSX.Element +} + +export const OpenInApp = Contract.define("file.openInApp") diff --git a/packages/gui-extensions/src/file/index.ts b/packages/gui-extensions/src/file/index.ts index 128375a1cb5..297f3e53dca 100644 --- a/packages/gui-extensions/src/file/index.ts +++ b/packages/gui-extensions/src/file/index.ts @@ -3,14 +3,14 @@ import { Browser } from "../browser/contract" import { Changes } from "../review/contract" import { Extension, Store } from "../sdk" import { OpenAppPreferences } from "./apps" -import { FileTree } from "./contract" +import { FileTree, OpenInApp } from "./contract" import en from "./i18n/en" const TreeState = Schema.Struct({ tab: Schema.Literals(["changes", "all"]) }) export default Extension.define({ id: "file", - provides: { tree: FileTree }, + provides: { tree: FileTree, openIn: OpenInApp }, // Without review the tree lists only the workspace files; without the browser, HTML opens as a file tab. uses: { changes: Changes, browser: Browser }, stores: { diff --git a/packages/gui-extensions/src/file/open-in-app.tsx b/packages/gui-extensions/src/file/open-in-app.tsx index c7117ea502f..8adfde501f3 100644 --- a/packages/gui-extensions/src/file/open-in-app.tsx +++ b/packages/gui-extensions/src/file/open-in-app.tsx @@ -9,7 +9,7 @@ import { showToast } from "@opencode/ui/toast" import { Tooltip } from "@opencode/ui/tooltip" import { createLatest, useExtension, type Context, type OS, type MountedSession, type SessionScreen } from "../sdk" import type { OpenApp } from "./apps" -import { useShared } from "./context" +import { useShared, type OpenRequest } from "./context" import { openInAppParentPath } from "./path" const MAC_OPEN_APPS = [ @@ -68,7 +68,7 @@ const showRequestError = (ctx: Context, err: Error | string) => { }) } -export function useOpenInApp(input: { session: MountedSession; path: () => string }) { +export function useOpenInApp(input: { session: MountedSession; path: () => string; request?: OpenRequest }) { const ctx = useExtension() const desktop = ctx.desktop const shared = useShared() @@ -110,7 +110,9 @@ export function useOpenInApp(input: { session: MountedSession; path: () => strin }) const [menu, setMenu] = createStore({ open: false }) - const [openRequest, setOpenRequest] = createStore<{ app?: OpenApp }>({}) + const [local, setLocal] = createStore<{ app?: OpenApp }>({}) + + const request = input.request ?? { app: () => local.app, set: (app: OpenApp | undefined) => setLocal("app", app) } const canOpen = createMemo(() => !!desktop && input.session.server.local) @@ -121,7 +123,7 @@ export function useOpenInApp(input: { session: MountedSession; path: () => strin ({ id: "finder", label: fileManager().label, icon: fileManager().icon } as const), ) - const opening = createMemo(() => openRequest.app !== undefined) + const opening = createMemo(() => request.app() !== undefined) const selectApp = (app: OpenApp | "finder") => { if (!options().some((item) => item.id === app)) return @@ -135,19 +137,19 @@ export function useOpenInApp(input: { session: MountedSession; path: () => strin const item = options().find((o) => o.id === app) const openWith = item && "openWith" in item ? item.openWith : undefined - setOpenRequest("app", app) + request.set(app) - const request = + const launched = app === "finder" && reveal ? desktop .reveal(target) .then((revealed) => (revealed ? undefined : desktop.launch(openInAppParentPath(target)))) : desktop.launch(target, openWith) - request + launched .catch((err) => showRequestError(ctx, err)) .finally(() => { - setOpenRequest("app", undefined) + request.set(undefined) }) } @@ -187,7 +189,7 @@ type OpenInAppState = ReturnType export default function OpenInAppButton(props: { session: MountedSession; screen: SessionScreen }) { const ctx = useExtension() const directory = () => props.screen.file.root - const state = useOpenInApp({ session: props.session, path: directory }) + const state = useOpenInApp({ session: props.session, path: directory, request: useShared().request }) return ( diff --git a/packages/gui-extensions/src/file/renderer.tsx b/packages/gui-extensions/src/file/renderer.tsx index 5e62664a0f4..8740d86e741 100644 --- a/packages/gui-extensions/src/file/renderer.tsx +++ b/packages/gui-extensions/src/file/renderer.tsx @@ -1,4 +1,16 @@ -import { batch, createMemo, lazy, on, onCleanup, Show, Suspense, type ParentProps } from "solid-js" +import { + batch, + createMemo, + createSignal, + lazy, + on, + onCleanup, + Show, + Suspense, + type ParentProps, + type Signal, +} from "solid-js" +import { createStore } from "solid-js/store" import { Icon } from "@opencode/ui/icon" import { encodeFilePath, getFilename } from "@opencode/util/path" import { @@ -22,7 +34,8 @@ import { import { artifactKind } from "@opencode/util/artifact" import { resolveArtifactPath } from "./artifact" import { FileContext, type FileShared } from "./context" -import { FileTree } from "./contract" +import type { OpenApp } from "./apps" +import { FileTree, OpenInApp } from "./contract" import type File from "./index" import { FileVisual } from "./label" import { fileTabId, fileTabPath, isFileTab, workspaceFileUrl } from "./path" @@ -48,6 +61,7 @@ const setup: Setup = (ctx) => { const tree = ctx.stores.tree const [handoff, setHandoff] = storage.memory("handoff", { initial: { sessions: {} } }) const preference = desktop ? ctx.stores.app : undefined + const [request, setRequest] = createStore<{ app?: OpenApp }>({}) // Tab objects per session screen, which stays while it routes another session, reused so neither strip updates nor a // session switch rebuild a trigger. `close` prunes them. @@ -105,6 +119,10 @@ const setup: Setup = (ctx) => { draft.app = app }), }, + request: { + app: () => request.app, + set: (app) => setRequest("app", app), + }, handoff: { get: (session, path) => handoff.sessions[session]?.[path], set: (session, files) => @@ -166,7 +184,6 @@ const setup: Setup = (ctx) => { draggable: false, closable: "hover", transient: true, - sidebar: "locked", group: GROUP, dom: { panel: TABPANEL }, } @@ -286,22 +303,58 @@ const setup: Setup = (ctx) => { }, }) - if (desktop) { - const OpenInAppButton = lazy(() => import("./open-in-app")) + const OpenInAppButton = lazy(() => import("./open-in-app")) + // How many "Open in" buttons each screen's panel headers show; the tab strip shows its own only while none do. + const headers = new WeakMap>() + + const headerButtons = (screen: SessionScreen) => { + const existing = headers.get(screen) + + if (existing) return existing + + const created = createSignal(0) + + headers.set(screen, created) + + return created + } + + if (desktop) { onCleanup(onIdle(() => void OpenInAppButton.preload())) ctx.add(Slot, { at: "session.panel.end", render: (input) => ( - - - - - + + + + + + + ), }) } + ctx.provide(OpenInApp, { + Button: bindExtension((props) => { + const count = headerButtons(props.screen) + + count[1]((value) => value + 1) + onCleanup(() => count[1]((value) => value - 1)) + + return ( + + + + + + + + ) + }), + }) + ctx.add(Slot, { at: "session.panel.sidebar", render: (input) => ( diff --git a/packages/gui-extensions/src/review/index.ts b/packages/gui-extensions/src/review/index.ts index 58cf2d2cda4..3fdbdad5024 100644 --- a/packages/gui-extensions/src/review/index.ts +++ b/packages/gui-extensions/src/review/index.ts @@ -1,5 +1,5 @@ import { Schema } from "effect" -import { FileTree } from "../file/contract" +import { FileTree, OpenInApp } from "../file/contract" import { Extension, Store } from "../sdk" import { Changes } from "./contract" import en from "./i18n/en" @@ -19,8 +19,9 @@ const SessionState = Schema.Struct({ export default Extension.define({ id: "review", provides: { changes: Changes }, - // The changed files list in the file browser's tree; without it the list stays empty. - uses: { tree: FileTree }, + // The changed files list in the file browser's tree; without it the list stays empty. The toolbar also shows the + // file extension's "Open in" button. + uses: { tree: FileTree, openIn: OpenInApp }, stores: { diff: Store.global( DiffState, diff --git a/packages/gui-extensions/src/review/panel.tsx b/packages/gui-extensions/src/review/panel.tsx index ae3852dbad5..8e2758360fd 100644 --- a/packages/gui-extensions/src/review/panel.tsx +++ b/packages/gui-extensions/src/review/panel.tsx @@ -6,6 +6,7 @@ import { SESSION_REVIEW_V2_SIDEBAR_WIDTH_MIN, SessionReviewV2, SessionReviewV2Sidebar, + SessionReviewV2SidebarToggle, type SessionReviewExpandMode, } from "@opencode/session-ui/v2/session-review-v2" import { SessionReviewFilePreviewV2 } from "@opencode/session-ui/v2/session-review-file-preview-v2" @@ -111,6 +112,14 @@ export default function ReviewPanelContent(props: { } function ReviewPanel(props: ReviewPanelProps) { + const ctx = useExtension() + + const openIn = () => { + const live = ctx.uses.openIn() + + return live.status === "active" ? live.value : undefined + } + const readFile = async (path: string) => props.session.server.client.file .read({ path, location: { directory: props.screen.file.root } }) @@ -188,6 +197,14 @@ function ReviewPanel(props: ReviewPanelProps) { stats={} empty={props.empty} sidebarOpen={props.state.sidebar.opened()} + sidebarToggle={ + + } + toolbarAction={ + + {(openIn) => } + + } sidebar={ // Always mounted: the sidebar header hosts the changes-mode dropdown, // which must stay reachable when the current mode has zero diffs. diff --git a/packages/gui-extensions/src/review/parts.tsx b/packages/gui-extensions/src/review/parts.tsx index eb1e4ba528d..7cf5f8d88ff 100644 --- a/packages/gui-extensions/src/review/parts.tsx +++ b/packages/gui-extensions/src/review/parts.tsx @@ -7,21 +7,36 @@ import type { ChangeMode, ReviewModel } from "./model" export function ReviewTitle(props: { review: ReviewModel }) { const ctx = useExtension() + const label = (option: ChangeMode) => { if (option === "git") return ctx.t("ui.sessionReview.title.git") + if (option === "branch") return ctx.t("ui.sessionReview.title.branch") + return ctx.t("ui.sessionReview.title.lastTurn") } + return ( 0}> -