Skip to content

Commit 9eee539

Browse files
committed
Downloads jump: reuse an open pane instead of duplicating the view
When a pane already shows the downloaded file's folder, the "jump to file" flows now reveal the file there instead of re-opening the same folder in the focused pane (which destroyed that pane's location and duplicated the view). User feedback: with jump-to-download on and Downloads already open in one pane, the other pane opened it again. - Add `revealFileInBestPane` / `navigateToDirInBestPane` in `navigate-and-select.ts`: pick the target pane in priority order — focused pane already shows the dir (cursor move only), else the other pane shows it (focus shift + cursor move), else navigate the focused pane (unchanged fallback). - Match is volume-safe and active-tab-only, mirroring `commitPathFromListing`'s drop-foreign-listings policy: a real local volume that contains the path (`isPathOnVolume`), never a `network`/`search-results`/MTP pane reporting a same-looking path string. - Route all downloads jump entry points through the new helpers: `goToLatestDownload` (⌘J, command palette, MCP), `goToDownload` (toast click + "Jump to file"), the global ⌃⌥⌘J hotkey, and the empty-Downloads toast's "Go to Downloads" action. The empty-toast action still snapshots the Downloads dir at toast-add time but re-evaluates which pane shows it at click time. - Leave the shared `navigateToFileInPane` / `navigateToDirInPane` primitives unchanged, so "Go to path" (⌘G) keeps its always-navigate behavior. - Expose `setFocusedPane` and `getPaneLocation` (active-tab volume id + volume path + path) on `ExplorerAPI`, wired through `DualPaneExplorer.svelte`. - Update the downloads `CLAUDE.md` (pane-reuse note + smoke steps) and navigation `DETAILS.md`.
1 parent 439d7fc commit 9eee539

7 files changed

Lines changed: 300 additions & 44 deletions

File tree

apps/desktop/src/lib/downloads/CLAUDE.md

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,16 @@ the NEXT toast's `initialCollapsed` reflects it. The `ToastItem` host extends th
108108
The split matters when a burst of downloads arrives: each toast must take the user to the file IT advertised, not to
109109
whichever file is most recent at click time.
110110

111+
**Pane reuse.** All jump entry points (`goToLatestDownload`, `goToDownload`, and the empty-toast "Go to Downloads"
112+
action) reveal the target through the `revealFileInBestPane` / `navigateToDirInBestPane` helpers in
113+
`file-explorer/navigation/navigate-and-select.ts`, NOT the plain `navigateToFileInPane`. If a pane already shows the
114+
file's `parentDir`, those helpers move the cursor there (focused pane) or shift focus to the other pane — no
115+
re-navigation, so an already-open Downloads view isn't duplicated. Only when neither pane shows the dir do they navigate
116+
the focused pane. The match is the pane's ACTIVE tab only and volume-safe (a real local volume that contains the path;
117+
an MTP or network pane at a same-looking path never counts). The empty-toast action snapshots the Downloads dir at
118+
toast-add time but re-evaluates which pane shows it at CLICK time. "Go to path" (⌘G) deliberately keeps the
119+
always-navigate behavior and does NOT reuse panes.
120+
111121
## FDA defense-in-depth
112122

