fix(ui): a picture's keys act only while its page is drawn

Found in review. Delete, Backspace and crop's Enter and Escape moved to
the app's keys, which run before anything is drawn and don't look at what
is on screen. So with Edit text and a picture selected:
- Delete in the page grid removed the hidden picture and kept the pages;
- Delete with the pointer on the Pages panel did the same;
- with the picture's page scrolled away or in single-page view on another
  page, Delete removed a picture the reader couldn't see, and Enter
  applied its crop.

`main` read these keys in the picture's own page pass, which none of
those cases reach. `picture_keys` runs there again, from
`page_tool_input`, under Edit text's gate (so no dialog gets them), and
before the group acts. Delete still works wherever on the pages the
pointer is.

Tests, each seen failing with the fix reverted:
`delete_in_the_page_grid_deletes_the_page_and_not_the_hidden_picture`,
`a_selected_pictures_keys_wait_for_its_page_to_be_on_screen`,
`delete_with_the_pointer_on_the_pages_panel_deletes_a_page`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
alehelandClaude Opus 5.5 committed 2026-11-04 19:37:47 +01:00
1 parent 79205fe4ac
commit 1ee332ae55
4 files changed
+83 -17

No files matched your search

-2
View File
@@ -66,8 +66,6 @@ impl PdfCraftApp {
{
object_ui::nudge_keys(ctx, &mut self.views[i], doc);
object_ui::delete_keys(ctx, &mut self.views[i]);
let held = ctx.input(|input| input.pointer.any_down());
edit_text_ui::picture_keys(ctx, &mut self.views[i], doc, held);
} 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);
}
+3
View File
@@ -193,6 +193,9 @@ pub(super) fn page_tool_input(ui: &mut egui::Ui, view: &mut DocView, frame: &mut
// pictures, each told what the press is to this page. Edit text ignores input while a dialog,
// the palette, another window or another tab has it (`gate`), as before.
let gate = !(tool == QuickTool::EditText && view.objects.input_blocked());
if tool == QuickTool::EditText && can_modify && gate {
crate::edit_text_ui::picture_keys(ui.ctx(), view, i, &images, ptr.any_down);
}
let on_objects = tool == QuickTool::EditText && can_modify && crate::object_ui::page_input(cx, press, view);
let picks_up = matches!(tool, QuickTool::Select | QuickTool::AddText | QuickTool::EditText);
let ccx = crate::content_ui::ContentCx {
+14 -15
View File
@@ -467,24 +467,24 @@ fn apply_crop_mode(view: &mut DocView, image: &pdfcraft_engine::PageImage) {
}
}
/// The selected picture's keys, from the app's own (none reach it under a dialog): Delete or
/// Backspace removes it, wherever the pointer is, unless it is being dragged or cropped. In crop
/// mode Enter applies the crop and Escape leaves it, though not while a button is down (`held`):
/// Escape then cancels the press the pages hold, if any (`canvas::cancel_on_escape`), and crop mode
/// stays. Nothing while a text field has the keyboard.
pub(crate) fn picture_keys(ctx: &egui::Context, view: &mut DocView, doc: &pdfcraft_engine::Document, held: bool) {
/// The keys of the picture selected on `page`, a page being drawn, so the keys never reach a picture
/// off the screen, and the page grid and the Pages panel have had theirs. Delete or Backspace removes
/// it, wherever on the pages the pointer is, unless it is being dragged or cropped. In crop mode
/// Enter applies the crop and Escape leaves it, though not while a button is down (`held`): Escape
/// then cancels the press the pages hold, if any (`canvas::cancel_on_escape`), and crop mode stays.
/// Nothing while a text field has the keyboard.
pub(crate) fn picture_keys(ctx: &egui::Context, view: &mut DocView, page: usize, images: &[pdfcraft_engine::PageImage], held: bool) {
if ctx.egui_wants_keyboard_input() {
return;
}
let Some(s) = view.image_selection.as_ref() else { return };
let (page, index) = (s.page, s.index);
let Some(s) = view.image_selection.as_ref().filter(|s| s.page == page) else { return };
let index = s.index;
let key = |k: egui::Key| ctx.input(|i| i.key_pressed(k));
// The page's pictures are read only when a key asks for one.
let image = || doc.page_images(page).into_iter().nth(index);
let image = images.get(index);
if s.crop.is_some() {
if key(egui::Key::Enter) {
if let Some(image) = image() {
apply_crop_mode(view, &image);
if let Some(image) = image {
apply_crop_mode(view, image);
}
} else if !held
&& key(egui::Key::Escape)
@@ -492,9 +492,8 @@ 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.
} 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.
ctx.input_mut(|i| {
i.consume_key(egui::Modifiers::NONE, egui::Key::Delete);
i.consume_key(egui::Modifiers::NONE, egui::Key::Backspace);
+66
View File
@@ -7377,6 +7377,72 @@ fn a_right_click_on_an_open_paragraphs_frame_over_a_picture_selects_the_picture(
assert!(h.query_by_label("Flip horizontal").is_some(), "its menu opens");
}
/// In the page grid, Delete deletes the page chosen there. The picture selected before the grid
/// opened is not on screen, and stays.
#[test]
fn delete_in_the_page_grid_deletes_the_page_and_not_the_hidden_picture() {
let mut h = picture_pages(false);
let id = h.state().views[0].id;
let pictures = h.state().session.get(id).unwrap().page_images(0).len();
let pages = h.state().session.get(id).unwrap().info.pages.len();
assert!(h.state_mut().execute("page.organize"));
h.run_steps(4);
h.get_by_label("Select page 2").click();
h.run_steps(2);
h.key_press(egui::Key::Delete);
h.run_steps(4);
let doc = h.state().session.get(id).unwrap();
assert_eq!(doc.info.pages.len(), pages - 1, "the page chosen in the grid is deleted");
assert_eq!(doc.page_images(0).len(), pictures, "the picture stays");
}
/// With the picture's page not on screen, its keys do nothing: Delete removes no picture the reader
/// can't see, and Enter applies no crop.
#[test]
fn a_selected_pictures_keys_wait_for_its_page_to_be_on_screen() {
use pdfcraft_ui_egui::canvas::PageLayout;
for (crop, key) in [(false, egui::Key::Delete), (true, egui::Key::Enter)] {
let mut h = picture_pages(crop);
let id = h.state().views[0].id;
let before = h.state().session.get(id).unwrap().page_images(0);
if crop {
let keep = before[0].visible_rect.unwrap();
let half = [keep[0], keep[1], (keep[0] + keep[2]) / 2.0, keep[3]];
h.state_mut().views[0].image_selection.as_mut().unwrap().crop.as_mut().unwrap().keep = half;
}
h.state_mut().views[0].layout = PageLayout::Single;
h.state_mut().views[0].continuous = false;
h.state_mut().views[0].go_to_page(1);
h.run_steps(4);
assert!(h.state().views[0].page_screen_rect(0).is_none(), "page 1 is off the screen");
h.key_press(key);
h.run_steps(4);
let after = h.state().session.get(id).unwrap().page_images(0);
assert_eq!(after.len(), before.len(), "the picture stays");
assert_eq!(after[0].visible_rect, before[0].visible_rect, "and is not cropped");
}
}
/// With the pointer on the Pages panel, Delete is the panel's: a page goes, though a picture is
/// selected on the pages.
#[test]
fn delete_with_the_pointer_on_the_pages_panel_deletes_a_page() {
let mut h = picture_pages(false);
h.state_mut().set_option("panel", "pages").unwrap();
h.run_steps(4);
let id = h.state().views[0].id;
let pages = h.state().session.get(id).unwrap().info.pages.len();
assert!(h.state().views[0].image_selection.is_some(), "the picture is selected");
assert!(h.state().views[0].page_screen_rect(0).is_some(), "on a page on screen");
let viewport = h.state().views[0].viewport_rect();
assert!(viewport.right() < 1300.0, "the panel lies right of the pages: {viewport:?}");
h.hover_at(egui::pos2(viewport.right() + 60.0, viewport.center().y));
h.run_steps(2);
h.key_press(egui::Key::Delete);
h.run_steps(4);
assert_eq!(h.state().session.get(id).unwrap().info.pages.len(), pages - 1, "a page goes");
}
/// In crop mode, a right-click on another picture selects it for its menu, as without crop mode. It
/// went to crop mode's whole-page hit, which unselected every picture.
#[test]