From d9dae17e97268916c75528d0ea69c34c448b5754 Mon Sep 17 00:00:00 2001 From: xxxigm <54813621+xxxigm@users.noreply.github.com> Date: Tue, 21 Jul 2026 21:17:32 +0700 Subject: [PATCH] =?UTF-8?q?fix(desktop):=20=E2=8C=98W=20closes=20visible?= =?UTF-8?q?=20file=20tab=20when=20preview=20selection=20is=20stale=20(#686?= =?UTF-8?q?39)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(desktop): make ⌘W close visible file tab on stale preview selection When the live preview target is gone but $rightRailActiveTabId still points at preview, file tabs remain on screen while ⌘W fell through to a workspace no-op. Close the visible file tab instead. * test(desktop): cover ⌘W close for file tabs and ghost preview selection Lock the happy path and the stale-preview regression so ⌘W keeps closing the file tab the rail is actually showing. --- apps/desktop/src/app/chat/close-tab.test.ts | 65 +++++++++++++++++++++ apps/desktop/src/app/chat/close-tab.ts | 15 +++-- apps/desktop/src/store/preview.ts | 45 +++++++++++++- 3 files changed, 118 insertions(+), 7 deletions(-) create mode 100644 apps/desktop/src/app/chat/close-tab.test.ts diff --git a/apps/desktop/src/app/chat/close-tab.test.ts b/apps/desktop/src/app/chat/close-tab.test.ts new file mode 100644 index 0000000000000..95847fd925c2f --- /dev/null +++ b/apps/desktop/src/app/chat/close-tab.test.ts @@ -0,0 +1,65 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { $rightRailActiveTabId, RIGHT_RAIL_PREVIEW_TAB_ID } from '@/store/layout' +import { + $filePreviewTabs, + $previewTarget, + clearSessionPreviewRegistry, + type PreviewTarget, + setCurrentSessionPreviewTarget +} from '@/store/preview' +import { $activeSessionId, $selectedStoredSessionId } from '@/store/session' + +import { closeActiveTab } from './close-tab' + +function fileTarget(path: string): PreviewTarget { + return { + kind: 'file', + label: path, + path, + previewKind: 'text', + source: path, + url: `file://${path}` + } +} + +describe('closeActiveTab', () => { + beforeEach(() => { + vi.stubGlobal('document', { activeElement: null }) + $activeSessionId.set('session-1') + $selectedStoredSessionId.set(null) + window.localStorage.clear() + clearSessionPreviewRegistry() + }) + + afterEach(() => { + vi.unstubAllGlobals() + $activeSessionId.set(null) + $selectedStoredSessionId.set(null) + clearSessionPreviewRegistry() + window.localStorage.clear() + }) + + it('closes the active file preview tab (⌘W happy path)', () => { + setCurrentSessionPreviewTarget(fileTarget('/work/notes.md'), 'manual') + + expect($filePreviewTabs.get()).toHaveLength(1) + expect($rightRailActiveTabId.get()).toBe('file:file:///work/notes.md') + + expect(closeActiveTab()).toBe(true) + expect($filePreviewTabs.get()).toHaveLength(0) + }) + + it('closes the visible file tab when active selection is a ghost preview', () => { + // Active tab id stuck on live-preview after that target was cleared, while + // file tabs remain (UI falls back to tabs[0] until React syncs). ⌘W must + // close the visible file tab instead of no-op'ing via closeWorkspaceTab(). + setCurrentSessionPreviewTarget(fileTarget('/work/notes.md'), 'manual') + $previewTarget.set(null) + $rightRailActiveTabId.set(RIGHT_RAIL_PREVIEW_TAB_ID) + + expect($filePreviewTabs.get()).toHaveLength(1) + expect(closeActiveTab()).toBe(true) + expect($filePreviewTabs.get()).toHaveLength(0) + }) +}) diff --git a/apps/desktop/src/app/chat/close-tab.ts b/apps/desktop/src/app/chat/close-tab.ts index 100e14365852f..d5a5a0bcae14b 100644 --- a/apps/desktop/src/app/chat/close-tab.ts +++ b/apps/desktop/src/app/chat/close-tab.ts @@ -1,12 +1,12 @@ import { closeActiveTerminal } from '@/app/right-sidebar/terminal/terminals' import { closeWorkspaceTab } from '@/components/pane-shell/tree/store' import { isFocusWithin } from '@/lib/keybinds/combo' -import { $filePreviewTarget, $previewTarget, closeActiveRightRailTab } from '@/store/preview' +import { $filePreviewTabs, $previewTarget, closeActiveRightRailTab } from '@/store/preview' /** * ⌘W — close the tab of the context you're in, by precedence: * 1. a focused terminal → its active terminal tab, - * 2. an open preview → its active preview tab (unchanged from pre-tiling), + * 2. right-rail tabs (live preview and/or file peeks), * 3. the MAIN zone → its active tab (a session tile stacked into the workspace). * Returns false when nothing closes, so ⌘W is a no-op — it never closes the * window (a bare workspace stays put). Shared by the keyboard path (Win/Linux) @@ -19,10 +19,13 @@ export function closeActiveTab(): boolean { return true } - if ($filePreviewTarget.get() || $previewTarget.get()) { - closeActiveRightRailTab() - - return true + // Prefer tab *presence* over the derived active file target. After the live + // preview is cleared, `$rightRailActiveTabId` can stay on `preview` while + // file tabs remain (the rail UI falls back to tabs[0]). Gating only on + // `$filePreviewTarget` made ⌘W fall through to closeWorkspaceTab() and look + // broken with a file tab still on screen. + if ($previewTarget.get() || $filePreviewTabs.get().length > 0) { + return closeActiveRightRailTab() } return closeWorkspaceTab() diff --git a/apps/desktop/src/store/preview.ts b/apps/desktop/src/store/preview.ts index c0533e719e4e1..a2365b523de0a 100644 --- a/apps/desktop/src/store/preview.ts +++ b/apps/desktop/src/store/preview.ts @@ -87,6 +87,16 @@ if ( selectRightRailTab(RIGHT_RAIL_PREVIEW_TAB_ID) } +// Inverse: persisted/default active id is still the live-preview tab, but that +// target isn't open and file tabs are. Point at the first file tab so ⌘W and +// the strip agree before React's fallback sync runs. +if ( + $rightRailActiveTabId.get() === RIGHT_RAIL_PREVIEW_TAB_ID && + $filePreviewTabs.get().length > 0 +) { + selectRightRailTab($filePreviewTabs.get()[0]!.id) +} + export const $filePreviewTarget = computed([$filePreviewTabs, $rightRailActiveTabId], (tabs, activeTabId) => { if (!activeTabId.startsWith('file:')) { return null @@ -460,7 +470,40 @@ export function closeRightRailTab(tabId: RightRailTabId) { closeFilePreviewTab(tabId) } -export const closeActiveRightRailTab = () => closeRightRailTab($rightRailActiveTabId.get()) +/** Close the tab the right rail is actually showing. Returns false when nothing + * closed (so ⌘W can fall through). Resolves a stale `preview` selection to the + * first file tab when the live preview target is already gone. */ +export function closeActiveRightRailTab(): boolean { + let tabId = $rightRailActiveTabId.get() + + if (tabId === RIGHT_RAIL_PREVIEW_TAB_ID && !$previewTarget.get()) { + const fallback = $filePreviewTabs.get()[0]?.id + + if (!fallback) { + return false + } + + tabId = fallback + } + + if (tabId === RIGHT_RAIL_PREVIEW_TAB_ID) { + if (!$previewTarget.get()) { + return false + } + + closeRightRailTab(tabId) + + return true + } + + if (!$filePreviewTabs.get().some(tab => tab.id === tabId)) { + return false + } + + closeRightRailTab(tabId) + + return true +} // The rail's visible tab order: the live preview tab (when present) first, then // the file tabs in their stored order. Mirrors `ChatPreviewRail`'s `tabs` memo