Skip to content

Commit d3db386

Browse files
committed
Favorites polish: reachable ext-sort, fixed rename keystroke leak, clearer shortcuts and tooltips
Polishes the just-shipped editable-favorites + show-extension features from David's QA pass: - **Sort-by-extension stays clickable when "show extension in name" is on.** Previously turning the setting on hid the Ext column AND its header, so the only sort-by-ext target vanished. Now the single Name-column header splits into two `SortableHeader` triggers in a `.header-name-ext` row: "Name" fills the space, "Ext" sits right-aligned shrunk to its label; both sort, each shows its caret when active. Data cells are unchanged (full filename in Name, no Ext cell), so the renderer stays in lockstep with the measurer (`ext: 0`). - **"Add to favorites" moves from ⌘D to ⌘⇧D** so it stops clashing with the dev-only "Open Debug window" (⌘D, non-configurable). Updated the registry default and the native-menu accelerator literals (`macos.rs` / `linux.rs`). - **Renaming a favorite no longer leaks keystrokes to the panes.** While the volume switcher dropdown is open it now owns keyboard focus: `+page.svelte`'s `isModalDialogOpen()` reads a new `explorerRef.isVolumeChooserOpen()` to suppress centralized dispatch, `DualPaneExplorer.routeToVolumeChooser` swallows keys from the pane behind it, and `VolumeBreadcrumb.handleKeyDown` bails during an active rename. So ⌘A, ⌥←/→, Backspace, arrows, etc. edit the rename textbox instead of moving the pane cursor or navigating. - **Keyboard-reorder hint reads ⌥↑ / ⌥↓** (macOS has no Alt key; Linux keeps "Alt+↑ / Alt+↓"), via the new pure `favorite-tooltip.ts`. - **The favorite row tooltip leads with the PATH**, so a renamed favorite ("Documents" → "Docs") still reveals where it points. The restricted-path tooltip variant is unchanged. - Drag-to-reorder was verified working end to end in the running app (cue + reorder + persist); no code change was needed there. Tests: pure `buildFavoriteTooltip` (path-first + platform fork), the combined Name+Ext header render + ext-sort caret + no-Ext-cell, and the favorite-rename keyboard guard. All verified live via the Tauri MCP bridge.
1 parent dbf4d70 commit d3db386

16 files changed

Lines changed: 461 additions & 32 deletions

apps/desktop/src-tauri/src/menu/linux.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ pub(crate) fn build_menu_linux<R: Runtime>(
273273
true,
274274
Some("Cmd+J"),
275275
)?;
276-
let favorites_add_item = MenuItem::with_id(app, FAVORITES_ADD_ID, "&Add to favorites", true, Some("Cmd+D"))?;
276+
let favorites_add_item = MenuItem::with_id(app, FAVORITES_ADD_ID, "&Add to favorites", true, Some("Cmd+Shift+D"))?;
277277

