From 641aa22c170d8c7705283642dc4642fbfbab8d4d Mon Sep 17 00:00:00 2001 From: vicrodh <103399283+vicrodh@users.noreply.github.com> Date: Mon, 8 Jun 2026 15:38:35 -0600 Subject: [PATCH] feat(slint): scroll-position restore on back/forward (Local Library) Scroll-position restore was never implemented (only a TODO in nav.rs); back/forward tracked which page was visited, not where it was scrolled. Add per-history-entry scroll memory: the mounted scroll container reports its live viewport-y via NavState.report-scroll, so every navigation (fresh record + back/forward) stamps the outgoing entry without touching the ~30 record call sites. go_back/go_forward hand the saved scroll back to the shell, which arms NavState.restore-scope + scroll-restore before switching the view. A container restores only when its scope matches, once per mount, and only after content has laid out (viewport overflows) to dodge the clamp-to-0 on async data load; it clears the scope once consumed. Wired the four Local Library lists (albums, tracks, folders, artists rail). Tracks is paginated, so its restore is best-effort within loaded pages. cc --- crates/qbz-slint/src/main.rs | 41 ++++- crates/qbz-slint/src/nav.rs | 167 ++++++++++++++---- .../ui/locallibrary/LocalLibraryView.slint | 58 +++++- crates/qbz-slint/ui/state.slint | 13 ++ 4 files changed, 243 insertions(+), 36 deletions(-) diff --git a/crates/qbz-slint/src/main.rs b/crates/qbz-slint/src/main.rs index 03b957215..1fdebb986 100644 --- a/crates/qbz-slint/src/main.rs +++ b/crates/qbz-slint/src/main.rs @@ -663,6 +663,34 @@ fn navigate_search( } /// Apply a history entry — set the view and re-load entity pages. +/// Stable scroll-restore id for an entry's primary list container, matching +/// the `scope` strings the Slint scroll containers compare against. Returns +/// `""` for views without a wired scroll memory (no container will match, so +/// nothing restores). Keep in sync with the `restore-scope` checks in the +/// `.slint` views. +fn scope_for(entry: &nav::NavEntry) -> &'static str { + match entry { + nav::NavEntry::LocalLibrary { tab } => match tab.as_str() { + "albums" => "ll:albums", + "tracks" => "ll:tracks", + "folders" => "ll:folders", + "artists" => "ll:artists", + _ => "", + }, + _ => "", + } +} + +/// Arm `NavState` so the destination scroll container restores its saved +/// position once it mounts. Must run before `apply_entry` switches the view. +fn arm_scroll_restore(weak: &slint::Weak, entry: &nav::NavEntry, scroll: f32) { + if let Some(w) = weak.upgrade() { + let ns = w.global::(); + ns.set_restore_scope(scope_for(entry).into()); + ns.set_scroll_restore(scroll); + } +} + fn apply_entry( entry: nav::NavEntry, runtime: &Arc>, @@ -2156,7 +2184,8 @@ fn main() -> Result<(), Box> { let handle = tokio_rt.handle().clone(); let image_cache = image_cache.clone(); window.global::().on_request_back(move || { - if let Some(entry) = nav::go_back() { + if let Some((entry, scroll)) = nav::go_back() { + arm_scroll_restore(&weak, &entry, scroll); apply_entry(entry, &runtime, &weak, &handle, &image_cache); } if let Some(w) = weak.upgrade() { @@ -2170,7 +2199,8 @@ fn main() -> Result<(), Box> { let handle = tokio_rt.handle().clone(); let image_cache = image_cache.clone(); window.global::().on_request_forward(move || { - if let Some(entry) = nav::go_forward() { + if let Some((entry, scroll)) = nav::go_forward() { + arm_scroll_restore(&weak, &entry, scroll); apply_entry(entry, &runtime, &weak, &handle, &image_cache); } if let Some(w) = weak.upgrade() { @@ -2178,6 +2208,13 @@ fn main() -> Result<(), Box> { } }); } + { + // The mounted scroll container reports its live viewport-y here so the + // nav module can stamp the outgoing entry on the next navigation. + window + .global::() + .on_report_scroll(|y| nav::set_live_scroll(y)); + } // Log out: clear the session and return to the login screen. { diff --git a/crates/qbz-slint/src/nav.rs b/crates/qbz-slint/src/nav.rs index c1befdbac..401217f00 100644 --- a/crates/qbz-slint/src/nav.rs +++ b/crates/qbz-slint/src/nav.rs @@ -4,10 +4,16 @@ //! `[<] [>]` button pair (and the mouse back/forward buttons) walk the //! stack. UI thread only, hence `thread_local`. //! -//! Scroll-position restoration per entry is a planned follow-up; for now -//! the stack tracks which page was visited, not where it was scrolled. +//! Scroll-position restoration: each entry remembers the viewport-y of the +//! scroll container that was showing it. The mounted view continuously +//! reports its live scroll via [`set_live_scroll`] (a NavState callback), so +//! every navigation — fresh [`record`] or [`go_back`]/[`go_forward`] — can +//! stamp the outgoing entry without touching the ~30 `record` call sites. +//! `go_back`/`go_forward` hand the restored scroll back to the shell, which +//! arms `NavState.restore-scope` + `scroll-restore`; the destination's scroll +//! container picks it up once its content has laid out. -use std::cell::RefCell; +use std::cell::{Cell, RefCell}; /// One navigable destination. #[derive(Clone, Debug, PartialEq)] @@ -94,17 +100,41 @@ pub enum NavEntry { }, } +/// One slot in the history stack: where we went, and how far it was +/// scrolled when we last left it. +#[derive(Clone, Debug)] +struct Entry { + nav: NavEntry, + /// Saved Flickable `viewport-y` (logical px; 0 at top, negative when + /// scrolled down — Slint's convention). + scroll: f32, +} + struct History { - entries: Vec, + entries: Vec, /// Index of the entry currently shown. cursor: usize, } thread_local! { static HISTORY: RefCell = RefCell::new(History { - entries: vec![NavEntry::Home], + entries: vec![Entry { nav: NavEntry::Home, scroll: 0.0 }], cursor: 0, }); + /// Live `viewport-y` of the scroll container currently on screen, kept + /// fresh by the mounted view via [`set_live_scroll`]. Read when leaving + /// a page so its entry can be stamped without per-call-site plumbing. + static LIVE_SCROLL: Cell = const { Cell::new(0.0) }; +} + +/// Record the on-screen scroll container's current `viewport-y`. Wired to +/// `NavState.report-scroll`, fired from the view's `changed viewport-y`. +pub fn set_live_scroll(y: f32) { + LIVE_SCROLL.with(|s| s.set(y)); +} + +fn live_scroll() -> f32 { + LIVE_SCROLL.with(|s| s.get()) } /// Push a Search history entry, OR replace the cursor entry in place @@ -114,14 +144,25 @@ thread_local! { pub fn push_or_replace_search(query: String) { HISTORY.with(|h| { let h = &mut *h.borrow_mut(); - match h.entries.get(h.cursor) { + match h.entries.get(h.cursor).map(|e| &e.nav) { Some(NavEntry::Search(_)) => { + // Replace in place: same Search page, keep its scroll. + let scroll = h.entries[h.cursor].scroll; h.entries.truncate(h.cursor + 1); - h.entries[h.cursor] = NavEntry::Search(query); + h.entries[h.cursor] = Entry { + nav: NavEntry::Search(query), + scroll, + }; } _ => { + if let Some(cur) = h.entries.get_mut(h.cursor) { + cur.scroll = live_scroll(); + } h.entries.truncate(h.cursor + 1); - h.entries.push(NavEntry::Search(query)); + h.entries.push(Entry { + nav: NavEntry::Search(query), + scroll: 0.0, + }); h.cursor = h.entries.len() - 1; } } @@ -132,41 +173,69 @@ pub fn push_or_replace_search(query: String) { /// no-op when the destination already is the current entry, so repeated /// clicks on the same page do not pile up. pub fn record(entry: NavEntry) { - HISTORY.with(|h| { + let pushed = HISTORY.with(|h| { let h = &mut *h.borrow_mut(); - if h.entries.get(h.cursor) == Some(&entry) { - return; + if h.entries.get(h.cursor).map(|e| &e.nav) == Some(&entry) { + return false; + } + // Stamp the page we are leaving with its live scroll position. + if let Some(cur) = h.entries.get_mut(h.cursor) { + cur.scroll = live_scroll(); } h.entries.truncate(h.cursor + 1); - h.entries.push(entry); + h.entries.push(Entry { + nav: entry, + scroll: 0.0, + }); h.cursor = h.entries.len() - 1; + true }); + // A fresh page starts at the top; the new view will report its own + // scroll as the user moves it. + if pushed { + set_live_scroll(0.0); + } } -/// Step back; returns the entry that is now current, or `None` at the -/// start of the stack. -pub fn go_back() -> Option { - HISTORY.with(|h| { +/// Step back; returns the entry that is now current plus its saved scroll +/// position, or `None` at the start of the stack. +pub fn go_back() -> Option<(NavEntry, f32)> { + let res = HISTORY.with(|h| { let h = &mut *h.borrow_mut(); if h.cursor == 0 { return None; } + // Stamp the page we are leaving before stepping away. + if let Some(cur) = h.entries.get_mut(h.cursor) { + cur.scroll = live_scroll(); + } h.cursor -= 1; - h.entries.get(h.cursor).cloned() - }) + h.entries.get(h.cursor).map(|e| (e.nav.clone(), e.scroll)) + }); + if let Some((_, scroll)) = &res { + set_live_scroll(*scroll); + } + res } -/// Step forward; returns the entry that is now current, or `None` at the -/// end of the stack. -pub fn go_forward() -> Option { - HISTORY.with(|h| { +/// Step forward; returns the entry that is now current plus its saved scroll +/// position, or `None` at the end of the stack. +pub fn go_forward() -> Option<(NavEntry, f32)> { + let res = HISTORY.with(|h| { let h = &mut *h.borrow_mut(); if h.cursor + 1 >= h.entries.len() { return None; } + if let Some(cur) = h.entries.get_mut(h.cursor) { + cur.scroll = live_scroll(); + } h.cursor += 1; - h.entries.get(h.cursor).cloned() - }) + h.entries.get(h.cursor).map(|e| (e.nav.clone(), e.scroll)) + }); + if let Some((_, scroll)) = &res { + set_live_scroll(*scroll); + } + res } /// Whether a back step is available. @@ -189,10 +258,20 @@ mod tests { fn reset() { HISTORY.with(|h| { *h.borrow_mut() = History { - entries: vec![NavEntry::Home], + entries: vec![Entry { + nav: NavEntry::Home, + scroll: 0.0, + }], cursor: 0, }; }); + set_live_scroll(0.0); + } + + /// Drop the scroll component for the assertions that only care about the + /// destination page. + fn nav_of(res: Option<(NavEntry, f32)>) -> Option { + res.map(|(e, _)| e) } #[test] @@ -203,10 +282,10 @@ mod tests { record(NavEntry::Artist("2".into())); assert!(can_back()); assert!(!can_forward()); - assert_eq!(go_back(), Some(NavEntry::Album("1".into()))); - assert_eq!(go_back(), Some(NavEntry::Home)); - assert_eq!(go_back(), None); - assert_eq!(go_forward(), Some(NavEntry::Album("1".into()))); + assert_eq!(nav_of(go_back()), Some(NavEntry::Album("1".into()))); + assert_eq!(nav_of(go_back()), Some(NavEntry::Home)); + assert_eq!(nav_of(go_back()), None); + assert_eq!(nav_of(go_forward()), Some(NavEntry::Album("1".into()))); } #[test] @@ -217,7 +296,7 @@ mod tests { go_back(); record(NavEntry::Artist("3".into())); assert!(!can_forward()); - assert_eq!(go_back(), Some(NavEntry::Album("1".into()))); + assert_eq!(nav_of(go_back()), Some(NavEntry::Album("1".into()))); } #[test] @@ -225,8 +304,11 @@ mod tests { reset(); record(NavEntry::Search("metallica".into())); record(NavEntry::Album("5".into())); - assert_eq!(go_back(), Some(NavEntry::Search("metallica".into()))); - assert_eq!(go_back(), Some(NavEntry::Home)); + assert_eq!( + nav_of(go_back()), + Some(NavEntry::Search("metallica".into())) + ); + assert_eq!(nav_of(go_back()), Some(NavEntry::Home)); } #[test] @@ -234,6 +316,25 @@ mod tests { reset(); record(NavEntry::Album("1".into())); record(NavEntry::Album("1".into())); - assert_eq!(go_back(), Some(NavEntry::Home)); + assert_eq!(nav_of(go_back()), Some(NavEntry::Home)); + } + + #[test] + fn scroll_is_stamped_on_leave_and_restored_on_return() { + reset(); + // On Home, scroll down a bit, then navigate away. + set_live_scroll(-420.0); + record(NavEntry::Album("1".into())); + // Fresh page starts at the top. + assert_eq!(live_scroll(), 0.0); + // Scroll the album page, then go back to Home. + set_live_scroll(-90.0); + let (entry, scroll) = go_back().expect("back to Home"); + assert_eq!(entry, NavEntry::Home); + assert_eq!(scroll, -420.0); + // Going forward returns to the album at its saved scroll. + let (entry, scroll) = go_forward().expect("forward to album"); + assert_eq!(entry, NavEntry::Album("1".into())); + assert_eq!(scroll, -90.0); } } diff --git a/crates/qbz-slint/ui/locallibrary/LocalLibraryView.slint b/crates/qbz-slint/ui/locallibrary/LocalLibraryView.slint index 266e2bce7..75b800b81 100644 --- a/crates/qbz-slint/ui/locallibrary/LocalLibraryView.slint +++ b/crates/qbz-slint/ui/locallibrary/LocalLibraryView.slint @@ -18,7 +18,7 @@ import { Theme } from "../foundation/semantic-colors.slint"; import { Typography } from "../foundation/typography.slint"; -import { LocalLibraryState, LocalLibraryActions, FolderNode, FolderSubcardItem, LocalArtistItem, AlphaJump, LibAlbumFilterState, AppearanceState } from "../state.slint"; +import { LocalLibraryState, LocalLibraryActions, FolderNode, FolderSubcardItem, LocalArtistItem, AlphaJump, LibAlbumFilterState, AppearanceState, NavState } from "../state.slint"; import { NavButtons } from "../shell/NavButtons.slint"; import { QbzIcon } from "../primitives/QbzIcon.slint"; import { CircleAction } from "../primitives/CircleAction.slint"; @@ -1163,6 +1163,21 @@ export component LocalLibraryView inherits Rectangle { albums-flick := Flickable { viewport-width: self.width; viewport-height: albums-page.preferred-height; + // Scroll-position restore on back/forward (nav::scope_for → "ll:albums"). + property restored: false; + function restore-scroll() { + if (!self.restored && NavState.restore-scope == "ll:albums" + && self.viewport-height > self.height) { + self.viewport-y = NavState.scroll-restore; + self.restored = true; + // Consume the arm so a later fresh re-mount of this + // tab does not replay a stale position. + NavState.restore-scope = ""; + } + } + init => { self.restore-scroll(); } + changed viewport-height => { self.restore-scroll(); } + changed viewport-y => { NavState.report-scroll(self.viewport-y); } albums-page := VerticalLayout { width: parent.width; padding-left: 32px; @@ -1325,7 +1340,22 @@ export component LocalLibraryView inherits Rectangle { tracks-list := ListView { vertical-stretch: 1; vertical-scrollbar-policy: ScrollBarPolicy.always-off; + // Scroll-position restore on back/forward (nav::scope_for → "ll:tracks"). + // Best-effort on this paginated list: restores when the + // saved position falls within already-loaded pages. + property restored: false; + function restore-scroll() { + if (!self.restored && NavState.restore-scope == "ll:tracks" + && self.viewport-height > self.height) { + self.viewport-y = NavState.scroll-restore; + self.restored = true; + NavState.restore-scope = ""; + } + } + init => { self.restore-scroll(); } + changed viewport-height => { self.restore-scroll(); } changed viewport-y => { + NavState.report-scroll(self.viewport-y); if (-self.viewport-y + self.visible-height >= self.viewport-height - 600px && LocalLibraryState.tracks-has-more && !LocalLibraryState.tracks-loading-more @@ -1462,6 +1492,19 @@ export component LocalLibraryView inherits Rectangle { folders-flick := Flickable { viewport-width: self.width; viewport-height: folders-page.preferred-height; + // Scroll-position restore on back/forward (nav::scope_for → "ll:folders"). + property restored: false; + function restore-scroll() { + if (!self.restored && NavState.restore-scope == "ll:folders" + && self.viewport-height > self.height) { + self.viewport-y = NavState.scroll-restore; + self.restored = true; + NavState.restore-scope = ""; + } + } + init => { self.restore-scroll(); } + changed viewport-height => { self.restore-scroll(); } + changed viewport-y => { NavState.report-scroll(self.viewport-y); } folders-page := VerticalLayout { width: parent.width; padding-left: 32px; @@ -2114,6 +2157,19 @@ export component LocalLibraryView inherits Rectangle { rail-flick := Flickable { viewport-width: self.width; viewport-height: rail-col.preferred-height; + // Scroll-position restore on back/forward (nav::scope_for → "ll:artists"). + property restored: false; + function restore-scroll() { + if (!self.restored && NavState.restore-scope == "ll:artists" + && self.viewport-height > self.height) { + self.viewport-y = NavState.scroll-restore; + self.restored = true; + NavState.restore-scope = ""; + } + } + init => { self.restore-scroll(); } + changed viewport-height => { self.restore-scroll(); } + changed viewport-y => { NavState.report-scroll(self.viewport-y); } rail-col := VerticalLayout { width: parent.width; padding-right: 22px; diff --git a/crates/qbz-slint/ui/state.slint b/crates/qbz-slint/ui/state.slint index 3a4a05ab9..44d00c135 100644 --- a/crates/qbz-slint/ui/state.slint +++ b/crates/qbz-slint/ui/state.slint @@ -1692,6 +1692,19 @@ export global NavState { callback request-back(); callback request-forward(); callback request-settings(); + + // ---- Scroll-position restore (browser-like, per history entry) ---- + // The mounted scroll container reports its live viewport-y here on every + // `changed viewport-y`; the Rust nav module stamps it onto the outgoing + // entry so back/forward can put it back. + callback report-scroll(length /* viewport-y */); + // Armed by Rust right before a back/forward view switch: `restore-scope` + // names which scroll container should restore (a stable per-list id, see + // nav::scope_for), and `scroll-restore` is the viewport-y to apply. A + // container restores only when its own scope matches, exactly once per + // mount, and only after its content has laid out (viewport overflows). + in-out property restore-scope: ""; + in-out property scroll-restore: 0; } // Artist detail screen state.