fix(ui): a right-click between the pages deselects the comment; Paste keeps to the picture's page

Two of CodeRabbit's findings on #5803. A right-click off the pages
cleared only the menu's anchor and kept the selected comment, so the
menu that opened was that comment's, not the current page's. A
right-click on a page's empty part deselects it, so one that no page
takes (`PageFrame.right_clicked`) now does the same. A selected Prepare
field keeps its menu off the pages, as it does on a page's empty part;
the comment there says so.

Where pages touch, the first page takes a right-click on the row they
share and anchors the menu there, but a picture on the second page under
it still opens its menu. Paste from that menu centred the image on page
1's point, which on page 2 is at its foot. The image menu now uses the
anchor only on the picture's own page, and otherwise pastes as Ctrl+V
does.

Tests, each seen failing with its fix reverted:
`a_right_click_between_the_pages_opens_the_page_menu_not_the_selected_comments`,
`paste_from_a_picture_menu_where_two_pages_touch_keeps_off_the_first_pages_point`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
alehelandClaude Opus 5.5 committed 2026-11-03 14:41:30 +01:00
1 parent d4c8b6639a
commit e69d1221ff
2 files changed
+81 -3

No files matched your search

+21 -3
View File
@@ -1662,7 +1662,9 @@ fn draw_pages(
// Set again below when the right-click lands on a link.
view.links.context = None;
// Set again below by the page right-clicked: a right-click off the pages (between them,
// beside one) anchors no page's menu, which then acts on the current page.
// beside one) anchors no page's menu, which then acts on the current page. It deselects the
// comment too (below), as one on a page's empty part does. A selected Prepare field keeps
// its menu, off the pages as on them.
view.comments.context_at = None;
let mut over_image = false;
if tool == QuickTool::EditText {
@@ -1734,8 +1736,22 @@ fn draw_pages(
draw_page(ui, view, &mut frame, i);
}
let PageFrame {
wanted, visible_now, mut current, best_overlap, current_overlap, current_height, pressed_page, calc_hover, out: page_out, ..
wanted,
visible_now,
mut current,
best_overlap,
current_overlap,
current_height,
pressed_page,
calc_hover,
out: page_out,
right_clicked,
..
} = frame;
// A right-click no page took deselects the comment, so the page's menu opens, not the comment's.
if resp.secondary_clicked() && !right_clicked {
view.comments.selected = None;
}
let PageOut {
mut canvas_action,
clicked_link,
@@ -1905,7 +1921,9 @@ fn draw_pages(
}));
if is_image_menu && let Some(sel) = view.image_selection.clone() {
let images = cached_edit_images(view, doc, sel.page).to_vec();
let at = view.comments.context_at.map(|(_, at)| at);
// Paste lands where the menu opened only on the picture's page: where pages touch, the
// first page takes the right-click, and its point means nothing on the second.
let at = view.comments.context_at.filter(|(page, _)| *page == sel.page).map(|(_, at)| at);
let rotation = (view.rotation % 360 + info.pages.get(sel.page).map_or(0, |p| p.rotation % 360)) % 360;
crate::edit_text_ui::image_context_menu(ui, view, &images, sel.page, sel.index, can_paste_image, &mut image_action, at, rotation);
return;
+60
View File
@@ -1726,6 +1726,66 @@ fn a_right_click_between_the_pages_anchors_no_pages_menu() {
assert_eq!(h.state().views[0].comments.context_at, None);
}
/// Two 300×400 pages, a rectangle comment on the first.
fn two_pages_and_a_comment_pdf() -> Vec<u8> {
pdf(&[
CATALOG,
"<< /Type /Pages /Kids [3 0 R 4 0 R] /Count 2 /MediaBox [0 0 300 400] >>",
"<< /Type /Page /Parent 2 0 R /Annots [5 0 R] >>",
"<< /Type /Page /Parent 2 0 R >>",
"<< /Type /Annot /Subtype /Square /Rect [30 290 200 320] /C [1 0 0] /F 4 /T (Reviewer) >>",
])
}
/// A right-click between the pages selects no comment, as one on a page's empty part does not, so
/// the menu that opens is the current page's. The selected comment's menu used to open there.
#[test]
fn a_right_click_between_the_pages_opens_the_page_menu_not_the_selected_comments() {
let mut h = open(two_pages_and_a_comment_pdf(), "100", false);
h.state_mut().views[0].comments.selected = Some((0, 0));
h.run_steps(2);
assert_eq!(h.state().views[0].comments.selected, Some((0, 0)), "the comment is selected");
let (one, two) = {
let v = &h.state().views[0];
(v.page_screen_rect(0).expect("page 1 on screen"), v.page_screen_rect(1).expect("page 2 on screen"))
};
let gap = egui::pos2(one.center().x, (one.max.y + two.min.y) / 2.0);
assert!(!one.contains(gap) && !two.contains(gap), "between the pages: {one:?} {two:?}");
right_click(&mut h, gap);
assert_eq!(h.state().views[0].comments.selected, None);
assert!(h.query_by_label("Rotate view counterclockwise").is_some(), "the page menu opens");
assert!(h.query_by_label("Reply").is_none(), "not the comment's");
}
/// Where pages touch, a right-click on the row they share is page 1's, and a picture on page 2 there
/// opens its menu, whose Paste then places the image as Ctrl+V does. It used to centre the image on
/// page 1's point, which on page 2 lies at its foot.
#[test]
fn paste_from_a_picture_menu_where_two_pages_touch_keeps_off_the_first_pages_point() {
let (mut h, seam) = touching_pages(|app| {
app.system_clipboard = false;
app.quick_tool = QuickTool::EditText;
});
let clip = || pdfcraft_ui_egui::Clip::Image { width: 60, height: 40, rgba: [50u8, 150, 250, 255].repeat(60 * 40) };
h.state_mut().clipboard_override = Some(clip());
assert!(h.state_mut().paste_image_at(1, [100.0, 400.0]), "a picture across page 2's top edge");
h.state_mut().clipboard_override = Some(clip());
h.state_mut().views[0].image_selection = None;
h.run_steps(4);
right_click(&mut h, seam);
assert_eq!(h.state().views[0].comments.context_at.map(|(page, _)| page), Some(0), "page 1 takes the right-click");
assert_eq!(h.state().views[0].image_selection.as_ref().map(|s| s.page), Some(1), "page 2's picture opens its menu");
h.get_by_label("Paste").click();
h.run_steps(3);
let pasted = {
let s = h.state();
s.session.get(s.views[0].id).unwrap().page_images(1)
};
assert_eq!(pasted.len(), 2, "the image is pasted on page 2");
let r = pasted[1].rect;
assert!((r[1] + r[3]) / 2.0 > 100.0, "not centred at page 2's foot: {r:?}");
}
/// With the camera tool, a click on another page moves the quadrilateral there, as a drag from it
/// does. The click was located by its press's origin, which egui has cleared by the release frame,
/// so it did nothing (on `main` too).