278278
let go_menu = Submenu::with_items(
279279
app,

apps/desktop/src-tauri/src/menu/macos.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ pub(crate) fn build_menu_macos<R: Runtime>(
296296
let go_to_path_item = MenuItem::with_id(app, GO_TO_PATH_ID, "Go to path\u{2026}", true, Some("Cmd+G"))?;
297297
let go_latest_download_item =
298298
MenuItem::with_id(app, GO_LATEST_DOWNLOAD_ID, "Go to latest download", true, Some("Cmd+J"))?;
299-
let favorites_add_item = MenuItem::with_id(app, FAVORITES_ADD_ID, "Add to favorites", true, Some("Cmd+D"))?;
299+
let favorites_add_item = MenuItem::with_id(app, FAVORITES_ADD_ID, "Add to favorites", true, Some("Cmd+Shift+D"))?;
300300

301301
let go_menu = Submenu::with_items(
302302
app,

apps/desktop/src-tauri/src/menu/mod.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,8 +153,9 @@ pub const GO_TO_PATH_ID: &str = "go_to_path";
153153
/// "Go to latest download" (⌘J): jumps the focused pane to the most recent download.
154154
pub const GO_LATEST_DOWNLOAD_ID: &str = "go_latest_download";
155155

156-
/// "Add to favorites" (⌘D), menu bar + palette: maps to the `favorites.add` command, which favorites
157-
/// the focused pane's current folder.
156+
/// "Add to favorites" (⌘⇧D), menu bar + palette: maps to the `favorites.add` command, which favorites
157+
/// the focused pane's current folder. ⌘⇧D, not ⌘D, because dev builds bind ⌘D to "Open Debug window"
158+
/// (non-configurable) and the two would clash.
158159
pub const FAVORITES_ADD_ID: &str = "favorites_add";
159160

160161
/// "Add to favorites", folder-row + parent-row CONTEXT menus: favorites `MenuState.context.path`

apps/desktop/src/lib/commands/command-registry.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ export const commands: Command[] = [
156156
name: 'Add to favorites',
157157
scope: 'Main window',
158158
showInPalette: true,
159-
shortcuts: ['⌘D'],
159+
shortcuts: ['⌘D'],
160160
description: "Add the focused pane's current folder to the switcher's Favorites.",
161161
keywords: ['bookmark', 'favorite', 'pin', 'shortcut'],
162162
},

apps/desktop/src/lib/file-explorer/navigation/CLAUDE.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,5 +40,10 @@ Browser-style back/forward history, path resolution, paged keyboard shortcuts, a
4040
`volume-grouping.ts` always renders (even empty, for the placeholder) — don't "tidy" it back into the hide-when-empty
4141
branch. Context-menu "Add to favorites" is handled in Rust (`FAVORITES_ADD_CONTEXT_ID`), not the `favorites.add`
4242
command. Full flow in [DETAILS.md](DETAILS.md) § Editable favorites.
43+
- **The favorite-rename `<input>` must not leak keystrokes to the panes.** Three guards work together; don't remove any:
44+
`VolumeBreadcrumb.handleKeyDown` bails while `renamingFavoriteId !== null`; `DualPaneExplorer.routeToVolumeChooser`
45+
swallows keys from the pane behind ANY open switcher dropdown (returns true even when the dropdown ignores the key);
46+
and `+page.svelte`'s `isModalDialogOpen()` reads `explorerRef.isVolumeChooserOpen()` to suppress centralized dispatch.
47+
The favorite reorder needs BOTH `dragover` `preventDefault()` and an `ondrop` handler, or the drop silently no-ops.
4348

4449
Architecture, flows, and decision detail: [DETAILS.md](DETAILS.md). Read it in whole before structural changes here.

apps/desktop/src/lib/file-explorer/navigation/DETAILS.md

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -218,18 +218,30 @@ of truth; see `src-tauri/src/favorites/CLAUDE.md`). All mutations go through the
218218
refresh. `stripFavoritePrefix(locationId)` recovers the bare favorite id (the remove / rename / reorder commands take
219219
the bare id, not the `fav-…` switcher id).
220220

221-
- **Add** has three surfaces: the `favorites.add` command (palette, the Go menu's "Add to favorites", ⌘D) favorites the
222-
focused pane's current dir (handler in `routes/(main)/command-handlers/misc-handlers.ts`); the folder-row and `..`
223-
context menus favorite a SPECIFIC path. The context-menu add is handled entirely in Rust (`menu/menu_handlers.rs`
224-
intercepts `FAVORITES_ADD_CONTEXT_ID` and favorites `MenuState.context.path`), so it never routes through
225-
`favorites.add` (which would favorite the wrong dir). The folder-row item lives in `build_context_menu` (directories
226-
only, not on search-results panes); the `..` row gets its own one-item menu via `show_parent_row_context_menu`
227-
(`FilePane.handleContextMenu` calls it with the parent dir path).
221+
- **Add** has three surfaces: the `favorites.add` command (palette, the Go menu's "Add to favorites", ⌘⇧D — ⌘D is the
222+
dev-only Debug window) favorites the focused pane's current dir (handler in
223+
`routes/(main)/command-handlers/misc-handlers.ts`); the folder-row and `..` context menus favorite a SPECIFIC path.
224+
The context-menu add is handled entirely in Rust (`menu/menu_handlers.rs` intercepts `FAVORITES_ADD_CONTEXT_ID` and
225+
favorites `MenuState.context.path`), so it never routes through `favorites.add` (which would favorite the wrong dir).
226+
The folder-row item lives in `build_context_menu` (directories only, not on search-results panes); the `..` row gets
227+
its own one-item menu via `show_parent_row_context_menu` (`FilePane.handleContextMenu` calls it with the parent dir
228+
path).
228229
- **Remove / Rename** are per-item, on the existing dropdown `row-menu` (right-click a favorite). Rename swaps the label
229230
for an inline `<input>` (Enter commits, Escape/blur cancels). Both strip the `fav-` prefix before calling the command.
231+
While a rename is active, `VolumeBreadcrumb.handleKeyDown` bails (`renamingFavoriteId !== null`) so the dropdown's
232+
list-nav keys (arrows / Home / End) don't steal them from the textbox; Enter/Escape never reach it (the input's own
233+
handler stops propagation). The broader keystroke-leak guard lives one level up: while ANY pane's switcher dropdown is
234+
open, `DualPaneExplorer.routeToVolumeChooser` swallows the key from the pane behind it (returns true even when the
235+
dropdown ignores the key), and `+page.svelte`'s `isModalDialogOpen()` reads `explorerRef.isVolumeChooserOpen()` to
236+
suppress centralized webview-keydown dispatch. So ⌘A, ⌥←/→, Backspace, etc. edit the rename textbox instead of acting
237+
on the pane.
230238
- **Reorder** is drag-to-reorder within the section (`draggingFavoriteId` / `dragOverFavoriteId` drive the drop-line
231-
cue) AND keyboard (Alt+Up / Alt+Down on a focused favorite, since the app is keyboard-first). Both compute the new
232-
order with the pure `favorites-reorder.ts` helpers and persist the FULL order via `reorderFavorites(barelIds)`.
239+
cue) AND keyboard (Option+Up / Option+Down on a focused favorite, since the app is keyboard-first; the row tooltip
240+
reads `⌥↑ / ⌥↓` on macOS, `Alt+↑ / Alt+↓` elsewhere, built by the pure `favorite-tooltip.ts`). HTML5 DnD needs BOTH
241+
`preventDefault()` on `dragover` (to mark a valid drop target, so the cue shows) AND an `ondrop` handler; keep both or
242+
the drop silently no-ops. Both paths compute the new order with the pure `favorites-reorder.ts` helpers and persist
243+
the FULL order via `reorderFavorites(bareIds)`. The favorite row's tooltip leads with the PATH (then the reorder hint)
244+
so a renamed favorite still reveals where it points.
233245
- **Empty state** is a real state (the user can remove every favorite). The `favorite` group in `volume-grouping.ts`
234246
always renders (unlike every other group, which hides when empty), and the switcher shows a single disabled,
235247
non-focusable placeholder row: "(Your favorites will show here)".

