feat: carry the drop-operation negotiation in both directions - #93
Open
AdrianEddy wants to merge 2 commits into
Open
AdrianEddy wants to merge 2 commits into
AdrianEddy wants to merge 2 commits into
Conversation
`DragResult::Dropped` now carries a `DropOperation` — a portable bit
mask (COPY | MOVE | LINK) each platform normalizes its native value
into: `DoDragDrop`'s out `DROPEFFECT` on Windows (at both call sites),
the `draggingSession:endedAtPoint:operation:` argument on macOS, and
the drag context's `selected_action` on GTK. The value tells the
SOURCE what to do with its original data; it was already present in a
local at all four callback sites and was discarded.
BREAKING CHANGE: `DragResult::Dropped` is a newtype variant, and with
the `serde` feature its wire form changes from "Dropped" to
{"Dropped": <mask>}. The TS unions in both plugin packages are updated
to match (they also never matched the `Cancel` arm, which serializes
as "Cancel", not "Cancelled").
`Options.mode` was a binary `Copy | Move` (never `Link`), and on Windows it became the whole of `DoDragDrop`'s `dwOKEffects` — so a conforming target could only ever negotiate that one operation or none. `Options.allowed_operations` is the same `DropOperation` mask the result now carries, feeding `dwOKEffects` (Windows), the `sourceOperationMaskForDraggingContext:` return (macOS) and the source's `GdkDragAction` (GTK). `Options::default()` still permits exactly a copy. The Windows `DragItem::Data` branch, which hardcoded `DROPEFFECT_COPY` and ignored `Options` entirely, now honours the mask like the `Files` branch. BREAKING CHANGE: `DragMode` is removed; an `allowed_operations` mask with no defined bits now makes the drag undroppable instead of silently offering a copy. The plugins' `mode` option additionally accepts "link".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
dragcurrently cannot express a drag-and-drop negotiation in either direction, and the two halves only make sense together.Inbound:
DragResultisDropped | Cancel, so a consumer cannot tell what the target did — only that something was dropped. That matters because the negotiated operation is an instruction to the source: after a move it must delete its original, after a copy it must not, and after a drop that resolved to "no effect" it must do neither. Today the only option is to assume the operation the source proposed, which is wrong whenever the target chose differently — and actively destructive in theDRAGDROP_S_DROP+DROPEFFECT_NONEcase (the target accepted the drop, performed nothing, and the source deletes its data anyway).Outbound:
Options.modeis a binaryCopy | Move(neverLink), and on Windows it becomes the whole ofDoDragDrop'sdwOKEffects— so a conforming target can only ever return that one effect or none. The negotiation is foreclosed before the target sees the drag.Neither half needs a new OS call. The result value is already sitting in a local at all four callback sites and is discarded (Windows twice — both
DoDragDropcalls; macOS — thedraggingSession:endedAtPoint:operation:argument; GTK —context.selected_action(), currently read only to trace-log it), and all three platform arms already compute their permission channel frommode— they just need a wider input.Change 1 —
DragResult::Dropped(DropOperation).DropOperationis a small portable bit mask (COPY | MOVE | LINK) each platform normalizes into. A mask rather than an enum becauseNSDragOperationisNS_OPTIONS,GdkDragActionis a flag set, and theDROPEFFECTdocumentation explicitly forbids equality comparison ("Your application should always mask values … never compare a DROPEFFECT against, say, DROPEFFECT_COPY"). Platform bits with no source-side obligation are dropped rather than invented into the portable set (DROPEFFECT_SCROLL,NSDragOperationPrivate,GdkDragAction::{DEFAULT, PRIVATE, ASK});NSDragOperationDeletefolds toMOVEandNSDragOperationGenerictoCOPY, matching what the source must do about them. On WindowspdwEffectis read only in theDRAGDROP_S_DROPbranch, per its documentation ("set only if the operation is not canceled"). Consumers that want a single action apply their own priority to the mask — which they must anyway, since the platforms disagree about the natural priority order.This is a
serdewire break that lands inside this repo: external tagging renders the newtype variant as{"Dropped": <mask>}where it was"Dropped".tauri-plugin-dragandtauri-plugin-drag-as-windowforwarddrag::DragResultto the webview verbatim, so this PR updates both TSDragResulttypes in the same pass — which also fixes a pre-existing bug in them: they declared"Cancelled", while the Rust variant serializes as"Cancel", so the cancel arm never matched. The alternative that avoids the wire break — keepingDroppeda unit variant plus a separateoperationfield — would make an operation representable for a cancelled drag; since the plugins ship in this repo and are updated in the same commit, the honest variant seemed better. Happy to do it the other way if JS wire stability matters more.Change 2 —
Options.mode: DragMode→Options.allowed_operations: DropOperation. The same type used for the question instead of the answer: the source declares the set of operations it permits, and each arm hands it to its existing permission channel —dwOKEffects(Windows), thesourceOperationMaskForDraggingContext:return (macOS), the source'sGdkDragAction(GTK) — as a three-bit fold that is the exact inverse of change 1's normalization.DragModeis deleted rather than deprecated: it answered the same question strictly worse, and keeping both would leave two vocabularies for one concept.Options::default()still permits exactly a copy, so default-options callers see no change. Behaviour change worth its changelog line: anallowed_operationswith no defined bits makes the drag undroppable (DROPEFFECT_NONE/ empty masks) — where the old design's only near-equivalent silently offered a copy and then reported the proposed action. Drive-by in the same lines: the WindowsDragItem::Databranch hardcodedDROPEFFECT_COPYand ignoredOptionsentirely; it now honours the mask like theFilesbranch. The plugins'modeoption gains"link".DropOperationhas noDefault(it could only be the empty mask, which as anallowed_operationsis an undroppable drag — not a value to reach by omitting a field), and under theserdefeature it deserializes throughfrom_bits_truncate(#[serde(from = "u32")]), so a wire value like4294967295cannot become a mask that intersects everything. Tests cover the mask semantics, the default, and the serde round trips including that clamp.One unrelated bug spotted next door, deliberately not included: in both Windows branches,
GetCursorPos(&mut pt)?runs betweenDoDragDropand the callback, so aGetCursorPosfailure makes a completed drop report as a failed drag (the callback never fires).A covector change file is included (major across the five packages).
This PR was generated by Claude