fix(ui): the page menu is the menu of what the right-click went to

The page's menu chose its entries from the selections, in a fixed order:
a selected Prepare field's menu, then a link's, then the picture's, then
a comment's. A selection left from before outranked what was just
right-clicked, so with a field selected, a right-click on the empty page
(or on a comment) opened the field's menu.

The right-click remembers what it went to (`DocView.menu_for`, from the
routed hits), and the menu shows that owner's entries when it has a
menu, or else the page's (the user's ruling). Add text's right-click is
the page's, as before; while a press is held only a link takes its menu.
The picture menu's flag and its geometric test go with it.

#5900's menu check (no image menu over the picture being cropped, #5835)
is the routed owner's too: a right-click that went to the crop (its
picture, box or grips) opens no menu, and `crop_contains_point` and
`crop_controls_contain`, its second point test, go.

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:46 +01:00
1 parent cbcf79acc7
commit 607635c157
5 files changed
+67 -86

No files matched your search

+28 -50
View File
@@ -255,22 +255,6 @@ impl ImageObb {
}
}
/// Whether pointer `p` is on an image being cropped, its crop box or its handles (#5835).
pub(crate) fn crop_contains_point(
img: &pdfcraft_engine::PageImage,
crop: &crate::edit_text_ui::CropMode,
xf: &PageXform,
info: &DocInfo,
page: usize,
p: Pos2,
) -> bool {
let obb = ImageObb::from_image(img, xf, info, page);
let full_obb = ImageObb::from_matrix(img.matrix, xf, info, page);
crate::edit_text_ui::quad_contains(obb.corners(), p)
|| crate::edit_text_ui::quad_contains(full_obb.corners(), p)
|| crate::edit_text_ui::crop_controls_contain(img, crop, xf, info, page, p)
}
/// The page display as it was before full screen, put back on leaving it
/// ([`DocView::enter_presentation`]).
#[derive(Clone, Copy, Debug, PartialEq)]
@@ -668,6 +652,9 @@ pub struct DocView {
pub(crate) cursor: crate::page_cursor::PageCursor,
/// Who the press the pages hold belongs to.
pub(crate) router: crate::page_router::PageRouter,
/// What the last right-click on a page went to, and on which page: the page's menu shows its
/// entries, or the page's own when it went to nothing that has a menu.
pub(crate) menu_for: Option<(usize, crate::page_router::Owner)>,
/// Escape cancelled the press the pages hold: the page ignores it until the button is up, so
/// its release neither clicks nor ends a drag.
pub(crate) press_cancelled: bool,
@@ -1040,6 +1027,7 @@ impl DocView {
signature_drag: Default::default(),
cursor: Default::default(),
router: Default::default(),
menu_for: None,
press_cancelled: false,
}
}
@@ -1675,6 +1663,7 @@ fn draw_pages(
// comment, and Edit text's picture, 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;
view.menu_for = None;
}
let mut frame = PageFrame {
env: *env,
@@ -1788,25 +1777,22 @@ fn draw_pages(
if view.continuous {
view.current = current;
}
// The menu of what the right-click went to (`menu_for`), or else the page's.
use crate::page_router::{CropPart, Owner, PicturePart};
let menu_for = view.menu_for.map(|(_, owner)| owner);
resp.context_menu(|ui| {
let is_crop_over_image = tool == QuickTool::EditText
&& view.image_selection.clone().is_some_and(|sel| {
let Some(ref crop) = sel.crop else { return false };
let pointer_pos = ptr.pos;
let Some(p) = pointer_pos else { return false };
let Some((_, xf)) = view.screen_xforms.iter().find(|(page_idx, _)| *page_idx == sel.page).cloned() else { return false };
let images = cached_edit_images(view, doc, sel.page).to_vec();
let Some(img) = images.get(sel.index) else { return false };
crop_contains_point(img, crop, &xf, info, sel.page, p)
});
if is_crop_over_image {
ui.ctx().data_mut(|d| d.insert_temp(crate::edit_text_ui::image_menu_id(), false));
// Crop mode has no menu: a right-click on the picture being cropped, its box or its grips is
// the crop's (#5835).
if matches!(menu_for, Some(Owner::Picture { part: PicturePart::Crop(part), .. }) if part != CropPart::Page) {
egui::Popup::close_all(ui.ctx());
ui.close();
return;
}
// Preparing a form: the selected field's menu.
if preparing && let Some((name, wi)) = view.prepare.selected.clone() {
// Preparing a form: the selected field's menu, on a field.
if preparing
&& matches!(menu_for, Some(Owner::PrepareField { .. } | Owner::PrepareCorner { .. }))
&& let Some((name, wi)) = view.prepare.selected.clone()
{
// Several fields: Align, Center, Distribute, Set Fields to Same Size.
if !view.prepare.also.is_empty() {
use crate::prepare::Arrange as A;
@@ -1853,7 +1839,9 @@ fn draw_pages(
}
return;
}
if let Some((page, index)) = view.links.context {
if matches!(menu_for, Some(Owner::Link { .. }))
&& let Some((page, index)) = view.links.context
{
if ui.button(tl!("Properties…")).clicked() {
view.links.open_existing = Some((page, index));
ui.close();
@@ -1874,28 +1862,21 @@ fn draw_pages(
return;
}
let can_paste_image = env.can_paste_image;
let is_image_menu = tool == QuickTool::EditText
&& (ui.ctx().data_mut(|d| d.get_temp::<bool>(crate::edit_text_ui::image_menu_id())).unwrap_or(false)
|| view.image_selection.clone().is_some_and(|sel| {
let pointer_pos = ptr.pos;
let Some(p) = pointer_pos else { return false };
let Some((_, xf)) = view.screen_xforms.iter().find(|(page_idx, _)| *page_idx == sel.page).cloned() else { return false };
let images = cached_edit_images(view, doc, sel.page).to_vec();
let Some(img) = images.get(sel.index) else { return false };
let obb = ImageObb::from_image(img, &xf, info, sel.page);
let b = image_screen_rect(img, &xf, info, sel.page);
obb.contains(b, p)
}));
if is_image_menu && let Some(sel) = view.image_selection.clone() {
if tool == QuickTool::EditText
&& matches!(menu_for, Some(Owner::Picture { .. }))
&& let Some(sel) = view.image_selection.clone()
{
let images = cached_edit_images(view, doc, sel.page).to_vec();
// 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.
// Paste lands where the menu opened, on the picture's page: the right-click that opened
// it went to that page (`page_input`), and another page's point means nothing there.
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;
}
if view.comments.selected.is_some() {
if let Some((page, Owner::Comment { index, .. })) = view.menu_for
&& view.comments.selected == Some((page, index))
{
canvas_action = comments::context_menu(ui, view, info, &|page, index| doc.annotation_deletable(page, index), prefs, allowed, can_modify);
return;
}
@@ -1944,9 +1925,6 @@ fn draw_pages(
};
default_page_context_menu(ui, page, at, perms, &mut page_action);
});
if !resp.context_menu_opened() {
ui.ctx().data_mut(|d| d.insert_temp(crate::edit_text_ui::image_menu_id(), false));
}
*out = PageOut {
canvas_action,
clicked_link,
+8 -1
View File
@@ -134,7 +134,7 @@ pub(super) fn page_tool_input(ui: &mut egui::Ui, view: &mut DocView, frame: &mut
Some(Owner::Picture { index, part }) if part != crate::page_router::PicturePart::Crop(crate::page_router::CropPart::Page) => Some(index),
_ => None,
};
crate::edit_text_ui::picture_context(ui.ctx(), i, picture, view);
crate::edit_text_ui::picture_context(i, picture, view);
}
// A mark already placed takes its own press (comments, below), as does one being moved on this
// page until its move ends; with no button down the Fill & Sign tool's preview hides where a
@@ -347,6 +347,13 @@ fn right_click(hits: &PageHits<'_>, under: &[crate::page_router::Hit], view: &mu
let context = hits.context_at(view, p, under);
// Delete page, Paste and Copy Image act where the menu opened, whoever takes the right-click.
comments::anchor(hits.pcx, p, view);
// The menu is what the right-click went to: Add text's is the page's. While a press is held only
// a link takes its menu, as only links take theirs then.
view.menu_for = match context {
Some(owner @ Owner::Link { .. }) => Some((hits.page, owner)),
_ if press_held || hits.tool == QuickTool::AddText && hits.can_modify => None,
owner => owner.map(|o| (hits.page, o)),
};
if press_held {
return context;
}
+3 -3
View File
@@ -21,10 +21,10 @@ mod image;
mod layout;
pub(crate) use background::TextBackground;
pub use image::{CropMode, DragTarget, ImageAction, ImageDrag, ImageHandle, ImageSelection, image_menu_id};
pub use image::{CropMode, DragTarget, ImageAction, ImageDrag, ImageHandle, ImageSelection};
pub(crate) use image::{
ImageCx, Matrix, body_hit as picture_hit, crop_controls_contain, crop_hit, handle_hit as picture_handle_hit, image_context_menu, image_input,
picture_context, picture_keys, quad_contains, user_to_screen,
ImageCx, Matrix, body_hit as picture_hit, crop_hit, handle_hit as picture_handle_hit, image_context_menu, image_input, picture_context,
picture_keys, quad_contains, user_to_screen,
};
#[cfg(test)]
use image::{rotate_knob, turn_degrees, turn_degrees_delta};
+7 -32
View File
@@ -264,20 +264,6 @@ fn same_rect(a: [f64; 4], b: [f64; 4]) -> bool {
a.into_iter().zip(b).all(|(a, b)| (a - b).abs() < 1e-3)
}
/// Whether `p` is on the crop box or one of its grips (#5835). The Done, Cancel and Remove
/// Crop pills are widgets of their own, so they take their clicks before the canvas does.
pub(crate) fn crop_controls_contain(
image: &pdfcraft_engine::PageImage,
mode: &CropMode,
xf: &PageXform,
info: &DocInfo,
page: usize,
p: Pos2,
) -> bool {
crop_frames(xf, info, page, image, mode.keep)
.is_some_and(|(_, k)| k.contains(p) || crop_handles(k).iter().any(|h| h.distance(p) < CROP_GRIP_REACH))
}
/// Crop mode for the selected image `i` whose visible frame is `b` (screen). Returns `true`
/// (the pointer is the crop's while it lasts; its keys are read by [`picture_keys`]).
#[allow(clippy::cognitive_complexity)]
@@ -678,11 +664,6 @@ pub(super) fn turn_degrees(centre: Pos2, pointer: Pos2, snap: bool) -> f32 {
turn_degrees_delta(centre, centre - egui::vec2(0.0, 10.0), pointer, snap)
}
#[doc(hidden)]
pub fn image_menu_id() -> egui::Id {
egui::Id::unique("image-menu")
}
/// Images on a page: the press that is a picture's (`press`, [`body_hit`] and [`handle_hit`])
/// selects it, a drag of the selected one moves, resizes or turns it. Returns `true` when the
/// pointer was used.
@@ -991,20 +972,15 @@ pub(crate) fn image_input(cx: &ImageCx<'_>, images: &[pdfcraft_engine::PageImage
}
/// A right-click on `page` that went to its picture `index` (on it, a grip of it, or the box kept
/// while it is cropped) selects it, keeping it as it is if it was selected, and opens the picture's
/// menu; one that went to anything else (`None`) unselects the picture selected and closes the menu.
pub(crate) fn picture_context(ctx: &egui::Context, page: usize, index: Option<usize>, view: &mut DocView) {
/// while it is cropped) selects it for its menu, keeping it as it is if it was selected; one that
/// went to anything else (`None`) unselects the picture selected.
pub(crate) fn picture_context(page: usize, index: Option<usize>, view: &mut DocView) {
match index {
Some(index) => {
if view.image_selection.as_ref().is_none_or(|s| (s.page, s.index) != (page, index)) {
view.image_selection = Some(ImageSelection::at(page, index));
}
ctx.data_mut(|d| d.insert_temp(image_menu_id(), true));
}
None => {
view.image_selection = None;
ctx.data_mut(|d| d.insert_temp(image_menu_id(), false));
Some(index) if view.image_selection.as_ref().is_none_or(|s| (s.page, s.index) != (page, index)) => {
view.image_selection = Some(ImageSelection::at(page, index));
}
Some(_) => {}
None => view.image_selection = None,
}
}
@@ -1029,7 +1005,6 @@ pub(crate) fn image_context_menu(
let picked = |ui: &mut egui::Ui, entry: egui::Response| {
let clicked = entry.clicked();
if clicked {
ui.ctx().data_mut(|d| d.insert_temp(image_menu_id(), false));
egui::Popup::close_all(ui.ctx());
ui.close();
}
+21
View File
@@ -1679,6 +1679,27 @@ fn with_prepare_a_click_that_began_off_the_page_keeps_the_selected_field() {
assert!(h.state().views[0].prepare.selected.is_some(), "a click that began off the page leaves it selected");
}
/// With Prepare open and a field selected, a right-click on the empty page opens the page's menu, and
/// one on the field the field's: the menu is what the right-click went to. The field's menu opened
/// wherever the page was right-clicked.
#[test]
fn with_prepare_a_right_click_beside_the_selected_field_opens_the_pages_menu() {
let mut h = open(comment_and_checkbox_pdf(), "150", false);
assert!(h.state_mut().execute("form.prepare"));
h.run_steps(4);
let on_box = at(&h, 110.0, 210.0);
mouse_click(&mut h, on_box);
assert!(h.state().views[0].prepare.selected.is_some(), "the field is selected");
let blank = at(&h, 60.0, 300.0);
right_click(&mut h, blank);
assert!(h.query_by_label("Duplicate…").is_none(), "no field menu on the empty page");
assert!(h.query_all_by_label_contains("Copy").next().is_some(), "the page's menu");
h.key_press(egui::Key::Escape);
h.run_steps(3);
right_click(&mut h, on_box);
assert!(h.query_by_label("Duplicate…").is_some(), "the field's menu on the field");
}
/// With the Edit panel open and the Note tool, a click on added text is the added text's: no note is
/// started under it, as before.
#[test]