113123
The watcher won't emit `download-detected` when the FDA gate is closed — that's the contract enforced in
@@ -171,11 +181,15 @@ or the settings rows. Each step is independent; you can stop after the ones that
171181
2. Wait for the FDA gate to open (existing onboarding). If FDA is already granted in System Settings, the gate clears
172182
automatically.
173183
3. Drop a file via Terminal: `touch ~/Downloads/test1.txt` → expect a Downloads toast in Cmdr.
174-
4. Click the toast body (anywhere outside the buttons) → the focused pane navigates to `~/Downloads` and selects
175-
`test1.txt`.
176-
5. Press `⌘J` from a Cmdr-focused window → the focused pane goes to the latest download (`test1.txt`).
177-
6. `Cmd-Tab` to Chrome, press `⌃⌥⌘J` → Cmdr foregrounds and goes to `test1.txt`. The first trigger of this session shows
178-
the warn toast ("The ⌃⌥⌘J shortcut jumps to your latest download from anywhere. Keep it on?").
184+
4. With neither pane on `~/Downloads`, click the toast body (anywhere outside the buttons) → the focused pane navigates
185+
to `~/Downloads` and selects `test1.txt`.
186+
5. Pane reuse: open `~/Downloads` in the LEFT pane, focus the RIGHT pane, then press `⌘J` → focus shifts to the left
187+
pane and the cursor lands on `test1.txt`; the right pane is untouched (no duplicate Downloads view). Now with the
188+
FOCUSED pane already on `~/Downloads`, press `⌘J` again → just the cursor moves, no re-navigation and no focus
189+
change.
190+
6. `Cmd-Tab` to Chrome, press `⌃⌥⌘J` → Cmdr foregrounds and reveals `test1.txt` (reusing a pane already on
191+
`~/Downloads`, else navigating the focused pane). The first trigger of this session shows the warn toast ("The ⌃⌥⌘J
192+
shortcut jumps to your latest download from anywhere. Keep it on?").
179193
7. Click "Keep it on" on the warn toast → `acknowledged` flips to `true`; subsequent triggers don't show the toast.
180194
8. Copy five files via Cmdr into `~/Downloads` (Cmd+C + Cmd+V or drag) → expect NO downloads toasts (Cmdr-own-write
181195
suppression).

apps/desktop/src/lib/downloads/go-to-latest.test.ts

Lines changed: 154 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ const {
1212
navigateMock,
1313
moveCursorMock,
1414
getFocusedPaneMock,
15+
getPaneLocationMock,
16+
setFocusedPaneMock,
1517
} = vi.hoisted(() => ({
1618
goToLatestDownloadMock: vi.fn(),
1719
downloadsWatcherStatusMock: vi.fn(),
@@ -21,6 +23,14 @@ const {
2123
navigateMock: vi.fn((): NavigateResult => ({ status: 'started', settled: Promise.resolve() })),
2224
moveCursorMock: vi.fn(() => Promise.resolve()),
2325
getFocusedPaneMock: vi.fn(() => 'left'),
26+
// Pane active-tab location getter. Default: both panes elsewhere, on the main
27+
// local volume, so the pane-reuse check fails and the focused pane navigates.
28+
getPaneLocationMock: vi.fn((): { volumeId: string; volumePath: string; path: string } => ({
29+
volumeId: 'root',
30+
volumePath: '/',
31+
path: '/Users/me/elsewhere',
32+
})),
33+
setFocusedPaneMock: vi.fn(),
2434
}))
2535

2636
vi.mock('$lib/ipc/bindings', () => ({
@@ -58,16 +68,33 @@ import LatestDownloadFdaToastContent from './LatestDownloadFdaToastContent.svelt
5868
import type { ExplorerAPI } from '../../routes/(main)/explorer-api'
5969

6070
function makeExplorerStub(): ExplorerAPI {
61-
// Only the three methods this helper touches need real stubs. Everything
62-
// else is unused; the typed stub avoids a `Partial<ExplorerAPI>` cast leaking
63-
// into the helper's call sites.
71+
// Only the methods these helpers touch need real stubs. Everything else is
72+
// unused; the typed stub avoids a `Partial<ExplorerAPI>` cast leaking into the
73+
// helper's call sites.
6474
return {
6575
getFocusedPane: getFocusedPaneMock,
6676
navigate: navigateMock,
6777
moveCursor: moveCursorMock,
78+
getPaneLocation: getPaneLocationMock,
79+
setFocusedPane: setFocusedPaneMock,
6880
} as unknown as ExplorerAPI
6981
}
7082

83+
/**
84+
* Helper: wire `getPaneLocation` so a given pane's active tab shows `path` on a
85+
* real local volume (so the pane-reuse match succeeds), while the other pane
86+
* sits elsewhere. `volumeId`/`volumePath` default to the main local volume.
87+
*/
88+
function paneShows(
89+
pane: 'left' | 'right',
90+
path: string,
91+
volume: { volumeId: string; volumePath: string } = { volumeId: 'root', volumePath: '/' },
92+
): void {
93+
getPaneLocationMock.mockImplementation((p: 'left' | 'right') =>
94+
p === pane ? { ...volume, path } : { volumeId: 'root', volumePath: '/', path: '/Users/me/elsewhere' },
95+
)
96+
}
97+
7198
describe('goToLatestDownload', () => {
7299
beforeEach(() => {
73100
goToLatestDownloadMock.mockReset()
@@ -77,6 +104,8 @@ describe('goToLatestDownload', () => {
77104
navigateMock.mockReset().mockReturnValue({ status: 'started', settled: Promise.resolve() })
78105
moveCursorMock.mockReset().mockResolvedValue(undefined)
79106
getFocusedPaneMock.mockReset().mockReturnValue('left')
107+
getPaneLocationMock.mockReset().mockReturnValue({ volumeId: 'root', volumePath: '/', path: '/Users/me/elsewhere' })
108+
setFocusedPaneMock.mockReset()
80109
})
81110

82111
it('navigates the focused pane and selects the file on success', async () => {
@@ -97,6 +126,93 @@ describe('goToLatestDownload', () => {
97126
expect(addToastMock).not.toHaveBeenCalled()
98127
})
99128

129+
it('moves the cursor only (no navigate, no focus shift) when the focused pane already shows the dir', async () => {
130+
goToLatestDownloadMock.mockResolvedValue({
131+
status: 'ok',
132+
data: { path: '/Users/me/Downloads/report.pdf', parentDir: '/Users/me/Downloads', fileName: 'report.pdf' },
133+
})
134+
getFocusedPaneMock.mockReturnValue('left')
135+
paneShows('left', '/Users/me/Downloads')
136+
137+
await goToLatestDownload(makeExplorerStub())
138+
139+
expect(navigateMock).not.toHaveBeenCalled()
140+
expect(setFocusedPaneMock).not.toHaveBeenCalled()
141+
expect(moveCursorMock).toHaveBeenCalledWith('left', 'report.pdf')
142+
})
143+
144+
it('shifts focus and moves the cursor (no navigate) when the OTHER pane already shows the dir', async () => {
145+
goToLatestDownloadMock.mockResolvedValue({
146+
status: 'ok',
147+
data: { path: '/Users/me/Downloads/report.pdf', parentDir: '/Users/me/Downloads', fileName: 'report.pdf' },
148+
})
149+
getFocusedPaneMock.mockReturnValue('left')
150+
paneShows('right', '/Users/me/Downloads')
151+
152+
await goToLatestDownload(makeExplorerStub())
153+
154+
expect(navigateMock).not.toHaveBeenCalled()
155+
expect(setFocusedPaneMock).toHaveBeenCalledWith('right')
156+
expect(moveCursorMock).toHaveBeenCalledWith('right', 'report.pdf')
157+
})
158+
159+
it('navigates the focused pane when neither pane shows the dir', async () => {
160+
goToLatestDownloadMock.mockResolvedValue({
161+
status: 'ok',
162+
data: { path: '/Users/me/Downloads/report.pdf', parentDir: '/Users/me/Downloads', fileName: 'report.pdf' },
163+
})
164+
getFocusedPaneMock.mockReturnValue('left')
165+
// Both panes sit elsewhere (the beforeEach default).
166+
167+
await goToLatestDownload(makeExplorerStub())
168+
169+
expect(navigateMock).toHaveBeenCalledWith({ pane: 'left', to: { path: '/Users/me/Downloads' }, source: 'user' })
170+
expect(setFocusedPaneMock).not.toHaveBeenCalled()
171+
expect(moveCursorMock).toHaveBeenCalledWith('left', 'report.pdf')
172+
})
173+
174+
it('does NOT count a pane at an equal-looking path on a virtual or device volume as showing the dir', async () => {
175+
goToLatestDownloadMock.mockResolvedValue({
176+
status: 'ok',
177+
data: { path: '/Users/me/Downloads/report.pdf', parentDir: '/Users/me/Downloads', fileName: 'report.pdf' },
178+
})
179+
getFocusedPaneMock.mockReturnValue('left')
180+
// The other pane's active tab reports the exact same path string, but it's on
181+
// a network volume (its volumePath is `smb://…`, not a real local mount). It
182+
// must not count, so the focused pane navigates as usual.
183+
getPaneLocationMock.mockImplementation((p: 'left' | 'right') =>
184+
p === 'right'
185+
? { volumeId: 'network', volumePath: 'smb://', path: '/Users/me/Downloads' }
186+
: { volumeId: 'root', volumePath: '/', path: '/Users/me/elsewhere' },
187+
)
188+
189+
await goToLatestDownload(makeExplorerStub())
190+
191+
expect(navigateMock).toHaveBeenCalledWith({ pane: 'left', to: { path: '/Users/me/Downloads' }, source: 'user' })
192+
expect(setFocusedPaneMock).not.toHaveBeenCalled()
193+
expect(moveCursorMock).toHaveBeenCalledWith('left', 'report.pdf')
194+
})
195+
196+
it('does NOT count an MTP pane whose path string matches the local dir', async () => {
197+
goToLatestDownloadMock.mockResolvedValue({
198+
status: 'ok',
199+
data: { path: '/Users/me/Downloads/report.pdf', parentDir: '/Users/me/Downloads', fileName: 'report.pdf' },
200+
})
201+
getFocusedPaneMock.mockReturnValue('left')
202+
// MTP device volume: volumePath is an `mtp://…` URL, so the local Downloads
203+
// path is not on it — `isPathOnVolume` rejects the match.
204+
getPaneLocationMock.mockImplementation((p: 'left' | 'right') =>
205+
p === 'right'
206+
? { volumeId: 'mtp-0-1', volumePath: 'mtp://mtp-0-1/65537', path: '/Users/me/Downloads' }
207+
: { volumeId: 'root', volumePath: '/', path: '/Users/me/elsewhere' },
208+
)
209+
210+
await goToLatestDownload(makeExplorerStub())
211+
212+
expect(navigateMock).toHaveBeenCalledWith({ pane: 'left', to: { path: '/Users/me/Downloads' }, source: 'user' })
213+
expect(setFocusedPaneMock).not.toHaveBeenCalled()
214+
})
215+
100216
it('shows the empty INFO toast with the dedup id on GoToLatestError::Empty', async () => {
101217
goToLatestDownloadMock.mockResolvedValue({
102218
status: 'error',
@@ -128,6 +244,27 @@ describe('goToLatestDownload', () => {
128244
expect(moveCursorMock).not.toHaveBeenCalled()
129245
})
130246

247+
it('empty-toast "Go to Downloads" reuses a pane already showing Downloads, evaluated at click time', async () => {
248+
goToLatestDownloadMock.mockResolvedValue({ status: 'error', error: { kind: 'empty' } })
249+
downloadsWatcherStatusMock.mockResolvedValue({
250+
status: 'ok',
251+
data: { running: true, downloadsDir: '/Users/me/Downloads', fdaPending: false },
252+
})
253+
getFocusedPaneMock.mockReturnValue('left')
254+
255+
await goToLatestDownload(makeExplorerStub())
256+
257+
const [, options] = addToastMock.mock.calls[0] as unknown as [unknown, { props?: { onGoToDownloads: () => void } }]
258+
// The other pane navigates to Downloads AFTER the toast was added, so the
259+
// action must re-evaluate which pane shows the dir at CLICK time.
260+
paneShows('right', '/Users/me/Downloads')
261+
options.props?.onGoToDownloads()
262+
263+
// Pane reuse: focus the pane that shows Downloads, no fresh navigation.
264+
expect(navigateMock).not.toHaveBeenCalled()
265+
expect(setFocusedPaneMock).toHaveBeenCalledWith('right')
266+
})
267+
131268
it('shows the FDA INFO toast with the dedup id on GoToLatestError::WatcherUnavailable', async () => {
132269
goToLatestDownloadMock.mockResolvedValue({
133270
status: 'error',
@@ -200,15 +337,28 @@ describe('goToDownload', () => {
200337
navigateMock.mockReset().mockReturnValue({ status: 'started', settled: Promise.resolve() })
201338
moveCursorMock.mockReset().mockResolvedValue(undefined)
202339
getFocusedPaneMock.mockReset().mockReturnValue('left')
340+
getPaneLocationMock.mockReset().mockReturnValue({ volumeId: 'root', volumePath: '/', path: '/Users/me/elsewhere' })
341+
setFocusedPaneMock.mockReset()
203342
})
204343

205-
it('navigates the focused pane to parentDir and selects the file', async () => {
344+
it('navigates the focused pane to parentDir and selects the file when neither pane shows it', async () => {
206345
await goToDownload(makeExplorerStub(), '/Users/me/Downloads', 'report.pdf')
207346

208347
expect(navigateMock).toHaveBeenCalledWith({ pane: 'left', to: { path: '/Users/me/Downloads' }, source: 'user' })
209348
expect(moveCursorMock).toHaveBeenCalledWith('left', 'report.pdf')
210349
})
211350

351+
it('reuses the focused pane (cursor only) when it already shows the dir', async () => {
352+
getFocusedPaneMock.mockReturnValue('left')
353+
paneShows('left', '/Users/me/Downloads')
354+
355+
await goToDownload(makeExplorerStub(), '/Users/me/Downloads', 'report.pdf')
356+
357+
expect(navigateMock).not.toHaveBeenCalled()
358+
expect(setFocusedPaneMock).not.toHaveBeenCalled()
359+
expect(moveCursorMock).toHaveBeenCalledWith('left', 'report.pdf')
360+
})
361+
212362
it('does nothing when the explorer handle is missing (HMR / pre-mount)', async () => {
213363
await goToDownload(undefined, '/Users/me/Downloads', 'report.pdf')
214364

apps/desktop/src/lib/downloads/go-to-latest.ts

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,17 @@
33
* palette, and the `go_to_latest_download` MCP tool).
44
*
55
* Calls the typed backend IPC and branches on the typed `GoToLatestError` enum —
6-
* no string matching. On success, navigates the focused pane to the file's
7-
* parent dir and moves the cursor to the file. On any error, surfaces a
8-
* single INFO toast using a stable dedup id so spamming ⌘J doesn't stack
9-
* copies.
6+
* no string matching. On success, reveals the file in the best pane: if a pane
7+
* already shows the file's parent dir, it reuses (and focuses) that pane instead
8+
* of duplicating the view; otherwise it navigates the focused pane (see
9+
* `revealFileInBestPane`). On any error, surfaces a single INFO toast using a
10+
* stable dedup id so spamming ⌘J doesn't stack copies.
1011
*/
1112

1213
import { commands } from '$lib/ipc/bindings'
1314
import { addToast } from '$lib/ui/toast'
1415
import { getAppLogger } from '$lib/logging/logger'
15-
import { navigateToFileInPane } from '$lib/file-explorer/navigation/navigate-and-select'
16+
import { revealFileInBestPane, navigateToDirInBestPane } from '$lib/file-explorer/navigation/navigate-and-select'
1617
import type { ExplorerAPI } from '../../routes/(main)/explorer-api'
1718

1819
import LatestDownloadEmptyToastContent from './LatestDownloadEmptyToastContent.svelte'
@@ -24,10 +25,12 @@ export { LATEST_DOWNLOAD_EMPTY_TOAST_ID, LATEST_DOWNLOAD_FDA_TOAST_ID }
2425
const log = getAppLogger('downloads')
2526

2627
/**
27-
* Go to the latest download in the focused pane.
28+
* Go to the latest download, reusing a pane that already shows its dir.
2829
*
2930
* Contract:
30-
* - Success: navigate the focused pane to `parentDir`, then select `fileName`.
31+
* - Success: reveal `fileName` in `parentDir` via `revealFileInBestPane` (reuse
32+
* an open pane when one already shows the dir; otherwise navigate the focused
33+
* pane), then select `fileName`.
3134
* - `empty`: INFO toast with a "Go to Downloads" action (resolves the
3235
* Downloads dir via `downloadsWatcherStatus` so the action knows where to go).
3336
* - `watcherUnavailable` / `downloadsDirUnresolved`: INFO toast with an
@@ -44,7 +47,7 @@ export async function goToLatestDownload(explorer: ExplorerAPI | undefined): Pro
4447

4548
const result = await commands.goToLatestDownload()
4649
if (result.status === 'ok') {
47-
await navigateToFileInPane(explorer, explorer.getFocusedPane(), result.data.parentDir, result.data.fileName)
50+
await revealFileInBestPane(explorer, result.data.parentDir, result.data.fileName)
4851
return
4952
}
5053

@@ -62,8 +65,9 @@ export async function goToLatestDownload(explorer: ExplorerAPI | undefined): Pro
6265
}
6366

6467
/**
65-
* Go to a SPECIFIC downloaded file (parent dir + file name) in the focused
66-
* pane, bypassing the latest-in-ring lookup.
68+
* Go to a SPECIFIC downloaded file (parent dir + file name), bypassing the
69+
* latest-in-ring lookup. Reuses a pane that already shows the dir
70+
* (`revealFileInBestPane`) rather than duplicating the view.
6771
*
6872
* `goToLatestDownload` consults the watcher's ring + scan fallback. The
6973
* downloads toast already knows the path it's for; jumping to the
@@ -83,7 +87,7 @@ export async function goToDownload(
8387
log.debug('goToDownload: no explorer; skipping (HMR or pre-mount)')
8488
return
8589
}
86-
await navigateToFileInPane(explorer, explorer.getFocusedPane(), parentDir, fileName)
90+
await revealFileInBestPane(explorer, parentDir, fileName)
8791
}
8892

8993
async function showEmptyToast(explorer: ExplorerAPI): Promise<void> {
@@ -92,18 +96,15 @@ async function showEmptyToast(explorer: ExplorerAPI): Promise<void> {
9296
// the prop closure logs and bails.
9397
const status = await commands.downloadsWatcherStatus()
9498
const downloadsDir = status.status === 'ok' ? status.data.downloadsDir : null
95-
// Snapshot the focused pane + Downloads dir at toast-add time so a later
96-
// pane focus change doesn't redirect the action somewhere unexpected.
97-
const focusedPane = explorer.getFocusedPane()
99+
// Snapshot the Downloads dir at toast-add time (it won't change), but pick the
100+
// target pane at CLICK time: `navigateToDirInBestPane` re-evaluates which pane
101+
// shows the dir then, since pane contents may shift while the toast sits there.
98102
const onGoToDownloads = () => {
99103
if (!downloadsDir) {
100104
log.warn('Go to Downloads pressed but Downloads dir is unresolved; no action')
101105
return
102106
}
103-
const result = explorer.navigate({ pane: focusedPane, to: { path: downloadsDir }, source: 'user' })
104-
if (result.status === 'refused') {
105-
log.warn('Go to Downloads: navigate refused: {reason}', { reason: result.reason.message })
106-
}
107+
void navigateToDirInBestPane(explorer, downloadsDir)
107108
}
108109
addToast(LatestDownloadEmptyToastContent, {
109110
id: LATEST_DOWNLOAD_EMPTY_TOAST_ID,

0 commit comments

Comments
 (0)