diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -498,6 +498,11 @@ pub struct DocView { grid_anchor: Option<(usize, f32)>, /// The page the grid scrolls into view on its next frame (Organize just opened on it). grid_reveal: Option, + /// The page the grid scrolls into view on its next frame only if it is out of view (an + /// arrow key moved to it). + grid_follow: Option, + /// Columns of the grid as last drawn, for ↑ / ↓. + grid_cols: usize, /// Sharp renders of the pages the grid shows larger than a thumbnail: only those in view, /// at the size drawn (device pixels wide). grid_pages: HashMap, @@ -694,6 +699,8 @@ impl DocView { grid_zoom_request: None, grid_anchor: None, grid_reveal: None, + grid_follow: None, + grid_cols: 1, grid_pages: HashMap::new(), last_scale: 0.0, sharp_hold_until: 0.0, diff --git a/crates/ui-egui/src/canvas/navigation.rs b/crates/ui-egui/src/canvas/navigation.rs --- a/crates/ui-egui/src/canvas/navigation.rs +++ b/crates/ui-egui/src/canvas/navigation.rs @@ -334,6 +334,9 @@ pub fn shortcuts(view: &mut DocView, ctx: &egui::Context, speed: crate::scroll_s // In the page grid ⌘+ / ⌘− / ⌘0 size its pages instead (looking is always allowed). let (zoom_in, zoom_out, zoom_reset) = (pressed(cmd(Key::Plus)) || pressed(cmd(Key::Equals)), pressed(cmd(Key::Minus)), pressed(cmd(Key::Num0))); if view.organize { + if arrows { + view.grid_arrow_keys(ctx); + } if zoom_in { view.grid_zoom_request = Some(view.grid_zoom * GRID_ZOOM_STEP); } diff --git a/crates/ui-egui/src/canvas/organize.rs b/crates/ui-egui/src/canvas/organize.rs --- a/crates/ui-egui/src/canvas/organize.rs +++ b/crates/ui-egui/src/canvas/organize.rs @@ -61,6 +61,62 @@ impl DocView { } } + /// The arrow keys in the organize grid: ← / → go to the previous / next page and ↑ / ↓ a + /// row up or down, selecting only that page; with ⇧ the selection runs from the anchor to + /// it instead (#3436). ⌘/Ctrl with an arrow is left to the other page keys. + pub(crate) fn grid_arrow_keys(&mut self, ctx: &egui::Context) { + use egui::{Event, Key, Modifiers}; + // Every arrow press this frame, in order, each with the Shift it was pressed with: the + // frame's modifier state can differ from a press's when Shift changes mid-frame. + let presses: Vec<(Key, bool)> = ctx.input_mut(|i| { + let mut presses = Vec::new(); + i.events.retain(|event| match event { + Event::Key { key, pressed: true, modifiers, .. } + if matches!(key, Key::ArrowLeft | Key::ArrowRight | Key::ArrowUp | Key::ArrowDown) + && modifiers.matches_logically(Modifiers::NONE) => + { + presses.push((*key, modifiers.shift)); + false + } + _ => true, + }); + presses + }); + for (key, shift) in presses { + self.grid_arrow(key, shift); + } + } + + /// One arrow press in the organize grid (see [`Self::grid_arrow_keys`]). + fn grid_arrow(&mut self, key: egui::Key, shift: bool) { + use egui::Key; + let Some(last) = self.page_count.checked_sub(1) else { return }; + let cols = self.grid_cols.max(1); + let current = self.current.min(last); + let (forward, step) = match key { + Key::ArrowLeft => (false, 1), + Key::ArrowRight => (true, 1), + Key::ArrowUp => (false, cols), + _ => (true, cols), + }; + let target = if forward { + // ↓ on the row above a short last row goes to the last page. + current.checked_add(step).map(|t| t.min(last)).filter(|t| *t != current && (step == 1 || t / cols > current / cols)) + } else { + current.checked_sub(step) + }; + let Some(target) = target else { return }; + if shift { + let anchor = self.select_anchor.filter(|a| *a <= last).unwrap_or(current); + self.selected = (anchor.min(target)..=anchor.max(target)).collect(); + self.select_anchor = Some(anchor); + } else { + self.clear_page_selection(Some(target)); + } + self.go_to_page(target); + self.grid_follow = Some(target); + } + /// Drop the page selection; a later ⇧-click ranges from `anchor`. pub fn clear_page_selection(&mut self, anchor: Option) { self.selected.clear(); @@ -216,6 +272,7 @@ pub(super) fn organize_grid(view: &mut DocView, info: &DocInfo, editable: bool, let cell = vec2(146.0 * view.grid_zoom + 44.0, 194.0 * view.grid_zoom + 88.0); let anchor = view.grid_anchor.take(); let reveal = view.grid_reveal.take(); + let follow = view.grid_follow.take(); let mut open_page = None; organize_toolbar(view, info, editable, dirty, unobstructed, ui, t); let viewport = ui.available_rect_before_wrap(); @@ -244,16 +301,24 @@ pub(super) fn organize_grid(view: &mut DocView, info: &DocInfo, editable: bool, } ui.add_space(20.0); let cols = ((ui.available_width() - 40.0) / cell.x).floor().max(1.0) as usize; + view.grid_cols = cols; let rows = info.pages.len().div_ceil(cols); let left = (ui.available_width() - cols as f32 * cell.x) / 2.0; - let (grid, _) = ui.allocate_exact_size(vec2(ui.available_width(), rows as f32 * cell.y), Sense::hover()); - // The page Organize opened on may lie in a row the virtualized grid has not laid out, - // so its cell is computed rather than found. + // Clicked where there is no page: the selection goes (the pages are on top of it). + let (grid, background) = ui.allocate_exact_size(vec2(ui.available_width(), rows as f32 * cell.y), Sense::click()); + if background.clicked() { + view.clear_page_selection(Some(view.current)); + } + // The page Organize opened on, or an arrow key moved to, may lie in a row the + // virtualized grid has not laid out, so its cell is computed rather than found. + let cell_of = |page: usize| { + let (row, col) = (page / cols, page % cols); + Rect::from_min_size(pos2(grid.left() + left + col as f32 * cell.x, grid.top() + row as f32 * cell.y), cell) + }; if let Some(page) = reveal.filter(|p| *p < info.pages.len()) { - let row = page / cols; - let col = page % cols; - let c = Rect::from_min_size(pos2(grid.left() + left + col as f32 * cell.x, grid.top() + row as f32 * cell.y), cell); - ui.scroll_to_rect_animation(c, Some(egui::Align::Center), egui::style::ScrollAnimation::none()); + ui.scroll_to_rect_animation(cell_of(page), Some(egui::Align::Center), egui::style::ScrollAnimation::none()); + } else if let Some(page) = follow.filter(|p| *p < info.pages.len()) { + ui.scroll_to_rect_animation(cell_of(page), None, egui::style::ScrollAnimation::none()); } for row in thumbnail_rows(clip.top() - 20.0, clip.bottom() - 20.0, cell.y, rows) { let row_rect = Rect::from_min_size(grid.min + vec2(0.0, row as f32 * cell.y), vec2(grid.width(), cell.y)); diff --git a/crates/ui-egui/tests/it/main.rs b/crates/ui-egui/tests/it/main.rs --- a/crates/ui-egui/tests/it/main.rs +++ b/crates/ui-egui/tests/it/main.rs @@ -55,6 +55,7 @@ mod object_selection; mod ocr; mod open_action; mod organize_drag; +mod organize_keys; mod pageboxes; mod panel_snapshots; mod pdf_links; diff --git a/crates/ui-egui/tests/it/organize_keys.rs b/crates/ui-egui/tests/it/organize_keys.rs new file mode 100644 --- /dev/null +++ b/crates/ui-egui/tests/it/organize_keys.rs @@ -0,0 +1,112 @@ +//! Organize Pages: the arrow keys move through the grid and ⇧ extends the selection with them; +//! a click where there is no page drops the selection (#3436). + +use egui::{Key, Modifiers, PointerButton}; +use egui_kittest::Harness; +use egui_kittest::kittest::Queryable; +use pdfcraft_engine::Session; +use pdfcraft_ui_egui::PdfCraftApp; + +const PAGES: usize = 20; + +fn organize() -> Harness<'static, PdfCraftApp> { + let bytes = Session::new().create_blank(200.0, 300.0, PAGES).unwrap(); + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(move |_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("pages.pdf", None, bytes.as_ref().clone()).unwrap(); + app.set_option("organize", "on").unwrap(); + app + }); + h.run_steps(4); + h +} + +fn state(h: &Harness<'static, PdfCraftApp>) -> (usize, Vec) { + let view = &h.state().views[0]; + (view.current, view.selected.iter().copied().collect()) +} + +/// Pages in the grid's first row: the pages drawn level with page 1. +fn columns(h: &Harness<'static, PdfCraftApp>) -> usize { + let top = h.get_by_label("Page 1").rect().top(); + (1..=PAGES).take_while(|p| h.get_by_label(&format!("Page {p}")).rect().top() == top).count() +} + +fn press(h: &mut Harness<'static, PdfCraftApp>, modifiers: Modifiers, key: Key) { + h.key_press_modifiers(modifiers, key); + h.run_steps(2); +} + +#[test] +fn arrow_keys_move_through_the_organize_grid_and_shift_extends_the_selection() { + let mut h = organize(); + let cols = columns(&h); + assert!((2..PAGES / 2).contains(&cols), "the test needs several rows of several pages: {cols}"); + assert_eq!(state(&h), (0, vec![])); + press(&mut h, Modifiers::NONE, Key::ArrowLeft); + assert_eq!(state(&h), (0, vec![]), "nothing before the first page"); + press(&mut h, Modifiers::NONE, Key::ArrowRight); + assert_eq!(state(&h), (1, vec![])); + press(&mut h, Modifiers::SHIFT, Key::ArrowRight); + press(&mut h, Modifiers::SHIFT, Key::ArrowRight); + assert_eq!(state(&h), (3, vec![1, 2, 3]), "⇧→ runs the selection from the page it started on"); + press(&mut h, Modifiers::SHIFT, Key::ArrowLeft); + assert_eq!(state(&h), (2, vec![1, 2]), "and ⇧← shortens it again"); + press(&mut h, Modifiers::SHIFT, Key::ArrowDown); + assert_eq!(state(&h), (2 + cols, (1..=2 + cols).collect()), "⇧↓ takes in a whole row"); + press(&mut h, Modifiers::NONE, Key::ArrowUp); + assert_eq!(state(&h), (2, vec![]), "a plain arrow selects only the page it lands on"); + press(&mut h, Modifiers::NONE, Key::ArrowUp); + assert_eq!(state(&h), (2, vec![]), "nothing above the first row"); + + // The last row is short: ↓ from the row above it lands on the last page; from the last row + // there is nowhere to go. + let last_row = (PAGES - 1) / cols * cols; + let above = last_row - 1; + h.state_mut().views[0].current = above; + press(&mut h, Modifiers::NONE, Key::ArrowDown); + assert_eq!(state(&h).0, (above + cols).min(PAGES - 1)); + let landed = state(&h).0; + press(&mut h, Modifiers::NONE, Key::ArrowDown); + assert_eq!(state(&h).0, landed, "nothing below the last row"); +} + +#[test] +fn clicking_between_pages_in_the_organize_grid_drops_the_selection() { + let mut h = organize(); + h.state_mut().views[0].select_pages(&[1, 2, 3]); + h.run_steps(2); + assert_eq!(state(&h).1, vec![1, 2, 3]); + // Left of the first page: the grid's margin, where there is no page. + let page = h.get_by_label("Page 1").rect(); + let empty = egui::pos2(page.left() - 8.0, page.center().y); + h.hover_at(empty); + h.run_steps(1); + for pressed in [true, false] { + h.event(egui::Event::PointerButton { pos: empty, button: PointerButton::Primary, pressed, modifiers: Modifiers::NONE }); + h.run_steps(1); + } + h.run_steps(1); + assert_eq!(state(&h).1, Vec::::new()); + assert_eq!(state(&h).0, 1, "the current page stays where it was"); +} + +/// Shift comes from each arrow press, not from the frame's modifier state: Shift pressed or let +/// go within one frame must not flip what an arrow does (#4993 review). +#[test] +fn each_arrow_press_uses_the_shift_it_was_pressed_with() { + let mut h = organize(); + let key = |key: Key, modifiers: Modifiers| egui::Event::Key { key, physical_key: None, pressed: true, repeat: false, modifiers }; + // The frame's modifiers stay NONE; only the events carry Shift. + h.event(key(Key::ArrowRight, Modifiers::SHIFT)); + h.event(key(Key::ArrowRight, Modifiers::SHIFT)); + h.run_steps(2); + assert_eq!(state(&h), (2, vec![0, 1, 2]), "both presses in one frame extend the selection"); + h.event(egui::Event::ModifiersChanged(Modifiers::SHIFT)); + h.event(key(Key::ArrowLeft, Modifiers::NONE)); + h.run_steps(2); + h.event(egui::Event::ModifiersChanged(Modifiers::NONE)); + h.run_steps(1); + assert_eq!(state(&h), (1, vec![]), "a plain press with Shift held later in the frame selects only its page"); +}