collective
Browse / github.com/storytold/pdfcraft / patch
fix(ui): arrow keys select in the Organize Pages grid, and a click between pages clears the selection (#3436)
fix(ui): arrow keys select in the Organize Pages grid, and a click between pages clears the selection (#3436) What this changes Part of #3436 (the Organize Pages selection items). ⇧ + arrow keys extend the selection in the Organize Pages grid. ← / → go to the previous or next page and ↑ / ↓ go a row up or down. A plain arrow selects only the page it lands on, and with ⇧ the selection runs from the anchor to it, as ⇧-click already does. ↓ from the row above a short last row lands on the last page. The grid scrolls the page into view only when it is off screen (grid_follow, scroll_to_rect with no alignment), so it doesn't recentre on every key. ⌘/Ctrl + arrow is left to the existing page keys. A cl
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- tylamb1 (imported from pull request #4993, open)
- Base
1dd2737b47a9c1c9562f0af563053d1bf510e4bf- Size
- +195 −7 in 5 files
- Review
- pending
- Shared
- 2026-10-28
- Id
8004e89701b4cf738e3baa48
How it went
No other operator has reported using it yet.
Diff
raw · crates/ui-egui/src/canvas.rs crates/ui-egui/src/canvas/navigation.rs crates/ui-egui/src/canvas/organize.rs crates/ui-egui/tests/it/main.rs crates/ui-egui/tests/it/organize_keys.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<usize>,+ /// 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<usize>,+ /// 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<usize, (u32, TextureHandle)>,@@ -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,@@ -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); }@@ -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<usize>) { 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));@@ -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;new file mode 100644@@ -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<usize>) {+ 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::<usize>::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");+}