Skip to content

Commit 608b8c8

Browse files
committed
Favorites: fix dead drag-reorder, dead ⌥↑/⌥↓ reorder, and rename keystroke leak
Three interactive bugs in the editable-favorites switcher, all found in real-mouse QA, all in `VolumeBreadcrumb.svelte`: - **Drag-to-reorder did nothing.** The reorder was built on HTML5 `draggable`/`ondragstart`/`ondrop`, whose events never fire under Tauri's `dragDropEnabled` (macOS intercepts drag gestures at the OS layer before the WKWebView sees them). Reimplemented as a self-contained pointer-drag: `onmousedown` arms `window` `mousemove`/`mouseup` listeners, a reorder begins only past a small move threshold (below it a mouseup is a plain click → navigate), and the live insertion slot (computed from each row's `getBoundingClientRect` midpoint via the new pure `pointerReorderTarget`) drives the existing drop-line cue. This mirrors why the native file-list drag is `onmousedown`-based, not `draggable`. The grabbed-row `is-dragging` style and reduced-motion are honored; favorite rows route navigation through mouseup (skipping `onclick`) so a click still works. - **Keyboard reorder (⌥↑ / ⌥↓) did nothing.** The dropdown navigates by a virtual `highlightedIndex` and rows aren't DOM-focused, so the per-item `onkeydown` never fired and `handleDropdownKey` ate the bare arrows first. Moved the Alt-arrow handling into the exported `handleKeyDown`, before `handleDropdownKey`, acting on the highlighted favorite and following the moved item so repeated ⌥↓ keeps moving it. Removed the dead per-item keydown path (and the now-unused `tabindex`/`role="button"`/HTML5-drag attrs on the rows). - **Space leaked from the rename textbox to the pane** (selected the file under the cursor). `handleRenameKeyDown` only stopped propagation for Enter/Escape; now it `stopPropagation()`s every key, since the focused rename `<input>` owns its keystrokes and the pane's raw Space-selection DOM listener isn't covered by the dispatch-level guard. Tests: extended the pure `favorites-reorder` helper tests for `pointerReorderTarget`, added `handleKeyDown` Alt-arrow reorder cases (move / top-edge no-op / non-favorite ignored), a rename-input all-keys-stop-propagation test, and updated the pre-existing `pane/volume-breadcrumb.test.ts` favorite tests to the pointer-based shape. Docs: rewrote the `navigation/CLAUDE.md` reorder must-know and `DETAILS.md` to record the Tauri/WKWebView HTML5-DnD-is-dead gotcha so nobody reintroduces HTML5 drag.
1 parent d3db386 commit 608b8c8

7 files changed

Lines changed: 342 additions & 102 deletions

