From 786fa481f4596a649c6b1c2c439df997114cb2c2 Mon Sep 17 00:00:00 2001 From: wstock Date: Thu, 8 Oct 2026 12:53:12 +0100 Subject: [PATCH] Issue #189: each document tab keeps its own scroll position (#259) The page view's ScrollArea had no id of its own, so egui derived it from the parent Ui, which is the same for every document. All tabs therefore shared one scroll offset: switching to another document showed it at wherever the previous one had been scrolled to (clamped to its length), and switching back showed the first document at the other one's offset, usually the top. Salt the ScrollArea with the document's id so egui keeps one offset per tab. Test: viewer.rs::each_document_keeps_its_own_scroll_position scrolls a.pdf to page 4, switches to b.pdf and scrolls it to page 2, then switches back and forth: before the fix b.pdf opened on page 2 instead of its own top. Cited as evidence on view.document-tabs. Co-authored-by: wstock <1394687+wstock@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 --- crates/ui-egui/src/canvas.rs | 2 ++ crates/ui-egui/tests/viewer.rs | 30 +++++++++++++++++++++++++++++- parity/acrobat-features.toml | 4 ++-- 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 5f8ae5d3..604c6e7e 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -1174,6 +1174,8 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { }, ..Default::default() }); + // Each document keeps its own scroll position when switching tabs (#189). + scroll = scroll.id_salt(("page-view", view.id)); if let Some((page, fx, fy, rel)) = view.zoom_anchor.take() { let r = rects[page.min(rects.len() - 1)]; let point = pos2(r.left() + fx * r.width(), r.top() - y_shift + fy * r.height()); diff --git a/crates/ui-egui/tests/viewer.rs b/crates/ui-egui/tests/viewer.rs index 3f915bdf..13a8f123 100644 --- a/crates/ui-egui/tests/viewer.rs +++ b/crates/ui-egui/tests/viewer.rs @@ -1,5 +1,5 @@ //! Viewer conveniences: close all, revert, fit height, view history, select all, find options, -//! cover page. +//! cover page, a scroll position per tab. use egui_kittest::Harness; use egui_kittest::kittest::Queryable; @@ -395,3 +395,31 @@ fn optimizer_audits_space_usage() { h.run_steps(2); assert_eq!(h.state().dialog, Some(Dialog::Optimize), "back to the optimizer"); } + +#[test] +fn each_document_keeps_its_own_scroll_position() { + // #189: every document's page view shared one scroll position, so switching tabs showed a + // document at wherever the other one had been scrolled to (usually back at the top). + use pdfcraft_ui_egui::canvas::Fit; + let mut h = harness(); + for v in &mut h.state_mut().views { + v.fit = Fit::Width; + } + let show = |h: &mut Harness<'static, PdfCraftApp>, i: usize| { + h.state_mut().active = Some(i); + h.run_steps(4); + h.state().views[i].current + }; + show(&mut h, 0); + h.state_mut().views[0].goto = Some((3, 0.0)); + h.run_steps(4); + assert_eq!(h.state().views[0].current, 3, "a.pdf scrolled to page 4"); + // b.pdf opens where it was (the top), and is scrolled to its own last page. + assert_eq!(show(&mut h, 1), 0); + h.state_mut().views[1].goto = Some((1, 0.0)); + h.run_steps(4); + assert_eq!(h.state().views[1].current, 1); + // Back to a.pdf: still on page 4, and b.pdf still on page 2. + assert_eq!(show(&mut h, 0), 3); + assert_eq!(show(&mut h, 1), 1); +} diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index d26e3865..0ea77eba 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -1166,8 +1166,8 @@ milestone = "M3" status = "shipped" commands = [] tools = ["doc_list"] -evidence = ["crates/ui-egui/tests/editing.rs::combining_files_opens_a_new_unsaved_tab", "crates/ui-egui/tests/control.rs::state_and_view_options"] -notes = "" +evidence = ["crates/ui-egui/tests/editing.rs::combining_files_opens_a_new_unsaved_tab", "crates/ui-egui/tests/control.rs::state_and_view_options", "crates/ui-egui/tests/viewer.rs::each_document_keeps_its_own_scroll_position"] +notes = "each tab keeps its own scroll position (#189)" [[feature]] id = "view.multiple-windows"