Skip to content

Commit 64db140

Browse files
committed
Drag&drop: Match Finder for the drop operation
- Default operation is now volume-aware, matching Finder: same volume → Move, cross-volume → Copy. Previously it was always Copy, which felt inconsistent with the outgoing-to-Desktop drag (where macOS picks Move). - Modifier overrides: Alt forces Copy (beats Cmd/Shift); Cmd forces Move (Finder); Shift also forces Move (Windows convention, kept as a friendly accelerator). - Same `pickDropOperation` feeds both the live overlay label and the actual drop, so they can no longer disagree. - Native side now reads Alt + Cmd + Shift bits from `[NSEvent modifierFlags]` and emits all three on the `drag-modifiers` event. - Decision and rationale recorded in `drag/CLAUDE.md`.
1 parent 450363e commit 64db140

6 files changed

Lines changed: 307 additions & 44 deletions

File tree

apps/desktop/src-tauri/src/drag_image_detection.rs

Lines changed: 41 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,16 @@
77
//!
88
//! Events emitted:
99
//! - `drag-image-size` `{ width, height }` — on drag enter
10-
//! - `drag-modifiers` `{ altHeld }` — on drag enter and every drag update (only when changed)
10+
//! - `drag-modifiers` `{ altHeld, cmdHeld, shiftHeld }` — on drag enter and every drag update (only when changed)
1111
//!
1212
//! ## Resilience
1313
//!
1414
//! All native API calls are guarded against method removal. The webview class is discovered
1515
//! from the actual webview instance (not a hardcoded name), so wry version changes are safe.
1616
//! If macOS deprecates APIs we rely on, the swizzle degrades gracefully:
1717
//! - Drag image detection disabled → the DOM overlay is always shown (redundant but functional)
18-
//! - Modifier key detection disabled → falls back to JS keydown/keyup (works when webview has focus)
18+
//! - Modifier key detection disabled → falls back to JS keydown/keyup (works when webview has focus,
19+
//! but not during OS-level drags initiated outside the window)
1920
//! - Image swapping disabled → self-drags show the OS drag image over the window (functional)
2021
//!
2122
//! Rust panics inside swizzled functions are caught via `catch_unwind` to prevent crashes
@@ -35,28 +36,38 @@ use tauri::{AppHandle, Emitter, Manager};
3536

3637
use crate::drag_image_swap;
3738

39+
/// NSEventModifierFlagShift = 1 << 17
40+
const NS_EVENT_MODIFIER_FLAG_SHIFT: usize = 1 << 17;
3841
/// NSEventModifierFlagOption = 1 << 19 (Option/Alt key)
3942
const NS_EVENT_MODIFIER_FLAG_OPTION: usize = 1 << 19;
43+
/// NSEventModifierFlagCommand = 1 << 20
44+
const NS_EVENT_MODIFIER_FLAG_COMMAND: usize = 1 << 20;
4045

4146
#[derive(Clone, Serialize)]
4247
struct DragImageSize {
4348
width: f64,
4449
height: f64,
4550
}
4651

47-
#[derive(Clone, Serialize)]
52+
#[derive(Clone, Copy, PartialEq, Eq, Serialize)]
4853
struct DragModifiers {
4954
#[serde(rename = "altHeld")]
5055
alt_held: bool,
56+
#[serde(rename = "cmdHeld")]
57+
cmd_held: bool,
58+
#[serde(rename = "shiftHeld")]
59+
shift_held: bool,
5160
}
5261

5362
static ORIGINAL_ENTERED_IMP: OnceLock<Imp> = OnceLock::new();
5463
static ORIGINAL_UPDATED_IMP: OnceLock<Imp> = OnceLock::new();
5564
static ORIGINAL_EXITED_IMP: OnceLock<Imp> = OnceLock::new();
5665
static APP_HANDLE: OnceLock<AppHandle> = OnceLock::new();
5766