apps/desktop/src/lib/file-explorer/navigation/VolumeBreadcrumb.svelte

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,13 @@
3030
import { tooltip } from '$lib/tooltip/tooltip'
3131
import { getCachedIcon, iconCacheVersion, prefetchIcons } from '$lib/icon-cache'
3232
import { isRestricted } from '$lib/stores/restricted-paths-store.svelte'
33+
import { isMacOS } from '$lib/shortcuts/key-capture'
3334
import InfoIcon from '~icons/lucide/info'
3435
import { describeUsbSpeed, type VolumeInfo } from '../types'
3536
import { isVolumeEjectable } from './eject-predicate'
37+
import { buildFavoriteTooltip } from './favorite-tooltip'
38+
39+
const favoriteTooltip = (volume: VolumeInfo): string => buildFavoriteTooltip(volume.path, isMacOS())
3640
3741
/** "USB 3.2 Gen 1 (Max. 625 MB/s)" - shared between the chip tooltip and the dropdown subline. */
3842
function usbSpeedDisplay(volume: VolumeInfo | undefined): string {
@@ -268,6 +272,13 @@
268272
export function handleKeyDown(e: KeyboardEvent): boolean {
269273
if (!isOpen) return false
270274
275+
// While renaming a favorite, the inline `<input>` owns every key: arrows,
276+
// Home/End, etc. move the text cursor, not the dropdown highlight. Bail so
277+
// the dropdown's list navigation doesn't steal them from the textbox. Enter
278+
// / Escape never reach here (the input's own handler stops propagation), so
279+
// commit / cancel still work.
280+
if (renamingFavoriteId !== null) return false
281+
271282
const submenuResult = handleSubmenuKey(e.key, {
272283
isOpen: () => submenu.volumeId !== null,
273284
close: () => { submenu.close(); },
@@ -795,7 +806,7 @@
795806
use:tooltip={isRestricted(volume.path)
796807
? RESTRICTED_FOLDER_TOOLTIP
797808
: isFavorite
798-
? 'Drag to reorder, or Alt+Up / Alt+Down. Right-click to rename or remove.'
809+
? favoriteTooltip(volume)
799810
: ''}
800811
onclick={() => {
801812
if (renamingFavoriteId === volume.id) return
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
/**
2+
* Behavioral tests for `VolumeBreadcrumb.svelte` focused on the favorite-rename
3+
* keyboard guard (Fix E): while a favorite is being renamed inline, the dropdown
4+
* must NOT consume arrow / Home / End / Enter keys, so the textbox keeps them and
5+
* the panes behind the dropdown stay inert. The cross-pane suppression itself
6+
* lives in `DualPaneExplorer.routeToVolumeChooser`; here we pin the leaf guard.
7+
*/
8+
9+
import { describe, it, expect, vi, beforeEach } from 'vitest'
10+
import { mount, tick, flushSync } from 'svelte'
11+
import VolumeBreadcrumb from './VolumeBreadcrumb.svelte'
12+
13+
vi.mock('$lib/tauri-commands', () => ({
14+
resolvePathVolume: vi.fn(() => Promise.resolve({ volume: { id: 'root', path: '/' } })),
15+
upgradeToSmbVolume: vi.fn(() => Promise.resolve({ status: 'success' })),
16+
ejectVolume: vi.fn(() => Promise.resolve()),
17+
getIpcErrorMessage: (e: unknown) => String(e),
18+
getVolumeSpace: vi.fn(() => Promise.resolve(null)),
19+
systemHasSavedSmbPassword: vi.fn(() => Promise.resolve(false)),
20+
upgradeToSmbVolumeUsingSavedPassword: vi.fn(() => Promise.resolve({ status: 'success' })),
21+
removeFavorite: vi.fn(() => Promise.resolve()),
22+
renameFavorite: vi.fn(() => Promise.resolve()),
23+
reorderFavorites: vi.fn(() => Promise.resolve()),
24+
stripFavoritePrefix: (id: string) => (id.startsWith('fav-') ? id.slice(4) : id),
25+
}))
26+
27+
vi.mock('$lib/stores/volume-store.svelte', () => ({
28+
getVolumes: () => [
29+
{ id: 'root', name: 'Macintosh HD', path: '/', category: 'main_volume', isEjectable: false },
30+
{ id: 'fav-1', name: 'Documents', path: '/Users/test/Documents', category: 'favorite', isEjectable: false },
31+
],
32+
getVolumesTimedOut: () => false,
33+
isVolumesRefreshing: () => false,
34+
isVolumeRetryFailed: () => false,
35+
requestVolumeRefresh: vi.fn(),
36+
}))
37+
38+
vi.mock('$lib/stores/volume-busy-store.svelte', () => ({ isVolumeBusy: () => false }))
39+
40+
vi.mock('$lib/ui/toast', () => ({ addToast: vi.fn(() => 'toast-id'), dismissToast: vi.fn() }))
41+
42+
vi.mock('$lib/settings/reactive-settings.svelte', () => ({
43+
formatFileSize: (n: number) => `${String(n)} B`,
44+
getFileSizeFormat: () => 'binary',
45+
getFileSizeUnit: () => 'bytes',
46+
getNetworkEnabled: () => true,
47+
getUseAppIconsForDocuments: () => false,
48+
}))
49+
50+
vi.mock('$lib/icon-cache', async () => {
51+
const { writable } = await import('svelte/store')
52+
return {
53+
getCachedIcon: vi.fn().mockReturnValue('/icons/dir.png'),
54+
iconCacheVersion: writable(0),
55+
prefetchIcons: vi.fn().mockResolvedValue(undefined),
56+
}
57+
})
58+
59+
interface BreadcrumbInstance {
60+
open: () => void
61+
getIsOpen: () => boolean
62+
handleKeyDown: (e: KeyboardEvent) => boolean
63+
}
64+
65+
function mountBreadcrumb(): { instance: BreadcrumbInstance; target: HTMLDivElement } {
66+
const target = document.createElement('div')
67+
document.body.appendChild(target)
68+
const instance = mount(VolumeBreadcrumb, {
69+
target,
70+
props: { volumeId: 'root', currentPath: '/Users/test' },
71+
}) as unknown as BreadcrumbInstance
72+
return { instance, target }
73+
}
74+
75+
describe('VolumeBreadcrumb favorite-rename keyboard guard', () => {
76+
beforeEach(() => {
77+
document.body.innerHTML = ''
78+
})
79+
80+
it('handleKeyDown returns false when the dropdown is closed', () => {
81+
const { instance } = mountBreadcrumb()
82+
expect(instance.handleKeyDown(new KeyboardEvent('keydown', { key: 'ArrowDown' }))).toBe(false)
83+
})
84+
85+
it('consumes ArrowDown when the dropdown is open and not renaming', async () => {
86+
const { instance } = mountBreadcrumb()
87+
instance.open()
88+
await tick()
89+
flushSync()
90+
expect(instance.getIsOpen()).toBe(true)
91+
expect(instance.handleKeyDown(new KeyboardEvent('keydown', { key: 'ArrowDown' }))).toBe(true)
92+
})
93+
94+
it('does NOT consume ArrowDown / Home / End while a favorite rename is active', async () => {
95+
const { instance, target } = mountBreadcrumb()
96+
instance.open()
97+
await tick()
98+
flushSync()
99+
100+
// Start the inline rename: right-click the favorite row, then click "Rename".
101+
const favRow = target.querySelector('.favorite-item') as HTMLElement
102+
expect(favRow).toBeTruthy()
103+
favRow.dispatchEvent(new MouseEvent('contextmenu', { bubbles: true, cancelable: true }))
104+
await tick()
105+
flushSync()
106+
const renameItem = [...target.querySelectorAll('.row-menu-item')].find(
107+
(el) => el.textContent.trim() === 'Rename',
108+
) as HTMLElement
109+
expect(renameItem).toBeTruthy()
110+
renameItem.click()
111+
await tick()
112+
flushSync()
113+
114+
expect(target.querySelector('.favorite-rename-input')).toBeTruthy()
115+
116+
// The guard: keys the dropdown would otherwise eat must fall through (false)
117+
// so the rename textbox keeps them.
118+
for (const key of ['ArrowDown', 'ArrowUp', 'Home', 'End']) {
119+
expect(instance.handleKeyDown(new KeyboardEvent('keydown', { key }))).toBe(false)
120+
}
121+
})
122+
})
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import { describe, it, expect } from 'vitest'
2+
import { buildFavoriteTooltip } from './favorite-tooltip'
3+
4+
describe('buildFavoriteTooltip', () => {
5+
it('leads with the path so a renamed favorite still reveals where it points', () => {
6+
const tip = buildFavoriteTooltip('/Users/dave/Documents', true)
7+
expect(tip.startsWith('/Users/dave/Documents\n')).toBe(true)
8+
})
9+
10+
it('uses the Option symbol and arrow glyphs on macOS (no "Alt")', () => {
11+
const tip = buildFavoriteTooltip('/Users/dave/Documents', true)
12+
expect(tip).toContain('⌥↑ / ⌥↓')
13+
expect(tip).not.toContain('Alt')
14+
})
15+
16+
it('spells out "Alt" on non-macOS platforms', () => {
17+
const tip = buildFavoriteTooltip('/home/dave/docs', false)
18+
expect(tip).toContain('Alt+↑ / Alt+↓')
19+
expect(tip).not.toContain('⌥')
20+
})
21+
22+
it('keeps the right-click hint on both platforms', () => {
23+
for (const isMac of [true, false]) {
24+
expect(buildFavoriteTooltip('/p', isMac)).toContain('Right-click to rename or remove.')
25+
}
26+
})
27+
28+
it('puts the path on its own first line (pre-line break)', () => {
29+
const [first] = buildFavoriteTooltip('/Applications', true).split('\n')
30+
expect(first).toBe('/Applications')
31+
})
32+
})
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/**
2+
* Pure builder for a favorite row's hover tooltip in the volume switcher. Kept
3+
* out of `VolumeBreadcrumb.svelte` so the path-first ordering and the
4+
* platform-forked reorder hint are unit-testable without a DOM.
5+
*/
6+
7+
/**
8+
* Tooltip for a favorite row. Leads with the PATH so a renamed favorite
9+
* ("Documents" → "Docs") still reveals where it points, then the
10+
* keyboard-reorder + context hints. The global tooltip CSS renders with
11+
* `white-space: pre-line`, so the `\n` becomes a real line break.
12+
*
13+
* macOS has no Alt key, so the reorder hint reads `⌥↑ / ⌥↓` (Option symbol +
14+
* arrow glyphs); other platforms spell out `Alt+↑ / Alt+↓`.
15+
*/
16+
export function buildFavoriteTooltip(path: string, isMac: boolean): string {
17+
const reorder = isMac ? '⌥↑ / ⌥↓' : 'Alt+↑ / Alt+↓'
18+
return `${path}\nDrag to reorder, or ${reorder}. Right-click to rename or remove.`
19+
}

0 commit comments

Comments
 (0)