diff --git a/web/src/lib/components/layout/TopBar.svelte b/web/src/lib/components/layout/TopBar.svelte index 95d6e8737..f251bc77f 100644 --- a/web/src/lib/components/layout/TopBar.svelte +++ b/web/src/lib/components/layout/TopBar.svelte @@ -17,6 +17,7 @@ import Menu from '$lib/components/common/Menu.svelte'; import MenuItem from '$lib/components/common/MenuItem.svelte'; import { workspaceRestoreTarget } from '$lib/utils/workspace-route'; + import { tabLanding } from '$lib/utils/tabLanding'; let { mobile = false }: { mobile?: boolean } = $props(); @@ -105,16 +106,14 @@ if (tab.ephemeral) void tabsStore.pin(tab.slug).catch(() => {}); } - // Closing a tab (PLAN-3002 Q2, Q3). Only closing the ACTIVE tab moves you: - // to its left neighbour, or, when it was the first tab, to the tab that - // becomes first (its right neighbour; lead ruling on TASK-3274), at that - // tab's last route. Closing the last tab lands on /console with "+" - // highlighted as the way back in. + // Closing a tab (PLAN-3002 Q2, Q3). Only closing the ACTIVE tab moves you, + // to where tabLanding says (the one copy of the Q3 rule, shared with the + // lost-workspace path; TASK-3280), at that tab's last route. Closing the + // last tab lands on /console with "+" highlighted as the way back in. async function closeTab(e: MouseEvent, tab: WorkspaceTab) { e.preventDefault(); e.stopPropagation(); - const before = tabsStore.tabs.map((t) => t.slug); - const idx = before.indexOf(tab.slug); + const before = tabsStore.tabs.slice(); const wasActive = tab.slug === currentSlug; try { await tabsStore.close(tab.slug); @@ -122,8 +121,8 @@ return; } if (!wasActive) return; - const after = tabsStore.tabs; - if (after.length === 0) { + const landing = tabLanding(before, tab.slug, tabsStore.tabs); + if (!landing) { // Set once the navigation has landed: /console has no TopBar, so // the console page shows it on its own create button and clears it // when it unmounts. @@ -131,8 +130,6 @@ uiStore.highlightAddWorkspace(); return; } - const neighbour = idx > 0 ? before[idx - 1] : before[idx + 1]; - const landing = after.find((t) => t.slug === neighbour) ?? after[0]; goto(workspaceRestoreTarget(landing)); } diff --git a/web/src/lib/components/layout/TopBar.svelte.test.ts b/web/src/lib/components/layout/TopBar.svelte.test.ts index 31224efec..79e188744 100644 --- a/web/src/lib/components/layout/TopBar.svelte.test.ts +++ b/web/src/lib/components/layout/TopBar.svelte.test.ts @@ -194,6 +194,18 @@ describe('TopBar tab bar: closing (PLAN-3002 Q2, Q3)', () => { expect(mocks.goto).toHaveBeenCalledWith('/u/alpha/docs'); }); + // The leg above closes the SECOND tab, whose left neighbour is also the + // first tab, so it cannot tell "left neighbour" from "first remaining" + // (TASK-3280: a closeTab that read the open set after the close passed it). + it('closing an active tab further right lands on its left neighbour, not on the first tab', async () => { + mocks.current = { slug: 'gamma', name: 'Gamma', owner_username: 'u' }; + await mountWith([ALPHA, BETA, GAMMA]); + mocks.tabs.close.mockResolvedValueOnce(answer([ALPHA, BETA])); + closeBtn('gamma').click(); + await settle(); + expect(mocks.goto).toHaveBeenCalledWith('/u/beta'); + }); + it('closing the active FIRST tab lands on the tab that becomes first (lead ruling on Q3)', async () => { mocks.current = { slug: 'alpha', name: 'Alpha', owner_username: 'u' }; await mountWith([ALPHA, BETA, GAMMA]); diff --git a/web/src/lib/utils/tabLanding.test.ts b/web/src/lib/utils/tabLanding.test.ts index ae9ab86c0..89ba973a6 100644 --- a/web/src/lib/utils/tabLanding.test.ts +++ b/web/src/lib/utils/tabLanding.test.ts @@ -37,6 +37,22 @@ describe('tabLanding (PLAN-3002 Q3)', () => { expect(tabLanding(before, 'a', tabs('c'))?.slug).toBe('c'); }); + // The two legs below give `after` a shape that is NOT `before` minus the + // gone tab, which is the only case where these rules differ from their + // look-alikes (TASK-3280: "first remaining" vs "last remaining", and + // "right neighbour" vs "first remaining"). + it('the fallback is the FIRST remaining tab, not any remaining tab', () => { + const before = tabs('a', 'b', 'c', 'd'); + // d went, and so did its neighbour c: two tabs remain. + expect(tabLanding(before, 'd', tabs('a', 'b'))?.slug).toBe('a'); + }); + + it('when the first tab goes, its right neighbour wins even if it is no longer first', () => { + // The refetch also opened x at the front of the bar. + const before = tabs('a', 'b', 'c'); + expect(tabLanding(before, 'a', tabs('x', 'b', 'c'))?.slug).toBe('b'); + }); + it('lands on the first tab when the gone workspace was never in the bar', () => { expect(tabLanding(tabs('a', 'b'), 'zz', tabs('a', 'b'))?.slug).toBe('a'); });