fix(navigation): include local library tabs in history

Record the outgoing tab state and scroll before switching, restore same-view history entries, and route sidebar tab requests through the same path without duplicating the initial landing. Add shared CI and local regressions for source filtering and tab transitions.

cx
This commit is contained in:
vicrodh committed 2026-09-06 14:34:41 -06:00
1 parent bf84f2e65b
commit f9578d042c
9 files changed
+229 -25

No files matched your search

+2 -1
View File
@@ -16,6 +16,7 @@
# qbz-qt`, the Slint-free dep-graph gate, and an OFFSCREEN BOOT of # qbz-qt`, the Slint-free dep-graph gate, and an OFFSCREEN BOOT of
# debug AND release binaries (including process liveness). # debug AND release binaries (including process liveness).
# Release also boots on Xvfb with a deliberately silent D-Bus. # Release also boots on Xvfb with a deliberately silent D-Bus.
# Node executes the actual Local Library filter/tab expressions.
# This is the thing `cargo check` cannot give: # This is the thing `cargo check` cannot give:
# a missing QML component or a call to a bridge member that does # a missing QML component or a call to a bridge member that does
# not exist compiles clean and throws when the user opens the # not exist compiles clean and throws when the user opens the
@@ -152,7 +153,7 @@ jobs:
build-essential pkg-config cmake clang libclang-dev lld nasm dbus \ build-essential pkg-config cmake clang libclang-dev lld nasm dbus \
libasound2-dev libjack-jackd2-dev libdbus-1-dev libssl-dev \ libasound2-dev libjack-jackd2-dev libdbus-1-dev libssl-dev \
libgl1-mesa-dev libegl1-mesa-dev libxkbcommon-dev libwayland-dev \ libgl1-mesa-dev libegl1-mesa-dev libxkbcommon-dev libwayland-dev \
xvfb libxcb-cursor0 libxcb-icccm4 libxcb-image0 libxcb-keysyms1 \ xvfb nodejs libxcb-cursor0 libxcb-icccm4 libxcb-image0 libxcb-keysyms1 \
libxcb-render-util0 libxcb-xinerama0 libxcb-xkb1 libxkbcommon-x11-0 libxcb-render-util0 libxcb-xinerama0 libxcb-xkb1 libxkbcommon-x11-0
# cxx-qt prefers LLD when present; without it, Ubuntu falls back to # cxx-qt prefers LLD when present; without it, Ubuntu falls back to
# the deprecated gold linker. Do not add workspace-wide RUSTFLAGS. # the deprecated gold linker. Do not add workspace-wide RUSTFLAGS.
+7 -2
View File
@@ -567,11 +567,14 @@ Item {
/// line number.) A handler runs once per item change and registers no /// line number.) A handler runs once per item change and registers no
/// dependency, so the guard survives and the cycle does not. /// dependency, so the guard survives and the cycle does not.
property bool _itemTabbed: false property bool _itemTabbed: false
property bool _itemTabRequest: false
Connections { Connections {
target: viewLoader target: viewLoader
function onItemChanged() { function onItemChanged() {
root._itemTabbed = viewLoader.item !== null root._itemTabbed = viewLoader.item !== null
&& typeof viewLoader.item.activeTab === "string" && typeof viewLoader.item.activeTab === "string"
root._itemTabRequest = viewLoader.item !== null
&& typeof viewLoader.item.tabNavigationRequest !== "undefined"
} }
} }
@@ -590,12 +593,14 @@ Item {
Binding { Binding {
target: viewLoader.item target: viewLoader.item
property: "activeTab" property: root._itemTabRequest ? "tabNavigationRequest" : "activeTab"
when: !root._armed when: !root._armed
&& root._itemTabbed && root._itemTabbed
&& QbzShell.navTab !== "" && QbzShell.navTab !== ""
&& QbzShell.navTabView === QbzShell.currentView && QbzShell.navTabView === QbzShell.currentView
value: (QbzShell.navTabSeq, QbzShell.navTab) value: root._itemTabRequest
? ({ "tab": QbzShell.navTab, "sequence": QbzShell.navTabSeq })
: (QbzShell.navTabSeq, QbzShell.navTab)
restoreMode: Binding.RestoreNone restoreMode: Binding.RestoreNone
} }
} }
+44 -10
View File
@@ -87,6 +87,19 @@ Rectangle {
return "top" return "top"
} }
property string activeTab: localDefaultTab property string activeTab: localDefaultTab
// Router requests go through the same history path as the in-view bar.
// The sequence makes reselecting a flyout tab after an in-view change work.
property var tabNavigationRequest: ({})
onTabNavigationRequestChanged: {
if (tabNavigationRequest.tab) activateTab(tabNavigationRequest.tab)
}
function activateTab(tab) {
if (tab === activeTab) return
if (localTabOrder.indexOf(tab) < 0 && !(tab === "ephemeral" && ephemeralActive)) return
if (_navigationReady && !_restoringNavigationState && QbzShell.currentView === "local")
QbzShell.recordLocalTab(tab, navigationStateJson)
activeTab = tab
}
// Session-only file reachability. Keys are `source:id`; values are the // Session-only file reachability. Keys are `source:id`; values are the
// user-facing reason. This is deliberately not persisted: a moved file or // user-facing reason. This is deliberately not persisted: a moved file or
// unavailable NAS must remain retryable and a rescan remains authoritative. // unavailable NAS must remain retryable and a rescan remains authoritative.
@@ -284,6 +297,28 @@ Rectangle {
root._restoringNavigationState = false root._restoringNavigationState = false
QbzShell.reportNavState("local", root.navigationStateJson) QbzShell.reportNavState("local", root.navigationStateJson)
} }
Connections {
target: QbzShell
function onStateRestoreChanged() {
// Back/Forward between tabs keeps this component mounted: there
// is no Component.onCompleted to consume the destination state.
if (root._navigationReady && QbzShell.currentView === "local"
&& QbzShell.restoreStateScope === "local" && QbzShell.stateRestore !== "") {
root.restoreNavigationState()
root.loadActiveTab()
}
}
}
Timer {
id: navigationSeed
interval: 0
onTriggered: {
// Initial persisted state, pending artist and flyout landing tab
// are one arrival, not separate history entries.
root._navigationReady = true
QbzShell.reportNavState("local", root.navigationStateJson)
}
}
// One quality/format/source funnel follows the user across Albums, // One quality/format/source funnel follows the user across Albums,
// Artists, Genres and Tracks. Favorites remains an Albums/Genres-only // Artists, Genres and Tracks. Favorites remains an Albums/Genres-only
@@ -473,7 +508,7 @@ Rectangle {
/// the tab you actually navigated to. /// the tab you actually navigated to.
Connections { Connections {
target: QbzLocal target: QbzLocal
function onLocalEphemeralOpenSeqChanged() { root.activeTab = "ephemeral" } function onLocalEphemeralOpenSeqChanged() { root.activateTab("ephemeral") }
} }
function loadTabForView(tab) { function loadTabForView(tab) {
@@ -621,7 +656,7 @@ Rectangle {
function consumePendingArtist() { function consumePendingArtist() {
var pending = QbzLocal.localPendingArtist var pending = QbzLocal.localPendingArtist
if (pending === "") return if (pending === "") return
activeTab = "artists" activateTab("artists")
selectedArtist = pending selectedArtist = pending
QbzLocal.clearPendingArtist() QbzLocal.clearPendingArtist()
} }
@@ -635,7 +670,7 @@ Rectangle {
if (raw === "") return if (raw === "") return
var route var route
try { route = JSON.parse(raw) } catch (e) { QbzLocal.clearPendingRoute(); return } try { route = JSON.parse(raw) } catch (e) { QbzLocal.clearPendingRoute(); return }
if (route.tab) activeTab = route.tab if (route.tab) activateTab(route.tab)
// The query pre-filters the TRACKS tab only; the albums and artists // The query pre-filters the TRACKS tab only; the albums and artists
// tabs have no search box of their own. // tabs have no search box of their own.
if (route.tab === "tracks" && route.query) { if (route.tab === "tracks" && route.query) {
@@ -657,19 +692,18 @@ Rectangle {
// one query; the Albums/Folders/Artists sets are bounded). // one query; the Albums/Folders/Artists sets are bounded).
Component.onCompleted: { Component.onCompleted: {
restoreNavigationState() restoreNavigationState()
root._navigationReady = true consumePendingArtist()
// Initial bindings are not a reliable change notification contract: consumePendingRoute()
// explicitly seed history even when this is a fresh entry whose state loadActiveTab()
// happens to equal every default. navigationSeed.restart()
QbzShell.reportNavState("local", root.navigationStateJson) }
function loadActiveTab() {
if (root.activeTab === "tracks") if (root.activeTab === "tracks")
QbzLocal.tracksSetFilterJson(JSON.stringify(root.tracksFilter)) QbzLocal.tracksSetFilterJson(JSON.stringify(root.tracksFilter))
else else
root.loadTabForView(root.activeTab) root.loadTabForView(root.activeTab)
if (root.activeTab === "tracks" && root.tracksSearch !== "") if (root.activeTab === "tracks" && root.tracksSearch !== "")
QbzLocal.tracksSearch(root.tracksSearch) QbzLocal.tracksSearch(root.tracksSearch)
consumePendingArtist()
consumePendingRoute()
} }
Component.onDestruction: { Component.onDestruction: {
if (QbzLocal.localAlbumsNativeActive) QbzLocal.albumsNativeClearSelection() if (QbzLocal.localAlbumsNativeActive) QbzLocal.albumsNativeClearSelection()
@@ -237,7 +237,7 @@ Item {
"count": root.view.ephemeralTrackCount }) "count": root.view.ephemeralTrackCount })
return t return t
} }
onSelected: function (id) { root.view.activeTab = id } onSelected: function (id) { root.view.activateTab(id) }
} }
LocalToolbar { LocalToolbar {
+89 -11
View File
@@ -62,16 +62,10 @@ struct Entry {
/// "album", "library:albums", "local:tracks". Opaque here: this module /// "album", "library:albums", "local:tracks". Opaque here: this module
/// never builds one and never parses one, it only carries it back. /// never builds one and never parses one, it only carries it back.
/// ///
/// THIS IS WHY THERE IS NO `scope_for(entry)` TABLE like the Slint build's /// The route id alone cannot identify a tab's scroll container. Local
/// (`crates/qbz/src/main.rs:3068`). Slint can derive the scope from the /// Library tab changes push entries with view-owned state, while the
/// entry because ITS history entries carry the tab — `Favorites { tab }` /// reporter supplies the exact scope to restore. An unmatched scope
/// and `LocalLibrary { tab }` are separate entries there. This port's /// restores nothing instead of applying another tab's scroll offset.
/// entry is a bare route id, so a derived scope would read `"library"` for
/// all five Library tabs, and coming back would drop the Albums grid's
/// offset into whatever tab the view happened to mount on. Letting the
/// reporter name its own scope buys tab granularity without turning tabs
/// into history entries, and the mismatch case degrades the right way: an
/// unmatched scope restores NOTHING instead of restoring the wrong thing.
scope: String, scope: String,
/// Saved `contentY` of that container, in Qt's convention: 0 at the top, /// Saved `contentY` of that container, in Qt's convention: 0 at the top,
/// POSITIVE downward. (Slint stores the same distance as a negative /// POSITIVE downward. (Slint stores the same distance as a negative
@@ -322,6 +316,32 @@ pub fn shell_entry_view() -> String {
} }
pub fn record(view: &str) { pub fn record(view: &str) {
record_entry(view, false);
}
/// A user-selected Local Library tab is a distinct history destination even
/// though ContentRouter keeps the same page mounted. Called before QML changes
/// the tab, so the outgoing filters, selection and scroll are still intact.
pub fn record_local_tab(tab: &str, state: &str) {
if current_view() != "local" {
return;
}
let Ok(mut next) = serde_json::from_str::<serde_json::Value>(state) else {
return;
};
let Some(previous) = next.get("activeTab").and_then(|value| value.as_str()) else {
return;
};
if previous == tab || tab.is_empty() {
return;
}
set_live_state("local", state);
record_entry("local", true);
next["activeTab"] = serde_json::json!(tab);
set_live_state("local", &next.to_string());
}
fn record_entry(view: &str, force: bool) {
// Persist BEFORE the history mutation so the write reflects the view the // Persist BEFORE the history mutation so the write reflects the view the
// user actually reached, and only for the safe set — a detail view leaves // user actually reached, and only for the safe set — a detail view leaves
// the stored value on the last safe root, which is exactly what the // the stored value on the last safe root, which is exactly what the
@@ -331,7 +351,7 @@ pub fn record(view: &str) {
} }
let (can_back, can_forward, current, pushed) = with_history(|h| { let (can_back, can_forward, current, pushed) = with_history(|h| {
let mut pushed = false; let mut pushed = false;
if h.entries[h.index].view != view { if force || h.entries[h.index].view != view {
// Stamp the page we are leaving with its live scroll position. // Stamp the page we are leaving with its live scroll position.
let (scope, y) = live_scroll(); let (scope, y) = live_scroll();
log::debug!( log::debug!(
@@ -444,6 +464,9 @@ fn step(delta: isize) {
String::new() String::new()
}; };
log::debug!("[qbz-qt] scroll: {current:?} arms scope {armed:?} y={scroll}"); log::debug!("[qbz-qt] scroll: {current:?} arms scope {armed:?} y={scroll}");
// A prior flyout request is not the destination of Back/Forward. Leaving
// it armed would override the restored tab when Local Library remounts.
crate::shell_bridge::ui(|mut b| b.as_mut().set_nav_tab(QString::default()));
publish( publish(
can_back, can_back,
can_forward, can_forward,
@@ -507,6 +530,61 @@ mod tests {
super::reset_live_state(); super::reset_live_state();
} }
#[test]
fn local_tabs_are_history_entries_with_independent_state_and_scroll() {
let _guard = TEST_LOCK.lock().unwrap();
reset();
super::record("home");
super::record("local");
let explorer = r#"{"activeTab":"genres","selectedGenres":{"rock":true}}"#;
super::set_live_state("local", explorer);
super::set_live_scroll("local:genres", 412.0);
super::record_local_tab("artists", explorer);
let artists = r#"{"activeTab":"artists","selectedArtist":"Led Zeppelin","artistsFilter":{"local":true}}"#;
super::set_live_state("local", artists);
super::set_live_scroll("local:artists", 620.0);
super::back();
assert_eq!(super::current_view(), "local");
assert_eq!(super::live_state(), ("local".into(), explorer.into()));
assert_eq!(super::live_scroll(), ("local:genres".into(), 412.0));
super::back();
assert_eq!(super::current_view(), "home");
super::forward();
assert_eq!(super::live_state(), ("local".into(), explorer.into()));
super::forward();
assert_eq!(super::live_state(), ("local".into(), artists.into()));
assert_eq!(super::live_scroll(), ("local:artists".into(), 620.0));
super::back();
super::record_local_tab("albums", explorer);
assert!(!super::with_history(|history| super::snapshot(history).1));
assert_eq!(
serde_json::from_str::<serde_json::Value>(&super::live_state().1).unwrap()["activeTab"],
"albums"
);
super::back();
assert_eq!(super::live_state(), ("local".into(), explorer.into()));
}
#[test]
fn local_tab_reselection_does_not_push_or_clear_forward_history() {
let _guard = TEST_LOCK.lock().unwrap();
reset();
super::record("local");
let explorer = r#"{"activeTab":"genres"}"#;
super::record_local_tab("artists", explorer);
super::back();
let before = super::with_history(|history| (history.entries.len(), history.index));
super::record_local_tab("genres", explorer);
super::record_local_tab("artists", "bad snapshot");
assert_eq!(
super::with_history(|history| (history.entries.len(), history.index)),
before
);
assert!(super::with_history(|history| super::snapshot(history).1));
}
#[test] #[test]
fn record_back_forward_cycle() { fn record_back_forward_cycle() {
let _guard = TEST_LOCK.lock().unwrap(); let _guard = TEST_LOCK.lock().unwrap();
+6
View File
@@ -519,6 +519,8 @@ pub mod qbz_shell {
#[qinvokable] #[qinvokable]
fn report_nav_state(self: Pin<&mut QbzShell>, scope: QString, state: QString); fn report_nav_state(self: Pin<&mut QbzShell>, scope: QString, state: QString);
#[qinvokable] #[qinvokable]
fn record_local_tab(self: Pin<&mut QbzShell>, tab: QString, state: QString);
#[qinvokable]
fn local_navigation_state(self: &QbzShell) -> QString; fn local_navigation_state(self: &QbzShell) -> QString;
/// Sidebar navigation: record a content view ("home" | "library") /// Sidebar navigation: record a content view ("home" | "library")
@@ -1307,6 +1309,10 @@ impl qbz_shell::QbzShell {
QString::from(crate::local_restore_qt::browser_json()) QString::from(crate::local_restore_qt::browser_json())
} }
pub fn record_local_tab(self: Pin<&mut Self>, tab: QString, state: QString) {
crate::nav_qt::record_local_tab(&tab.to_string(), &state.to_string());
}
pub fn navigate_to(self: Pin<&mut Self>, view: QString) { pub fn navigate_to(self: Pin<&mut Self>, view: QString) {
crate::navigate_to(&view.to_string()); crate::navigate_to(&view.to_string());
} }
+1
View File
@@ -34,6 +34,7 @@
# QbzCore initialized, and process still alive at the deadline. # QbzCore initialized, and process still alive at the deadline.
# Native Qt SDK content participates in the C++ dependency cache. # Native Qt SDK content participates in the C++ dependency cache.
# 6. release xcb boot against a private silent D-Bus (requires Xvfb). # 6. release xcb boot against a private silent D-Bus (requires Xvfb).
# The shared runtime gate also executes Local Library QML logic with Node.
# #
# Usage: # Usage:
# ./scripts/cargo-test.sh # job `test` # ./scripts/cargo-test.sh # job `test`
+1
View File
@@ -3,6 +3,7 @@
set -euo pipefail set -euo pipefail
cd "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/.." cd "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/.."
export QBZ_PREBUILT_SHADERS=1 export QBZ_PREBUILT_SHADERS=1
node scripts/test_qt_local_views.mjs
python3 scripts/qt-cargo.py test --manifest-path crates/Cargo.toml -p qbz-qt --no-fail-fast python3 scripts/qt-cargo.py test --manifest-path crates/Cargo.toml -p qbz-qt --no-fail-fast
target_dir="${CARGO_TARGET_DIR:-$PWD/crates/target}" target_dir="${CARGO_TARGET_DIR:-$PWD/crates/target}"
logs="$(mktemp -d "${TMPDIR:-/tmp}/qbz-qt-gate-XXXXXX")" logs="$(mktemp -d "${TMPDIR:-/tmp}/qbz-qt-gate-XXXXXX")"
+78
View File
@@ -0,0 +1,78 @@
#!/usr/bin/env node
// Execute the actual LocalLibraryView expressions against mixed-source rows.
// No Qt window, account, database, or audio input is required.
import assert from 'node:assert/strict';
import fs from 'node:fs';
import vm from 'node:vm';
import { fileURLToPath } from 'node:url';
const qml = fs.readFileSync(process.argv[2] || fileURLToPath(
new URL('../crates/qbz-qt/qml/views/LocalLibraryView.qml', import.meta.url)), 'utf8');
function body(marker) {
const offset = qml.indexOf(marker);
assert.ok(offset >= 0, `missing expression: ${marker}`);
const begin = qml.indexOf('{', offset);
for (let depth = 1, pos = begin + 1; pos < qml.length; ++pos) {
if (qml[pos] === '{') ++depth;
if (qml[pos] === '}' && --depth === 0) return qml.slice(begin + 1, pos);
}
throw new Error(`unclosed expression: ${marker}`);
}
const rows = [
{id: 'local-copy', source: 'user', qualityTier: 'cd', format: 'flac'},
{id: 'plex-copy', source: 'plex', qualityTier: 'cd', format: 'flac'},
{id: 'jellyfin-copy', source: 'jellyfin', qualityTier: 'hires', format: 'flac'},
{id: 'mixed', mediaVariants: [
{source: 'local', qualityTier: 'cd', format: 'flac'},
{source: 'qobuz_purchase', qualityTier: 'dsd', format: 'dsd'},
]},
];
const context = vm.createContext({
console, albums: rows, selectedArtist: 'Led Zeppelin', artistsFilter: {},
QbzLocal: {localArtistsNativeActive: false, artistAlbumIds: () => JSON.stringify(rows.map(r => r.id))},
albumFavorite: () => false,
});
context.root = context;
vm.runInContext(`function sourceBucket(word) {${body('function sourceBucket(')}}
function applyFilter(rows, selectedFilter) {${body('function applyFilter(')}}
function artistAlbums() {${body('readonly property var artistAlbums:')}}`, context);
function albumIds(filter) {
context.artistsFilter = filter;
return Array.from(context.artistAlbums(), row => row.id);
}
assert.deepEqual(albumIds({local: true}), ['local-copy', 'mixed']);
assert.deepEqual(albumIds({plex: true}), ['plex-copy']);
assert.deepEqual(albumIds({jellyfin: true}), ['jellyfin-copy']);
assert.deepEqual(albumIds({local: true, plex: true}), ['local-copy', 'plex-copy', 'mixed']);
assert.deepEqual(albumIds({local: true, dsd: true}), []);
assert.deepEqual(albumIds({offline: true, dsd: true}), ['mixed']);
assert.deepEqual(albumIds({}), rows.map(r => r.id));
context.selectedArtist = '';
assert.deepEqual(albumIds({local: true}), []);
const transitions = [];
Object.assign(context, {
activeTab: 'genres', localTabOrder: ['genres', 'albums', 'artists', 'folders', 'tracks'],
ephemeralActive: false, _navigationReady: true, _restoringNavigationState: false,
QbzShell: {currentView: 'local', recordLocalTab: (tab, state) => {
assert.equal(JSON.parse(state).activeTab, context.activeTab, 'stamp before switching');
transitions.push([context.activeTab, tab]);
}},
});
Object.defineProperty(context, 'navigationStateJson', {
get: () => JSON.stringify({activeTab: context.activeTab}),
});
vm.runInContext(`function activateTab(tab) {${body('function activateTab(')}}`, context);
context.activateTab('artists');
assert.equal(context.activeTab, 'artists');
context.activateTab('artists');
context.activateTab('not-a-local-tab');
assert.deepEqual(transitions, [['genres', 'artists']]);
context._restoringNavigationState = true;
context.activateTab('genres');
assert.equal(transitions.length, 1, 'history restore must not record itself');
context._restoringNavigationState = false;
context._navigationReady = false;
context.activateTab('tracks');
assert.equal(transitions.length, 1, 'initial landing must be one entry');
console.log('Local view regressions OK: source-filtered artist albums and tab transitions');