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 {