From caf364670fcb8deb5b849c7cc089ecce7290bb51 Mon Sep 17 00:00:00 2001 From: alehel Date: Tue, 3 Nov 2026 09:39:26 +0100 Subject: [PATCH] fix(ui): Delete on a selected added picture deletes it alone `refactor(ui): crop mode is a hit of its picture` moved the picture's Delete to the app's key handling, which runs before the page view, so its edit applied before added content read the keys. Added content's selection mirrors the picture's when the picture is added content (`sync_image_selection`), and once the picture was gone that index named the next added item on the page: added content's own Delete, seeing the same key press, deleted it too. The picture's Delete consumes the key, and drops the mirrored selection with the picture's. Found in review. Co-Authored-By: Claude Opus 5.5 --- .../ui-egui/src/tools/edit_text_ui/image.rs | 9 ++++++ crates/ui-egui/tests/it/editing.rs | 32 +++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/crates/ui-egui/src/tools/edit_text_ui/image.rs b/crates/ui-egui/src/tools/edit_text_ui/image.rs index 2115e88faf..50ec5ec78b 100644 --- a/crates/ui-egui/src/tools/edit_text_ui/image.rs +++ b/crates/ui-egui/src/tools/edit_text_ui/image.rs @@ -493,7 +493,16 @@ pub(crate) fn picture_keys(ctx: &egui::Context, view: &mut DocView, doc: &pdfcra s.crop = None; } } else if !s.dragging() && !s.turning && (key(egui::Key::Delete) || key(egui::Key::Backspace)) && image().is_some() { + // The key is the picture's: added content's own Delete runs later this frame, after this + // edit has applied. + ctx.input_mut(|i| { + i.consume_key(egui::Modifiers::NONE, egui::Key::Delete); + i.consume_key(egui::Modifiers::NONE, egui::Key::Backspace); + }); view.image_selection = None; + // Added content's selection follows the picture's (`content_ui::sync_image_selection`), and + // goes with it: once the picture is gone it would be the next added item's. + view.content.selected = None; view.pending_edit = Some(Edit::EditPageImage { page, index, change: pdfcraft_engine::ImageEdit::Delete }); } } diff --git a/crates/ui-egui/tests/it/editing.rs b/crates/ui-egui/tests/it/editing.rs index f3d9fef095..af0d02b9c6 100644 --- a/crates/ui-egui/tests/it/editing.rs +++ b/crates/ui-egui/tests/it/editing.rs @@ -7369,6 +7369,38 @@ fn delete_removes_the_selected_picture_with_the_pointer_on_a_paragraph() { assert!(h.state().views[0].image_selection.is_none()); } +/// Delete on a selected added picture deletes it alone: the added text placed after it stays. The +/// key's second reader, added content's own Delete, took the text that the picture's index then +/// named. +#[test] +fn delete_on_a_selected_added_picture_deletes_it_alone() { + let mut h = edit_page(pdfcraft_ui_egui::QuickTool::EditText); + let mut png = Vec::new(); + image::RgbImage::from_pixel(30, 20, image::Rgb([20, 90, 200])).write_to(&mut std::io::Cursor::new(&mut png), image::ImageFormat::Png).unwrap(); + h.state_mut().add_image("logo.png".into(), png); + h.run_steps(3); + assert!(h.state_mut().apply_edit(pdfcraft_engine::Edit::AddText { + page: 0, + text: pdfcraft_engine::AddedText { rect: [20.0, 20.0, 120.0, 35.0], text: "Caption".into(), ..Default::default() }, + })); + h.run_steps(3); + let id = h.state().views[0].id; + let images = h.state().session.get(id).unwrap().page_images(0).len(); + assert_eq!(images, 1, "the added picture is the page's picture"); + h.state_mut().views[0].image_selection = Some(pdfcraft_ui_egui::ImageSelection::at(0, 0)); + h.run_steps(3); + h.key_press(Key::Delete); + h.run_steps(4); + let doc = h.state().session.get(id).unwrap(); + assert!(doc.page_images(0).is_empty(), "the picture is deleted"); + let texts: Vec<&str> = doc + .added() + .iter() + .filter_map(|a| if let pdfcraft_engine::AddedContent::Text(t) = &a.content { Some(t.text.as_str()) } else { None }) + .collect(); + assert_eq!(texts, ["Caption"], "and the text placed after it stays"); +} + /// With a dialog open, Delete is the dialog's: the selected picture stays, and so does crop mode /// under Escape and Enter. #[test]