diff --git a/packages/app/component-tests/browser-pane-restore.spec.ts b/packages/app/component-tests/browser-pane-restore.spec.ts index 2a78ca5da30f..e1177d3d645e 100644 --- a/packages/app/component-tests/browser-pane-restore.spec.ts +++ b/packages/app/component-tests/browser-pane-restore.spec.ts @@ -521,7 +521,7 @@ story("reloads the browser tab with ⌘R inside the pane and leaves it to the wi await expect(reloads).toHaveText("1") }) -story("adds a second new tab with ⌘T on a new tab page", async ({ page }) => { +story("keeps blank browser tabs when adding tabs and selecting another panel", async ({ page }) => { const root = page.getByTestId("browser-region-fixture") const mac = await page.evaluate(() => /Mac|iPhone|iPad|iPod/.test(navigator.platform)) const address = root.getByRole("combobox", { name: "Browser address", exact: true }) @@ -536,6 +536,12 @@ story("adds a second new tab with ⌘T on a new tab page", async ({ page }) => { await expect(tabs).toHaveCount(2) await expect(tabs.nth(1)).toHaveAttribute("aria-selected", "true") await expect(root.getByTestId("browser-closes")).toHaveText("[]") + + // Selecting a different panel must not treat blank browser tabs as disposable placeholders. + await address.press(`${mac ? "Meta" : "Control"}+1`) + await expect(root.getByRole("tab", { name: "beta.ts", exact: true })).toHaveAttribute("aria-selected", "true") + await expect(tabs).toHaveCount(2) + await expect(root.getByTestId("browser-closes")).toHaveText("[]") }) story("reopens a closed browser tab with ⌘⇧T after the side panel and a session tab elsewhere", async ({ page }) => { diff --git a/packages/app/e2e/regression/review.spec.ts b/packages/app/e2e/regression/review.spec.ts index e175c46740bb..9cbf05982d97 100644 --- a/packages/app/e2e/regression/review.spec.ts +++ b/packages/app/e2e/regression/review.spec.ts @@ -528,6 +528,7 @@ test("file tree expands Windows paths and scrolls long names in both directions" await expect(status).toHaveText("A") await expect.poll(() => statusInset(status, "right")).toBeLessThanOrEqual(24) const treeScroll = panel.locator(".opencode-pierre-file-tree") + const scrollTree = (position: { left?: number; top?: number }) => treeScroll.evaluate((element, position) => { const scroll = element.shadowRoot!.querySelector("[data-file-tree-virtualized-scroll]")! @@ -538,6 +539,7 @@ test("file tree expands Windows paths and scrolls long names in both directions" return { left: scroll.scrollLeft, overflow: scroll.scrollWidth - scroll.clientWidth } }, position) + await scrollTree({ left: 1_000_000 }) await expect.poll(() => statusInset(status, "right")).toBeLessThanOrEqual(24) // The tree keeps the long row's width after it scrolls out of view, so the sideways position holds. @@ -1287,18 +1289,60 @@ test("names a single change source and keeps the review header rows at 48px", as await expect(header).toHaveCSS("height", "48px") await expect(toolbar).toHaveCSS("height", "48px") - // Closing the last tab closes the side region; it opens empty, on its launchers, next time. + // Closing the last tab closes the side region; it reopens on a New tab view listing its launchers. const region = page.getByRole("button", { name: "Toggle review", exact: true }) await panel.locator("#session-side-panel-review-tab").hover() await panel.getByRole("button", { name: "Close tab", exact: true }).click() await expect(region).toHaveAttribute("aria-expanded", "false") await region.click() - await expect(panel.getByRole("tab")).toHaveCount(0) + await expect(panel.getByRole("tab")).toHaveCount(1) + await expect(panel.getByRole("tab", { name: "New tab", exact: true })).toHaveAttribute("aria-selected", "true") + // "+" adds another New tab beside it instead of replacing it. + await panel.getByRole("button", { name: "Add tab", exact: true }).click() + await expect(panel.getByRole("tab", { name: "New tab", exact: true })).toHaveCount(2) // Only its launchers: "Recently closed" lists files, not the closed Review. await expect(panel.getByRole("button", { name: /^Review/ })).toBeVisible() await expect(panel.getByRole("region", { name: "Recently closed", exact: true })).toHaveCount(0) }) +test("Review from a new tab keeps the file browser and preview open", async ({ page }) => { + const directory = "C:/OpenCode/ReviewNewTabFile" + await openSession(page, { + name: "ReviewNewTabFile", + fileList: (path) => (path ? [] : [fileNode(directory, "README.md")]), + fileContent: (path) => ({ type: "text", content: `contents:${path}` }), + seed: { + panes: { ses_reviewnewtabfile: { review: true } }, + storage: { "opencode.global.dat:review-panel-v2": { sidebarOpened: false } }, + }, + }) + const panel = page.locator("#review-panel") + const tab = (name: string) => panel.getByRole("tab", { name, exact: true }) + + await openFileLauncher(panel) + await expect(tab("Open file")).toHaveAttribute("aria-selected", "true") + await panel.getByRole("button", { name: "Add tab", exact: true }).click() + await expect(tab("New tab")).toHaveAttribute("aria-selected", "true") + await panel.getByRole("button", { name: /^Review/ }).click() + await expect(tab("Review")).toHaveAttribute("aria-selected", "true") + await expect(tab("Open file")).toHaveAttribute("aria-selected", "false") + await tab("Review").hover() + await panel + .locator('div[data-value="review:changes"]') + .getByRole("button", { name: "Close tab", exact: true }) + .click() + await tab("Open file").click() + await panel.getByRole("treeitem", { name: "README.md" }).click() + await expect(tab("README.md")).toHaveAttribute("aria-selected", "true") + + // Choosing Review also leaves a single-click file preview open beside it. + await panel.getByRole("button", { name: "Add tab", exact: true }).click() + await expect(tab("New tab")).toHaveAttribute("aria-selected", "true") + await panel.getByRole("button", { name: /^Review/ }).click() + await expect(panel.locator("#session-side-panel-review-tab")).toHaveAttribute("aria-selected", "true") + await expect(tab("README.md")).toHaveCount(1) +}) + 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/panels.tsx b/packages/app/src/runtime/extension/panels.tsx index e086305c4e47..f1f5d3ccb2ce 100644 --- a/packages/app/src/runtime/extension/panels.tsx +++ b/packages/app/src/runtime/extension/panels.tsx @@ -309,16 +309,27 @@ export function createRegion(input: { return entry ? paneOf(entry) : "side" }) - // The host's new tab view: the pane it shows in, and the session and selection it opened over, so a tab that opens - // and selects itself replaces it. - const [adding, setAdding] = createSignal<{ pane: SessionPane; session: string; over: string | undefined }>() + // The host's new tab views: each shows in a pane, over the session and selection it opened over, so a tab that opens + // and selects itself replaces it. A view stops being live once the selection moves off what it opened over. + type NewTab = { id: string; pane: SessionPane; session: string; over: string | undefined } - const newTab = (pane: SessionPane) => { - const value = adding() + const [adding, setAdding] = createSignal([]) + const [chosen, setChosen] = createSignal() + let serial = 0 - return value?.pane === pane && value.session === input.view().key && value.over === panes[pane].active() + const live = (value: NewTab) => value.session === input.view().key && value.over === panes[value.pane].active() + const newTabs = (pane: SessionPane) => adding().filter((value) => value.pane === pane && live(value)) + + const shownNewTab = (pane: SessionPane) => { + const list = newTabs(pane) + + return list.find((value) => value.id === chosen()) ?? list.at(-1) } + createEffect(() => { + if (adding().some((value) => !live(value))) setAdding((list) => list.filter(live)) + }) + return { /** Whether a key event belongs to the side region rather than the session around it. */ focus: createSideFocus(), @@ -347,20 +358,38 @@ export function createRegion(input: { select(key: string) { input.tabs().setActive(key) }, - newTab, - /** Shows the new tab view in a pane, by default the one holding the selected tab. */ - openNewTab(pane: SessionPane = activePane()) { - setAdding({ pane, session: input.view().key, over: panes[pane].active() }) + /** The live new tab views of a pane, in strip order. */ + newTabs, + /** The new tab view a pane shows: the selected one, else the newest. */ + shownNewTab, + newTab: (pane: SessionPane) => shownNewTab(pane) !== undefined, + selectNewTab(id: string) { + setChosen(id) }, - /** Closes the new tab view, in one pane or wherever it is; returns whether it was showing. */ + /** Adds a new tab view in a pane, by default the one holding the selected tab, and shows it. */ + openNewTab(pane: SessionPane = activePane()) { + const id = `new:${++serial}` + + setAdding((list) => [...list, { id, pane, session: input.view().key, over: panes[pane].active() }]) + setChosen(id) + }, + dismissNewTab(id: string) { + setAdding((list) => list.filter((value) => value.id !== id)) + }, + /** Closes the shown new tab view, in one pane or wherever it is; returns whether one was showing. */ closeNewTab(pane?: SessionPane) { - const value = adding() + const panesToCheck: SessionPane[] = pane ? [pane] : ["side", "split", "bottom"] - if (!value || (pane && value.pane !== pane)) return false - const shown = newTab(value.pane) - setAdding(undefined) + const value = panesToCheck.flatMap((id) => { + const shown = shownNewTab(id) - return shown + return shown ? [shown] : [] + })[0] + + if (!value) return false + setAdding((list) => list.filter((entry) => entry.id !== value.id)) + + return true }, /** Opens or closes the side region for the user. Opening moves focus into it once it lands; closing gives focus back. */ toggle() { diff --git a/packages/app/src/runtime/extension/side-region.tsx b/packages/app/src/runtime/extension/side-region.tsx index b15fbb7f5338..b786dcc73284 100644 --- a/packages/app/src/runtime/extension/side-region.tsx +++ b/packages/app/src/runtime/extension/side-region.tsx @@ -6,7 +6,6 @@ import { createSignal, on, onCleanup, - untrack, type Accessor, type JSX, } from "solid-js" @@ -66,16 +65,13 @@ export function SideRegion(props: { const contentMounted = createMemo((mounted) => mounted || tabsVisible(), false) const panelWidth = createMemo(() => (tabsVisible() ? "auto" : "0px")) - const menu = createSessionPanelMenu() - const primary = createMemo(() => menu().find((item) => item.primary)) - - // Opening the side region with no tabs starts a new tab, once per opening and session. Keyed by the session, not the - // item, whose object changes with every routed session. + // Opening the side region with no tabs shows a new tab view, once per opening and session. Keyed by the session, not + // the item, whose object changes with every routed session. const starting = createMemo(() => - tabsOpen() && layout.ready() && props.region.empty() && primary() ? props.view.key : undefined, + tabsOpen() && layout.ready() && props.region.empty() ? props.view.key : undefined, ) - createEffect(on(starting, (key) => key && untrack(primary)?.run())) + createEffect(on(starting, (key) => key && props.region.openNewTab("side"))) // The split pane shows while it holds tabs; when the side pane has none, the split pane takes the whole height. const panes = createMemo(() => { @@ -225,21 +221,18 @@ export function PaneTabs(props: { const modifier = closedTabsModifier() // Without a new tab item, "+" opens a host-owned new tab view listing the launchers, as ⌘T does inside the region. - const newTab = createMemo(() => props.region.newTab(props.pane.id)) + const current = createMemo(() => props.region.shownNewTab(props.pane.id)) const add = () => props.region.openNewTab(props.pane.id) - const dismiss = () => props.region.closeNewTab(props.pane.id) - - // Once replaced, the view is gone: returning to the tab or session it opened over does not show it again. - createEffect(on(newTab, (shown) => !shown && dismiss(), { defer: true })) let selectionEvent: Event | undefined return ( { - if (value === "new") return + if (props.region.newTabs(props.pane.id).some((entry) => entry.id === value)) + return props.region.selectNewTab(value) // Kobalte selects the first tab while triggers register, including while the "+" menu's click is still // dispatching. Persist input events on a tab only; the region owns fallback selection. @@ -249,7 +242,7 @@ export function PaneTabs(props: { selectionEvent.target instanceof Element && selectionEvent.target.closest('[role="tab"]') ) { - dismiss() + props.region.newTabs(props.pane.id).forEach((entry) => props.region.dismissNewTab(entry.id)) props.region.select(value) } }} @@ -284,33 +277,35 @@ export function PaneTabs(props: { )} - - { - event.preventDefault() - event.stopPropagation() - }} - onClick={(event) => { - event.preventDefault() - event.stopPropagation() - dismiss() - }} - icon={} - aria-label={language.t("common.closeTab")} - /> - } - hideCloseButton - > - {language.t("session.tab.new")} - - + + {(entry) => ( + props.region.dismissNewTab(entry.id)} + closeButton={ + { + event.preventDefault() + event.stopPropagation() + }} + onClick={(event) => { + event.preventDefault() + event.stopPropagation() + props.region.dismissNewTab(entry.id) + }} + icon={} + aria-label={language.t("common.closeTab")} + /> + } + hideCloseButton + > + {language.t("session.tab.new")} + + )} +
- - - - + + {(entry) => ( + + + + )} - + {/* With a new tab item, the new tab it opens is on its way. */} @@ -407,13 +404,13 @@ export function PaneTabs(props: { {/* Kept mounted under the new tab view, so terminals and editors keep their state. */} -
+
props.shown() && !newTab(), + shown: () => props.shown() && !current(), present: props.present, placement: () => "side", reserve: () => false, diff --git a/packages/gui-extensions/src/browser/model.ts b/packages/gui-extensions/src/browser/model.ts index 1adde2c0b5fe..d12d21ba02c0 100644 --- a/packages/gui-extensions/src/browser/model.ts +++ b/packages/gui-extensions/src/browser/model.ts @@ -73,9 +73,6 @@ type PaneHandle = { export type Model = ReturnType -/** A tab showing the new tab page: no page, and none loading. */ -const blank = (tab: Browser.Tab) => (!tab.url || tab.url === "about:blank") && !tab.loading - // Attachments belong to the shell session tab, not the session route: native pages and the agent's // browser survive visiting Settings or another tab and close when the session tab does. export function createModel(ctx: SetupContext) { @@ -304,31 +301,6 @@ export function createModel(ctx: SetupContext) { }), ) - // A new tab holds the place of the tab the user picks next: once another tab is selected, it closes unless a - // page started loading in it, or the pick is another new tab (mod+t on a new tab page adds one, as in a - // browser). It leaves the strip in the same update, so the picked tab takes its place without waiting for the - // desktop to close its page. - const shownBlank = () => - state.attachments[id]?.browser?.tabs.find((item) => { - const placement = layout.state(key(item.id), ref) - - return blank(item) && (placement === "visible" || placement === "active") - })?.id - - createKeyed(shownBlank, (tabID) => - onCleanup(() => { - const item = state.attachments[id]?.browser?.tabs.find((candidate) => candidate.id === tabID) - - if (live.get(id) !== entry || !item || !blank(item) || layout.state(key(tabID), ref) !== "open") return - - if (shownBlank()) return - layout.close(key(tabID), ref) - - // The host calls the panel's `close` for the routed session only; another session's page closes here. - if (sessions.current()?.key !== ref.key) command(ref, { type: "tabs.close", tabID }) - }), - ) - // Previews made while another session was on screen open once the user returns to this one. createKeyed( () => sessions.current()?.key === ref.key && ctx.screen.current(), diff --git a/packages/gui-extensions/src/review/renderer.tsx b/packages/gui-extensions/src/review/renderer.tsx index ab4e812faa95..b50ada3e4d2f 100644 --- a/packages/gui-extensions/src/review/renderer.tsx +++ b/packages/gui-extensions/src/review/renderer.tsx @@ -210,7 +210,7 @@ const setup: Setup = (ctx) => { const show = () => { const session = sessions.current() - if (session?.project && !layout.narrow()) layout.open(KEY, session) + if (session?.project && !layout.narrow()) layout.open(KEY, session, { tab: "select" }) } const mac = /(Mac|iPhone|iPad)/.test(navigator.platform)