From 83c1aec4f5ec99099e8d445768e3876df78b2ef8 Mon Sep 17 00:00:00 2001 From: vicrodh <103399283+vicrodh@users.noreply.github.com> Date: Thu, 30 Jul 2026 18:41:06 -0600 Subject: [PATCH] feat(qt): collapsible rows in the My QBZ detail, and ephemeral leaves no trace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **The accordion the owner asked for.** Each expandable row gets a chevron; clicking it opens THAT row's tracks inline and nothing else. Verified by driving the real app: in list mode every row is collapsed with the chevron pointing right, clicking row 3 — a PLEX row, so the hybrid path — flips its chevron, renders its three tracks with durations and quality badges, and leaves rows 1, 2, 4 and 5 untouched. This is a DELIBERATE divergence from both references, decided by the owner with the facts in hand: neither Slint nor Tauri has a per-row accordion. Both render inline tracks only as a view MODE, for every row at once, and both say "no chevron" in their own comments (MixtapeDetailView.slint:1139, MixtapeCollectionDetailView.svelte:124-127). The owner first believed Tauri had it, asked for the code to be checked rather than the comment, and on the evidence chose the accordion anyway. Do not "restore parity" by removing it. Built additively on machinery that already existed: the document already carried `canExpand` / `inlineTracks` / `expandLoading` / `tracksLoaded`. What was added is per-row OPEN state in Rust, a toggle invokable, and lazy per-row fetching — opening a row fetches only its tracks, once. The "expanded" view mode survives and now means "open all", sharing ONE notion of open rather than a second parallel state. Also here: - `body`'s `anchors.bottomMargin: 100` shrank the list VIEWPORT; in the .slint that 100px is padding INSIDE the Flickable, i.e. scroll runway. Measured on screen before the fix: the last row clipped mid-height with ~85px of dead band under it. - `apply_resolved` cloned and serialised the WHOLE document per resolved item (34 serialize→parse→patch cycles for one collection, 200 for a big one, on the GUI thread), and `patch_expanded` did the same while the document carried every loaded item's inline tracks — O(n²). - The DEAD affordances the audit filed against the modals and the grids. **Ephemeral honours its name** (owner rule): an ephemeral folder plays and queues, and writes nothing durable. Recently Played and Most Played are now guarded — an ephemeral album played through would previously have outranked real albums in Most Played, one row per track start. The Slint reference does NOT have this exemption; that is a gap in both frontends, not a Qt regression, and this does not restore parity by removing it. Two stores were deliberately NOT changed and need the owner's word: the scrobblers and Discord presence. Neither is "las bibliotecas" — nothing is stored on our side — but both publish the track to a third party, so if "ni now playing ni nada" is meant literally they belong in the same decision. The one-line fix location is in the lane report. cc --- .../qbz-qt/qml/controls/AddToMixtapeModal.qml | 37 +- crates/qbz-qt/qml/controls/MyQbzModals.qml | 9 +- crates/qbz-qt/qml/views/myqbz/MyQbzCard.qml | 9 +- .../qbz-qt/qml/views/myqbz/MyQbzDetailRow.qml | 66 ++- .../qml/views/myqbz/MyQbzDetailView.qml | 146 +++++- .../qbz-qt/qml/views/myqbz/MyQbzGridView.qml | 130 ++++- crates/qbz-qt/src/myqbz_bridge.rs | 65 ++- crates/qbz-qt/src/myqbz_detail_qt.rs | 461 ++++++++++++++++-- crates/qbz-qt/src/playback_qt.rs | 22 +- crates/qbz-qt/src/recently_qt.rs | 68 ++- 10 files changed, 912 insertions(+), 101 deletions(-) diff --git a/crates/qbz-qt/qml/controls/AddToMixtapeModal.qml b/crates/qbz-qt/qml/controls/AddToMixtapeModal.qml index 21daf1c75..92df0c61e 100644 --- a/crates/qbz-qt/qml/controls/AddToMixtapeModal.qml +++ b/crates/qbz-qt/qml/controls/AddToMixtapeModal.qml @@ -346,11 +346,18 @@ Item { ListView { id: pickList visible: !root.loading && root.rows.length > 0 + // The .slint's `padding: 8` on the list layout, on all + // FOUR sides. The horizontal half is an ANCHOR margin on + // the view, never an `x` on the delegate: a vertical + // ListView writes its delegates' `x`, so a literal `x: 8` + // there is silently clobbered to 0 and the rows sat flush + // against the card's left border with a 16px gap opening + // on the right (the hover fill and the zebra band touched + // the border on one side only). The vertical half stays a + // Flickable content margin. anchors.fill: parent - // The .slint's `padding: 8` on the list layout. The - // horizontal half is on the DELEGATE, not as a Flickable - // margin: a vertical ListView's leftMargin moves the - // content origin, it does not inset the rows. + anchors.leftMargin: 8 + anchors.rightMargin: 8 topMargin: 8 bottomMargin: 8 spacing: 2 @@ -363,8 +370,7 @@ Item { delegate: AddPickRow { required property var modelData required property int index - x: 8 - width: Math.max(0, pickList.width - 16) + width: pickList.width rowData: modelData rowIndex: index } @@ -417,6 +423,25 @@ Item { verticalAlignment: Text.AlignVCenter clip: true selectByMouse: true + // LOAD-BEARING, and the exact shape `searchInput` + // above uses (:242 + its Binding). With ONLY the + // focus-gated Binding below, a re-entered create + // panel kept the previously typed name on screen + // while `createName` had already been reset to "" + // — the field never re-seeds because it keeps + // activeFocus across the hide/show (the panel is + // only `visible: false` for a frame, focus comes + // straight back), so the `when:` guard never + // fires. `canCreate` was then false with a name + // visibly in the box: "Create & Add" sat at 50 % + // with a dead MouseArea and Enter did nothing. + // Repros: chip -> type -> Back -> chip; and + // type -> close -> reopen into the create panel. + // A declarative `text:` survives user editing (the + // edit does not break the binding), and + // `onTextEdited` writes the same value straight + // back, so typing is unaffected. + text: root.createName onTextEdited: root.createName = text onAccepted: { if (root.canCreate) diff --git a/crates/qbz-qt/qml/controls/MyQbzModals.qml b/crates/qbz-qt/qml/controls/MyQbzModals.qml index 905890769..33abfc5f0 100644 --- a/crates/qbz-qt/qml/controls/MyQbzModals.qml +++ b/crates/qbz-qt/qml/controls/MyQbzModals.qml @@ -315,7 +315,14 @@ Item { QbzTextArea { visible: editPanel.isDescription width: parent.width - height: visible ? 96 : 0 + // A FLAT 96, never `visible ? 96 : 0`: a Column does not + // position invisible children, so the hidden arm already + // costs no layout height, while a 0-height text area + // hands its inner Flickable height = 0 - 8 - 8 = -16 — + // the silent negative-size trap QbzLineEdit.qml:166-167 + // documents. The sibling QbzLineEdit is single-line and + // lands at a clean 0, so it keeps its ternary. + height: 96 wrapMode: TextEdit.WordWrap text: editPanel.draftDescription onEdited: function (v) { editPanel.draftDescription = v } diff --git a/crates/qbz-qt/qml/views/myqbz/MyQbzCard.qml b/crates/qbz-qt/qml/views/myqbz/MyQbzCard.qml index e0cd918e0..42423cf9a 100644 --- a/crates/qbz-qt/qml/views/myqbz/MyQbzCard.qml +++ b/crates/qbz-qt/qml/views/myqbz/MyQbzCard.qml @@ -49,7 +49,14 @@ Rectangle { border.width: root.compact ? 0 : 1 border.color: cardArea.containsMouse ? theme.surfaceHover : theme.surfaceElevated - readonly property string subtitle: (root.item.label || "") + " · " + (root.item.meta || "") + // `label · meta`, but never a bare " · ": a default/half-built `item` (a + // standalone instance, or a pre-publish frame) has neither half and would + // otherwise render the separator on its own. + readonly property string subtitle: { + var l = root.item.label || "" + var m = root.item.meta || "" + return (l !== "" && m !== "") ? (l + " · " + m) : (l + m) + } Loader { anchors.fill: parent diff --git a/crates/qbz-qt/qml/views/myqbz/MyQbzDetailRow.qml b/crates/qbz-qt/qml/views/myqbz/MyQbzDetailRow.qml index 6f91c3d61..a0f0c6928 100644 --- a/crates/qbz-qt/qml/views/myqbz/MyQbzDetailRow.qml +++ b/crates/qbz-qt/qml/views/myqbz/MyQbzDetailRow.qml @@ -5,6 +5,11 @@ // spacing 12, cells 40 / 1fr / 140 / 80 / 160 / 72 / 60 / 40. Row height 56, // bottom hairline `Theme.alpha-4`, hover/selected fill `surface-hover`. // +// Plus a NINTH, leading 24px cell that no reference has: the accordion +// chevron (owner-authorised divergence, see its own comment below). It shifts +// the eight reference columns 36px right; `titleColWidth` and the host's +// `ColHead` row are retuned together. +// // --- TWO THINGS THAT ARE LOAD-BEARING ----------------------------------- // // 1. Z-ORDER. `MixtapeDetailView.slint:203-217` documents the bug: the @@ -78,6 +83,13 @@ Rectangle { readonly property string yearText: root.rev >= 0 ? (root.item.yearText || "") : "" readonly property string artPath: root.rev >= 0 ? (root.item.artPath || "") : "" readonly property bool selected: root.rev >= 0 && root.item.selected === true + /// Albums and playlists have an inline track list; a single track has + /// nothing to show, so it gets no chevron (Rust decides — `canExpand`). + readonly property bool canExpand: root.rev >= 0 && root.item.canExpand === true + /// THE accordion state, and the ONLY one: Rust derives it from the shared + /// `OPEN_ROWS` set that the `expanded` view-mode segment also writes, so + /// this row never needs to know what the view mode is. + readonly property bool rowOpen: root.rev >= 0 && root.item.rowOpen === true /// track → music, playlist → list-music, else disc (.slint :263-267). readonly property string typeGlyph: root.itemType === "track" ? "music" @@ -88,13 +100,20 @@ Rectangle { // same reason `rows/TrackRow.qml:213-214` OR's its four. readonly property bool hovered: rowArea.containsMouse || artArea.containsMouse || titleArea.containsMouse || subArea.containsMouse || menuArea.containsMouse + || chevArea.containsMouse width: parent ? parent.width : 0 height: 56 color: (root.hovered || root.selected) ? theme.surfaceHover : "transparent" - /// Title column = rowWidth − padding(24) − fixed cells(592) − gaps(84). - readonly property int titleColWidth: Math.max(0, root.width - 700) + /// Title column = rowWidth − padding(24) − fixed cells(616) − gaps(96). + /// + /// 736, not the reference's 700: the accordion chevron added a NINTH cell + /// (24px) and a ninth 12px gap in front of the ordinal, shifting the + /// 8-column template 36px right. `MyQbzDetailView.qml`'s `ColHead` row + /// carries the identical 24px leader and the identical `- 736`, and the two + /// must be changed together or the header stops lining up with the rows. + readonly property int titleColWidth: Math.max(0, root.width - 736) // Bottom hairline — `#ffffff0a` in the .slint (:200) == `alpha-4`, taken // from the polarity-baked ramp so it is visible on light themes too. The @@ -135,6 +154,49 @@ Rectangle { anchors.rightMargin: 12 spacing: 12 + // --- Col 0 (24) — THE ACCORDION CHEVRON -------------------------- + // + // ADDITIVE, and a deliberate divergence from BOTH references + // (owner-authorised 2026-07-30): neither `MixtapeDetailView.slint` nor + // the Tauri build ever had a per-row accordion — both render inline + // tracks only as a whole-list VIEW MODE, explicitly "no chevron". The + // owner asked for the accordion anyway. The view mode survives + // untouched and now means "open all", through the same per-row state. + // + // The MouseArea is INVISIBLE on a non-expandable row, and an invisible + // item is not hit-tested — so a track row's chevron cell falls through + // to `rowArea` behind it and clicking there still opens the item, which + // is the gesture the owner asked to keep. On an expandable row this + // area sits at the default z, above `rowArea`'s `z: -1`, and swallows + // the click: the chevron toggles ONLY this row, it never navigates. + Item { + width: 24 + height: parent.height + QbzIcon { + visible: root.canExpand + anchors.centerIn: parent + name: "chevron-right" + width: 14 + height: 14 + tintName: (root.rowOpen || chevArea.containsMouse) ? "textPrimary" : "muted" + // Points right when closed, down when open. `rotation` is a + // plain QQuickItem property and QbzIcon is an Image, so this + // costs no effect pass and no extra glyph. + rotation: root.rowOpen ? 90 : 0 + Behavior on rotation { + NumberAnimation { duration: 140; easing.type: Easing.OutCubic } + } + } + MouseArea { + id: chevArea + visible: root.canExpand + anchors.fill: parent + hoverEnabled: true + cursorShape: Qt.PointingHandCursor + onClicked: QbzMyQbz.detailToggleRowExpand(root.sourceItemId) + } + } + // --- Col 1 (40) — ordinal / selection checkbox ------------------ Item { width: 40 diff --git a/crates/qbz-qt/qml/views/myqbz/MyQbzDetailView.qml b/crates/qbz-qt/qml/views/myqbz/MyQbzDetailView.qml index e7ac5bb6d..593491acc 100644 --- a/crates/qbz-qt/qml/views/myqbz/MyQbzDetailView.qml +++ b/crates/qbz-qt/qml/views/myqbz/MyQbzDetailView.qml @@ -22,6 +22,18 @@ // paddings, the 32px hero→body gap and the 10px toolbar tail are all kept; // the toolbar simply became literally sticky, which is what §7.2 calls it. // +// --- PER-ROW COLLAPSIBLE ROWS (owner-authorised divergence) ------------- +// Every album / playlist row carries a chevron that opens ITS OWN inline track +// list (`MyQbzDetailRow.qml`, col 0). NEITHER the .slint NOR the Tauri build +// had this — both render inline tracks only as a whole-list view MODE, +// explicitly "no chevron". The owner asked for the accordion anyway +// (2026-07-30); do not "restore" it toward either reference. +// It is ADDITIVE on machinery that already existed: Rust already published +// per-row `canExpand` / `inlineTracks` / `expandLoading` / `tracksLoaded`, so +// the accordion added exactly one thing — `rowOpen`, THE single notion of open. +// The `expanded` segment writes the same state and now means "open all"; there +// is deliberately no second, QML-side definition of "is this row open". +// // --- THE MODEL IS PATCHED, NEVER REPLACED, AND PARSED ONCE -------------- // `QbzMyQbz` carries no signals: every Rust-side change arrives as a whole new // `detailJson`. `refresh()` is the single entry point: ONE `JSON.parse` per @@ -215,13 +227,66 @@ Rectangle { root.doc = d root.syncRows(d) } + /// Merge a PARTIAL row batch from `QbzMyQbz.detailRowsPatched`. + /// + /// Rust no longer republishes the whole document for a one-row change + /// (resolved quality, an inline track list landing, a chevron toggle) — it + /// sends `{ id, rows: [ { position, sourceItemId, ...changed } ] }`. The + /// document in Rust stays authoritative; this is purely the cheap path to + /// the SAME live row objects `syncRows` patches, so `patchRev` is bumped + /// ONCE for the batch and every delegate binding re-evaluates exactly like + /// after a republish. + /// + /// `position` is the join column because it is the stable persisted key + /// (spec 02 §5.2) and it survives a re-derive; the array INDEX does not. + /// A batch addressed at another collection, or at rows the current + /// derivation filtered out, is silently dropped — the same guard Rust + /// makes before building it. + /// + /// Being a signal, it is a DELTA: an instance mounted after it was emitted + /// never saw it. That is what `QbzMyQbz.detailResync()` below is for. + function applyRowPatch(patchJson) { + var p + try { + p = JSON.parse(patchJson) + } catch (e) { + return + } + if (!p || !p.rows || p.rows.length === 0) return + if ((p.id || "") !== (root.doc.id || "")) return + var byPosition = ({}) + for (var i = 0; i < root.rows.length; i++) + byPosition[root.rows[i].position] = root.rows[i] + var touched = false + for (var j = 0; j < p.rows.length; j++) { + var patch = p.rows[j] + var target = byPosition[patch.position] + if (target === undefined) continue + Object.assign(target, patch) + touched = true + } + if (touched) root.patchRev = root.patchRev + 1 + } + Connections { target: QbzMyQbz function onDetailJsonChanged() { root.refresh() } + function onDetailRowsPatched(patchJson) { root.applyRowPatch(patchJson) } } // `doc`'s initial binding has already parsed at this point — reuse it // instead of parsing a third time. - Component.onCompleted: root.syncRows(root.doc) + // + // Then ask Rust to re-hand the document. This view is a `Loader` child + // (`shell/AppShell.qml:192`), so nav-away DESTROYS it and nav-back builds a + // NEW instance whose `detailJson` is whatever was last fully published — + // every row patch since (resolved quality badges, loaded inline tracks, + // which rows are open) went to the dead instance. One full publish per + // mount closes that, and it is the only full publish the patch channel + // still costs. + Component.onCompleted: { + root.syncRows(root.doc) + QbzMyQbz.detailResync() + } // --- ONE loading-dots clock for the whole view (spec 01 §7.5 / §15.5) -- // Slint animates each row's LoadingDots off `animation-tick`; a windowed @@ -506,7 +571,15 @@ Rectangle { anchors.bottom: parent.bottom anchors.leftMargin: 32 anchors.rightMargin: 32 - anchors.bottomMargin: 100 + // NOT `bottomMargin: 100`. The .slint's 100px is `padding-bottom` + // INSIDE the Flickable (:657) — scroll RUNWAY past the last row, + // clearing the player bar. As an anchor margin here it shrank the + // ListView/GridView VIEWPORT instead: the last row was clipped + // mid-height and ~85px of dead band sat under it, unusable and + // unscrollable, on every collection. The runway now lives on the + // flickables themselves (`bottomMargin: 100` below), which is what + // the sibling `MyQbzGridView.qml:336,358` already does right. + anchors.bottomMargin: 0 // ================= BODY: LIST / EXPANDED ================= Column { @@ -527,9 +600,17 @@ Rectangle { anchors.leftMargin: 12 anchors.rightMargin: 12 spacing: 12 + // The accordion chevron's leading cell (see + // MyQbzDetailRow.qml). Unlabelled — a header for a + // disclosure control would be noise — but it MUST be + // here, at exactly the row's 24px, or the eight + // reference columns below drift 36px out of the header. + Item { width: 24; height: 1 } ColHead { width: 40; text: "#"; horizontalAlignment: Text.AlignHCenter } ColHead { - width: Math.max(0, listHost.width - 700) + // Paired with MyQbzDetailRow's `titleColWidth`: + // 24 padding + 616 fixed cells + 96 gaps. + width: Math.max(0, listHost.width - 736) text: root.trs("Item") } ColHead { width: 140; text: root.trs("Type") } @@ -554,13 +635,21 @@ Rectangle { boundsBehavior: Flickable.StopAtBounds cacheBuffer: 500 reuseItems: true + // The .slint's 100px page bottom padding, as scroll + // runway past the last row (clears the player bar). + bottomMargin: 100 model: root.rows // EXPANDED mode is this SAME recycled list with the // inline block unhidden — a nested ListView inside a // recycled delegate cannot size itself (spec 02 §9.1), // and switching the delegate per mode would reset - // contentY. + // contentY. The per-row ACCORDION rides the identical + // machinery: the outer list stays recycled and windowed + // whatever is open, and an open row's tracks stay a + // bounded `Repeater` inside a `Column` delegate, which + // is content-sized and therefore safe. Do not "upgrade" + // that Repeater to a nested ListView. delegate: Column { id: rowCell required property var modelData @@ -597,8 +686,24 @@ Rectangle { readonly property var live: root.patchRev >= 0 ? (root.rows[index] || modelData) : modelData - readonly property bool expanded: root.viewMode === "expanded" - && (root.patchRev >= 0 && rowCell.live.canExpand === true) + /// ONE notion of open, and it is Rust's `rowOpen`. + /// + /// It used to be `viewMode === "expanded" && + /// canExpand`, i.e. a second, QML-side definition + /// of the same thing. With the accordion there are + /// two writers (the chevron and the segment) and + /// two definitions would disagree the first time a + /// row was closed inside expanded mode — so the + /// view mode is not read here at all. Rust sets + /// `rowOpen` on every expandable row while the mode + /// is `expanded` (open all) and clears it on the + /// way out; `canExpand` is implied, because a + /// non-expandable row is never marked open. + /// + /// Through `live`, like every other row read — see + /// the note above. + readonly property bool expanded: root.patchRev >= 0 + && rowCell.live.rowOpen === true readonly property var inlineTracks: (root.patchRev >= 0) ? (rowCell.live.inlineTracks || []) : [] readonly property bool expandLoading: root.patchRev >= 0 @@ -617,15 +722,24 @@ Rectangle { // Inline tracks (.slint :1151-1218): padding // 52 / 12 / 4 / 8, spacing 0. + // + // 88, not 52: the reference's 52 is `12 padding + + // 40 ordinal cell`, i.e. exactly where the row's + // artwork column starts. The chevron cell added + // `24 + 12` in front of the ordinal, so the same + // alignment is 88 and the right inset stays 12 + // (width - 88 - 12). Keep it tied to the row's + // template or the inline block stops hanging under + // the title it belongs to. Item { visible: rowCell.expanded width: parent.width height: visible ? inlineCol.height + 12 : 0 Column { id: inlineCol - x: 52 + x: 88 y: 4 - width: Math.max(0, parent.width - 64) + width: Math.max(0, parent.width - 100) spacing: 0 Item { @@ -683,14 +797,14 @@ Rectangle { // spec 01 §7.5 / OQ#9. routeGoToExternally: true onPlayRequested: QbzMyQbz.inlineTrackAction( - rowCell.modelData.sourceItemId || "", + rowCell.live.sourceItemId || "", modelData.id || "", "play") // `next|later|queue` -> the wire's // `play-next|play-later|queue`. // Unmapped is a SILENT no-op. onEnqueueRequested: function (mode) { QbzMyQbz.inlineTrackAction( - rowCell.modelData.sourceItemId || "", + rowCell.live.sourceItemId || "", modelData.id || "", mode === "next" ? "play-next" : mode === "later" ? "play-later" : "queue") @@ -698,12 +812,12 @@ Rectangle { onGoToRequested: function (goKind) { if (goKind === "artist") QbzMyQbz.openArtist( - rowCell.modelData.source || "", + rowCell.live.source || "", modelData.artist || "", modelData.artistId || "") else QbzMyQbz.inlineTrackAction( - rowCell.modelData.sourceItemId || "", + rowCell.live.sourceItemId || "", modelData.id || "", "go-to-album") } } @@ -747,6 +861,8 @@ Rectangle { cacheBuffer: 500 reuseItems: true boundsBehavior: Flickable.StopAtBounds + // Same runway as the list arm — see `body`'s note. + bottomMargin: 100 model: root.rows delegate: MyQbzDetailCard { @@ -787,8 +903,10 @@ Rectangle { } // ========================= BULK ACTION BAR ========================== - // A SIBLING of the scroller, floating over the body's 100px bottom - // gutter. `parent.width - 18 - 22` is asymmetric on purpose (.slint :1253). + // A SIBLING of the scroller, floating over the last 100px of scroll runway + // (which is now inside the flickables, not carved out of the viewport — + // see `body`). `parent.width - 18 - 22` is asymmetric on purpose + // (.slint :1253). QbzMultiSelectBar { visible: root.selectMode && root.selectedCount > 0 x: 18 diff --git a/crates/qbz-qt/qml/views/myqbz/MyQbzGridView.qml b/crates/qbz-qt/qml/views/myqbz/MyQbzGridView.qml index 5715bc802..7ea5bc064 100644 --- a/crates/qbz-qt/qml/views/myqbz/MyQbzGridView.qml +++ b/crates/qbz-qt/qml/views/myqbz/MyQbzGridView.qml @@ -52,7 +52,13 @@ // rather than reinvented as a MyQBZ-only mechanism. // 5. Slint reads `MyQbzState.loading` in neither grid; the cardGrid skeleton // below is the same deliberate improvement LibraryView.qml:1163-1225 -// documents. The empty state stays gated on `!loading`. +// documents. The empty state stays gated on `!loading`, and the skeleton is +// ARMED after 250ms so a fresh (empty) account does not flash a page of +// fake cards on its way to "No mixtapes yet" — see `skelArmed`. +// 6. One live instance serves BOTH routes, so per-route UI state is reset by +// hand on `kind` change (`onKindChanged`), and the two body views take a +// QML-side model (`activeItems`) so a republish does not reset the viewport +// and the inactive view builds no delegates. import QtQuick import QtQuick.Window @@ -95,6 +101,73 @@ Rectangle { } } readonly property var items: root.doc.cards || [] + // The model the two body views actually consume. NOT `root.items` + // directly, for two reasons that both showed up as "the grid fights me": + // + // 1. SCROLL RESET. `items` is a FRESH JS array out of `JSON.parse` on + // every publish, and assigning a new array to a view's `model` is a + // full model reset — the view drops to `contentY 0`. Rust publishes + // three times per visit (`set_loading(true)` -> `apply` -> + // `artwork_pass`, the last one behind a NETWORK round trip), again on + // every `reload_grids()` / `republish_all()`, and again on every + // keystroke in the search box. So the user scrolled, and a beat later + // the artwork pass yanked them back to the top. `applyItems()` carries + // the viewport across the reset, which is what Slint gets for free by + // diffing its VecModel (MixtapesView.slint:36-50 even restores it). + // 2. DOUBLE DELEGATES. `visible: false` does NOT stop delegate creation, + // so the inactive view used to build the whole card set too: 12 cards + // meant 24 `MyQbzCard`s, 24 mosaics and TWO artwork dispatches per + // card. The ternaries below hand the model to the ACTIVE view only, + // which is Slint's `if …mix-view == "grid"` / `== "list"` + // (MixtapesView.slint:186,202) — exactly one subtree alive. + property var activeItems: [] + property real keepGridY: 0 + property real keepListY: 0 + // Set while a route flip is in flight: the viewport must NOT be carried + // from Mixtapes into Collections (finding 5 — one instance serves both). + property bool suppressKeep: false + + function applyItems() { + if (!root.suppressKeep) { + root.keepGridY = grid.contentY + root.keepListY = list.contentY + } + root.activeItems = root.items + // `forceLayout()` flushes the pending re-layout synchronously, so + // `contentHeight` below is the NEW one and the clamp is honest. + if (grid.visible) { + grid.forceLayout() + grid.contentY = root.clampY(grid, root.keepGridY) + } + if (list.visible) { + list.forceLayout() + list.contentY = root.clampY(list, root.keepListY) + } + } + function clampY(view, y) { + var max = Math.max(0, view.contentHeight + view.bottomMargin - view.height) + return Math.max(0, Math.min(y, max)) + } + function releaseKeep() { root.suppressKeep = false } + + onItemsChanged: root.applyItems() + Component.onCompleted: root.applyItems() + + // Mixtapes and Collections are ONE live component (AppShell.qml:199-223 + // only flips `kind`), so every scrap of per-route UI state has to be reset + // by hand or it bleeds across the flip: the expandable search stayed OPEN + // showing the other route's query, and the viewport carried over. + // `suppressKeep` covers both possible orderings of this handler and the + // `items` re-parse that the same `kind` change triggers. + onKindChanged: { + root.suppressKeep = true + root.keepGridY = 0 + root.keepListY = 0 + grid.contentY = 0 + list.contentY = 0 + searchBox.open = false + Qt.callLater(root.releaseKeep) + } readonly property bool loading: root.doc.loading === true readonly property string search: root.doc.search || "" readonly property string sortField: root.doc.sort || "position" @@ -134,7 +207,24 @@ Rectangle { ? (root.Window.window.visibility !== Window.Minimized && root.Window.window.visibility !== Window.Hidden) : true - readonly property bool gridPending: root.loading && root.items.length === 0 + // The skeleton is ARMED, not immediate. A first load with nothing to show + // yet is indistinguishable from an account that simply has no mixtapes, so + // the old immediate `loading && items.length === 0` flashed a full page of + // fake cards at every empty account before "No mixtapes yet" resolved. The + // grid docs come out of local SQLite in a few ms, so a 250ms arming delay + // removes the flash entirely while a genuinely slow load still gets its + // skeleton. (Slint has no skeleton at all here — divergence 5.) + readonly property bool gridLoadingEmpty: root.loading && root.items.length === 0 + property bool skelArmed: false + onGridLoadingEmptyChanged: if (!root.gridLoadingEmpty) root.skelArmed = false + Timer { + interval: 250 + repeat: false + running: root.gridLoadingEmpty && !root.skelArmed + && root.visible && root.windowShowing + onTriggered: root.skelArmed = true + } + readonly property bool gridPending: root.gridLoadingEmpty && root.skelArmed Timer { interval: 900 repeat: true @@ -153,7 +243,12 @@ Rectangle { Text { anchors.left: parent.left anchors.verticalCenter: parent.verticalCenter - width: Math.max(0, parent.width - 28 - 16) + // The 28px button + its 16px gap are reserved ONLY when the button + // is there. Slint's `HorizontalLayout` with the `if`-gated button + // gives the title the whole row when it is absent + // (MixtapesView.slint:65-87); reserving it unconditionally elided + // an empty grid's title 44px early. + width: Math.max(0, parent.width - (root.populated ? 28 + 16 : 0)) text: root.isMix ? root.t("Mixtapes") : root.t("Collections") color: theme.textPrimary font.pixelSize: theme.fontSection @@ -182,6 +277,8 @@ Rectangle { layoutDirection: Qt.LeftToRight QbzLineEdit { + // Named so the route flip can collapse it — see onKindChanged. + id: searchBox anchors.verticalCenter: parent.verticalCenter searchMode: true expandable: true @@ -336,7 +433,8 @@ Rectangle { bottomMargin: 100 clip: true boundsBehavior: Flickable.StopAtBounds - model: root.items + // Only the ACTIVE view carries a model — see `activeItems`. + model: root.viewMode === "grid" ? root.activeItems : [] delegate: MyQbzCard { required property var modelData @@ -358,7 +456,7 @@ Rectangle { bottomMargin: 100 clip: true boundsBehavior: Flickable.StopAtBounds - model: root.items + model: root.viewMode === "list" ? root.activeItems : [] delegate: MyQbzCard { required property var modelData @@ -373,23 +471,29 @@ Rectangle { // Thin auto-hiding scrollbars (the ListScrollbar replica), one per body // view — LibraryView.qml:1443-1459's shape. Children of the ROOT, not of - // `body`: the .slint pins the bar at `parent.width - width - 4` of the VIEW - // and spans its FULL height (MixtapesView.slint:213-221), i.e. outside the - // 32px content padding, and the 32px right pad exists precisely so the grid - // never rides under it. + // `body`, so they sit OUTSIDE the 32px content padding (the .slint pins the + // bar at `parent.width - width - 4`, MixtapesView.slint:213-221, and the + // 32px right pad exists precisely so the grid never rides under it). + // + // VERTICALLY they track `body`, not the root. In Slint the bar and the + // flickable are the same box, so the mapping is exact; here the body starts + // 118px down. Spanning the full root height over-sized the thumb by ~18% + // and mapped the track's top 118px — visually the header/toolbar band — to + // content that is not there, so a click in the gutter beside the TOOLBAR + // scrolled the grid. QbzScrollBar { anchors.right: parent.right anchors.rightMargin: 4 - anchors.top: parent.top - anchors.bottom: parent.bottom + anchors.top: body.top + anchors.bottom: body.bottom target: grid visible: grid.visible && grid.contentHeight > grid.height } QbzScrollBar { anchors.right: parent.right anchors.rightMargin: 4 - anchors.top: parent.top - anchors.bottom: parent.bottom + anchors.top: body.top + anchors.bottom: body.bottom target: list visible: list.visible && list.contentHeight > list.height } diff --git a/crates/qbz-qt/src/myqbz_bridge.rs b/crates/qbz-qt/src/myqbz_bridge.rs index ae7bc63e2..b86e0c286 100644 --- a/crates/qbz-qt/src/myqbz_bridge.rs +++ b/crates/qbz-qt/src/myqbz_bridge.rs @@ -20,10 +20,13 @@ //! post-boot push — the same reason `theme_json` / `window_width` are seeded at //! construction (shell_bridge.rs "Seeded at CONSTRUCTION"). //! -//! No `#[qsignal]`: every user-visible outcome goes through `QbzShell.toastJson` -//! (toast_qt.rs) and every state change is a property republish, so the -//! auto-generated `Changed` signals are the only `Connections` targets QML -//! needs. +//! ONE `#[qsignal]`, `detail_rows_patched`: every user-visible outcome goes +//! through `QbzShell.toastJson` (toast_qt.rs) and every state change is a +//! property republish, so the auto-generated `Changed` signals are +//! normally the only `Connections` targets QML needs. The exception is the +//! detail list's per-row DELTA channel, where republishing the whole document +//! per row was measured quadratic — the reasoning is at the signal's own +//! declaration, and `QbzHome.foryouArtReady` is the precedent. //! //! Invokable bodies are one-line forwards into the domain controllers, exactly //! as album_bridge.rs forwards. Nothing here holds state or does work. @@ -120,9 +123,26 @@ pub mod qbz_myqbz_bridge { fn detail_toggle_source_filter(self: Pin<&mut QbzMyQbz>, kind: QString); /// `mode` ∈ list | grid | expanded; `expanded` also fires /// `ensure_expanded`. No re-derive — caches, selection and scroll - /// position all survive (myqbz_detail.rs:558-562). + /// position all survive (myqbz_detail.rs:558-562). Since the accordion + /// landed, `expanded` = OPEN ALL and leaving it = close all, through + /// the same per-row state `detail_toggle_row_expand` writes. #[qinvokable] fn detail_set_view_mode(self: Pin<&mut QbzMyQbz>, mode: QString); + /// THE CHEVRON: toggle ONE row's inline block open/closed, by its + /// `sourceItemId`. A first open fetches only that row's tracks; a + /// second is instant off `INLINE_CACHE`. Non-expandable rows (tracks) + /// are a no-op. Owner-authorised divergence from both references, + /// which have no per-row accordion at all. + #[qinvokable] + fn detail_toggle_row_expand(self: Pin<&mut QbzMyQbz>, source_item_id: QString); + /// Re-publish the open detail document unchanged, for a FRESH view + /// instance. `AppShell.qml:192` mounts this view through a `Loader`, so + /// nav-away destroys it; the `detail_rows_patched` deltas below were + /// delivered to the dead instance and the new one would come back with + /// stale quality badges, no inline tracks and every row collapsed. + /// Called from `Component.onCompleted` — once per mount. + #[qinvokable] + fn detail_resync(self: Pin<&mut QbzMyQbz>); /// Leaving select mode clears the selection. #[qinvokable] fn detail_toggle_select_mode(self: Pin<&mut QbzMyQbz>); @@ -258,6 +278,33 @@ pub mod qbz_myqbz_bridge { /// Clears the stored path (does NOT store a default path). #[qinvokable] fn branding_reset_icon(self: Pin<&mut QbzMyQbz>); + + // --- Detail: the row-patch channel ---------------------------------- + + /// PARTIAL row updates for the detail list, as + /// `{ "id": , "rows": [ { "position", "sourceItemId", + /// ...only the changed fields } ] }`. The QML host `Object.assign`s + /// each entry onto the live row object it finds by `position` and bumps + /// its `patchRev` once for the whole batch. + /// + /// This is the ONE `#[qsignal]` in this bridge and the module header's + /// "every state change is a property republish" has a stated exception + /// here, for the same reason `QbzHome.foryouArtReady` exists: a + /// per-item republish of a whole document is quadratic. `apply_resolved` + /// used to clone + serialise the ENTIRE `DetailDoc` per resolved item + /// (200 serialize -> parse -> patch cycles on the GUI thread for a big + /// artist_collection) and `patch_expanded` did it while the document + /// carried every loaded row's `inlineTracks`. + /// + /// A SIGNAL rather than an eighth `#[qproperty]`: a property setter + /// compares before emitting, so two identical consecutive deltas would + /// drop the second. And because a signal is a delta, a view mounted + /// after it has missed it — `detail_resync` closes that hole. + /// + /// `#[auto_cxx_name]` makes the QML spelling `detailRowsPatched`, i.e. + /// the handler is `onDetailRowsPatched(patchJson)`. + #[qsignal] + fn detail_rows_patched(self: Pin<&mut QbzMyQbz>, patch_json: QString); } impl cxx_qt::Threading for QbzMyQbz {} @@ -385,6 +432,14 @@ impl qbz_myqbz_bridge::QbzMyQbz { crate::myqbz_detail_qt::set_view_mode(&mode.to_string()); } + pub fn detail_toggle_row_expand(self: Pin<&mut Self>, source_item_id: QString) { + crate::myqbz_detail_qt::toggle_row_expand(&source_item_id.to_string()); + } + + pub fn detail_resync(self: Pin<&mut Self>) { + crate::myqbz_detail_qt::resync(); + } + pub fn detail_toggle_select_mode(self: Pin<&mut Self>) { crate::myqbz_detail_qt::toggle_select_mode(); } diff --git a/crates/qbz-qt/src/myqbz_detail_qt.rs b/crates/qbz-qt/src/myqbz_detail_qt.rs index 457beff67..af1c7e6d2 100644 --- a/crates/qbz-qt/src/myqbz_detail_qt.rs +++ b/crates/qbz-qt/src/myqbz_detail_qt.rs @@ -146,6 +146,19 @@ pub struct DetailRow { pub selected: bool, #[serde(rename = "canExpand")] pub can_expand: bool, + /// THE per-row accordion state — is this row's inline block open? + /// + /// ADDITIVE, and a deliberate divergence from BOTH references + /// (owner-authorised 2026-07-30): neither `MixtapeDetailView.slint` nor the + /// Tauri build had a per-row accordion, they render inline tracks only as a + /// whole-list VIEW MODE ("no chevron"). The owner asked for the accordion + /// anyway; the view mode SURVIVES and now simply means "open all". + /// + /// It is ONE notion of open, not two: the chevron and the `expanded` + /// segment both write `OPEN_ROWS`, and this flag is derived from that set + /// in `to_item`. Do not add a second `viewMode === "expanded"` test in QML. + #[serde(rename = "rowOpen")] + pub row_open: bool, #[serde(rename = "tracksLoaded")] pub tracks_loaded: bool, #[serde(rename = "expandLoading")] @@ -154,6 +167,71 @@ pub struct DetailRow { pub inline_tracks: Vec, } +/// A PARTIAL row update for the `detailRowsPatched` channel — only the fields +/// that actually changed travel. QML `Object.assign`s each entry onto the live +/// row object it finds by `position` and bumps `patchRev` once for the batch. +/// +/// This is the shape fix for the republish storm the audit measured +/// (`2026-07-30-myqbz-audit/detail.md` #7/#8): `apply_resolved` cloned and +/// `serde_json::to_string`'d the ENTIRE `DetailDoc` once per resolved item — +/// 34 full serialize -> parse -> patch cycles for one collection, 200 for a big +/// one, all on the GUI thread — and `patch_expanded` did the same while the +/// document carried every already-loaded row's `inlineTracks`, i.e. O(n²). +/// The QML side already patches row objects IN PLACE, so the document never +/// needed to travel whole for a one-row change. +/// +/// `position` is the stable persisted key and the only join column QML needs; +/// `sourceItemId` rides along for logging and for a QML-side sanity check. +/// A patch is a DELTA — a freshly mounted view has missed every delta already +/// sent, which is why `detail_resync` exists. +#[derive(Clone, Default, Serialize)] +struct RowPatch { + position: i32, + #[serde(rename = "sourceItemId")] + source_item_id: String, + #[serde(rename = "sourceKind", skip_serializing_if = "Option::is_none")] + source_kind: Option, + #[serde(rename = "qualityTier", skip_serializing_if = "Option::is_none")] + quality_tier: Option, + #[serde(rename = "qualityDetail", skip_serializing_if = "Option::is_none")] + quality_detail: Option, + #[serde(rename = "typeLabel", skip_serializing_if = "Option::is_none")] + type_label: Option, + #[serde(rename = "artistId", skip_serializing_if = "Option::is_none")] + artist_id: Option, + #[serde(rename = "qualityResolving", skip_serializing_if = "Option::is_none")] + quality_resolving: Option, + #[serde(rename = "artUrl", skip_serializing_if = "Option::is_none")] + art_url: Option, + #[serde(rename = "artPath", skip_serializing_if = "Option::is_none")] + art_path: Option, + /// The alias twin of `artUrl` — `publish` fills both in one place, so a row + /// patch that moves one MUST move the other or the two go out of step. + #[serde(rename = "artworkUrl", skip_serializing_if = "Option::is_none")] + artwork_url: Option, + #[serde(rename = "artworkPath", skip_serializing_if = "Option::is_none")] + artwork_path: Option, + #[serde(rename = "rowOpen", skip_serializing_if = "Option::is_none")] + row_open: Option, + #[serde(rename = "tracksLoaded", skip_serializing_if = "Option::is_none")] + tracks_loaded: Option, + #[serde(rename = "expandLoading", skip_serializing_if = "Option::is_none")] + expand_loading: Option, + #[serde(rename = "inlineTracks", skip_serializing_if = "Option::is_none")] + inline_tracks: Option>, +} + +impl RowPatch { + /// An empty patch addressed at `row`. Fill only what changed. + fn of(row: &DetailRow) -> Self { + Self { + position: row.position, + source_item_id: row.source_item_id.clone(), + ..Self::default() + } + } +} + /// The published detail document (spec 02 §5.2 `DetailDoc`). #[derive(Clone, Serialize)] pub struct DetailDoc { @@ -281,6 +359,16 @@ static FULL_ITEMS: Mutex> = Mutex::new(Vec::new()); static INLINE_CACHE: LazyLock>>> = LazyLock::new(|| Mutex::new(HashMap::new())); +/// THE per-row accordion OPEN set, same key as `INLINE_CACHE`. +/// +/// ONE notion of "is this row open", written by BOTH mechanisms: the chevron +/// (`toggle_row_expand`, one key) and the `expanded` view-mode segment +/// (`set_view_mode` / `rebuild`, every expandable key = "open all"). Two +/// parallel states would disagree the first time a user closed a row inside +/// expanded mode. Surviving a re-derive is deliberate: a sort or a search must +/// not fold the rows the user opened. +static OPEN_ROWS: LazyLock>> = LazyLock::new(|| Mutex::new(HashSet::new())); + /// resolveItems results, same key. A MISS is what flags a row /// `qualityResolving` (the skeleton). static RESOLVE_CACHE: LazyLock>> = @@ -338,6 +426,46 @@ fn publish_current() { publish(&doc); } +/// Send a batch of PARTIAL row updates over `QbzMyQbz.detailRowsPatched` +/// instead of republishing the whole document (see `RowPatch`). +/// +/// A SIGNAL, not a property: cxx-qt property setters compare before emitting, +/// so two identical consecutive patches would silently drop the second, and a +/// delta must never be deduplicated. The `id` is the collection the patch was +/// computed against — QML drops a batch addressed at another document, which is +/// the same guard `apply_resolved` makes on the Rust side. +fn publish_rows(collection_id: &str, patches: Vec) { + if patches.is_empty() { + return; + } + #[derive(Serialize)] + struct Envelope<'a> { + id: &'a str, + rows: &'a [RowPatch], + } + let json = serde_json::to_string(&Envelope { + id: collection_id, + rows: &patches, + }) + .unwrap_or_else(|_| "{}".into()); + crate::myqbz_bridge::ui(move |mut b| { + b.as_mut() + .detail_rows_patched(QString::from(json.as_str())); + }); +} + +/// Re-hand the current document to a FRESH view instance. +/// +/// `AppShell.qml:192` mounts this view through a `Loader { source: ... }`, so +/// nav-away DESTROYS it and nav-back builds a new one whose only input is the +/// last published `detailJson`. Every `RowPatch` since that publish is a delta +/// the new instance never saw — resolved quality badges, loaded inline tracks +/// and open rows would all come back missing. One full publish per mount is the +/// price of the patch channel, and it is paid exactly once. +pub(crate) fn resync() { + publish_current(); +} + // --------------------------------------------------------------------------- // String helpers (myqbz_detail.rs:177-249) // --------------------------------------------------------------------------- @@ -489,6 +617,7 @@ fn to_item( resolved_cache: &HashMap, inline_cache: &HashMap>, selected: &HashSet, + open_rows: &HashSet, ) -> DetailRow { let source = source_str(item.source); // `small_qobuz_url` only rewrites Qobuz CDN `_.jpg` urls; running it @@ -539,6 +668,10 @@ fn to_item( None => (Vec::new(), false), }; + // A track has nothing to show, so it never carries an open state — the + // chevron cell stays empty for it and its row body keeps the whole gesture. + let can_expand = matches!(item.item_type, ItemType::Album | ItemType::Playlist); + DetailRow { position: item.position, item_type: item_type_str(item.item_type).to_string(), @@ -565,7 +698,11 @@ fn to_item( artwork_url: String::new(), artwork_path: String::new(), selected: selected.contains(&item.position), - can_expand: matches!(item.item_type, ItemType::Album | ItemType::Playlist), + can_expand, + // Re-derives (sort / search / filter) rebuild every row, so the open + // state has to be re-read off the set here or the accordion would snap + // shut on the first keystroke. + row_open: can_expand && open_rows.contains(&key), tracks_loaded, expand_loading: false, inline_tracks, @@ -576,12 +713,39 @@ fn build_rows(items: &[MixtapeCollectionItem]) -> Vec { let resolved = RESOLVE_CACHE.lock().unwrap(); let inline = INLINE_CACHE.lock().unwrap(); let selected = SELECTED.lock().unwrap(); + let open_rows = OPEN_ROWS.lock().unwrap(); items .iter() - .map(|it| to_item(it, &resolved, &inline, &selected)) + .map(|it| to_item(it, &resolved, &inline, &selected, &open_rows)) .collect() } +/// Force every expandable row OPEN, in the document AND in the shared set — the +/// `expanded` view-mode's whole meaning. Returns the rows it changed as +/// patches, so a caller that is not already republishing can send just those. +/// +/// Called from `rebuild` as well as `set_view_mode` because a re-derive can +/// REVEAL a row (a filter cleared, a search narrowed then widened) that has +/// never been opened: without this it would render collapsed among its open +/// neighbours and the segment would be lying. +fn open_all_expandable(d: &mut DetailDoc) -> Vec { + let mut open = OPEN_ROWS.lock().unwrap(); + let mut patches = Vec::new(); + for row in d.items.iter_mut() { + if !row.can_expand { + continue; + } + open.insert(cache_key(&row.source, &row.source_item_id)); + if !row.row_open { + row.row_open = true; + let mut patch = RowPatch::of(row); + patch.row_open = Some(true); + patches.push(patch); + } + } + patches +} + /// The source word a RESOLVED track renders as — the glyph kind in /// `SourceIcon.qml` and the `sourceKind` field of a detail row. /// @@ -739,6 +903,14 @@ fn rebuild(d: &mut DetailDoc) { let view = filtered_sorted(d); d.items = build_rows(&view); + // `expanded` means OPEN ALL. The rows were just rebuilt off `OPEN_ROWS`, + // so this only ever adds the ones the re-derive revealed. (A row closed by + // its chevron WHILE in expanded mode therefore re-opens on the next + // re-derive — the price of one shared state, and the honest reading of a + // segment whose meaning is "all of them".) + if d.view_mode == "expanded" { + open_all_expandable(d); + } d.selected_count = SELECTED.lock().unwrap().len() as i32; let source_count = i32::from(d.src_qobuz) + i32::from(d.src_plex) + i32::from(d.src_local); @@ -774,9 +946,12 @@ pub(crate) fn refresh_view() { let missing = attach_artwork(); publish_current(); dispatch_downloads(missing); - if with_doc(|d| d.view_mode == "expanded") { - ensure_expanded(); - } + // Unconditional now that `ensure_expanded` keys off the per-row OPEN state + // rather than the view mode: with nothing open it is one pass over the + // rows and no fetch, and it covers the accordion in LIST mode too (a row + // the user opened, then filtered out and back in, has lost its in-flight + // spinner and must be re-fetched). + ensure_expanded(); } // --------------------------------------------------------------------------- @@ -968,6 +1143,7 @@ fn reset() { FULL_ITEMS.lock().unwrap().clear(); INLINE_CACHE.lock().unwrap().clear(); RESOLVE_CACHE.lock().unwrap().clear(); + OPEN_ROWS.lock().unwrap().clear(); SELECTED.lock().unwrap().clear(); *LAST_FILTER_KEY.lock().unwrap() = None; // Close the persist gate until `apply` restores this collection's prefs. @@ -1096,6 +1272,12 @@ fn resolve_from_tracks(item: &MixtapeCollectionItem, tracks: &[QueueTrack]) -> R /// array replacement would reset `ListView.contentY` and throw the user to the /// top (spec 02 §9.1) — and this is not a re-derive, so it must not clear the /// selection either. +/// +/// The patch now travels as a `RowPatch` over `detailRowsPatched` instead of a +/// clone + full serialisation of the document per resolved item (audit #7): the +/// document stays authoritative in `DOC`, only the changed fields cross the +/// hop. Rows are matched with `filter`, not `find`, so a collection holding the +/// same album twice hydrates both copies rather than only the first. fn apply_resolved(item: &MixtapeCollectionItem, resolved: ResolvedItem, collection_id: &str) { let key = cache_key(source_str(item.source), &item.source_item_id); RESOLVE_CACHE @@ -1109,15 +1291,16 @@ fn apply_resolved(item: &MixtapeCollectionItem, resolved: ResolvedItem, collecti .map(|u| u.is_empty()) .unwrap_or(true); - let (doc, backfill) = with_doc(|d| { + let (patches, backfill) = with_doc(|d| { + let mut patches: Vec = Vec::new(); if d.id != collection_id { - return (None, None); + return (patches, None); } let mut backfill: Option = None; - if let Some(row) = d + for row in d .items .iter_mut() - .find(|r| r.source_item_id == item.source_item_id) + .filter(|r| r.source_item_id == item.source_item_id) { row.source_kind = resolved.source_kind.clone(); row.quality_tier = resolved.quality_tier.clone(); @@ -1125,19 +1308,32 @@ fn apply_resolved(item: &MixtapeCollectionItem, resolved: ResolvedItem, collecti row.type_label = resolved.type_label.clone(); row.artist_id = resolved.artist_id.clone(); row.quality_resolving = false; + + let mut patch = RowPatch::of(row); + patch.source_kind = Some(row.source_kind.clone()); + patch.quality_tier = Some(row.quality_tier.clone()); + patch.quality_detail = Some(row.quality_detail.clone()); + patch.type_label = Some(row.type_label.clone()); + patch.artist_id = Some(row.artist_id.clone()); + patch.quality_resolving = Some(false); + if row.art_url.is_empty() && !resolved.artwork_url.is_empty() { row.art_url = resolved.artwork_url.clone(); row.art_path = crate::artwork_qt::cached_path(&row.art_url); - if row.art_path.is_empty() && stored_art_empty { + if row.art_path.is_empty() && stored_art_empty && backfill.is_none() { backfill = Some(row.art_url.clone()); } + // Both spellings, exactly as `publish` fills them. + patch.art_url = Some(row.art_url.clone()); + patch.art_path = Some(row.art_path.clone()); + patch.artwork_url = Some(row.art_url.clone()); + patch.artwork_path = Some(row.art_path.clone()); } + patches.push(patch); } - (Some(d.clone()), backfill) + (patches, backfill) }); - if let Some(doc) = doc { - publish(&doc); - } + publish_rows(collection_id, patches); if let Some(url) = backfill { crate::spawn(async move { crate::artwork_qt::download_missing(vec![url]).await; @@ -1212,29 +1408,41 @@ fn full_item_by_source_id(source_item_id: &str) -> Option .cloned() } -/// Ensure every expandable row's inline tracks are loaded. Marks the pending -/// rows loading in ONE pass, publishes, then resolves each. Idempotent — -/// already-cached rows are skipped, so re-entering expanded mode is instant. +/// Ensure every OPEN row's inline tracks are loaded. Marks the pending rows +/// loading in ONE pass, sends that as a row patch, then resolves each. +/// Idempotent — already-cached rows are skipped, so re-entering expanded mode +/// is instant. +/// +/// The gate is `row_open`, not `can_expand`: with the accordion, "expandable" +/// is no longer "will be shown". In expanded mode every expandable row IS open +/// (`rebuild` / `set_view_mode`), so the old behaviour is unchanged; in list +/// mode this fetches only what the user actually opened, which is what makes +/// the chevron lazy. `toggle_row_expand` fetches its own single row directly +/// rather than routing through here, so one chevron never sweeps the list. /// /// DEVIATION (reported): the fan-out is capped at `MAX_RESOLVE_CONCURRENCY`, /// the same cap T18 mandates for `resolve_items`. The reference is uncapped and /// an expanded 200-item artist_collection would open 200 sockets; the visible /// behaviour (rows filling in progressively) is identical. pub(crate) fn ensure_expanded() { - let (collection_id, pending) = with_doc(|d| { + let (collection_id, pending, patches) = with_doc(|d| { let mut pending: Vec = Vec::new(); + let mut patches: Vec = Vec::new(); for row in d.items.iter_mut() { - if row.can_expand && !row.tracks_loaded && !row.expand_loading { + if row.row_open && !row.tracks_loaded && !row.expand_loading { row.expand_loading = true; + let mut patch = RowPatch::of(row); + patch.expand_loading = Some(true); + patches.push(patch); pending.push(row.source_item_id.clone()); } } - (d.id.clone(), pending) + (d.id.clone(), pending, patches) }); if pending.is_empty() { return; } - publish_current(); + publish_rows(&collection_id, patches); let runtime = crate::app(); crate::spawn(async move { @@ -1246,19 +1454,7 @@ pub(crate) fn ensure_expanded() { let collection_id = collection_id.clone(); handles.push(tokio::spawn(async move { let _permit = sem.acquire().await.expect("semaphore open"); - if current_id() != collection_id { - return; - } - let Some(item) = full_item_by_source_id(&source_item_id) else { - // No backing item (should not happen) — clear the spinner. - patch_expanded(&source_item_id, &collection_id, None); - return; - }; - let tracks = crate::myqbz_play_qt::fetch_item_tracks(&runtime, &item).await; - let rows: Vec = tracks.iter().map(track_to_item).collect(); - let key = cache_key(source_str(item.source), &item.source_item_id); - INLINE_CACHE.lock().unwrap().insert(key, rows.clone()); - patch_expanded(&source_item_id, &collection_id, Some(rows)); + fetch_row_tracks(&runtime, source_item_id, collection_id).await; })); } for handle in handles { @@ -1267,30 +1463,124 @@ pub(crate) fn ensure_expanded() { }); } -/// Patch ONE row's expansion state in place (never a re-derive). -fn patch_expanded( - source_item_id: &str, - collection_id: &str, - rows: Option>, +/// Fetch ONE row's tracks, cache them, and patch that row. The single body +/// behind both entry points — `ensure_expanded`'s capped fan-out and the +/// chevron's `toggle_row_expand` — so the two can never drift. +/// +/// Re-checks the open collection first: the user may have navigated away while +/// this task queued behind the concurrency cap, and the result is then dead +/// weight (`patch_expanded` drops it a second time on the same test). +async fn fetch_row_tracks( + runtime: &Arc>, + source_item_id: String, + collection_id: String, ) { - let doc = with_doc(|d| { + if current_id() != collection_id { + return; + } + let Some(item) = full_item_by_source_id(&source_item_id) else { + // No backing item (should not happen) — clear the spinner. + patch_expanded(&source_item_id, &collection_id, None); + return; + }; + let tracks = crate::myqbz_play_qt::fetch_item_tracks(runtime, &item).await; + let rows: Vec = tracks.iter().map(track_to_item).collect(); + let key = cache_key(source_str(item.source), &item.source_item_id); + INLINE_CACHE.lock().unwrap().insert(key, rows.clone()); + patch_expanded(&source_item_id, &collection_id, Some(rows)); +} + +/// Patch ONE row's expansion state in place (never a re-derive). +/// +/// Sends a `RowPatch`, not the document (audit #8): the document carries EVERY +/// loaded row's `inlineTracks`, so republishing it per expanded item made +/// entering expanded mode on a 200-item collection quadratic — item 200's +/// publish serialised ~3000 track objects that QML then re-parsed. +fn patch_expanded(source_item_id: &str, collection_id: &str, rows: Option>) { + let patches = with_doc(|d| { + let mut patches: Vec = Vec::new(); if d.id != collection_id { - return None; + return patches; } - let row = d + for row in d .items .iter_mut() - .find(|r| r.source_item_id == source_item_id)?; - row.expand_loading = false; - if let Some(rows) = rows { - row.tracks_loaded = true; - row.inline_tracks = rows; + .filter(|r| r.source_item_id == source_item_id) + { + row.expand_loading = false; + let mut patch = RowPatch::of(row); + patch.expand_loading = Some(false); + if let Some(ref rows) = rows { + row.tracks_loaded = true; + row.inline_tracks = rows.clone(); + patch.tracks_loaded = Some(true); + patch.inline_tracks = Some(rows.clone()); + } + patches.push(patch); } - Some(d.clone()) + patches }); - if let Some(doc) = doc { - publish(&doc); + publish_rows(collection_id, patches); +} + +/// THE CHEVRON — toggle ONE row's inline block (the feature the owner asked +/// for: "una fila colapsable con el contenido dentro"). +/// +/// Opening fetches only THIS row's tracks, and only the first time: a second +/// open is a pure state flip because `INLINE_CACHE` survives, `to_item` +/// re-hydrates `inlineTracks` from it, and `tracks_loaded` short-circuits the +/// fetch. Closing never drops the cache. +/// +/// It writes the SAME `OPEN_ROWS` set the `expanded` view-mode segment writes, +/// which is what keeps "open all" and "open this one" from disagreeing. The +/// view mode itself is NOT touched — a chevron in list mode does not flip the +/// segment, and closing every row by hand does not leave the segment stranded. +pub(crate) fn toggle_row_expand(source_item_id: &str) { + let mut fetch = false; + let (collection_id, patches) = with_doc(|d| { + let mut patches: Vec = Vec::new(); + let id = d.id.clone(); + let Some(row) = d + .items + .iter_mut() + .find(|r| r.source_item_id == source_item_id) + else { + return (id, patches); + }; + if !row.can_expand { + return (id, patches); + } + let now_open = !row.row_open; + { + let key = cache_key(&row.source, &row.source_item_id); + let mut open = OPEN_ROWS.lock().unwrap(); + if now_open { + open.insert(key); + } else { + open.remove(&key); + } + } + row.row_open = now_open; + + let mut patch = RowPatch::of(row); + patch.row_open = Some(now_open); + if now_open && !row.tracks_loaded && !row.expand_loading { + row.expand_loading = true; + patch.expand_loading = Some(true); + fetch = true; + } + patches.push(patch); + (id, patches) + }); + publish_rows(&collection_id, patches); + if !fetch || collection_id.is_empty() { + return; } + let source_item_id = source_item_id.to_string(); + let runtime = crate::app(); + crate::spawn(async move { + fetch_row_tracks(&runtime, source_item_id, collection_id).await; + }); } // --------------------------------------------------------------------------- @@ -1354,9 +1644,26 @@ pub(crate) fn reset_filters() { /// `list` | `grid` | `expanded`. The reference does NOT re-derive here (the /// item list is unchanged, only the layout switches), so neither do we: mutate, /// persist, PUBLISH. The caches, the selection and the scroll position survive. -/// `expanded` additionally fires `ensure_expanded`. +/// +/// The segment now drives the SAME per-row open state the chevron does: +/// `expanded` = open all, and leaving it = close all. Without the close-all arm +/// the list would keep every inline block rendered after switching back and the +/// segment would appear to do nothing. Toggling list <-> grid leaves the set +/// alone, so a row opened in list mode is still open when you come back from +/// the grid. pub(crate) fn set_view_mode(mode: &str) { - with_doc(|d| d.view_mode = mode.to_string()); + let was_expanded = with_doc(|d| d.view_mode == "expanded"); + with_doc(|d| { + d.view_mode = mode.to_string(); + if mode == "expanded" { + open_all_expandable(d); + } else if was_expanded { + OPEN_ROWS.lock().unwrap().clear(); + for row in d.items.iter_mut() { + row.row_open = false; + } + } + }); persist_prefs(); publish_current(); if mode == "expanded" { @@ -1560,6 +1867,7 @@ pub(crate) fn teardown() { FULL_ITEMS.lock().unwrap().clear(); INLINE_CACHE.lock().unwrap().clear(); RESOLVE_CACHE.lock().unwrap().clear(); + OPEN_ROWS.lock().unwrap().clear(); SELECTED.lock().unwrap().clear(); *LAST_FILTER_KEY.lock().unwrap() = None; PREFS_HYDRATED.store(false, Ordering::SeqCst); @@ -1715,6 +2023,55 @@ mod tests { assert_ne!(cache_key("qobuz", "1"), cache_key("local", "1")); } + /// The accordion's ONE state: `rowOpen` is derived from the shared set on + /// every rebuild (so a sort or a keystroke cannot fold an open row), and a + /// TRACK never carries it — it has nothing to show, and its row body must + /// keep the whole click gesture. + #[test] + fn row_open_is_derived_from_the_shared_open_set() { + let resolved = HashMap::new(); + let inline = HashMap::new(); + let selected = HashSet::new(); + let album = item(0, "A", ItemType::Album, AlbumSource::Qobuz); + let track = item(1, "B", ItemType::Track, AlbumSource::Qobuz); + + let mut open = HashSet::new(); + open.insert(cache_key("qobuz", &album.source_item_id)); + open.insert(cache_key("qobuz", &track.source_item_id)); + + let album_row = to_item(&album, &resolved, &inline, &selected, &open); + assert!(album_row.can_expand); + assert!(album_row.row_open); + + let track_row = to_item(&track, &resolved, &inline, &selected, &open); + assert!(!track_row.can_expand); + assert!(!track_row.row_open); + + let closed: HashSet = HashSet::new(); + assert!(!to_item(&album, &resolved, &inline, &selected, &closed).row_open); + } + + /// A patch carries the join key plus ONLY what changed — the point of the + /// channel. An untouched field must be absent, not `null`, so QML's + /// `Object.assign` cannot blank a value it was not told about. + #[test] + fn row_patch_serialises_only_the_changed_fields() { + let mut row = DetailRow { + position: 7, + source_item_id: "abc".into(), + ..DetailRow::default() + }; + row.row_open = true; + let mut patch = RowPatch::of(&row); + patch.row_open = Some(true); + let json = serde_json::to_string(&patch).unwrap(); + assert!(json.contains("\"position\":7")); + assert!(json.contains("\"sourceItemId\":\"abc\"")); + assert!(json.contains("\"rowOpen\":true")); + assert!(!json.contains("qualityTier")); + assert!(!json.contains("inlineTracks")); + } + #[test] fn filter_key_tracks_every_derive_input() { let mut a = DetailDoc::default(); diff --git a/crates/qbz-qt/src/playback_qt.rs b/crates/qbz-qt/src/playback_qt.rs index 1e6fd5986..74936a6c5 100644 --- a/crates/qbz-qt/src/playback_qt.rs +++ b/crates/qbz-qt/src/playback_qt.rs @@ -18,7 +18,7 @@ //! live-updated by Settings > Audio (settings_qt). The #638 device-cap clamp //! lives in the Slint glue and is NOT ported. //! - Offline-cache tier (offline bytes), prefetch warming, stop-after, -//! infinite refill, QConnect branches, recently-played recording. +//! infinite refill, QConnect branches. //! //! DONE, previously listed here — kept as one line each so the list above //! only ever names REAL gaps: @@ -36,6 +36,10 @@ //! published as `np_seekable_max`. //! - Shuffle-play reorders the queue instead of latching the global shuffle //! MODE (#17) — `xorshift_shuffle` in `play_track_list_in`. +//! - Recently-played / Most-played recording — `recently_qt::record_queue_track` +//! on the de-duped track edge, EPHEMERAL-EXEMPT: an ephemeral folder plays +//! and leaves no row behind (owner rule, 2026-07-30). The Slint reference +//! does NOT have that exemption; do not "restore parity" by removing it. use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; use std::sync::{Arc, RwLock}; @@ -1479,8 +1483,20 @@ pub fn start_poll_loop(runtime: Arc>) { // same file the other frontends write; without this the Qt build // shows their history and never adds to it. Same de-duped edge // as the scrobblers, never from refresh_now_playing. - if let Some(track) = runtime.core().get_queue_state().await.current_track { - crate::recently_qt::record_queue_track(&track); + // + // EPHEMERAL (owner rule): an ephemeral folder "solo se reproduce + // y ya" and "no deja rastros en las bibliotecas" — it may be + // added to the queue and to nothing else, so a play of it must + // NOT land in Recently Played / Most Played. The player carries + // the synthetic id verbatim (`local_ephemeral::play_file` hands + // `row_id` to the engine), so the poll's own `track_id` is the + // cheapest place to decide, before the queue-state fetch. + // `record_queue_track` re-checks the same predicate, so the rule + // holds even if another call site appears. + if !crate::local_ephemeral::is_ephemeral_id(track_id as i64) { + if let Some(track) = runtime.core().get_queue_state().await.current_track { + crate::recently_qt::record_queue_track(&track); + } } last_track_id = track_id; // The engine may have reached this track through a GAPLESS diff --git a/crates/qbz-qt/src/recently_qt.rs b/crates/qbz-qt/src/recently_qt.rs index fee8c437c..acbbd3f8e 100644 --- a/crates/qbz-qt/src/recently_qt.rs +++ b/crates/qbz-qt/src/recently_qt.rs @@ -119,6 +119,17 @@ fn derive_albums(tracks: &[RecentTrack]) -> Vec { albums } +/// True when a stored track id names an EPHEMERAL (in-memory, session-only) +/// track and therefore must never be persisted here. +/// +/// Ephemeral ids are synthetic and `>= EPHEMERAL_ID_FLOOR` (2^48). Qobuz ids +/// and `local_tracks` row ids parse far below the floor; Plex rating_keys and +/// the empty id do not parse as `i64` at all — all of those keep recording. +fn is_ephemeral_track_id(id: &str) -> bool { + id.parse::() + .is_ok_and(crate::local_ephemeral::is_ephemeral_id) +} + /// Read the whole store. Missing / unreadable -> empty (the rails then render /// their placeholders, never an error). fn read_store() -> RecentStore { @@ -170,15 +181,22 @@ pub(crate) fn load_albums() -> Vec { /// id, capped at [`MAX_RECENT`]) and its album at the front of the album /// history (dedup by album id, capped at [`MAX_RECENT_ALBUMS`]). /// -/// GLUE: this is the write half; nothing in the Qt port calls it yet, so the -/// rails currently render whatever the Slint/Tauri builds recorded. The call -/// site is the playback track-start edge in `playback_qt.rs` — see the "GLUE -/// NEEDED" note in the handoff. +/// WIRED: the only caller is [`record_queue_track`], from the de-duped +/// playback track-start edge in `playback_qt.rs`. Ephemeral ids are rejected +/// here as well as there — see [`is_ephemeral_track_id`]. #[allow(dead_code)] pub(crate) fn record(track: RecentTrack) { if track.id.is_empty() { return; } + // EPHEMERAL — store invariant, not just a call-site courtesy. An ephemeral + // id must never reach this JSON, whoever calls. Real ids parse well below + // `EPHEMERAL_ID_FLOOR` (2^48) and Plex rating_keys do not parse as i64 at + // all, so this rejects exactly the synthetic ids and nothing else. See the + // long note on `record_queue_track` for the why. + if is_ephemeral_track_id(&track.id) { + return; + } let mut store = read_store(); if !track.album_id.is_empty() { store.albums.retain(|a| a.id != track.album_id); @@ -222,6 +240,29 @@ pub(crate) fn record(track: RecentTrack) { /// every album play/enqueue entry point has to route a group key to the local /// path instead of `get_album` — see `playback_qt::is_local_album`. pub(crate) fn record_queue_track(track: &qbz_models::QueueTrack) { + // EPHEMERAL — owner rule: "ephemeral solo se reproduce y ya […] honramos el + // nombre ephemeral: no deja rastros en las bibliotecas, ni now playing ni + // nada." An ephemeral folder lives in memory for one session and may be + // added to the QUEUE and to nothing else. Recording it here would write it + // into TWO app-wide persistent stores that outlive the process — + // `recently_played.json` (Recently Played Albums / Tracks) and + // `album_play_history.db` (Most Played Albums) — naming a folder that is + // not in the library, under synthetic ids that are meaningless after + // restart. Neither store can express "temporary", so the only correct move + // is to never write. Guarded HERE, above both writes, so any future caller + // inherits the rule; the poll-loop call site skips it earlier too, purely + // to save the queue-state fetch. + // + // NOTE: the Slint reference (`qbz/src/playback.rs::record_recent`) has no + // such guard — this is the rule being implemented for the first time, not + // a Qt-only regression being patched. + if crate::local_ephemeral::is_ephemeral_id(track.id as i64) { + log::debug!( + "[qbz-qt] ephemeral track {} not recorded in play history (no library traces)", + track.id + ); + return; + } let tier = crate::home_qt::quality_tier_from_depth(track.bit_depth); let detail = crate::home_qt::quality_detail_from_parts(track.bit_depth, track.sample_rate); let quality_label = if tier.is_empty() { @@ -300,6 +341,25 @@ mod tests { assert_eq!(derived[1].id, "b"); } + /// The owner's ephemeral rule: a session-only folder leaves no trace in the + /// play history. Pure predicate test — it never touches the real store. + #[test] + fn ephemeral_ids_are_rejected_by_the_history_store() { + let floor = qbz_library::ephemeral::EPHEMERAL_ID_FLOOR; + assert!(is_ephemeral_track_id(&floor.to_string())); + assert!(is_ephemeral_track_id(&(floor + 42).to_string())); + } + + #[test] + fn real_ids_still_record() { + // Qobuz track id, local_tracks row id, a Plex rating_key and the empty + // id: none of them may be mistaken for ephemeral. + assert!(!is_ephemeral_track_id("139578884")); + assert!(!is_ephemeral_track_id("2954")); + assert!(!is_ephemeral_track_id("plex-12345")); + assert!(!is_ephemeral_track_id("")); + } + #[test] fn tracks_without_an_album_are_skipped_by_the_derive() { let tracks = vec![RecentTrack {