From 54b91598a3ea9773562902c4837d478940ec1f06 Mon Sep 17 00:00:00 2001
From: FredApps <153173854+FredApps@users.noreply.github.com>
Date: Sat, 14 Nov 2026 22:25:50 +0100
Subject: [PATCH] Move selected comments together with arrow keys (#7165)

* Move selected comments together with arrow keys

* Test keyboard nudging of selected lines

* test(ui): cover locked and duplicate nudge selections
---
 crates/ui-egui/src/tools/comments/input.rs | 22 ++++++-
 crates/ui-egui/tests/it/comments.rs        | 77 ++++++++++++++++++++++
 2 files changed, 97 insertions(+), 2 deletions(-)

diff --git a/crates/ui-egui/src/tools/comments/input.rs b/crates/ui-egui/src/tools/comments/input.rs
index 79b69a6eba..a23b8e26ae 100644
--- a/crates/ui-egui/src/tools/comments/input.rs
+++ b/crates/ui-egui/src/tools/comments/input.rs
@@ -788,7 +788,7 @@ pub(crate) fn delete_selected(cv: &mut CommentView, info: &DocInfo) -> Option<Ed
     }
 }
 
-/// The arrow keys move the selected comment a screen pixel; with ⌘/Ctrl or Shift, ten. Up is up
+/// The arrow keys move selected comments 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.
@@ -838,7 +838,25 @@ pub(crate) fn nudge_keys(ctx: &egui::Context, keys: PageKeys, view: &mut DocView
     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 });
+        // Match a pointer drag: move the eligible selection on this page as one undo step.
+        // Text markup and locked comments cannot be moved and would reject a batch.
+        let mut indices = vec![index];
+        for &(other_page, other_index) in &view.comments.also {
+            if other_page == page
+                && !indices.contains(&other_index)
+                && info.annotations.iter().any(|a| a.page == page && a.index == other_index && !a.is_reply() && !on_text(a) && !a.locked)
+            {
+                indices.push(other_index);
+            }
+        }
+        view.pending_edit = Some(if indices.len() == 1 {
+            Edit::MoveAnnotation { page, index, dx, dy }
+        } else {
+            Edit::Batch {
+                label: tl!("Move comments").to_owned(),
+                edits: indices.into_iter().map(|index| Edit::MoveAnnotation { page, index, dx, dy }).collect(),
+            }
+        });
     }
 }
 
diff --git a/crates/ui-egui/tests/it/comments.rs b/crates/ui-egui/tests/it/comments.rs
index 655e25b50c..de14b01525 100644
--- a/crates/ui-egui/tests/it/comments.rs
+++ b/crates/ui-egui/tests/it/comments.rs
@@ -1127,6 +1127,58 @@ fn arrow_keys_nudge_the_selected_comment() {
     assert_eq!(h.state().views[0].current, 1, "and the view goes to the next page");
 }
 
