From 89545ce475f2d4777812e2d0d217269a4798bd31 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Mon, 24 Aug 2026 13:45:36 -0700 Subject: [PATCH] Restore previously-active tab when closing a browser tab Closing a browser tab now activates the most-recently-used (MRU) tab instead of the visual neighbor. This provides more predictable navigation behavior when switching between tabs. --- src/renderer/src/components/Terminal.tsx | 21 +--- src/renderer/src/store/slices/browser.ts | 13 +-- .../store-active-worktree-tab-close.test.ts | 102 ++++++++++++++++++ .../slices/tabs-open-close-lifecycle.test.ts | 8 +- 4 files changed, 118 insertions(+), 26 deletions(-) diff --git a/src/renderer/src/components/Terminal.tsx b/src/renderer/src/components/Terminal.tsx index 49bc620914a..05f62faa0a9 100644 --- a/src/renderer/src/components/Terminal.tsx +++ b/src/renderer/src/components/Terminal.tsx @@ -1774,9 +1774,12 @@ function Terminal(): React.JSX.Element | null { } const currentTabs = state.browserTabsByWorktree[owningWorktreeId] ?? [] if (currentTabs.length <= 1) { + const hasUnifiedEntry = Object.values(state.unifiedTabsByWorktree).some((tabs) => + tabs.some((tab) => tab.contentType === 'browser' && tab.entityId === tabId) + ) destroyWorkspaceWebviews(state.browserPagesByWorkspace, tabId) closeBrowserTab(tabId) - if (state.activeWorktreeId === owningWorktreeId) { + if (!hasUnifiedEntry && state.activeWorktreeId === owningWorktreeId) { const worktreeFile = state.openFiles.find((file) => file.worktreeId === owningWorktreeId) if (worktreeFile) { setActiveFile(worktreeFile.id) @@ -1793,24 +1796,10 @@ function Terminal(): React.JSX.Element | null { } return } - if (state.activeWorktreeId === owningWorktreeId && tabId === state.activeBrowserTabId) { - const idx = currentTabs.findIndex((tab) => tab.id === tabId) - const nextTab = currentTabs[idx + 1] ?? currentTabs[idx - 1] - if (nextTab) { - setActiveBrowserTab(nextTab.id) - } - } destroyWorkspaceWebviews(state.browserPagesByWorkspace, tabId) closeBrowserTab(tabId) }, - [ - closeBrowserTab, - setActiveBrowserTab, - setActiveFile, - setActiveTab, - setActiveTabType, - setActiveWorktree - ] + [closeBrowserTab, setActiveFile, setActiveTab, setActiveTabType, setActiveWorktree] ) const handlePtyExit = useCallback( diff --git a/src/renderer/src/store/slices/browser.ts b/src/renderer/src/store/slices/browser.ts index 0017d42c7c8..231ebfd916a 100644 --- a/src/renderer/src/store/slices/browser.ts +++ b/src/renderer/src/store/slices/browser.ts @@ -23,13 +23,13 @@ import { normalizeBrowserHistoryEntries, normalizeBrowserHistoryUrl } from '../../../../shared/workspace-session-browser-history' -import { pickNeighbor } from './tab-group-state' import { destroyWorkspaceWebviews } from './browser-webview-cleanup' import { getRecentlyClosedTabPosition, restoreRecentlyClosedTabPosition, pushRecentlyClosedTabKind } from './recently-closed-tabs' +import { pickNeighbor } from './tab-group-state' import type { RecentlyClosedTabPosition } from './recently-closed-tabs' import { callRuntimeRpc, type RuntimeClientTarget } from '@/runtime/runtime-rpc-client' import { toRuntimeWorktreeSelector } from '@/runtime/runtime-worktree-selector' @@ -846,13 +846,14 @@ export const createBrowserSlice: StateCreator = delete nextRemoteBrowserPageHandlesByPageId[page.id] } - const nextActiveBrowserTabIdByWorktree = { ...s.activeBrowserTabIdByWorktree } const remainingBrowserTabs = nextBrowserTabsByWorktree[owningWorktreeId] ?? [] - const tabBarOrder = s.tabBarOrderByWorktree[owningWorktreeId] ?? [] - const neighborTabId = pickNeighbor(tabBarOrder, tabId) + const nextActiveBrowserTabIdByWorktree = { ...s.activeBrowserTabIdByWorktree } if (nextActiveBrowserTabIdByWorktree[owningWorktreeId] === tabId) { + const neighborId = pickNeighbor(s.tabBarOrderByWorktree[owningWorktreeId] ?? [], tabId) nextActiveBrowserTabIdByWorktree[owningWorktreeId] = - neighborTabId ?? remainingBrowserTabs[0]?.id ?? null + (neighborId && remainingBrowserTabs.some((tab) => tab.id === neighborId) + ? neighborId + : remainingBrowserTabs[0]?.id) ?? null } const nextTabBarOrder = { @@ -916,7 +917,7 @@ export const createBrowserSlice: StateCreator = browserPagesByWorkspace: nextBrowserPagesByWorkspace, activeBrowserTabId: s.activeBrowserTabId === tabId - ? (neighborTabId ?? remainingBrowserTabs[0]?.id ?? null) + ? (nextActiveBrowserTabIdByWorktree[owningWorktreeId] ?? null) : s.activeBrowserTabId, activeBrowserTabIdByWorktree: nextActiveBrowserTabIdByWorktree, tabBarOrderByWorktree: nextTabBarOrder, diff --git a/src/renderer/src/store/slices/store-active-worktree-tab-close.test.ts b/src/renderer/src/store/slices/store-active-worktree-tab-close.test.ts index 7ac3fef8d82..1ccc070b008 100644 --- a/src/renderer/src/store/slices/store-active-worktree-tab-close.test.ts +++ b/src/renderer/src/store/slices/store-active-worktree-tab-close.test.ts @@ -424,6 +424,108 @@ describe('setActiveWorktree', () => { expect(s.activeBrowserTabIdByWorktree[backgroundWt]).toBe(browserTab.id) }) + it('uses unified MRU selection when closing an active browser tab', () => { + const store = createTestStore() + const wt = 'repo1::/path/wt1' + + seedStore(store, { + worktreesByRepo: { + repo1: [makeWorktree({ id: wt, repoId: 'repo1', path: '/path/wt1' })] + }, + activeWorktreeId: wt + }) + + const previous = store.getState().createBrowserTab(wt, 'https://previous.example.com') + store.getState().createBrowserTab(wt, 'https://neighbor.example.com') + const closing = store.getState().createBrowserTab(wt, 'https://closing.example.com') + store.getState().setActiveBrowserTab(previous.id) + store.getState().setActiveBrowserTab(closing.id) + + store.getState().closeBrowserTab(closing.id) + + expect(store.getState().activeBrowserTabId).toBe(previous.id) + expect(store.getState().activeBrowserTabIdByWorktree[wt]).toBe(previous.id) + }) + + it('keeps the unified MRU target when closing the last browser tab', () => { + const store = createTestStore() + const wt = 'repo1::/path/wt1' + + seedStore(store, { + worktreesByRepo: { + repo1: [makeWorktree({ id: wt, repoId: 'repo1', path: '/path/wt1' })] + }, + activeWorktreeId: wt + }) + + store.getState().createTab(wt) + const previous = store.getState().createTab(wt) + const closing = store.getState().createBrowserTab(wt, 'https://closing.example.com') + + store.getState().closeBrowserTab(closing.id) + + expect(store.getState().activeTabId).toBe(previous.id) + expect(store.getState().activeTabType).toBe('terminal') + }) + + it('keeps a valid browser target for an inactive worktree after close', () => { + const store = createTestStore() + const activeWt = 'repo1::/path/active' + const backgroundWt = 'repo1::/path/background' + + seedStore(store, { + worktreesByRepo: { + repo1: [ + makeWorktree({ id: activeWt, repoId: 'repo1', path: '/path/active' }), + makeWorktree({ id: backgroundWt, repoId: 'repo1', path: '/path/background' }) + ] + }, + activeWorktreeId: activeWt + }) + + const previous = store + .getState() + .createBrowserTab(backgroundWt, 'https://previous.example.com', { + activate: false + }) + const closing = store.getState().createBrowserTab(backgroundWt, 'https://closing.example.com', { + activate: false + }) + store.setState({ activeBrowserTabIdByWorktree: { [backgroundWt]: closing.id } }) + + store.getState().closeBrowserTab(closing.id) + + expect(store.getState().activeBrowserTabIdByWorktree[backgroundWt]).toBe(previous.id) + }) + + it('keeps the global browser target when closing a legacy tab without a unified wrapper', () => { + const store = createTestStore() + const wt = 'repo1::/path/wt1' + + seedStore(store, { + worktreesByRepo: { + repo1: [makeWorktree({ id: wt, repoId: 'repo1', path: '/path/wt1' })] + }, + activeWorktreeId: wt + }) + + const previous = store.getState().createBrowserTab(wt, 'https://previous.example.com', { + activate: false + }) + const closing = store.getState().createBrowserTab(wt, 'https://closing.example.com', { + activate: false + }) + store.setState({ + unifiedTabsByWorktree: {}, + activeBrowserTabId: closing.id, + activeBrowserTabIdByWorktree: { [wt]: closing.id } + }) + + store.getState().closeBrowserTab(closing.id) + + expect(store.getState().activeBrowserTabId).toBe(previous.id) + }) + it('queues and consumes a one-shot address-bar focus request for a fresh blank browser tab', () => { const store = createTestStore() const wt = 'repo1::/path/wt1' diff --git a/src/renderer/src/store/slices/tabs-open-close-lifecycle.test.ts b/src/renderer/src/store/slices/tabs-open-close-lifecycle.test.ts index 7e10c3c1e04..ecd9fed0c2c 100644 --- a/src/renderer/src/store/slices/tabs-open-close-lifecycle.test.ts +++ b/src/renderer/src/store/slices/tabs-open-close-lifecycle.test.ts @@ -347,12 +347,12 @@ describe('TabsSlice', () => { expect(buildMobileSessionTabSnapshots(store.getState())[0]?.tabs ?? []).toEqual([]) }) - it('activates the previously-active tab (MRU) instead of the visual neighbor', () => { + it('activates the previously-active tab across content types', () => { const t1 = store.getState().createUnifiedTab(WT, 'terminal') - const t2 = store.getState().createUnifiedTab(WT, 'terminal') - const t3 = store.getState().createUnifiedTab(WT, 'terminal') + const t2 = store.getState().createUnifiedTab(WT, 'browser') + const t3 = store.getState().createUnifiedTab(WT, 'editor') - // Visit order ...→t3→t1→t3; closing t3 should jump to t1 (MRU previous), not the visual neighbor t2. + // Visit order ...→t3→t1→t3; closing t3 should jump to t1, not browser neighbor t2. store.getState().activateTab(t1.id) store.getState().activateTab(t3.id) store.getState().closeUnifiedTab(t3.id)