From 9a76f90f070e0b3d11f61bf42e4e30a7da26b43f Mon Sep 17 00:00:00 2001 From: Oliver Hagmann <268387159+ohagmann@users.noreply.github.com> Date: Tue, 20 Oct 2026 18:38:28 +0200 Subject: [PATCH] M5: the arrow keys move the selected comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With a comment selected, the arrow keys scrolled the view (↑ ↓) or turned the page (← →), so a text box, rectangle or line could only be placed with the mouse. Now an arrow key moves the selected comment a screen pixel, and ⌘/Ctrl or Shift and an arrow ten, as Edit text nudges objects; up is up on screen whatever the zoom, the page's rotation and the view's. Each press is one undo step (Edit::MoveAnnotation). The keys are taken before the view's navigation, which has them back once nothing is selected. Text markup stays with its text, and a locked comment where it is. Co-Authored-By: Claude Opus 5.5 --- ROADMAP.md | 2 ++ crates/ui-egui/src/comments.rs | 49 +++++++++++++++++++++++++++++ crates/ui-egui/src/lib.rs | 2 ++ crates/ui-egui/tests/it/comments.rs | 44 ++++++++++++++++++++++++++ docs/ui-parity.md | 2 +- parity/acrobat-features.toml | 12 +++++++ 6 files changed, 110 insertions(+), 1 deletion(-) diff --git a/ROADMAP.md b/ROADMAP.md index bc057209c..9164899c0 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -225,6 +225,8 @@ Ranked; detail and the full milestone table in [docs/roadmap.md](docs/roadmap.md - **2026-10-20 (M3, #3541/#3620):** Comment hover follows the individual marked lines instead of the enclosing multiline rectangle, and overlapping comments follow paint order. Synthetic UI regressions cover all four text-markup kinds, page/view rotations, neighboring highlights, legacy rectangle fallback and unchanged PDF bytes. Estimates unchanged. - **2026-10-20 (M10, #3532 partial):** Vector printing retains existing widget artwork when its appearance lacks Subtype. A print-owned copy adds Form dispatch metadata. The source stream and all explicit subtypes stay unchanged. Synthetic source/output raster checks cover resources, selected/shared appearances, placement, flags and content modes. Appearance regeneration and physical printer validation remain outside this fix. Estimates unchanged. +- **2026-10-20 (M5, arrow keys move comments):** With a comment selected, the arrow keys move it a screen pixel and ⌘/Ctrl or Shift and an arrow ten, instead of scrolling the view or turning the page; up is up on screen at any zoom and rotation, and each press is one undo step. Text markup and locked comments stay put. Estimates unchanged. + - **2026-10-19 (M14, #3373):** The main menu and its long submenus scroll in short windows, keeping their commands reachable. Estimates unchanged. - **2026-10-19 (M3, #3387):** Save-prompt shortcuts yield to later consent and utility dialogs; a synthetic UI regression checks that an edited document stays open. Estimates unchanged. diff --git a/crates/ui-egui/src/comments.rs b/crates/ui-egui/src/comments.rs index d80f988b7..541e1c8ad 100644 --- a/crates/ui-egui/src/comments.rs +++ b/crates/ui-egui/src/comments.rs @@ -1823,6 +1823,55 @@ pub(crate) fn keys(ctx: &egui::Context, view: &mut DocView, tool: &mut QuickTool } } +/// The arrow keys move the selected comment a screen pixel; with ⌘/Ctrl or Shift, ten. Up is up +/// on screen whatever the zoom, the page's rotation and the view's; the keys are taken before +/// the view scrolls or turns the page with them. Each press is one undo step. Text markup stays +/// with its text, and a locked comment where it is. +pub(crate) fn nudge_keys(ctx: &egui::Context, view: &mut DocView, info: &DocInfo) { + if ctx.egui_wants_keyboard_input() || view.pending_edit.is_some() { + return; + } + let cv = &view.comments; + if cv.gesture.is_some() || cv.composer.is_some() { + return; + } + let Some((page, index)) = cv.selected else { return }; + let movable = + info.annotations.iter().any(|a| a.page == page && a.index == index && a.in_reply_to.is_none() && !is_markup(&a.subtype) && !a.locked); + let (Some(xf), Some(p)) = (view.page_xform(page), info.pages.get(page)) else { return }; + if !movable { + return; + } + use egui::{Key, Modifiers}; + let mut step = egui::Vec2::ZERO; + ctx.input_mut(|i| { + for (key, direction) in + [(Key::ArrowLeft, vec2(-1.0, 0.0)), (Key::ArrowRight, vec2(1.0, 0.0)), (Key::ArrowUp, vec2(0.0, -1.0)), (Key::ArrowDown, vec2(0.0, 1.0))] + { + // The modified patterns first: a plain one also matches a Shift press. + if i.consume_key(Modifiers::COMMAND, key) || i.consume_key(Modifiers::SHIFT, key) { + step += direction * 10.0; + } else if i.consume_key(Modifiers::NONE, key) { + step += direction; + } + } + }); + if step == egui::Vec2::ZERO { + return; + } + // The step on screen, in user space: from the page's centre to `step` pixels away. + let user = |s: Pos2| { + let (vx, vy) = xf.screen_to_view(s); + let u = p.view_to_user(vx, vy); + [f64::from(u[0]), f64::from(u[1])] + }; + let (a, b) = (user(xf.rect.center()), user(xf.rect.center() + step)); + let (dx, dy) = (b[0] - a[0], b[1] - a[1]); + if dx.is_finite() && dy.is_finite() && (dx != 0.0 || dy != 0.0) { + view.pending_edit = Some(Edit::MoveAnnotation { page, index, dx, dy }); + } +} + /// Save attachment… and, for a PDF, Open in a new tab, first in a file attachment comment's /// menus as in the Attachments panel: the comment is where the reader finds its file. Reading /// the file changes nothing, so a read-only document offers them too. diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 64b22ff18..d618e4ba2 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -3023,6 +3023,8 @@ impl PdfCraftApp { && let Some(doc) = self.session.get(self.views[i].id).filter(|d| d.allows_modification()) { object_ui::nudge_keys(ctx, &mut self.views[i], doc); + } else if let Some(doc) = self.session.get(self.views[i].id).filter(|d| d.allows_annotation()) { + comments::nudge_keys(ctx, &mut self.views[i], &doc.info); } // A selected field moves with the arrows too (Prepare a form, #2608), before the view // turns the page with them. diff --git a/crates/ui-egui/tests/it/comments.rs b/crates/ui-egui/tests/it/comments.rs index 6194c7516..5e6227843 100644 --- a/crates/ui-egui/tests/it/comments.rs +++ b/crates/ui-egui/tests/it/comments.rs @@ -860,6 +860,50 @@ fn line_ends_are_dragged_to_change_length_and_angle() { assert!((p[1][0] - 200.0).abs() < 1.0 && (p[1][1] - 160.0).abs() < 1.0, "{p:?}"); } +/// The arrow keys move the selected comment a screen pixel, ⌘/Ctrl or Shift and an arrow ten, +/// instead of scrolling or turning the page; each press is one undo step. Without a selection +/// the arrows scroll and turn pages as before. +#[test] +fn arrow_keys_nudge_the_selected_comment() { + use egui::{Key, Modifiers}; + let mut h = harness(|app| app.set_option("quick", "square").unwrap()); + drag_pt(&mut h, (40.0, 100.0), (140.0, 40.0)); + h.state_mut().set_option("quick", "select").unwrap(); + click_pt(&mut h, (90.0, 70.0)); + assert_eq!(h.state().views[0].comments.selected, Some((0, 0))); + // One screen pixel in user space, at this zoom. + let pixel = 300.0 / h.state().views[0].page_screen_rect(0).unwrap().width(); + let before = comments(&h)[0].rect; + h.key_press(Key::ArrowRight); + h.run_steps(3); + let r = comments(&h)[0].rect; + assert!((r[0] - before[0] - pixel).abs() < 0.01 && (r[1] - before[1]).abs() < 0.01, "one pixel right: {r:?} from {before:?}"); + assert_eq!(h.state().views[0].current, 0, "the page didn't turn"); + let id = h.state().views[0].id; + assert_eq!(h.state().session.get(id).unwrap().can_undo(), Some("Move comment")); + // ⌘/Ctrl and Shift: ten pixels (down on screen is down in user space here). + h.key_press_modifiers(Modifiers::COMMAND, Key::ArrowDown); + h.run_steps(3); + let r2 = comments(&h)[0].rect; + assert!((r[1] - r2[1] - 10.0 * pixel).abs() < 0.01 && (r2[0] - r[0]).abs() < 0.01, "ten pixels down: {r2:?} from {r:?}"); + h.key_press_modifiers(Modifiers::SHIFT, Key::ArrowLeft); + h.run_steps(3); + let r3 = comments(&h)[0].rect; + assert!((r2[0] - r3[0] - 10.0 * pixel).abs() < 0.01, "ten pixels left: {r3:?} from {r2:?}"); + h.state_mut().undo(); + h.run_steps(2); + assert!((comments(&h)[0].rect[0] - r2[0]).abs() < 0.01, "each press is one undo step"); + // Nothing selected: → turns the page again. + h.key_press(Key::Escape); + h.run_steps(2); + assert_eq!(h.state().views[0].comments.selected, None); + let rect = comments(&h)[0].rect; + h.key_press(Key::ArrowRight); + h.run_steps(3); + assert_eq!(comments(&h)[0].rect, rect, "the comment stays"); + assert_eq!(h.state().views[0].current, 1, "and the view goes to the next page"); +} + #[test] fn a_shape_just_drawn_resizes_from_its_handles_without_the_select_tool() { // #2338: the new shape showed its handles, but only the Select tool answered them. diff --git a/docs/ui-parity.md b/docs/ui-parity.md index c12461793..703f1e9d8 100644 --- a/docs/ui-parity.md +++ b/docs/ui-parity.md @@ -52,7 +52,7 @@ No dump of Acrobat's menu tree has been diffed against the registry yet (gap 24) | Rulers, grids, snap to grid, guides | yes | no | planned | | Cursor coordinates, line weights | yes | no | planned | | Measuring snap to paths, endpoints, midpoints, intersections | yes | yes | shipped | -| Nudge selected objects with arrow keys (Shift for larger steps) | yes | yes in Edit text: a point, Shift ten points, for selected objects, images and an open paragraph's box (#1520); and in Prepare a form, for the selected field (#2608) | shipped | +| Nudge selected objects with arrow keys (Shift for larger steps) | yes | yes in Edit text: a point, Shift ten points, for selected objects, images and an open paragraph's box (#1520); and in Prepare a form, for the selected field (#2608); a selected comment: a screen pixel, ⌘/Ctrl or Shift ten | shipped | | Single-key tool accelerators (H, V, Z, …) | yes, opt-in | no | planned | | Context menus on pages, thumbnails, comments, fields | yes | thumbnails, comments, fields | partial | | Comment pop-ups as movable windows, connector lines | yes | inline in the panel | planned | diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 362c2aa77..d55fe67f6 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -3971,6 +3971,18 @@ tools = ["comment_delete"] evidence = ["crates/ui-egui/tests/it/comments.rs::comments_are_selected_moved_and_deleted_with_the_select_tool", "crates/annot/src/tests.rs::delete_takes_popup_and_reply_chain"] notes = "deletes the pop-up and the reply thread" +[[feature]] +id = "comment.keyboard-nudge" +title = "Move the selected comment with the arrow keys" +area = "E" +tier = "P2" +milestone = "M5" +status = "shipped" +commands = [] +tools = ["comment_edit"] +evidence = ["crates/ui-egui/tests/it/comments.rs::arrow_keys_nudge_the_selected_comment"] +notes = "With a comment selected (Select tool, or a Fill & Sign mark picked up), an arrow key moves it a screen pixel and ⌘/Ctrl or Shift and an arrow ten, up being up on screen whatever the zoom and rotation; each press is one undo step, and the arrows scroll and turn pages again once nothing is selected. Text markup stays with its text and a locked comment where it is. Headless, comment_edit's `move` takes the offset in points." + [[feature]] id = "comment.reshape" title = "Change a line's ends and a polygon's corners after drawing"