From e125a2d659b77dc512d4c9cf50ddca43c333c7a6 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo <103679419+marcoarnulfo@users.noreply.github.com> Date: Tue, 13 Oct 2026 06:12:07 +0200 Subject: [PATCH] fix(edit): move existing text by its frame, and selections with the arrow keys (#1600) * fix(edit): move existing text by its frame, and selections with the arrow keys (fixes #1520) * docs(parity): arrow-key nudging and the paragraph frame are no longer gaps (#1520) --- crates/ui-egui/src/edit_text_ui.rs | 6 ++ crates/ui-egui/src/lib.rs | 5 ++ crates/ui-egui/src/object_ui.rs | 93 ++++++++++++++++++++++--- crates/ui-egui/tests/editing.rs | 107 +++++++++++++++++++++++++++++ docs/gaps.md | 2 +- docs/ui-parity.md | 2 +- parity/acrobat-features.toml | 4 +- 7 files changed, 206 insertions(+), 13 deletions(-) diff --git a/crates/ui-egui/src/edit_text_ui.rs b/crates/ui-egui/src/edit_text_ui.rs index 2352249b..65c50528 100644 --- a/crates/ui-egui/src/edit_text_ui.rs +++ b/crates/ui-egui/src/edit_text_ui.rs @@ -319,6 +319,12 @@ pub struct ImageSelection { drag: Option<(Pos2, Option)>, } +impl ImageSelection { + pub(crate) fn dragging(&self) -> bool { + self.drag.is_some() + } +} + /// Once a selected corner is grabbed, text under it cannot take over the gesture. The same /// applies after the pointer leaves the image/page while dragging. pub(crate) fn image_owns_pointer(ui: &egui::Ui, xf: &PageXform, page: usize, info: &DocInfo, view: &DocView) -> bool { diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 3418fcdf..fac33652 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -2539,6 +2539,11 @@ impl PdfCraftApp { } } self.tool_keys(i, ctx); + if self.quick_tool == QuickTool::EditText + && 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); + } canvas::shortcuts(&mut self.views[i], ctx); self.copy_key(i, ctx); self.image_pick_keys(i, ctx); diff --git a/crates/ui-egui/src/object_ui.rs b/crates/ui-egui/src/object_ui.rs index feb81c8c..118946bd 100644 --- a/crates/ui-egui/src/object_ui.rs +++ b/crates/ui-egui/src/object_ui.rs @@ -101,6 +101,16 @@ impl ObjectSelection { } } + /// The move of the whole selection by `offset` (user space). The selection follows it. + fn move_by(&mut self, page: usize, offset: [f64; 2]) -> Edit { + let mut expected = self.selected.clone(); + for object in &mut expected { + object.rect = [object.rect[0] + offset[0], object.rect[1] + offset[1], object.rect[2] + offset[0], object.rect[3] + offset[1]]; + } + self.pending = Some(expected); + Edit::MoveObjects { page, objects: self.selected.iter().map(|o| o.target).collect(), offset } + } + fn inventory(&mut self, doc: &Document, page: usize) -> Result, String> { if self.cache.len() >= 8 && !self.cache.contains_key(&page) { self.cache.clear(); @@ -215,15 +225,7 @@ fn input(s: &mut ObjectSelection, ui: &egui::Ui, resp: &egui::Response, xf: &Pag } else if gesture.start.distance(gesture.now) > 3.0 && view.pending_edit.is_none() { let (a, b) = (xf.screen_to_view(gesture.start), xf.screen_to_view(gesture.now)); match doc.object_move_offset(page, [f64::from(b.0 - a.0), f64::from(b.1 - a.1)]) { - Ok(offset) => { - let mut expected = s.selected.clone(); - for object in &mut expected { - object.rect = - [object.rect[0] + offset[0], object.rect[1] + offset[1], object.rect[2] + offset[0], object.rect[3] + offset[1]]; - } - s.pending = Some(expected); - view.pending_edit = Some(Edit::MoveObjects { page, objects: s.selected.iter().map(|o| o.target).collect(), offset }); - } + Ok(offset) => view.pending_edit = Some(s.move_by(page, offset)), Err(_) => s.notice = Some(tl!("These objects could not be moved. Select them again and try a smaller move.").into()), } } @@ -294,6 +296,20 @@ fn input(s: &mut ObjectSelection, ui: &egui::Ui, resp: &egui::Response, xf: &Pag return true; } } + // The frame of the paragraph open for typing takes hold of the box, as in Acrobat: a click + // selects it (for the arrow keys), a drag moves it. Its text keeps the caret. + if let Some(block) = view.line_editor.as_ref().filter(|e| e.page == page && !e.has_changes()).map(|e| e.block) + && let Some(object) = objects.iter().find(|o| o.target == ObjectTarget { kind: ObjectKind::Text, index: block }) + && on_frame(screen(xf, info, page, object), p) + { + s.clear(); + s.page = Some(page); + s.selected.push(object.clone()); + s.gesture = Some(Gesture { start: p, now: p, marquee: false, additive: false }); + leave_single(view); + view.current = page; + return true; + } s.clear(); if hit.is_some() { return false; @@ -305,6 +321,65 @@ fn input(s: &mut ObjectSelection, ui: &egui::Ui, resp: &egui::Response, xf: &Pag true } +/// Whether `p` is on the frame drawn around a paragraph open for typing (`r`, screen): just +/// outside the text, where a press would otherwise close the paragraph. +fn on_frame(r: Rect, p: Pos2) -> bool { + r.expand(2.0 + FRAME_REACH).contains(p) && !r.expand(2.0).contains(p) +} + +/// How far outside a paragraph's frame a press still grabs it (screen pixels). +const FRAME_REACH: f32 = 6.0; + +/// The arrow keys move the selected image or objects a point, Shift and an arrow ten points, +/// as in Acrobat. They are taken before the view turns the page with them. +pub(crate) fn nudge_keys(ctx: &egui::Context, view: &mut DocView, doc: &Document) { + if ctx.egui_wants_keyboard_input() || view.pending_edit.is_some() { + return; + } + let image = view.image_selection.as_ref().filter(|s| !s.dragging()).map(|s| (s.page, s.index)); + let objects = view.objects.page.filter(|_| !view.objects.selected.is_empty() && view.objects.gesture.is_none()); + let Some(page) = objects.or(image.map(|(page, _)| page)) else { return }; + let Some(xf) = view.page_xform(page) else { return }; + use egui::{Key, Modifiers}; + let mut step = egui::Vec2::ZERO; + ctx.input_mut(|i| { + for (key, direction) in [ + (Key::ArrowLeft, egui::vec2(-1.0, 0.0)), + (Key::ArrowRight, egui::vec2(1.0, 0.0)), + (Key::ArrowUp, egui::vec2(0.0, -1.0)), + (Key::ArrowDown, egui::vec2(0.0, 1.0)), + ] { + // Shift first: a plain key pattern also matches a Shift press. + if 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; + } + // A point on screen, whatever the zoom and the view's rotation: one screen pixel's step in + // view space, at unit length. + let c = xf.rect.center(); + let unit = |d: egui::Vec2| { + let (a, b) = (xf.screen_to_view(c), xf.screen_to_view(c + d)); + let v = egui::vec2(b.0 - a.0, b.1 - a.1); + if v.length() > 0.0 { v / v.length() } else { egui::Vec2::ZERO } + }; + let delta = unit(egui::vec2(1.0, 0.0)) * step.x + unit(egui::vec2(0.0, 1.0)) * step.y; + let Ok(offset) = doc.object_move_offset(page, [f64::from(delta.x), f64::from(delta.y)]) else { return }; + if objects.is_some() { + view.pending_edit = Some(view.objects.move_by(page, offset)); + } else if let Some((page, index)) = image + && let Some(r) = doc.page_images(page).get(index).map(|im| im.rect) + { + let rect = [r[0] + offset[0], r[1] + offset[1], r[2] + offset[0], r[3] + offset[1]]; + view.pending_edit = Some(Edit::EditPageImage { page, index, change: pdfcraft_engine::ImageEdit::Move(rect) }); + } +} + pub(crate) fn paint(painter: &egui::Painter, xf: &PageXform, page: usize, info: &DocInfo, selection: &ObjectSelection) { if selection.page != Some(page) { return; diff --git a/crates/ui-egui/tests/editing.rs b/crates/ui-egui/tests/editing.rs index 56e7cce3..882f19dd 100644 --- a/crates/ui-egui/tests/editing.rs +++ b/crates/ui-egui/tests/editing.rs @@ -2651,6 +2651,113 @@ fn dragging_a_paragraph_moves_it_and_its_edge_rewraps_it() { assert!(h.state().views[0].line_editor.is_none()); } +/// Press and release at `at` without moving. +fn click_at(h: &mut Harness<'static, PdfCraftApp>, at: egui::Pos2) { + h.hover_at(at); + h.run_steps(1); + h.drag_at(at); + h.run_steps(1); + h.drop_at(at); + h.run_steps(4); +} + +fn press(h: &mut Harness<'static, PdfCraftApp>, modifiers: Modifiers, key: Key) { + h.key_press_modifiers(modifiers, key); + h.run_steps(4); +} + +#[test] +fn arrow_keys_nudge_a_selected_image_instead_of_turning_the_page() { + use pdfcraft_cos::{Dict, Document, Object, SaveOptions, Stream, write_incremental}; + let mut doc = Document::open(std::sync::Arc::new(fixture(2))).unwrap(); + let page = pdfcraft_model::pages(&doc)[0].clone(); + let mut form = Dict::new(); + form.set(b"Subtype".to_vec(), Object::name("Form")); + form.set(b"BBox".to_vec(), Object::Array([0, 0, 200, 300].map(Object::Int).to_vec())); + let artwork = doc.add(Object::Stream(Stream::flate(form, b"0.2 0.5 0.9 rg 40 200 120 80 re f"))); + let contents = doc.add(Object::Stream(Stream::flate(Dict::new(), b"/Figure Do"))); + doc.update_dict(page.obj, |d| { + d.set(b"Contents".to_vec(), Object::Ref(contents)); + let mut xo = Dict::new(); + xo.set(b"Figure".to_vec(), Object::Ref(artwork)); + let mut resources = Dict::new(); + resources.set(b"XObject".to_vec(), Object::Dict(xo)); + d.set(b"Resources".to_vec(), Object::Dict(resources)); + }) + .unwrap(); + let bytes = write_incremental(&doc, &SaveOptions::default()).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("figure.pdf", None, bytes.clone()).expect("opens"); + app + }); + h.run_steps(4); + assert!(h.state_mut().execute("edit.edit_text")); + h.run_steps(2); + let id = h.state().views[0].id; + let rect = |h: &Harness<'static, PdfCraftApp>| h.state().session.get(id).unwrap().page_images(0)[0].rect; + let before = rect(&h); + let r = h.state().views[0].page_screen_rect(0).expect("on screen"); + click_at(&mut h, egui::pos2(r.left() + 100.0 / 200.0 * r.width(), r.top() + (300.0 - 240.0) / 300.0 * r.height())); + assert!(h.state().views[0].image_selection.is_some(), "selected"); + // As in Acrobat: an arrow moves it a point, Shift and an arrow ten. + press(&mut h, Modifiers::NONE, Key::ArrowRight); + press(&mut h, Modifiers::SHIFT, Key::ArrowDown); + let moved = rect(&h); + let near = |a: f64, b: f64| (a - b).abs() < 0.01; + assert!(near(moved[0], before[0] + 1.0) && near(moved[1], before[1] - 10.0), "{before:?} → {moved:?}"); + assert!(near(moved[2] - moved[0], before[2] - before[0]), "moved, not resized"); + assert_eq!(h.state().session.get(id).unwrap().can_undo(), Some("Move image")); + assert_eq!(h.state().views[0].current, 0, "the arrows moved the image, not the page"); + assert!(h.state().views[0].image_selection.is_some(), "it stays selected for the next nudge"); +} + +#[test] +fn an_open_paragraph_moves_by_its_frame_and_by_the_arrow_keys() { + let mut h = harness(2, |_| {}); + assert!(h.state_mut().execute("edit.edit_text")); + h.run_steps(2); + let id = h.state().views[0].id; + let block = |h: &Harness<'static, PdfCraftApp>| h.state().session.get(id).unwrap().text_blocks(0)[0].clone(); + let r = h.state().views[0].page_screen_rect(0).expect("on screen"); + let k = r.width() / 200.0; + let screen = |x: f64, y: f64| egui::pos2(r.left() + x as f32 * k, r.top() + (300.0 - y as f32) * k); + // Just above the paragraph's frame: on the frame, off its text. + let frame = |b: &pdfcraft_engine::TextBlock| screen((b.rect[0] + b.rect[2]) / 2.0, b.rect[3]) - egui::vec2(0.0, 4.0); + let near = |a: f64, b: f64| (a - b).abs() < 0.01; + + let before = block(&h); + click_at(&mut h, screen((before.rect[0] + before.rect[2]) / 2.0, (before.rect[1] + before.rect[3]) / 2.0)); + assert!(h.state().views[0].line_editor.is_some(), "a click on the text opens it for typing"); + // A click on its frame selects the box instead, and the arrows move it. + click_at(&mut h, frame(&before)); + assert!(h.state().views[0].line_editor.is_none(), "the frame selects the box, it doesn't type"); + press(&mut h, Modifiers::NONE, Key::ArrowRight); + press(&mut h, Modifiers::SHIFT, Key::ArrowUp); + let nudged = block(&h); + assert_eq!(nudged.text, "Page 1"); + assert!(near(nudged.rect[0], before.rect[0] + 1.0) && near(nudged.rect[1], before.rect[1] + 10.0), "{:?} → {:?}", before.rect, nudged.rect); + assert_eq!(h.state().session.get(id).unwrap().can_undo(), Some("Move object")); + assert_eq!(h.state().views[0].current, 0, "the arrows moved the paragraph, not the page"); + + // Open it again and drag it by its frame: it follows the pointer. + press(&mut h, Modifiers::NONE, Key::Escape); + click_at(&mut h, screen((nudged.rect[0] + nudged.rect[2]) / 2.0, (nudged.rect[1] + nudged.rect[3]) / 2.0)); + assert!(h.state().views[0].line_editor.is_some()); + let from = frame(&nudged); + drag(&mut h, from, from + egui::vec2(30.0 * k, -50.0 * k)); + assert!(h.state().views[0].line_editor.is_none()); + let dragged = block(&h); + assert!( + (dragged.rect[0] - nudged.rect[0] - 30.0).abs() < 1.5 && (dragged.rect[1] - nudged.rect[1] - 50.0).abs() < 1.5, + "{:?} → {:?}", + nudged.rect, + dragged.rect + ); + assert_eq!(h.state().session.get(id).unwrap().text_lines(0).len(), 1, "moved, not retyped"); +} + /// Contributor-original text, with an off-origin CropBox. Keep the moving paragraph last in /// source order so an ordinary hit picks it again when two independent boxes fully overlap. fn paragraph_drag_fixture(content: &str, rotation: i64, unit: i64) -> Vec { diff --git a/docs/gaps.md b/docs/gaps.md index 0e8b583d..1b8e6c9f 100644 --- a/docs/gaps.md +++ b/docs/gaps.md @@ -249,7 +249,7 @@ update the numbers. ### 16. UI precision gaps -- **Missing:** moving several selected objects across pages (same-page moves shipped, #844), nudging with arrow keys, single-key tool accelerators, +- **Missing:** moving several selected objects across pages (same-page moves shipped, #844), single-key tool accelerators, rulers, grids and guides, loupe and dynamic zoom, multiple windows and split view, comment pop-ups that can be moved, connector lines, customisable quick tools; shortcut display and tool-mode bugs (#746, #721), macOS window controls (#789). diff --git a/docs/ui-parity.md b/docs/ui-parity.md index a7bac6fd..b5b93452 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 | no: arrow keys scroll and turn pages | gap | +| 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) | 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 08e4a47c..4da4fbca 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -1866,8 +1866,8 @@ milestone = "M7" status = "shipped" commands = ["edit.edit_text"] tools = ["text_edit"] -evidence = ["crates/edit/src/tests.rs::paragraphs_move_and_rewrap_to_a_new_width", "crates/ui-egui/tests/editing.rs::dragging_a_paragraph_moves_it_and_its_edge_rewraps_it", "crates/automation/tests/automation.rs::editing_existing_text_through_tools"] -notes = "existing paragraphs: drag the box to move it, drag the handle on its right edge to rewrap it to a new width (upright pages); the move is applied in page space through the paragraph's CTM, in its own q … Q; a click still opens it for typing (#40)" +evidence = ["crates/edit/src/tests.rs::paragraphs_move_and_rewrap_to_a_new_width", "crates/ui-egui/tests/editing.rs::dragging_a_paragraph_moves_it_and_its_edge_rewraps_it", "crates/ui-egui/tests/editing.rs::an_open_paragraph_moves_by_its_frame_and_by_the_arrow_keys", "crates/ui-egui/tests/editing.rs::arrow_keys_nudge_a_selected_image_instead_of_turning_the_page", "crates/automation/tests/automation.rs::editing_existing_text_through_tools"] +notes = "existing paragraphs: drag the box to move it, drag the handle on its right edge to rewrap it to a new width (upright pages); the move is applied in page space through the paragraph's CTM, in its own q … Q; a click still opens it for typing (#40); once open, its frame grabs the box (click to select, drag to move) while the text keeps the caret, and the arrow keys nudge the selected box or image a point, Shift ten points (#1520)" [[feature]] id = "edit.paragraph-detection"