58-
/// Tracks previous alt state so we only emit `drag-modifiers` when it changes.
67+
/// Tracks previous modifier state so we only emit `drag-modifiers` when something changes.
5968
static LAST_ALT_HELD: AtomicBool = AtomicBool::new(false);
69+
static LAST_CMD_HELD: AtomicBool = AtomicBool::new(false);
70+
static LAST_SHIFT_HELD: AtomicBool = AtomicBool::new(false);
6071

6172
// Warn-once flags to prevent log spam for issues that recur on every drag event.
6273
static WARNED_NSEVENT_MISSING: AtomicBool = AtomicBool::new(false);
@@ -178,41 +189,53 @@ unsafe fn install_swizzles(webview_ptr: *mut std::ffi::c_void) {
178189

179190
// --- Modifier key detection ---
180191

181-
/// Reads the current Option/Alt key state from `[NSEvent modifierFlags]`.
192+
/// Reads the current Alt/Cmd/Shift modifier state from `[NSEvent modifierFlags]`.
182193
/// This is a class method that reads hardware state — works even when the webview isn't focused.
183-
/// Returns `false` if NSEvent can't be found (graceful degradation).
184-
fn is_option_held() -> bool {
194+
/// Returns all flags as `false` if NSEvent can't be found (graceful degradation).
195+
fn read_modifiers() -> DragModifiers {
185196
let Some(cls) = AnyClass::get(c"NSEvent") else {
186197
warn_once(
187198
&WARNED_NSEVENT_MISSING,
188-
"drag_image_detection: NSEvent class not found — Alt/Option detection during drags \
199+
"drag_image_detection: NSEvent class not found — modifier key detection during drags \
189200
is disabled. This is a core AppKit class and shouldn't disappear; if it did, check \
190201
whether macOS moved it to a different framework or renamed it. Modifier detection \
191202
falls back to JS keydown/keyup, which doesn't work during OS-level drags.",
192203
);
193-
return false;
204+
return DragModifiers {
205+
alt_held: false,
206+
cmd_held: false,
207+
shift_held: false,
208+
};
194209
};
195210
let flags: usize = unsafe { msg_send![cls, modifierFlags] };
196-
flags & NS_EVENT_MODIFIER_FLAG_OPTION != 0
211+
DragModifiers {
212+
alt_held: flags & NS_EVENT_MODIFIER_FLAG_OPTION != 0,
213+
cmd_held: flags & NS_EVENT_MODIFIER_FLAG_COMMAND != 0,
214+
shift_held: flags & NS_EVENT_MODIFIER_FLAG_SHIFT != 0,
215+
}
197216
}
198217

199-
/// Emits `drag-modifiers` if the alt state changed since last emission.
218+
/// Emits `drag-modifiers` if any tracked modifier changed since last emission.
200219
fn emit_modifiers_if_changed() {
201-
let alt_held = is_option_held();
202-
let prev = LAST_ALT_HELD.swap(alt_held, Ordering::Relaxed);
203-
if alt_held != prev
220+
let mods = read_modifiers();
221+
let prev_alt = LAST_ALT_HELD.swap(mods.alt_held, Ordering::Relaxed);
222+
let prev_cmd = LAST_CMD_HELD.swap(mods.cmd_held, Ordering::Relaxed);
223+
let prev_shift = LAST_SHIFT_HELD.swap(mods.shift_held, Ordering::Relaxed);
224+
if (mods.alt_held != prev_alt || mods.cmd_held != prev_cmd || mods.shift_held != prev_shift)
204225
&& let Some(app_handle) = APP_HANDLE.get()
205226
{
206-
let _ = app_handle.emit("drag-modifiers", DragModifiers { alt_held });
227+
let _ = app_handle.emit("drag-modifiers", mods);
207228
}
208229
}
209230

210231
/// Emits `drag-modifiers` unconditionally (used on drag enter to set initial state).
211232
fn emit_modifiers_forced() {
212-
let alt_held = is_option_held();
213-
LAST_ALT_HELD.store(alt_held, Ordering::Relaxed);
233+
let mods = read_modifiers();
234+
LAST_ALT_HELD.store(mods.alt_held, Ordering::Relaxed);
235+
LAST_CMD_HELD.store(mods.cmd_held, Ordering::Relaxed);
236+
LAST_SHIFT_HELD.store(mods.shift_held, Ordering::Relaxed);
214237
if let Some(app_handle) = APP_HANDLE.get() {
215-
let _ = app_handle.emit("drag-modifiers", DragModifiers { alt_held });
238+
let _ = app_handle.emit("drag-modifiers", mods);
216239
}
217240
}
218241

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

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,11 @@ Key files:
4646
- `drop-target-hit-testing.ts` — Pure logic: `document.elementFromPoint()` + `data-drop-target-path` walk
4747
- `drop-target-validation.ts` — Pure logic: blocks drops onto the source itself or into a descendant
4848
- `DragOverlay.svelte` + `drag-overlay.svelte.ts` — Floating label near cursor
49-
- `../modifier-key-tracker.svelte.ts` — Alt/Option state (DragDropEvent doesn't include modifiers; lives in parent
49+
- `../modifier-key-tracker.svelte.ts` — Alt/Cmd/Shift state (DragDropEvent doesn't include modifiers; lives in parent
5050
`file-explorer/` directory)
51+
- `drop-operation.ts` — Pure logic: resolves the `'move' | 'copy'` operation from source/target paths, the volumes list,
52+
and the current modifier state. Same function feeds the overlay label and the actual drop, so the displayed and
53+
executed operation can never disagree.
5154
- `drag-position.ts` — Corrects Tauri coords for docked DevTools (dev-only, zero overhead in prod)
5255
- Integration in `DualPaneExplorer.svelte`
5356

@@ -68,7 +71,7 @@ Key files:
6871
## Key decisions
6972

7073
- **Decision**: Always show confirmation dialog on drop
71-
- **Why**: Drag-and-drop is imprecise. Default operation is copy (safer than move).
74+
- **Why**: Drag-and-drop is imprecise. The dialog is the safety net regardless of which operation is preselected.
7275
- **Decision**: Same-pane pane-level drops are no-ops
7376
- **Why**: Dropping onto a subfolder within the same pane is valid.
7477
- **Decision**: Block drops onto the source itself or into a descendant
@@ -92,6 +95,21 @@ Key files:
9295
arbitrate the operation natively.
9396
- **Decision**: Modifier keys tracked via NSEvent.modifierFlags
9497
- **Why**: Tauri doesn't expose modifier state in DragDropEvent. Emits `drag-modifiers` event only when state changes.
98+
- **Decision**: Drop operation follows Finder's volume-aware default plus Alt/Cmd/Shift modifiers
99+
- **Default**: same volume → Move, cross-volume → Copy (matches Finder's behavior on a stock macOS install).
100+
- **Alt (Option)** held → force Copy. Beats Cmd/Shift if both are held — the user is asking for Copy.
101+
- **Cmd** held → force Move. Matches Finder's force-move modifier.
102+
- **Shift** held → force Move. Windows convention; included as a friendly accelerator for cross-platform users.
103+
- **Why**: Earlier we kept Copy-as-default for safety, but it created a confusing inconsistency: dragging out of Cmdr
104+
to the Desktop becomes Move (macOS arbitrates the outgoing operation from the source mask + modifiers), so the same
105+
gesture meant different things inside vs. outside the app. Matching Finder removes that surprise. The confirmation
106+
dialog still catches mistakes, so we don't lose the safety net.
107+
- The same `pickDropOperation` runs for both the live overlay (`handleDragOver`) and the actual drop (`handleDrop`),
108+
so the two can't diverge.
109+
- **Decision**: Internal-drop modifier semantics align with the outgoing arbitration macOS does for us
110+
- **Why**: For drags out to other apps, AppKit arbitrates the operation from the source mask plus modifiers (Alt →
111+
Copy, Cmd → Move, Ctrl-Alt → Link). We own the choice for internal drops, so we replicate the same Alt/Cmd
112+
convention. Shift is an extra Move accelerator for Windows-trained users; Link isn't supported.
95113
- **Decision**: Viewport position correction only in dev mode
96114
- **Why**: DevTools docked mode shrinks viewport but Tauri reports window-relative positions. Offset computed via
97115
`outerSize()` vs `innerHeight`. Zero overhead in prod.
Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
import { describe, expect, it } from 'vitest'
2+
import type { VolumeInfo } from '$lib/file-explorer/types'
3+
import { findVolumeIdForPath, isSameVolume, pickDropOperation } from './drop-operation'
4+
5+
const root: VolumeInfo = { id: 'boot', name: 'Macintosh HD', path: '/', category: 'main_volume', isEjectable: false }
6+
const usb: VolumeInfo = {
7+
id: 'usb',
8+
name: 'MyDrive',
9+
path: '/Volumes/MyDrive',
10+
category: 'attached_volume',
11+
isEjectable: true,
12+
}
13+
const usbPrefix: VolumeInfo = {
14+
id: 'usbprefix',
15+
name: 'MyDrive2',
16+
path: '/Volumes/MyDrive2',
17+
category: 'attached_volume',
18+
isEjectable: true,
19+
}
20+
const volumes: VolumeInfo[] = [root, usb, usbPrefix]
21+
22+
const noMods = { altHeld: false, cmdHeld: false, shiftHeld: false }
23+
24+
describe('findVolumeIdForPath', () => {
25+
it('matches the longest-prefix volume', () => {
26+
expect(findVolumeIdForPath('/Volumes/MyDrive/foo.txt', volumes)).toBe('usb')
27+
})
28+
29+
it('falls back to root for paths not under any mount', () => {
30+
expect(findVolumeIdForPath('/Users/dave/notes.md', volumes)).toBe('boot')
31+
})
32+
33+
it('does not match a sibling whose name is a prefix', () => {
34+
// /Volumes/MyDrive2/foo must not match the /Volumes/MyDrive volume
35+
expect(findVolumeIdForPath('/Volumes/MyDrive2/foo', volumes)).toBe('usbprefix')
36+
})
37+
38+
it('returns null when no volume matches', () => {
39+
expect(findVolumeIdForPath('/Users/dave', [usb])).toBeNull()
40+
})
41+
42+
it('matches a path equal to the volume path itself', () => {
43+
expect(findVolumeIdForPath('/Volumes/MyDrive', volumes)).toBe('usb')
44+
})
45+
})
46+
47+
describe('isSameVolume', () => {
48+
it('returns true for two paths on the boot volume', () => {
49+
expect(isSameVolume('/Users/a', '/Users/b/c', volumes)).toBe(true)
50+
})
51+
52+
it('returns false for cross-volume paths', () => {
53+
expect(isSameVolume('/Users/a', '/Volumes/MyDrive/x', volumes)).toBe(false)
54+
})
55+
56+
it('returns false when the source resolves to no volume', () => {
57+
expect(isSameVolume('/orphan', '/Users/a', [usb])).toBe(false)
58+
})
59+
})
60+
61+
describe('pickDropOperation', () => {
62+
const baseOpts = {
63+
sourcePath: '/Users/a/file.txt',
64+
targetPath: '/Users/b',
65+
volumes,
66+
}
67+
68+
it('defaults to Move when source and target share a volume', () => {
69+
expect(pickDropOperation({ ...baseOpts, modifiers: noMods })).toBe('move')
70+
})
71+
72+
it('defaults to Copy when source and target are on different volumes', () => {
73+
expect(
74+
pickDropOperation({
75+
...baseOpts,
76+
targetPath: '/Volumes/MyDrive/dst',
77+
modifiers: noMods,
78+
}),
79+
).toBe('copy')
80+
})
81+
82+
it('Alt forces Copy even on same-volume drops', () => {
83+
expect(
84+
pickDropOperation({
85+
...baseOpts,
86+
modifiers: { altHeld: true, cmdHeld: false, shiftHeld: false },
87+
}),
88+
).toBe('copy')
89+
})
90+
91+
it('Cmd forces Move even on cross-volume drops', () => {
92+
expect(
93+
pickDropOperation({
94+
...baseOpts,
95+
targetPath: '/Volumes/MyDrive/dst',
96+
modifiers: { altHeld: false, cmdHeld: true, shiftHeld: false },
97+
}),
98+
).toBe('move')
99+
})
100+
101+
it('Shift forces Move even on cross-volume drops', () => {
102+
expect(
103+
pickDropOperation({
104+
...baseOpts,
105+
targetPath: '/Volumes/MyDrive/dst',
106+
modifiers: { altHeld: false, cmdHeld: false, shiftHeld: true },
107+
}),
108+
).toBe('move')
109+
})
110+
111+
it('Alt beats Cmd when both are held (force-Copy wins)', () => {
112+
expect(
113+
pickDropOperation({
114+
...baseOpts,
115+
modifiers: { altHeld: true, cmdHeld: true, shiftHeld: false },
116+
}),
117+
).toBe('copy')
118+
})
119+
120+
it('falls back to Copy when source path is missing', () => {
121+
expect(
122+
pickDropOperation({
123+
sourcePath: null,
124+
targetPath: '/Users/b',
125+
volumes,
126+
modifiers: noMods,
127+
}),
128+
).toBe('copy')
129+
})
130+
131+
it('falls back to Copy when target path is missing', () => {
132+
expect(
133+
pickDropOperation({
134+
sourcePath: '/Users/a',
135+
targetPath: null,
136+
volumes,
137+
modifiers: noMods,
138+
}),
139+
).toBe('copy')
140+
})
141+
})
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
// Picks the operation (move or copy) for a drag-and-drop drop, following Finder conventions:
2+
// - Option (Alt) held → Copy (force, beats other modifiers)
3+
// - Cmd or Shift held → Move (force; Cmd is Finder, Shift is the Windows accelerator)
4+
// - Otherwise, same volume → Move
5+
// - Otherwise → Copy
6+
7+
import type { VolumeInfo } from '$lib/file-explorer/types'
8+
9+
export interface ModifierState {
10+
altHeld: boolean
11+
cmdHeld: boolean
12+
shiftHeld: boolean
13+
}
14+
15+
/**
16+
* Returns the volume ID whose `path` is the longest prefix of `path`, or `null` if none matches.
17+
* Treats `/` specially so it matches everything but loses to longer mounts (`/Volumes/Foo`).
18+
*/
19+
export function findVolumeIdForPath(path: string, volumes: readonly VolumeInfo[]): string | null {
20+
let best: VolumeInfo | null = null
21+
for (const v of volumes) {
22+
if (!v.path) continue
23+
const matches = v.path === '/' ? path.startsWith('/') : path === v.path || path.startsWith(`${v.path}/`)
24+
if (matches && (!best || v.path.length > best.path.length)) {
25+
best = v
26+
}
27+
}
28+
return best?.id ?? null
29+
}
30+
31+
/** True when source and target paths resolve to the same volume. False if either can't be resolved. */
32+
export function isSameVolume(sourcePath: string, targetPath: string, volumes: readonly VolumeInfo[]): boolean {
33+
const a = findVolumeIdForPath(sourcePath, volumes)
34+
if (a === null) return false
35+
return a === findVolumeIdForPath(targetPath, volumes)
36+
}
37+
38+
/**
39+
* Picks the drop operation. The `sourcePath` should be the first path in the drag (volume affinity is
40+
* deterministic and matches the common case of single-volume selections). When the source can't be
41+
* resolved to a volume, falls back to Copy (the safer default).
42+
*/
43+
export function pickDropOperation(opts: {
44+
sourcePath: string | null
45+
targetPath: string | null
46+
volumes: readonly VolumeInfo[]
47+
modifiers: ModifierState
48+
}): 'move' | 'copy' {
49+
const { altHeld, cmdHeld, shiftHeld } = opts.modifiers
50+
if (altHeld) return 'copy'
51+
if (cmdHeld || shiftHeld) return 'move'
52+
if (opts.sourcePath && opts.targetPath && isSameVolume(opts.sourcePath, opts.targetPath, opts.volumes)) {
53+
return 'move'
54+
}
55+
return 'copy'
56+
}

0 commit comments

Comments
 (0)