From f5a8afc0f2bf97f5235c45adeb7d327fb6067b8d Mon Sep 17 00:00:00 2001 From: vicrodh <103399283+vicrodh@users.noreply.github.com> Date: Mon, 13 Jul 2026 02:45:10 -0600 Subject: [PATCH] fix(playlist): reorder via the existing row drag gesture, not a bespoke grip The prior reorder reused only the DragGhost but re-implemented the drag gesture with its own grip + absolute-position math, which placed the ghost several px off the cursor. Remove the grip entirely and drive reorder off the SAME row-body drag that already adds a track to a sidebar playlist: it captures the source index on start, the shared DragGhost (positioned by the Rust DragActions path) follows the cursor exactly, the drop slot is read from DragState.pointer-y against the ListView's absolute-position (no row double-count), and releasing WITHIN the list reorders while releasing on a sidebar playlist still adds. Reverted the sidebar drop suppression + removed DragState.reorder. cc --- crates/qbz-ui/ui/playlist/PlaylistView.slint | 160 +++++++++-------- crates/qbz-ui/ui/primitives/TrackRow.slint | 173 ++++++------------- crates/qbz-ui/ui/shell/Sidebar.slint | 3 - crates/qbz-ui/ui/state.slint | 6 - 4 files changed, 145 insertions(+), 197 deletions(-) diff --git a/crates/qbz-ui/ui/playlist/PlaylistView.slint b/crates/qbz-ui/ui/playlist/PlaylistView.slint index d9006ea8b..d97b54a3d 100644 --- a/crates/qbz-ui/ui/playlist/PlaylistView.slint +++ b/crates/qbz-ui/ui/playlist/PlaylistView.slint @@ -338,40 +338,84 @@ export component PlaylistView inherits Rectangle { : @tr("Default"); // --- Custom-order drag-reorder state (issue #589) ------------------- - // Slot bookkeeping is component-LOCAL; the floating ghost REUSES the - // app-wide DragGhost via the DragState global (with DragState.reorder - // set, so sidebar rows skip drop-target detection). DragActions is - // never called, so the Rust add-to-playlist drop path never runs. + // Reorder RIDES the normal row-body drag (TrackRow's `ta` + + // DragActions — the same gesture that drags a track onto a sidebar + // playlist; no separate grip/gesture). TrackRow reports the source + // index via body-drag-started right as the shared drag starts; from + // there this view reads everything off DragState — the Rust-fed + // WINDOW-coord pointer the app-wide DragGhost is positioned with (no + // per-row absolute-position math). Releasing inside the track list + // reorders; releasing on a sidebar playlist is the unchanged Rust + // add-to-playlist drop (DragActions.end). // All indices are VISIBLE-list indices into PlaylistState.tracks. - property reorder-active: false; - property reorder-from: -1; // visible index of the dragged row - property reorder-over: -1; // insertion slot 0..N - // Window-Y of the track ListView's TOP EDGE, captured at drag-begin - // (row window-top - row*50px - viewport-y). Unlike the queue's fixed - // list-top, the live viewport-y is folded back in below so the slot - // math stays scroll-aware. - property reorder-list-top; + property reorder-from: -1; // source row; -1 = this drag is not a reorder + property reorder-over: -1; // insertion slot 0..N; -1 = pointer outside the list + // Sidebar drop target captured WHILE the drag is active. Rust's + // DragActions.end clears over-playlist-id in the same turn it clears + // `active`, and `changed` callbacks run deferred — by the time the + // drag-active handler below fires, over-playlist-id always reads "". + // This guarded mirror keeps the value the drop actually saw. + property reorder-drop-playlist: ""; + // DragState mirrors — a `changed` callback needs a component-local + // property (same pattern as Sidebar's drop detection). + property drag-active: DragState.active; + property drag-px: DragState.pointer-x; + property drag-py: DragState.pointer-y; + property drag-over-id: DragState.over-playlist-id; - // Pointer window-Y -> insertion slot [0..N]. Rows are a fixed 50px - // stacked with 0 spacing; the +0.5 rounds boundaries to row centers - // (top half of row k -> slot k, bottom half -> slot k+1). - pure function slot-from-pointer(win-y: length) -> int { - return Math.max(0, Math.min(PlaylistState.tracks.length, - Math.floor((win-y - root.reorder-list-top - track-list.viewport-y) / 50px + 0.5))); + // True while the drag pointer sits inside the track list's rect. + // track-list is a regular element (NOT a row inside the ListView), so + // its absolute-position is exact — no row double-count. Only called + // from event handlers (reading absolute-position in a declarative + // binding is a layout recursion). + function pointer-in-list() -> bool { + return DragState.pointer-x >= track-list.absolute-position.x + && DragState.pointer-x <= track-list.absolute-position.x + track-list.width + && DragState.pointer-y >= track-list.absolute-position.y + && DragState.pointer-y <= track-list.absolute-position.y + track-list.height; } - function commit-reorder() { - root.reorder-active = false; - // Release the shared ghost. - DragState.active = false; - DragState.reorder = false; - // Fire only a real move. `to == from` and `to == from+1` both drop - // back onto the same gap (no-op). - if (root.reorder-over != root.reorder-from - && root.reorder-over != root.reorder-from + 1) { - PlaylistActions.reorder-track(root.reorder-from, root.reorder-over); + // DragState.pointer-y -> insertion slot [0..N]. Rows are a fixed 50px + // stacked with 0 spacing; round() lands boundaries on row centers + // (top half of row k -> slot k, bottom half -> slot k+1). viewport-y + // (<= 0) folds the scroll back in, so the math stays scroll-aware. + function slot-from-pointer() -> int { + return Math.max(0, Math.min(PlaylistState.tracks.length, + Math.round((DragState.pointer-y - track-list.absolute-position.y + - track-list.viewport-y) / 50px))); + } + function update-reorder-slot() { + if (root.reorder-from >= 0 && DragState.active) { + root.reorder-over = root.pointer-in-list() ? root.slot-from-pointer() : -1; + } + } + changed drag-px => { root.update-reorder-slot(); } + changed drag-py => { root.update-reorder-slot(); } + changed drag-over-id => { + // Only while active — see the reorder-drop-playlist doc above. + if (DragState.active) { + root.reorder-drop-playlist = root.drag-over-id; + } + } + changed drag-active => { + if (root.drag-active) { + return; + } + // Drag ended (the Rust DragActions.end already ran — it handled a + // sidebar-playlist drop, if any). A reorder drag released INSIDE + // the list and NOT on a sidebar playlist moves the track; the slot + // is recomputed here from the FINAL pointer (the deferred drag-py + // handler may not have run for the last move). `slot == from` and + // `slot == from + 1` are the two gaps around the source row (no-op). + if (root.reorder-from >= 0 + && root.reorder-drop-playlist == "" + && root.pointer-in-list() + && root.slot-from-pointer() != root.reorder-from + && root.slot-from-pointer() != root.reorder-from + 1) { + PlaylistActions.reorder-track(root.reorder-from, root.slot-from-pointer()); } root.reorder-from = -1; root.reorder-over = -1; + root.reorder-drop-playlist = ""; } background: Theme.surface-main; @@ -1026,42 +1070,17 @@ export component PlaylistView inherits Rectangle { local-goto-actions: (track.source == "local" || track.source == "plex") && track.id.is-float(); multi-select-mode: PlaylistState.multi-select-mode; - // Drag-reorder (issue #589): the row's grip handle - // reports the gesture; this view owns the state. - reorder-index: i; - reorder-source: root.reorder-active && root.reorder-from == i; - reorder-begin(idx, row-top, gx, gy) => { - root.reorder-active = true; + // Drag-reorder (issue #589): rides the normal + // row-body drag — the row only reports the + // source index as the shared drag starts; the + // view-level DragState handlers above do the rest. + // TODO: auto-scroll during drag when the pointer + // nears the list's top/bottom edge (the drag + // currently reorders within the visible viewport). + body-drag-started(idx) => { root.reorder-from = idx; - root.reorder-over = idx; - root.reorder-list-top = - row-top - idx * 50px - track-list.viewport-y; - // Reuse the app-wide DragGhost (proven - // window-coord positioning) for the floating - // pill; reorder=true keeps sidebar rows from - // claiming a drop target. Pointer first, then - // active, so the ghost never flashes at a - // stale position. - DragState.count = 1; - DragState.ghost-title = track.title; - DragState.ghost-subtitle = track.album != "" - ? track.artist + " · " + track.album - : track.artist; - DragState.pointer-x = gx; - DragState.pointer-y = gy; - DragState.reorder = true; - DragState.active = true; - } - reorder-update(gx, gy) => { - root.reorder-over = root.slot-from-pointer(gy); - DragState.pointer-x = gx; - DragState.pointer-y = gy; - // TODO: auto-scroll during drag when the pointer - // nears the list's top/bottom edge (the drag - // currently reorders within the visible viewport). - } - reorder-end => { - root.commit-reorder(); + root.reorder-over = -1; + root.reorder-drop-playlist = ""; } media-action(kind, id, action) => { if (action == "play") { @@ -1074,12 +1093,15 @@ export component PlaylistView inherits Rectangle { } } // Drop indicator: a 2px accent line at the insertion slot, - // hidden on the two no-op slots (from, from+1). The slot's + // hidden while the pointer is outside the list (slot -1 — + // e.g. over the sidebar, where the drop ADDS instead) and + // on the two no-op slots (from, from+1). The slot's // content-space position folds back through viewport-y; // clamped to the list rect so a slot scrolled out of view // never paints outside it. Width stops before the 14px // scrollbar gutter. - if root.reorder-active + if root.reorder-from >= 0 + && root.reorder-over >= 0 && root.reorder-over != root.reorder-from && root.reorder-over != root.reorder-from + 1: Rectangle { x: 0; @@ -1447,8 +1469,8 @@ export component PlaylistView inherits Rectangle { } } - // The floating reorder ghost is the app-wide DragGhost (AppShell), - // driven through DragState by the reorder-begin/update handlers above — - // no component-local ghost (its absolute-position math double-counted - // the row offset inside the ListView and drifted off the cursor). + // The floating reorder ghost is the app-wide DragGhost (AppShell): the + // reorder IS the normal row-body drag, so the ghost is positioned by + // the proven Rust DragActions window coords — no component-local ghost, + // no row absolute-position math. } diff --git a/crates/qbz-ui/ui/primitives/TrackRow.slint b/crates/qbz-ui/ui/primitives/TrackRow.slint index a914bd6b5..3ac4556cd 100644 --- a/crates/qbz-ui/ui/primitives/TrackRow.slint +++ b/crates/qbz-ui/ui/primitives/TrackRow.slint @@ -34,11 +34,10 @@ export component TrackRow inherits Rectangle { // single shared interaction gate every blacklist surface reuses. property row-inert: root.track.is-blacklisted; // Fade out when the row is being removed (favorites un-favorite); a - // blacklisted row that isn't fading dims to 0.4, and so does the row - // currently being drag-reordered (queue-drag parity). + // blacklisted row that isn't fading dims to 0.4. opacity: root.track.removing ? 0.0 - : ((root.row-inert || root.reorder-source) ? 0.4 : 1.0); + : (root.row-inert ? 0.4 : 1.0); animate opacity { duration: 250ms; } in property index; in property show-artwork: true; @@ -87,17 +86,16 @@ export component TrackRow inherits Rectangle { // Custom-order reorder controls (up/down chevrons) at the leading // edge — used by the playlist custom sort. in property show-reorder: false; - // Drag-reorder (custom-order lists, issue #589). Mirrors QueueRow's - // drag contract: the grip handle in the reorder gutter reports the - // gesture in WINDOW coords; the parent view owns the list-level state. - // NOT the row-body drag (ta + DragActions = drag onto sidebar - // playlists) — the handle has its OWN TouchArea. - in property reorder-index; - in property reorder-source: false; // this row is being dragged - callback reorder-begin(int /* visible index */, length /* row window-top */, - length /* pointer window-x */, length /* pointer window-y */); - callback reorder-update(length /* pointer window-x */, length /* pointer window-y */); - callback reorder-end(); + // Drag-reorder (custom-order lists, issue #589) RIDES the normal + // row-body drag (ta + DragActions — the same gesture that drags a + // track onto a sidebar playlist; there is NO separate reorder + // gesture/grip). On a reorderable surface (show-reorder) the row + // reports which visible index the drag left from, right before + // DragActions.start; the parent view then reads the drag from + // DragState (the Rust-fed window-coord pointer the shared DragGhost + // uses) — a drop inside the list reorders, a drop on a sidebar + // playlist stays the unchanged add-to-playlist path. + callback body-drag-started(int /* visible index */); // Whether the row can be dragged onto sidebar playlists. in property draggable: true; callback media-action(string, string, string); @@ -225,6 +223,13 @@ export component TrackRow inherits Rectangle { && (Math.abs(root.absolute-position.x - root.x + ta.mouse-x - ta.down-x) > 6px || Math.abs(root.absolute-position.y - root.y + ta.mouse-y - ta.down-y) > 6px)) { ta.dragging = true; + // Reorderable surface: tell the parent view which row + // this drag left from BEFORE the shared drag starts + // (issue #589 — see body-drag-started above). The + // DragActions call itself is untouched. + if (root.show-reorder) { + root.body-drag-started(root.index); + } DragActions.start( root.track.id, root.track.title, @@ -248,115 +253,45 @@ export component TrackRow inherits Rectangle { padding-right: 12px; spacing: 14px; - // Reorder gutter (custom-order mode) — leading edge: a drag GRIP - // handle next to the up/down chevrons. The grip's TouchArea is - // deliberately separate from the row-body `ta` (which drives the - // drag-onto-sidebar-playlists ghost via DragActions) — this one - // implements the queue's drag-reorder gesture (QueueRow) and - // reports in window coords via the reorder-* callbacks. - if root.show-reorder: HorizontalLayout { - spacing: 2px; - VerticalLayout { - alignment: center; - Rectangle { - width: 18px; - height: 36px; - border-radius: Radius.sm; - background: grip-ta.has-hover ? Theme.surface-elevated : transparent; - QbzIcon { - source: @image-url("../assets/icons/grip-vertical.svg"); - width: 14px; - height: 14px; - x: Math.round((parent.width - self.width) / 2 / 1px) * 1px; - y: Math.round((parent.height - self.height) / 2 / 1px) * 1px; - tint: grip-ta.has-hover ? Theme.text-primary : Theme.text-muted; - } - grip-ta := TouchArea { - mouse-cursor: grab; - property down-y; - property dragging: false; - pointer-event(e) => { - if (e.kind == PointerEventKind.down) { - // absolute-position double-counts the ROW's own - // layout offset inside the ListView; subtracting - // root.{y} recovers true window coords — and the - // correction propagates unchanged to this nested - // element (if/for offsets are exact). See - // reference_slint_absolute_position_flickable. - self.down-y = self.absolute-position.y - root.y + self.mouse-y; - self.dragging = false; - } - if (e.kind == PointerEventKind.up && self.dragging) { - root.reorder-end(); - self.dragging = false; - } - } - // Safety net: the playlist ListView stays scrollable, so - // it can steal the grab mid-drag and `up` never arrives. - // `pressed` flips false on release AND grab-cancel. - changed pressed => { - if (!self.pressed && self.dragging) { - root.reorder-end(); - self.dragging = false; - } - } - moved => { - if (self.pressed) { - if (!self.dragging - && Math.abs(self.absolute-position.y - root.y + self.mouse-y - self.down-y) > 6px) { - self.dragging = true; - root.reorder-begin(root.reorder-index, - root.absolute-position.y - root.y, - self.absolute-position.x - root.x + self.mouse-x, - self.absolute-position.y - root.y + self.mouse-y); - } - if (self.dragging) { - root.reorder-update( - self.absolute-position.x - root.x + self.mouse-x, - self.absolute-position.y - root.y + self.mouse-y); - } - } - } + // Reorder gutter (custom-order mode) — leading edge: the up/down + // chevrons. Drag-reorder needs no control here — it rides the + // row-body drag (see body-drag-started above). + if root.show-reorder: VerticalLayout { + alignment: center; + spacing: 0px; + Rectangle { + width: 22px; + height: 18px; + QbzIcon { + source: @image-url("../assets/icons/chevron-up.svg"); + width: 14px; + height: 14px; + x: Math.round((parent.width - self.width) / 2 / 1px) * 1px; + y: Math.round((parent.height - self.height) / 2 / 1px) * 1px; + tint: up-ta.has-hover ? Theme.text-primary : Theme.text-muted; + } + up-ta := TouchArea { + mouse-cursor: pointer; + clicked => { + root.media-action("track", root.track.id, "move-up"); } } } - VerticalLayout { - alignment: center; - spacing: 0px; - Rectangle { - width: 22px; - height: 18px; - QbzIcon { - source: @image-url("../assets/icons/chevron-up.svg"); - width: 14px; - height: 14px; - x: Math.round((parent.width - self.width) / 2 / 1px) * 1px; - y: Math.round((parent.height - self.height) / 2 / 1px) * 1px; - tint: up-ta.has-hover ? Theme.text-primary : Theme.text-muted; - } - up-ta := TouchArea { - mouse-cursor: pointer; - clicked => { - root.media-action("track", root.track.id, "move-up"); - } - } + Rectangle { + width: 22px; + height: 18px; + QbzIcon { + source: @image-url("../assets/icons/chevron-down.svg"); + width: 14px; + height: 14px; + x: Math.round((parent.width - self.width) / 2 / 1px) * 1px; + y: Math.round((parent.height - self.height) / 2 / 1px) * 1px; + tint: down-ta.has-hover ? Theme.text-primary : Theme.text-muted; } - Rectangle { - width: 22px; - height: 18px; - QbzIcon { - source: @image-url("../assets/icons/chevron-down.svg"); - width: 14px; - height: 14px; - x: Math.round((parent.width - self.width) / 2 / 1px) * 1px; - y: Math.round((parent.height - self.height) / 2 / 1px) * 1px; - tint: down-ta.has-hover ? Theme.text-primary : Theme.text-muted; - } - down-ta := TouchArea { - mouse-cursor: pointer; - clicked => { - root.media-action("track", root.track.id, "move-down"); - } + down-ta := TouchArea { + mouse-cursor: pointer; + clicked => { + root.media-action("track", root.track.id, "move-down"); } } } diff --git a/crates/qbz-ui/ui/shell/Sidebar.slint b/crates/qbz-ui/ui/shell/Sidebar.slint index 1d69db846..c5108b0f5 100644 --- a/crates/qbz-ui/ui/shell/Sidebar.slint +++ b/crates/qbz-ui/ui/shell/Sidebar.slint @@ -159,9 +159,6 @@ component SidebarRow inherits Rectangle { // to get the true window bounds (the drag pointer in DragState is // already in true window coords — TrackRow corrects it the same way). root.drop-hot = root.drag-on - // A within-list reorder drag (playlist custom order) reuses the - // DragState ghost but is NOT an add-to-playlist drop. - && !DragState.reorder && !root.is-folder && root.drag-px >= root.absolute-position.x - root.x && root.drag-px <= root.absolute-position.x - root.x + root.width diff --git a/crates/qbz-ui/ui/state.slint b/crates/qbz-ui/ui/state.slint index 373a8938c..0d48d61a8 100644 --- a/crates/qbz-ui/ui/state.slint +++ b/crates/qbz-ui/ui/state.slint @@ -968,12 +968,6 @@ export global DragState { in-out property pointer-y; // The sidebar playlist id currently under the pointer (drop target). in-out property over-playlist-id; - // WITHIN-LIST reorder drag (playlist custom order, issue #589): the - // gesture reuses this global purely for the floating ghost (DragGhost's - // window-coord positioning), driven from .slint only — DragActions is - // never called, so the Rust add-to-playlist drop path never runs. - // Sidebar rows skip drop-target detection while this is set. - in-out property reorder: false; } export global DragActions {