fix(app): preserve empty side panel tabs (#58440)

This commit is contained in:
usrnk1 2026-10-23 14:05:45 +02:00 • committed by GitHub
parent d7a08170e7
commit 79a143f82b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 146 additions and 98 deletions

View file

@ -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 }) => {

View file

@ -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<HTMLElement>("[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 })

View file

@ -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<NewTab[]>([])
const [chosen, setChosen] = createSignal<string>()
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() {

View file

@ -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<boolean>((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 (
<Tabs
data-pane-content={props.pane.id}
value={newTab() ? "new" : (props.pane.active() ?? "empty")}
value={current()?.id ?? props.pane.active() ?? "empty"}
onChange={(value) => {
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: {
</Show>
)}
</For>
<Show when={newTab()}>
<Tabs.Trigger
value="new"
class="group"
onMiddleClick={dismiss}
closeButton={
<IconButton
size="small"
variant="ghost-muted"
onPointerDown={(event) => {
event.preventDefault()
event.stopPropagation()
}}
onClick={(event) => {
event.preventDefault()
event.stopPropagation()
dismiss()
}}
icon={<Icon name="xmark-small" />}
aria-label={language.t("common.closeTab")}
/>
}
hideCloseButton
>
{language.t("session.tab.new")}
</Tabs.Trigger>
</Show>
<For each={props.region.newTabs(props.pane.id)}>
{(entry) => (
<Tabs.Trigger
value={entry.id}
class="group"
onMiddleClick={() => props.region.dismissNewTab(entry.id)}
closeButton={
<IconButton
size="small"
variant="ghost-muted"
onPointerDown={(event) => {
event.preventDefault()
event.stopPropagation()
}}
onClick={(event) => {
event.preventDefault()
event.stopPropagation()
props.region.dismissNewTab(entry.id)
}}
icon={<Icon name="xmark-small" />}
aria-label={language.t("common.closeTab")}
/>
}
hideCloseButton
>
{language.t("session.tab.new")}
</Tabs.Trigger>
)}
</For>
<div class="h-full shrink-0 sticky end-0 z-10 flex items-center justify-center bg-v2-background-bg-base">
<Show
when={primary() ?? (launchers().length === 0 && menu().length === 1 ? menu()[0] : undefined)}
@ -379,12 +374,14 @@ export function PaneTabs(props: {
</div>
</div>
<Show when={newTab()}>
<Tabs.Content value="new" class="flex flex-col h-full overflow-hidden contain-strict">
<Launchers items={launchers()} onChoose={target} closed={closed} />
</Tabs.Content>
<Show when={current()}>
{(entry) => (
<Tabs.Content value={entry().id} class="flex flex-col h-full overflow-hidden contain-strict">
<Launchers items={launchers()} onChoose={target} closed={closed} />
</Tabs.Content>
)}
</Show>
<Show when={props.placeholder && !newTab() && props.pane.active() === undefined}>
<Show when={props.placeholder && !current() && props.pane.active() === undefined}>
<Tabs.Content value="empty" class="flex flex-col h-full overflow-hidden contain-strict">
{/* With a new tab item, the new tab it opens is on its way. */}
<Show when={!primary()}>
@ -407,13 +404,13 @@ export function PaneTabs(props: {
</Tabs.Content>
</Show>
{/* Kept mounted under the new tab view, so terminals and editors keep their state. */}
<div class="contents" classList={{ hidden: newTab() }} inert={newTab() || undefined}>
<div class="contents" classList={{ hidden: !!current() }} inert={!!current() || undefined}>
<RegionContent
pane={props.pane}
view={props.view}
screen={props.screen}
frame={{
shown: () => props.shown() && !newTab(),
shown: () => props.shown() && !current(),
present: props.present,
placement: () => "side",
reserve: () => false,

View file

@ -73,9 +73,6 @@ type PaneHandle = {
export type Model = ReturnType<typeof createModel>
/** 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<typeof definition>) {
@ -304,31 +301,6 @@ export function createModel(ctx: SetupContext<typeof definition>) {
}),
)
// 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(),

View file

@ -210,7 +210,7 @@ const setup: Setup<typeof Review> = (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)