Fix #273: ↓ / ↑ turn pages in single-page view (#550)

In single-page view ↓ / ↑ only added to the pending keyboard scroll, so on a
page that fits the window (or once scrolled to the page's end) they did
nothing, while the mouse wheel turned the page there.

The page view now records, after each frame in single-page view, which page
it drew and whether it was at the top and at the bottom of it. ↓ at the
bottom (always, on a page that fits) steps to the next page; ↑ at the top
steps to the previous page and lands on its bottom, so holding ↑ keeps
scrolling back through the document. The edges only count when they belong
to the current page with no jump pending, so a stale frame can't turn two
pages for one press.

Tested by a kittest: Fit page in single-page view, ↓ / ↑ turn one page per
press; Fit width, ↓ scrolls through page 1 before turning, and ↑ from the
top of page 2 returns to the bottom of page 1.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Brandon Thomas
2026-10-09 10:50:59 -04:00
committed by GitHub
co-authored by Claude Opus 5.5
parent 6fce73f902
commit 653f8268c2
3 changed files with 74 additions and 3 deletions
+32 -2
View File
@@ -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);
+41
View File
@@ -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.
+1 -1
View File
@@ -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"