diff --git a/packages/app/e2e/regression/tabs.spec.ts b/packages/app/e2e/regression/tabs.spec.ts index f413c7fb5d15..df428935c84f 100644 --- a/packages/app/e2e/regression/tabs.spec.ts +++ b/packages/app/e2e/regression/tabs.spec.ts @@ -1235,7 +1235,7 @@ test("⌘W closes the session tab again once the window loses focus after the si await expect(sessions.alphaTab).toHaveCount(0) }) -test("⌘W in the side panel closes its New tab, then the panel, and only then the session tab", async ({ page }) => { +test("⌘W in the side panel closes its last New tab with the panel, and only then the session tab", async ({ page }) => { await page.setViewportSize({ width: 1440, height: 900 }) const alpha = { id: "ses_side_new_alpha", title: "Side new alpha" } const beta = { id: "ses_side_new_beta", title: "Side new beta" } @@ -1254,14 +1254,14 @@ test("⌘W in the side panel closes its New tab, then the panel, and only then t // A launcher row in the New tab view holds focus as ⌘W removes it. await panel.getByRole("button", { name: /^Open file/ }).focus() + // The New tab was the panel's last tab, so closing it closes the panel too, and focus goes back. await page.keyboard.press("ControlOrMeta+w") await expect(newTab).toHaveCount(0) - await expect(toggle).toHaveAttribute("aria-expanded", "true") - - // Nothing is left to close in the panel, so ⌘W closes the panel rather than the session, and focus goes back. - await page.keyboard.press("ControlOrMeta+w") await expect(toggle).toHaveAttribute("aria-expanded", "false") await expect(composer).toBeFocused() + + // One press too many closes nothing rather than the session. + await page.keyboard.press("ControlOrMeta+w") await expect(alphaTab).toHaveCount(1) await expectPath(page, sessionHref(alpha.id)) diff --git a/packages/app/src/runtime/plugin/panels.tsx b/packages/app/src/runtime/plugin/panels.tsx index a33a98b1f49a..c4ca26abbafb 100644 --- a/packages/app/src/runtime/plugin/panels.tsx +++ b/packages/app/src/runtime/plugin/panels.tsx @@ -340,12 +340,27 @@ export function createRegion(input: { 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. + const giveBackFocus = () => { + const inside = !!document.getElementById("review-panel")?.contains(document.activeElement) + const back = returnFocus + returnFocus = undefined + + if (inside && back?.isConnected) back.focus({ preventScroll: true }) + } + + // Emptying the region closes it, so focus goes back as the user's own close would give it. + const emptied = () => { + giveBackFocus() + input.onEmptied?.() + } + // 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(() => { setState("adding", (list) => list.filter((value) => value.id !== id)) - if (empty() && !state.adding.some(live)) input.onEmptied?.() + if (empty() && !state.adding.some(live)) emptied() }) return { @@ -409,12 +424,7 @@ export function createRegion(input: { const focused = document.activeElement if (side.opened()) { - const inside = !!document.getElementById("review-panel")?.contains(focused) - const back = returnFocus - returnFocus = undefined - - // Focus leaves before the close starts, back to where it was; otherwise the panel hands it to its toggle. - if (inside && back?.isConnected) back.focus({ preventScroll: true }) + giveBackFocus() side.toggle() return @@ -444,7 +454,7 @@ export function createRegion(input: { batch(() => { input.tabs().close(key) - if (last) input.onEmptied?.() + if (last) emptied() }) if (entry) diff --git a/packages/app/src/session/commands/use-session-commands.tsx b/packages/app/src/session/commands/use-session-commands.tsx index 3d2ca6ccf909..2d953398c3f3 100644 --- a/packages/app/src/session/commands/use-session-commands.tsx +++ b/packages/app/src/session/commands/use-session-commands.tsx @@ -241,8 +241,10 @@ export const useSessionCommands = (actions: SessionCommandContext) => { if (actions.region.closeNewTab(focusedPane())) { const panel = document.getElementById("review-panel") - // A launcher row that held focus is gone; focus stays in the panel so closing it next returns focus. - if (panel && !panel.contains(document.activeElement)) panel.focus({ preventScroll: true, focusVisible: false }) + // A launcher row that held focus is gone; focus stays in the panel so closing it next returns focus. Closing the + // strip's last New tab closes the panel too, which already gave focus back. + if (panel && actions.session.layout.view().side.opened() && !panel.contains(document.activeElement)) + panel.focus({ preventScroll: true, focusVisible: false }) return }