From a4b705491995927142b910adf786b06e5c2e9caf Mon Sep 17 00:00:00 2001 From: Aleksander Helgaker Date: Thu, 15 Oct 2026 08:34:52 +0200 Subject: [PATCH] Issue #1910: Organize opens on the current page (#1975) Every way into the page grid (toolbar, tool panel, page.organize, --organize, Create multiple) goes through DocView::open_organize, which drops a selection left from earlier that does not include the page being read and scrolls that page's cell into view, computed from the row so virtualized rows need not be laid out. Grid clicks navigate the document as Pages-panel clicks do, so the viewer no longer resets the current page from its scroll position when the grid closes. Regression: crates/ui-egui/tests/editing.rs::organize_opens_on_the_current_page (middle page, last page, reopening after a selection elsewhere). Co-authored-by: Claude Fable 5.1 Co-authored-by: Brandon Thomas --- ROADMAP.md | 2 + crates/ui-egui/src/canvas.rs | 32 ++++++++++++++-- crates/ui-egui/src/commands.rs | 8 +++- crates/ui-egui/src/create_multiple_ui.rs | 2 +- crates/ui-egui/src/lib.rs | 16 ++++++-- crates/ui-egui/tests/editing.rs | 47 ++++++++++++++++++++++++ docs/gaps.md | 6 ++- parity/acrobat-features.toml | 4 +- 8 files changed, 105 insertions(+), 12 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 4e2626fb5..1999e02b8 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2,6 +2,7 @@ **Stage: alpha** · next: beta, ~25% (ready for real work 50% → 75%) and ~400–700 h away +> **Last reviewed:** 2026-10-13 · **Last updated:** 2026-10-13 · **Change:** minor (M4 #1910: Organize Pages opens on the current page; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) > **Last reviewed:** 2026-10-13 · **Last updated:** 2026-10-13 · **Change:** minor (M3 candidate: native opens retain chosen tools through password retries and defer startup intake while preparation is pending; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) > **Last reviewed:** 2026-10-12 · **Last updated:** 2026-10-12 · **Change:** minor (M4 #1310 #1311 #1312: undo, revert and the next save keep the last saved revision; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) > **Last reviewed:** 2026-10-12 · **Last updated:** 2026-10-12 · **Change:** minor (M13: CLI check visits each directory once through aliases and cycles; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) @@ -159,6 +160,7 @@ Ranked; detail and the full milestone table in [docs/roadmap.md](docs/roadmap.md - **2026-10-12 (M3, page-label navigation):** The page box displays logical labels and keeps the current page when its unchanged value is submitted, including duplicate labels. Editable-control regressions also cover numeric fallback, invalid input, active typing and focused same-document replacement with changed labels or fewer pages. Clean file-watcher reload preserves the chosen scrolling mode, preventing a Single Page reader from jumping to page 1 during replacement. Estimates unchanged. - **2026-10-13 (community, M2/M6, form-heavy documents):** An image whose pixels hold at most 16 bits of samples (an 8-bit Separation or DeviceN spot-colour image, a 1-4-bit gray or indexed one) is converted once per possible pixel value instead of once per pixel (vendored hayro-interpret patch 28). A fillable character sheet whose 3304 × 4105 DeviceN background ran a PostScript tint transform per pixel drew each page in 3.8 s and 430 MB, on opening and again after every field was filled; it now draws in 0.11 s and 100 MB, pixel for pixel the same. The inspector places widgets without `/P` in one pass over the pages (a form of 8,000 such fields opened in 1.17 s), and the Fields panel pairs its rows with the form through one map a frame and draws only the rows in view (8,000 fields: about 15× less CPU a frame with the panel open). Regressions fail before each fix. - **2026-10-14 (M9, signature appearance labels):** A visible signature's labels ("Digitally signed by", "Reason: ", …) follow the interface language when Helvetica can draw all of them, and fall back to English as a set otherwise, with a note under the preview; the preview shows the same labels and the signing time. A drawn label the font can't draw is refused naming the label and character, never written as `?`. `sign_document` takes `labels` and `appearance`. Sign, automation and kittest regressions cover German, Chinese and the refusals. Estimates unchanged. +- **2026-10-13 (M4, #1910 Organize on the current page):** Opening Organize Pages from the toolbar, tool panel, command or `--organize` selects the page being read and scrolls its cell into view, replacing a stale grid selection that did not include it. One UI regression covers a middle page, the last page and reopening after an earlier selection. Estimates unchanged. - **2026-10-13 (M3, native-opening candidate):** Chosen tools survive preparation, conversion and password retries; startup intake waits for pending preparation. Regression tests cover completion, cancellation and document identity. Estimates unchanged. - **2026-10-13 (community, M3, #531 touchpad momentum):** On Wayland a two-finger swipe keeps gliding after the fingers lift and slows to a stop, measured from the gesture's last 0.1 s and decaying with a 0.35 s time constant through the #759 1:1 path. Any wheel input, click, key, zoom or edge ends it (fingers resting on the touchpad don't yet: the vendored winit binds no Wayland hold gestures); single-page paging, macOS (which sends its own momentum), the web, and Animation effects Off never glide. Unit tests for the physics and two headless view tests. Estimates unchanged. - **2026-10-13 (M3, native-open admission):** Later prepared replies stay queued while a modal or native picker owns the foreground. Picker completion, cancellation and abandonment wake retained opens. Regressions cover recipient ownership, exactly-once admission, source/history preservation and one-shot repaint requests. Estimates unchanged. diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index f9e9aeca9..c19d90c69 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -476,6 +476,8 @@ pub struct DocView { /// The page to keep in place after a grid zoom, and how far below the top of the grid its /// cell was. grid_anchor: Option<(usize, f32)>, + /// The page the grid scrolls into view on its next frame (Organize just opened on it). + grid_reveal: Option, /// 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, @@ -677,6 +679,7 @@ impl DocView { grid_zoom: 1.0, grid_zoom_request: None, grid_anchor: None, + grid_reveal: None, grid_pages: HashMap::new(), last_scale: 0.0, sharp_hold_until: 0.0, @@ -765,6 +768,18 @@ impl DocView { self.page_input_reselect = true; } + /// Open Organize Pages on the page being read (issue #1910). An empty selection means the + /// current page, so a selection left from earlier that does not include it is dropped, and + /// the grid scrolls its cell into view. Every way into the grid goes through here; closing + /// it is a plain `organize = false`. + pub fn open_organize(&mut self) { + self.organize = true; + if !self.selected.is_empty() && !self.selected.contains(&self.current) { + self.clear_page_selection(Some(self.current)); + } + self.grid_reveal = Some(self.current); + } + /// Pages an organize command acts on: the selection, or the current page. pub fn target_pages(&self) -> Vec { if self.selected.is_empty() { vec![self.current] } else { self.selected.iter().copied().collect() } @@ -4333,6 +4348,7 @@ fn organize_grid(view: &mut DocView, info: &DocInfo, editable: bool, dirty: bool // The page image scales with the zoom; the padding and the page number don't. 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 mut open_page = None; organize_toolbar(view, info, editable, dirty, unobstructed, ui, t); let viewport = ui.available_rect_before_wrap(); @@ -4364,6 +4380,14 @@ fn organize_grid(view: &mut DocView, info: &DocInfo, editable: bool, dirty: bool 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. + 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()); + } 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)); for col in 0..cols { @@ -4447,7 +4471,9 @@ fn organize_grid(view: &mut DocView, info: &DocInfo, editable: bool, dirty: bool check.widget_info(|| egui::WidgetInfo::selected(egui::WidgetType::Checkbox, true, checked, label.clone())); if check.on_hover_text(label).changed() { view.click_page(i, egui::Modifiers::COMMAND, false); - view.current = i; + // The document shows the page picked here when the grid closes, as after a + // Pages-panel click, so reopening the grid keeps the pick (issue #1910). + view.go_to_page(i); } // Local actions always target the labelled page. The toolbar above the grid // remains the place for actions on the whole selection. @@ -4477,7 +4503,7 @@ fn organize_grid(view: &mut DocView, info: &DocInfo, editable: bool, dirty: bool if resp.clicked() { let m = ui.input(|i| i.modifiers); view.click_page(i, m, false); - view.current = i; + view.go_to_page(i); } if resp.double_clicked() { open_page = Some(i); @@ -4486,7 +4512,7 @@ fn organize_grid(view: &mut DocView, info: &DocInfo, editable: bool, dirty: bool resp.context_menu(|ui| { if !view.selected.contains(&i) { view.selected = [i].into(); - view.current = i; + view.go_to_page(i); } if ui.button(tl!("Extract pages")).clicked() { view.pending_action = Some(ViewAction::Extract); diff --git a/crates/ui-egui/src/commands.rs b/crates/ui-egui/src/commands.rs index 5a28ce9b2..7f974e7d3 100644 --- a/crates/ui-egui/src/commands.rs +++ b/crates/ui-egui/src/commands.rs @@ -317,8 +317,12 @@ impl PdfCraftApp { self.apply_edit(Edit::ResetForm { names: None }); } "page.organize" => { - if let Some(i) = active { - self.views[i].organize = !self.views[i].organize; + if let Some(v) = active.and_then(|i| self.views.get_mut(i)) { + if v.organize { + v.organize = false; + } else { + v.open_organize(); + } } } "page.rotate" => { diff --git a/crates/ui-egui/src/create_multiple_ui.rs b/crates/ui-egui/src/create_multiple_ui.rs index a89f5d592..54d2181d0 100644 --- a/crates/ui-egui/src/create_multiple_ui.rs +++ b/crates/ui-egui/src/create_multiple_ui.rs @@ -148,7 +148,7 @@ impl PdfCraftApp { if self.views.len() > before && let Some(view) = self.active.and_then(|i| self.views.get_mut(i)) { - view.organize = true; + view.open_organize(); } } } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 07030210b..d45f609e5 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -1315,7 +1315,7 @@ impl PdfCraftApp { && self.left == LeftPanel::Tool("organize") && let Some(view) = self.views.last_mut() { - view.organize = true; + view.open_organize(); } if activate && let Some(mode) = self.mode_override { // Explicit mode options do not reset independent --tool / --left choices. @@ -2067,7 +2067,11 @@ impl PdfCraftApp { self.left = LeftPanel::Tool(id); self.reveal_tools(); if let Some(view) = self.active.and_then(|i| self.views.get_mut(i)) { - view.organize = id == "organize"; + if id == "organize" { + view.open_organize(); + } else { + view.organize = false; + } } } @@ -2506,7 +2510,13 @@ impl PdfCraftApp { "right" => true, _ => return Err("tools-side must be left or right".into()), })?, - ("organize", Some(v)) => v.organize = value != "off", + ("organize", Some(v)) => { + if value == "off" { + v.organize = false; + } else { + v.open_organize(); + } + } // `--grid-zoom 150`: the size of the pages in the organize grid, in percent. ("grid-zoom", Some(v)) => { let percent = value.trim_end_matches('%').parse::().map_err(|e| e.to_string())?; diff --git a/crates/ui-egui/tests/editing.rs b/crates/ui-egui/tests/editing.rs index 3d365761d..78448dfb9 100644 --- a/crates/ui-egui/tests/editing.rs +++ b/crates/ui-egui/tests/editing.rs @@ -149,6 +149,53 @@ fn organize_tool_opens_the_overview_directly_and_can_be_reentered() { assert!(!h.state().views[0].organize, "another tool returns to the document"); } +/// Issue #1910: the page grid opens on the page the user was reading: it is selected and +/// scrolled into view, whether the document was on a middle page, its last page or a page +/// other than an earlier grid selection. +#[test] +fn organize_opens_on_the_current_page() { + let mut h = harness(60, |_| {}); + let in_view = |h: &Harness<'static, PdfCraftApp>, page: usize| { + let viewport = h.state().views[0].viewport_rect(); + let label = format!("Page {}", page + 1); + h.query_by_label(&label).is_some_and(|n| viewport.contains_rect(n.rect())) + }; + // From a middle page. + h.state_mut().views[0].go_to_page(30); + h.run_steps(2); + assert!(h.state_mut().execute("page.organize")); + h.run_steps(4); + assert!(h.state().views[0].organize); + assert_eq!(h.state().views[0].target_pages(), [30]); + assert!(h.state().views[0].selected.is_empty(), "an empty selection is the current page, so checkboxes still toggle"); + assert!(in_view(&h, 30), "the current page is scrolled into view"); + assert!(!in_view(&h, 0), "the grid does not open on page 1"); + // An earlier grid selection on another page does not win over the page the user went to. + h.get_by_label("Select page 32").click(); + h.run_steps(2); + assert_eq!(h.state().views[0].target_pages(), [31]); + h.get_by_label("Close").click(); + h.run_steps(2); + assert!(!h.state().views[0].organize); + h.state_mut().views[0].go_to_page(45); + h.run_steps(2); + h.state_mut().set_option("tool", "organize").unwrap(); + h.run_steps(4); + assert_eq!(h.state().views[0].target_pages(), [45]); + assert!(in_view(&h, 45)); + assert!(!in_view(&h, 1)); + // From the last page. + h.get_by_label("Close").click(); + h.run_steps(2); + h.state_mut().views[0].go_to_page(59); + h.run_steps(2); + h.state_mut().set_option("organize", "on").unwrap(); + h.run_steps(4); + assert_eq!(h.state().views[0].target_pages(), [59]); + assert!(in_view(&h, 59)); + assert!(!in_view(&h, 0)); +} + #[test] fn organize_tool_chosen_before_opening_a_document_shows_its_pages() { let mut app = PdfCraftApp::new(); diff --git a/docs/gaps.md b/docs/gaps.md index 9f016bf0d..9717c14e0 100644 --- a/docs/gaps.md +++ b/docs/gaps.md @@ -1,6 +1,8 @@ # Where PdfCraft falls short of Acrobat Pro > **Last reviewed:** 2026-10-12 · **Last updated:** 2026-10-13 · **Change:** minor (gap 14: Vietnamese catalog shipped; Hindi/Indonesian/Korean and Arabic RTL remain) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) +> **Last reviewed:** 2026-10-13 · **Last updated:** 2026-10-13 · **Change:** minor (gap 6: #1910 Organize Pages opens on the current page; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) +> **Last reviewed:** 2026-10-12 · **Last updated:** 2026-10-12 · **Change:** minor (gap 6: #1398 stamps read upright on rotated pages; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) > **Last reviewed:** 2026-10-12 · **Last updated:** 2026-10-12 · **Change:** minor (gap 6: #1317, #1327, #1302, #1315 and #1340; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) > **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-12 · **Change:** minor (#1316: replacing a line keeps a neighbouring line whose glyph box overlaps; gap 3 remains) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) > **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-12 · **Change:** gap 4 closed: the web build prints through the browser's print dialog; exclusive image-crop resize follow-up; minor (Word invoice regression coverage, #773; Office fidelity still partial; gap 2: hayro patch counts after the native-gradient backport) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) @@ -135,7 +137,8 @@ update the numbers. batch keeps a cloudy comment flatten cannot draw (#1317), leaves a refused text-box restyle or resize unchanged (#1327), keeps arrow defaults off plain lines (#1302), stops a custom text match on the separator instead of the next glyph (#1315), and states the vertical-text - redaction refusal without claiming an overlap (#1340). The rest of the backlog is untriaged. + redaction refusal without claiming an overlap (#1340). Organize Pages opens on the page being + read, selected and scrolled into view (#1910). The rest of the backlog is untriaged. - **Impact:** every one is a user who lost trust. Crash reports outrank feature work (`AGENTS.md` §4). - **Estimate:** 45–80 h (≈ 0.2–0.4 h per issue). **Doc:** this file. @@ -361,6 +364,7 @@ update the numbers. | Date | Change | Summary | |---|---|---| +| 2026-10-13 | minor | Gap 6: Organize Pages selects and reveals the current page on open, including after an earlier grid selection elsewhere (#1910). Estimates unchanged. | | 2026-10-12 | minor | Gap 6: rubber stamps, explicit image rectangles and PDF-page stamps read upright on rotated pages (#1398). Estimates unchanged. | | 2026-10-12 | minor | #1316: replacing a line or paragraph keeps a neighbouring line whose glyph box overlaps. Gap 3 remains. Estimates unchanged. | | 2026-10-12 | minor | Exclusive image crops resize with their pixels across nested states, intervening operators and content streams; shared clips remain fixed. Synthetic engine/tool regressions and private image save/reopen checks; estimates unchanged. | diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 31ec9d336..283147bc6 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -2608,8 +2608,8 @@ milestone = "M4" status = "shipped" commands = ["page.organize"] tools = [] -evidence = ["crates/ui-egui/tests/editing.rs::shift_click_selects_a_range_and_moves_it", "crates/ui-egui/tests/editing.rs::organize_tool_opens_the_overview_directly_and_can_be_reentered", "crates/ui-egui/tests/control.rs::organize_overview_is_reachable_through_the_control_channel"] -notes = "Opening the Organize pages tool enters the overview directly. Hovered, focused and selected pages expose individual rotate/delete actions; the toolbar acts on the selection." +evidence = ["crates/ui-egui/tests/editing.rs::shift_click_selects_a_range_and_moves_it", "crates/ui-egui/tests/editing.rs::organize_tool_opens_the_overview_directly_and_can_be_reentered", "crates/ui-egui/tests/editing.rs::organize_opens_on_the_current_page", "crates/ui-egui/tests/control.rs::organize_overview_is_reachable_through_the_control_channel"] +notes = "Opening the Organize pages tool enters the overview directly, on the current page: selected and scrolled into view (#1910). Hovered, focused and selected pages expose individual rotate/delete actions; the toolbar acts on the selection." [[feature]] id = "organize.multi-select-pages"