fix(app): keep side panel New tabs until they are closed or used (#61702)

This commit is contained in:
DS
2026-11-04 17:06:50 -05:00
committed by GitHub
Unverified
parent ded4c290d8
commit bf937be15b
5 changed files with 133 additions and 36 deletions
+3 -2
View File
@@ -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)
@@ -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")
+1 -1
View File
@@ -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)
+63 -15
View File
@@ -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<Record<SessionPane, string>>
}>({ 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)
+15 -15
View File
@@ -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)
}}
>
<div class="session-review-v2-tabs-bar sticky top-0 shrink-0 flex items-center">