diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index da2a03737..346f1d0fb 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -203,6 +203,9 @@ pub struct DocView { /// Pending keyboard scrolling, in points down (negative: up): ↓ / ↑, and Page Down / /// Page Up where the pages scroll. pub key_scroll: f32, + /// Single-page view, as last drawn: the page shown and whether the view was at its top and + /// at its bottom, so ↑ / ↓ turn the page there instead of doing nothing (#273). + single_edges: Option<(usize, bool, bool)>, /// Briefly outline an annotation after navigating to it from a panel. pub flash: Option<(usize, [f32; 4], f64)>, /// Compare files: differences shaded on this document's pages (page, user-space box, colour). @@ -353,6 +356,7 @@ impl DocView { forward: Vec::new(), goto: None, key_scroll: 0.0, + single_edges: None, flash: None, compare_marks: Vec::new(), pages: HashMap::new(), @@ -1166,12 +1170,31 @@ pub fn shortcuts(view: &mut DocView, ctx: &egui::Context) { if key(Key::ArrowLeft) || (key(Key::PageUp) && (command || !scrolls)) { view.step_page(false); } - // ↓ / ↑ scroll a line, and Page Down / Page Up a screen where the pages scroll. + // ↓ / ↑ scroll a line, and Page Down / Page Up a screen where the pages scroll. In + // single-page view, ↓ / ↑ turn the page once there is nothing left to scroll that way, as + // the wheel does (#273): always on a page that fits. if !command { let screen = (view.viewport_h - KEY_SCROLL_LINE).max(KEY_SCROLL_LINE); let steps = [(Key::ArrowDown, KEY_SCROLL_LINE), (Key::ArrowUp, -KEY_SCROLL_LINE), (Key::PageDown, screen), (Key::PageUp, -screen)]; for (k, by) in steps { - if key(k) && (scrolls || matches!(k, Key::ArrowDown | Key::ArrowUp)) { + let arrow = matches!(k, Key::ArrowDown | Key::ArrowUp); + if !key(k) || !(scrolls || arrow) { + continue; + } + let forward = k == Key::ArrowDown; + // Only an up-to-date view counts: the page drawn is the current one, with no jump pending. + let at_end = !scrolls + && view.goto.is_none() + && view.single_edges.is_some_and(|(page, top, bottom)| page == view.current && if forward { bottom } else { top }); + if at_end { + let before = view.current; + view.step_page(forward); + if !forward && view.current != before { + // Going back up lands on the bottom of the previous page. + view.goto = Some((view.current, 1.0)); + } + view.single_edges = None; + } else { view.key_scroll += by; } } @@ -1910,6 +1933,13 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { }); (wanted, visible_now) }); + view.single_edges = visible_pages.first().copied().filter(|_| view.layout == PageLayout::Single).map(|page| { + // At the top once the page's top edge shows where going to a page puts it (a gap + // below the window's top). Within a point, as for wheel paging, so layout rounding + // can't hide an edge. + let max_y = (out.content_size.y - out.inner_rect.height()).max(0.0); + (page, out.state.offset.y <= MARGIN - GAP + 1.0, out.state.offset.y >= max_y - 1.0) + }); view.auto_scroll.paint(ui, avail); diff --git a/crates/ui-egui/tests/viewer.rs b/crates/ui-egui/tests/viewer.rs index 67dcecb2d..ffbabc10c 100644 --- a/crates/ui-egui/tests/viewer.rs +++ b/crates/ui-egui/tests/viewer.rs @@ -475,6 +475,47 @@ fn arrow_and_page_keys_move_through_a_scrolling_document() { assert_eq!(press(&mut h, Key::PageDown, 1), 3); } +#[test] +fn up_and_down_turn_pages_in_single_page_view() { + // #273: in single-page view ↓ / ↑ only scrolled, so on a page that fits (or once scrolled to + // the end) they did nothing, while the wheel turned the page there. + use egui::Key; + use pdfcraft_ui_egui::canvas::{Fit, PageLayout}; + let mut h = harness(); + h.state_mut().active = Some(0); + h.state_mut().views[0].layout = PageLayout::Single; + h.state_mut().views[0].fit = Fit::Page; + h.run_steps(4); + let press = |h: &mut Harness<'static, PdfCraftApp>, key, times: usize| { + for _ in 0..times { + h.key_press(key); + h.run_steps(2); + } + h.run_steps(2); + h.state().views[0].current + }; + // A page that fits: each press turns one page. + assert_eq!(press(&mut h, Key::ArrowDown, 1), 1); + assert_eq!(press(&mut h, Key::ArrowDown, 1), 2); + assert_eq!(press(&mut h, Key::ArrowUp, 1), 1); + assert_eq!(press(&mut h, Key::ArrowUp, 1), 0); + assert_eq!(press(&mut h, Key::ArrowUp, 1), 0, "nothing before the first page"); + // A page taller than the window scrolls first, then turns at its bottom. + h.state_mut().views[0].fit = Fit::Width; + h.run_steps(4); + let mut lines = 0; + while press(&mut h, Key::ArrowDown, 1) == 0 { + lines += 1; + assert!(lines < 500, "↓ never reached page 2"); + } + assert!(lines > 2, "↓ scrolled through page 1 before turning ({lines} lines)"); + // ↑ at the top of page 2 goes back to the bottom of page 1, then scrolls up it. + assert_eq!(press(&mut h, Key::ArrowUp, 1), 0); + assert_eq!(press(&mut h, Key::ArrowDown, 1), 1, "back at the bottom of page 1"); + assert_eq!(press(&mut h, Key::ArrowUp, 2), 0); + assert_eq!(press(&mut h, Key::ArrowDown, 1), 0, "scrolled up from the bottom of page 1"); +} + #[test] fn v_h_and_space_pick_the_quick_tools() { // The toolbar's tooltips promise "Select (V)" and "Hand (H)", but the keys did nothing. diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index f223feead..04ea2600c 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -1119,7 +1119,7 @@ status = "shipped" commands = [] tools = [] evidence = ["crates/ui-egui/tests/control.rs::state_and_view_options", "crates/ui-egui/tests/ui.rs::theme_and_view_options_apply", "crates/ui-egui/tests/viewer.rs::two_page_view_steps_a_spread_at_a_time", "crates/ui-egui/tests/viewer.rs::arrow_and_page_keys_move_through_a_scrolling_document"] -notes = "in two-page view, next and previous page move a whole spread (with or without a cover page); → / ← turn pages in every layout, ↓ / ↑ scroll a line and Page Down / Page Up a screen (the page in single-page view), as in Acrobat" +notes = "in two-page view, next and previous page move a whole spread (with or without a cover page); → / ← turn pages in every layout, ↓ / ↑ scroll a line and Page Down / Page Up a screen (the page in single-page view), as in Acrobat; in single-page view ↓ / ↑ turn the page once there is nothing left to scroll that way, like the wheel" [[feature]] id = "view.goto-page-label"