From bf937be15b7aa2be8918d661055677436724732c Mon Sep 17 00:00:00 2001 From: DS <78942835+Tarquinen@users.noreply.github.com> Date: Wed, 4 Nov 2026 17:06:50 -0500 Subject: [PATCH] fix(app): keep side panel New tabs until they are closed or used (#61702) --- packages/app/e2e/regression/review.spec.ts | 5 +- .../app/e2e/regression/side-panels.spec.ts | 54 ++++++++++++- packages/app/e2e/regression/terminal.spec.ts | 2 +- packages/app/src/runtime/plugin/panels.tsx | 78 +++++++++++++++---- .../app/src/runtime/plugin/side-region.tsx | 30 +++---- 5 files changed, 133 insertions(+), 36 deletions(-) diff --git a/packages/app/e2e/regression/review.spec.ts b/packages/app/e2e/regression/review.spec.ts index 94456f7393c..e402562a6c6 100644 --- a/packages/app/e2e/regression/review.spec.ts +++ b/packages/app/e2e/regression/review.spec.ts @@ -1746,8 +1746,9 @@ test("Review from a new tab keeps the file browser and preview open", async ({ p await openFileLauncher(panel) await expect(tab("Open file")).toHaveAttribute("aria-selected", "true") + // The empty panel's own New tab stays beside the one the file browser replaced, so the added one is the last. await panel.getByRole("button", { name: "Add tab", exact: true }).click() - await expect(tab("New tab")).toHaveAttribute("aria-selected", "true") + await expect(tab("New tab").last()).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") @@ -1762,7 +1763,7 @@ test("Review from a new tab keeps the file browser and preview open", async ({ p // 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 expect(tab("New tab").last()).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) diff --git a/packages/app/e2e/regression/side-panels.spec.ts b/packages/app/e2e/regression/side-panels.spec.ts index ef129849eb1..b7ce2e8d93f 100644 --- a/packages/app/e2e/regression/side-panels.spec.ts +++ b/packages/app/e2e/regression/side-panels.spec.ts @@ -127,6 +127,54 @@ test("closing the last New tab closes the side region, and only a New tab lists } }) +test("New tabs stay when a launcher in another one opens a tab, or when the tab before them closes", async ({ + page, +}) => { + await page.setViewportSize({ width: 1440, height: 900 }) + await openSession(page, { name: "ManyNewTabs", seed: { panes: { ses_manynewtabs: { review: true } } } }) + + const panel = page.locator("#review-panel") + const toggle = page.getByRole("button", { name: "Toggle review", exact: true }) + const newTabs = panel.getByRole("tab", { name: "New tab", exact: true }) + const review = panel.getByRole("tab", { name: "Review", exact: true }) + + await expect(newTabs).toHaveCount(1) + await panel.getByRole("button", { name: "Add tab", exact: true }).click() + await panel.getByRole("button", { name: "Add tab", exact: true }).click() + await expect(newTabs).toHaveCount(3) + await expect(newTabs.nth(2)).toHaveAttribute("aria-selected", "true") + + // A launcher in the middle New tab replaces only that one. + await newTabs.nth(1).click() + await openReviewLauncher(panel) + await expect(review).toHaveAttribute("aria-selected", "true") + await expect(newTabs).toHaveCount(2) + + // Selecting another tab keeps the New tabs too. + await newTabs.nth(0).click() + await expect(newTabs.nth(0)).toHaveAttribute("aria-selected", "true") + await review.click() + await expect(review).toHaveAttribute("aria-selected", "true") + await expect(newTabs).toHaveCount(2) + + // Closing the strip's first tab shows a New tab instead of closing the region. + await review.hover() + await panel + .locator('div[data-value="review:changes"]') + .getByRole("button", { name: "Close tab", exact: true }) + .click() + await expect(review).toHaveCount(0) + await expect(toggle).toHaveAttribute("aria-expanded", "true") + await expect(newTabs).toHaveCount(2) + await expect(newTabs.nth(1)).toHaveAttribute("aria-selected", "true") + + // The region closes with its last New tab. + await newTabs.nth(0).click({ button: "middle" }) + await expect(newTabs).toHaveCount(1) + await newTabs.click({ button: "middle" }) + await expect(toggle).toHaveAttribute("aria-expanded", "false") +}) + test("tabs in a narrow side strip shrink to keep their close buttons, and a tab's menu closes the others", async ({ page, }) => { @@ -238,12 +286,12 @@ test("the new tab view lists recently closed tabs, expands to the rest, and keep await expect(recent.getByRole("button", { name: "Show more", exact: true })).toHaveCount(0) await expect(recent.getByRole("button", { name: "Show less", exact: true })).toHaveCount(0) - // Closing the last tab closes the region; it opens on its launchers with that tab listed first. + // Closing the last tab keeps the region open on the New tab beside it, which lists that tab first. const readme = panel.getByRole("tab", { name: "README.md" }) await readme.click() + await expect(panel.getByRole("tab", { name: "New tab", exact: true })).toHaveAttribute("aria-selected", "false") await readme.locator("..").getByRole("button", { name: "Close tab" }).click() - await expect(toggle).toHaveAttribute("aria-expanded", "false") - await toggle.click() + await expect(toggle).toHaveAttribute("aria-expanded", "true") await expect(panel.getByRole("tab", { name: "New tab", exact: true })).toHaveAttribute("aria-selected", "true") await expect(rows.first()).toContainText("README.md") diff --git a/packages/app/e2e/regression/terminal.spec.ts b/packages/app/e2e/regression/terminal.spec.ts index 6223c2473c3..7e857a4846d 100644 --- a/packages/app/e2e/regression/terminal.spec.ts +++ b/packages/app/e2e/regression/terminal.spec.ts @@ -266,7 +266,7 @@ test("after a server restart, showing the terminal again restores it from a new // Showing the tab again takes a new snapshot and reconnects. await side(page).getByRole("button", { name: "Add tab", exact: true }).click() - await expect(side(page).getByRole("tab", { name: "New tab", exact: true })).toHaveAttribute("aria-selected", "true") + await expect(side(page).getByRole("tab", { name: "Build", exact: true })).toHaveAttribute("aria-selected", "false") await side(page).getByRole("tab", { name: "Build", exact: true }).click() await expect.poll(() => terminalText(page, screen)).toBe("before-restart\nafter-restart") await expect(side(page).getByText("Terminal disconnected", { exact: true })).toHaveCount(0) diff --git a/packages/app/src/runtime/plugin/panels.tsx b/packages/app/src/runtime/plugin/panels.tsx index 1a63edac8e3..3558d695e8e 100644 --- a/packages/app/src/runtime/plugin/panels.tsx +++ b/packages/app/src/runtime/plugin/panels.tsx @@ -223,6 +223,12 @@ export function createRegion(input: { return sibling ? sessionTabPane(tabs, sibling) : "side" } + const paneOfKey = (key: string) => { + const entry = byKey().get(key) + + return entry ? paneOf(entry) : sessionTabPane(input.tabs().tabs(), key) + } + // Narrow screens never select a transient tab: a stored one falls back like a missing tab. const desktop = createMediaQuery("(min-width: 768px)") @@ -314,30 +320,62 @@ export function createRegion(input: { return entry ? paneOf(entry) : "side" }) - // 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 } + // The host's new tab views, each listing the launchers in a pane until the user closes it or a tab selected from it + // replaces it. They belong to the session they opened in. A split or bottom pane closes with its last tab, and its + // New tabs go with it. + type NewTab = { id: string; pane: SessionPane; session: string } const [state, setState] = createStore<{ adding: NewTab[] + // The New tab each pane shows over its selected tab; none shows the selected tab. chosen: Partial> }>({ adding: [], chosen: {} }) let serial = 0 - const live = (value: NewTab) => value.session === input.view().key && value.over === panes[value.pane].active() + const live = (value: NewTab) => + value.session === input.view().key && (value.pane === "side" || panes[value.pane].keys().length > 0) + const newTabs = (pane: SessionPane) => state.adding.filter((value) => value.pane === pane && live(value)) + // A pane with no tab of its own shows its newest New tab. const shownNewTab = (pane: SessionPane) => { const list = newTabs(pane) + const chosen = list.find((value) => value.id === state.chosen[pane]) - return list.find((value) => value.id === state.chosen[pane]) ?? list.at(-1) + if (chosen) return chosen + + if (panes[pane].active() !== undefined) return undefined + + return list.at(-1) } createEffect(() => { if (state.adding.some((value) => !live(value))) setState("adding", (list) => list.filter(live)) }) + // A tab that selects itself while a New tab shows replaces that New tab, as one a launcher in it opens does. The other + // New tabs stay. Selecting a tab in the strip unchooses the New tab first, and the pane falling back to another tab + // after its selected tab closes or moves away keeps it. + for (const pane of ["side", "split", "bottom"] as const) { + createEffect( + on( + () => ({ + key: panes[pane].active(), + keys: panes[pane].keys(), + chosen: state.chosen[pane], + shown: shownNewTab(pane)?.id, + }), + (next, prev) => { + if (!prev?.shown || !next.key || next.key === prev.key || next.chosen !== prev.chosen) return + + if (prev.key && !next.keys.includes(prev.key)) return + setState("adding", (list) => list.filter((value) => value.id !== prev.shown)) + }, + ), + ) + } + const empty = createMemo(() => strip().length === 0) // Focus leaves before the close starts, back to where it was; otherwise the panel hands it to its toggle. @@ -355,9 +393,18 @@ export function createRegion(input: { input.onEmptied?.() } - // Closing the last New tab of a strip with no tabs closes the region, as closing its last tab does. + // Closing the shown New tab shows the New tab after it, else the one before, as a browser does. Closing the last New + // tab of a strip with no tabs closes the region, as closing its last tab does. const dismissNewTab = (id: string) => batch(() => { + const closing = state.adding.find((value) => value.id === id) + + if (closing && state.chosen[closing.pane] === id) { + const list = newTabs(closing.pane) + const index = list.findIndex((value) => value.id === id) + setState("chosen", closing.pane, (list[index + 1] ?? list[index - 1])?.id) + } + setState("adding", (list) => list.filter((value) => value.id !== id)) if (empty() && !state.adding.some(live)) emptied() @@ -379,21 +426,21 @@ export function createRegion(input: { return key ? byKey().get(key) : undefined }), pane: (pane: SessionPane) => panes[pane], - paneOf: (key: string): SessionPane => { - const entry = byKey().get(key) - - return entry ? paneOf(entry) : sessionTabPane(input.tabs().tabs(), key) - }, + paneOf: paneOfKey, wide: createMemo(() => providers().some((item) => item.value.wide)), /** A plugin's tab ids in the stored strip. */ openFor: (plugin: string) => stored().flatMap((key) => (key.startsWith(`${plugin}:`) ? [key.slice(plugin.length + 1)] : [])), + /** Selects a tab and shows it over the New tab its pane showed, which stays in the strip. */ select(key: string) { - input.tabs().setActive(key) + batch(() => { + setState("chosen", paneOfKey(key), undefined) + input.tabs().setActive(key) + }) }, /** 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. */ + /** The new tab view a pane shows: the chosen one, else the newest while the pane selects no tab. */ shownNewTab, newTab: (pane: SessionPane) => shownNewTab(pane) !== undefined, selectNewTab(pane: SessionPane, id: string) { @@ -404,7 +451,7 @@ export function createRegion(input: { const id = `new:${++serial}` batch(() => { - setState("adding", (list) => [...list, { id, pane, session: input.view().key, over: panes[pane].active() }]) + setState("adding", (list) => [...list, { id, pane, session: input.view().key }]) setState("chosen", pane, id) }) }, @@ -451,10 +498,11 @@ export function createRegion(input: { const closed = entry && closedEntry(key, entry.provider, panelInput(input.view, input.screen, { tab: entry.tab })) if (closed) input.closed.push(input.view(), closed) + // New tabs left in the strip keep the region open. batch(() => { input.tabs().close(key) - if (last) emptied() + if (last && !state.adding.some(live)) emptied() }) if (entry) diff --git a/packages/app/src/runtime/plugin/side-region.tsx b/packages/app/src/runtime/plugin/side-region.tsx index d5b4b0f7dc0..ae990057f02 100644 --- a/packages/app/src/runtime/plugin/side-region.tsx +++ b/packages/app/src/runtime/plugin/side-region.tsx @@ -81,13 +81,15 @@ export function SideRegion(props: { createEffect(on(starting, (key) => key && !props.region.newTab("side") && 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. + // The split pane shows while it holds tabs; when the side pane has none, New tabs included, the split pane takes the + // whole height. const panes = createMemo(() => { const split = props.region.pane("split").keys().length > 0 if (!split) return ["side"] as const - if (props.region.pane("side").keys().length === 0) return ["split"] as const + if (props.region.pane("side").keys().length === 0 && props.region.newTabs("side").length === 0) + return ["split"] as const return ["side", "split"] as const }) @@ -252,21 +254,19 @@ export function PaneTabs(props: { data-pane-content={props.pane.id} value={current()?.id ?? props.pane.active() ?? "empty"} onChange={(value) => { + // Kobalte selects the first tab while triggers register, including while the "+" click is still dispatching. + // Only input on a tab selects one; the region owns fallback selection. + if ( + !selectionEvent || + selectionEvent.eventPhase === Event.NONE || + !(selectionEvent.target instanceof Element) || + !selectionEvent.target.closest('[role="tab"]') + ) + return + if (props.region.newTabs(props.pane.id).some((entry) => entry.id === value)) return props.region.selectNewTab(props.pane.id, 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. - if ( - selectionEvent && - selectionEvent.eventPhase !== Event.NONE && - selectionEvent.target instanceof Element && - selectionEvent.target.closest('[role="tab"]') - ) { - // Select first: dismissing the only New tab with nothing selected lets Kobalte pick the first tab meanwhile. - props.region.select(value) - props.region.newTabs(props.pane.id).forEach((entry) => props.region.dismissNewTab(entry.id)) - } + props.region.select(value) }} >