+#[test]
+fn arrow_keys_nudge_all_selected_comments_as_one_undo_step() {
+    use egui::Key;
+    let mut h = harness(|app| app.set_option("quick", "square").unwrap());
+    drag_pt(&mut h, (30.0, 110.0), (80.0, 70.0));
+    drag_pt(&mut h, (130.0, 110.0), (180.0, 70.0));
+    h.state_mut().set_option("quick", "select").unwrap();
+    let before: Vec<_> = comments(&h).iter().map(|a| a.rect).collect();
+    assert_eq!(before.len(), 2);
+    h.state_mut().views[0].comments.selected = Some((0, 0));
+    h.state_mut().views[0].comments.also = vec![(0, 1)];
+    h.key_press(Key::ArrowRight);
+    h.run_steps(3);
+    let moved: Vec<_> = comments(&h).iter().map(|a| a.rect).collect();
+    for (old, new) in before.iter().zip(&moved) {
+        assert!(new[0] > old[0], "each selected comment moves: {before:?} -> {moved:?}");
+    }
+    h.state_mut().undo();
+    h.run_steps(2);
+    let undone: Vec<_> = comments(&h).iter().map(|a| a.rect).collect();
+    assert_eq!(undone, before, "one undo restores the whole selection");
+}
+
+#[test]
+fn arrow_nudge_skips_locked_comments_and_duplicate_selection_entries() {
+    let mut h = harness(|app| app.set_option("quick", "square").unwrap());
+    for (a, b) in [((20.0, 20.0), (60.0, 60.0)), ((100.0, 20.0), (140.0, 60.0)), ((180.0, 20.0), (220.0, 60.0))] {
+        drag_pt(&mut h, a, b);
+    }
+    h.state_mut().set_option("quick", "select").unwrap();
+    assert!(h.state_mut().apply_edit(pdfcraft_engine::Edit::LockAnnotation { page: 0, index: 1, locked: true }));
+    h.run_steps(2);
+    let before = comments(&h);
+    h.state_mut().views[0].comments.selected = Some((0, 0));
+    h.state_mut().views[0].comments.also = vec![(0, 1), (0, 2), (0, 2), (0, 0)];
+    h.key_press(egui::Key::ArrowRight);
+    h.run_steps(3);
+    let after = comments(&h);
+    let left = |list: &[Annotation], i: usize| list.iter().find(|a| a.index == i).expect("comment still exists").rect[0];
+    let primary_delta = left(&after, 0) - left(&before, 0);
+    let secondary_delta = left(&after, 2) - left(&before, 2);
+    assert!(primary_delta > 0.0, "primary comment moves");
+    assert!((secondary_delta - primary_delta).abs() < 0.01, "duplicate entries must not move the second comment twice");
+    assert_eq!(left(&after, 1), left(&before, 1), "locked comment stays in place");
+    h.state_mut().undo();
+    h.run_steps(2);
+    let undone = comments(&h);
+    for i in 0..3 {
+        assert_eq!(left(&undone, i), left(&before, i), "one undo restores comment {i}");
+    }
+}
+
 /// #3632: with a comment selected, Alt+← / Alt+→ still go back and forward through the view
 /// history instead of nudging it (egui's key patterns let an extra Alt through).
 #[test]
@@ -5848,3 +5900,28 @@ fn properties_dialog_makes_the_text_box_font_size_default() {
         assert_eq!(font_size_of_last(&fresh).font_size, Some(size), "{quick}: the default font size persisted");
     }
 }
+
+#[test]
+fn ctrl_selected_lines_nudge_together_and_undo() {
+    let mut h = harness(|app| app.set_option("quick", "line").unwrap());
+    for y in [40.0, 80.0, 120.0] {
+        drag_pt(&mut h, (40.0, y), (160.0, y));
+    }
+    assert_eq!(comments(&h).len(), 3);
+    h.state_mut().set_option("quick", "select").unwrap();
+    h.run_steps(2);
+    click_pt(&mut h, (100.0, 40.0));
+    click_pt_with(&mut h, (100.0, 80.0), egui::Modifiers::COMMAND);
+    click_pt_with(&mut h, (100.0, 120.0), egui::Modifiers::COMMAND);
+    assert_eq!(h.state().views[0].comments.selection().len(), 3);
+    let before: Vec<_> = comments(&h).iter().map(|a| a.rect).collect();
+    h.key_press(egui::Key::ArrowRight);
+    h.run_steps(3);
+    let moved: Vec<_> = comments(&h).iter().map(|a| a.rect).collect();
+    for (old, new) in before.iter().zip(&moved) {
+        assert!(new[0] > old[0], "each selected line moves: {before:?} -> {moved:?}");
+    }
+    h.state_mut().undo();
+    h.run_steps(2);
+    assert_eq!(comments(&h).iter().map(|a| a.rect).collect::<Vec<_>>(), before);
+}