File tree

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

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,19 @@ 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.
43+
- **The favorite-rename `<input>` must not leak keystrokes to the panes.** Four guards work together; don't remove any:
44+
`VolumeBreadcrumb.handleRenameKeyDown` calls `e.stopPropagation()` for EVERY key (the focused input owns its
45+
keystrokes; the pane's Space-selection DOM listener isn't covered by the dispatch-level guard, so without this a Space
46+
typed into the box also selects the file under the cursor); `VolumeBreadcrumb.handleKeyDown` bails while
47+
`renamingFavoriteId !== null`; `DualPaneExplorer.routeToVolumeChooser` swallows keys from the pane behind ANY open
48+
switcher dropdown (returns true even when the dropdown ignores the key); and `+page.svelte`'s `isModalDialogOpen()`
49+
reads `explorerRef.isVolumeChooserOpen()` to suppress centralized dispatch.
50+
- **Favorite reorder is POINTER-based, not HTML5 drag.** HTML5 `draggable`/`ondragstart`/`ondrop` never fire under
51+
Tauri's `dragDropEnabled` (the OS intercepts drag gestures before the WKWebView sees them), so the reorder uses
52+
`onmousedown` + window `mousemove`/`mouseup` listeners with a small move threshold (below it, a mouseup is a plain
53+
click → navigate). Don't reintroduce HTML5 drag here. Keyboard reorder (Alt+↑ / Alt+↓) lives in the exported
54+
`handleKeyDown` and acts on the virtual `highlightedIndex` (the rows aren't DOM-focused), so it must run before
55+
`handleDropdownKey` consumes the bare arrows. Both paths persist the FULL order via `reorderFavorites(bareIds)` using
56+
the pure `favorites-reorder.ts` helpers.
4857

4958
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: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -228,20 +228,35 @@ the bare id, not the `fav-…` switcher id).
228228
path).
229229
- **Remove / Rename** are per-item, on the existing dropdown `row-menu` (right-click a favorite). Rename swaps the label
230230
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.
238-
- **Reorder** is drag-to-reorder within the section (`draggingFavoriteId` / `dragOverFavoriteId` drive the drop-line
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.
231+
`handleRenameKeyDown` calls `e.stopPropagation()` on EVERY key: the focused input owns its keystrokes, and the pane's
232+
Space-selection / type-to-jump DOM listeners aren't covered by the dispatch-level guard, so a leaked Space would
233+
select the file under the cursor while the user types into the box. Enter commits, Escape cancels, everything else
234+
edits the text. While a rename is active, `VolumeBreadcrumb.handleKeyDown` also bails (`renamingFavoriteId !== null`)
235+
so the dropdown's list-nav keys (arrows / Home / End) don't steal them. The broader keystroke-leak guard lives one
236+
level up: while ANY pane's switcher dropdown is open, `DualPaneExplorer.routeToVolumeChooser` swallows the key from
237+
the pane behind it (returns true even when the dropdown ignores the key), and `+page.svelte`'s `isModalDialogOpen()`
238+
reads `explorerRef.isVolumeChooserOpen()` to suppress centralized webview-keydown dispatch.
239+
- **Reorder** is pointer-drag within the section AND keyboard (Option+Up / Option+Down, since the app is keyboard-first;
240+
the row tooltip reads `⌥↑ / ⌥↓` on macOS, `Alt+↑ / Alt+↓` elsewhere, built by the pure `favorite-tooltip.ts`).
241+
- **Pointer drag** uses `onmousedown` on the row + `window` `mousemove`/`mouseup` listeners (armed on mousedown,
242+
removed on mouseup and `onDestroy`), NOT HTML5 drag-and-drop. A reorder begins only once the pointer moves past a
243+
small threshold (`DRAG_THRESHOLD_PX`); below it, a mouseup is a plain click that navigates. So favorite rows skip
244+
the `onclick` navigate path (it would double-fire with the mouseup) and `handleVolumeSelect` runs from mouseup
245+
instead. During the drag, `favoriteRowMidpoints()` + the pure `pointerReorderTarget()` compute the live insertion
246+
slot (`dragOverIndex`) that drives the `is-drag-over` drop-line cue; the grabbed row carries `is-dragging`.
247+
- **Why pointer and not HTML5 DnD:** under Tauri's `dragDropEnabled` (on by default), macOS intercepts drag gestures
248+
at the OS layer before the WKWebView sees `dragstart`/`dragover`/`drop`, so an HTML5-`draggable` reorder silently
249+
never fires (the events don't arrive). This is the same reason the native file-list drag (`views/FullList.svelte`)
250+
is `onmousedown`-based, not `draggable`. Don't reintroduce HTML5 drag here; it'll look wired-up and do nothing.
251+
Synthetic MCP/test events bypass the OS interception, so "it works under MCP" is not proof it works with a real
252+
mouse.
253+
- **Keyboard** (Alt+↑ / Alt+↓) is handled in the exported `handleKeyDown`, BEFORE `handleDropdownKey` consumes the
254+
bare arrows, and acts on the highlighted favorite (`allVolumes[highlightedIndex]`) since the rows aren't DOM-focused
255+
(the dropdown navigates by a virtual `highlightedIndex`). After persisting, it moves the highlight to follow the
256+
moved favorite so repeated Alt+↓ keeps moving the same item.
257+
- Both paths compute the new order with the pure `favorites-reorder.ts` helpers (`moveItem`, `clampedReorderTarget`,
258+
`pointerReorderTarget`) and persist the FULL order via `reorderFavorites(bareIds)`. The favorite row's tooltip leads
259+
with the PATH (then the reorder hint) so a renamed favorite still reveals where it points.
245260
- **Empty state** is a real state (the user can remove every favorite). The `favorite` group in `volume-grouping.ts`
246261
always renders (unlike every other group, which hides when empty), and the switcher shows a single disabled,
247262
non-focusable placeholder row: "(Your favorites will show here)".

0 commit comments

Comments
 (0)