From ae62b66304a3e4b089fe7d3da7138cc5d44b885c Mon Sep 17 00:00:00 2001 From: alehel Date: Mon, 2 Nov 2026 13:16:14 +0100 Subject: [PATCH] fix(ui): right-clicks with the Link tool and the camera as before; true docs Routing right-clicks changed a few more cases beyond the rulings, found in review: - **The Link tool.** A right-click on a link also unselected the selected comment and moved the page menu's anchor; the Link tool used to take it whole. On a document it can't change, its right-click on a note over a link opened the plain page menu instead of the note's. Now the Link tool's right-click is what its press there would take, on a document it can change, and on a link it leaves comments alone. - **The camera.** A corner, on the handle layer, took right-clicks from a comment under it, though it has no menu. Corners are no longer right-click targets. - **The Link tool's hand** showed in the 2 px margin past the page's edge, where a press is no page's. `link_ui::hit` now finds nothing off the page. Docs made true: what a comment's press is (`comments::page_input`), what reaches the free-press gate (`page_tool_input`), and when every page is `PagePress::Free`. Tests: those three, and two that pin right-clicks kept as before: Edit text's on a paragraph under a comment, and a Prepare field tool's. Each seen failing with its fix reverted. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/canvas/page_input.rs | 34 +++++---- crates/ui-egui/src/canvas/page_router.rs | 3 +- crates/ui-egui/src/tools/comments/input.rs | 5 +- crates/ui-egui/src/tools/link_ui.rs | 4 + crates/ui-egui/tests/it/pointer.rs | 85 ++++++++++++++++++++++ 5 files changed, 113 insertions(+), 18 deletions(-) diff --git a/crates/ui-egui/src/canvas/page_input.rs b/crates/ui-egui/src/canvas/page_input.rs index f1d458257e..eda260451d 100644 --- a/crates/ui-egui/src/canvas/page_input.rs +++ b/crates/ui-egui/src/canvas/page_input.rs @@ -35,9 +35,10 @@ pub(crate) struct PageToolContext<'a> { pub info: &'a DocInfo, } -/// The tools' input on one page. The hits at the press point decide who holds the press, and each -/// module is told what the press is to this page (`PagePress`): its own, nobody's (the active tool's, -/// through `tool_input`), or another page's. Text selection runs after, on what nobody took. +/// The tools' input on one page. The hits at the press point decide who holds the press, and the +/// modules are told what the press is to this page (`PagePress`): their own, nobody's (the active +/// tool's: `tool_input`, and the tools still called here), or another page's. The Edit text group +/// still asks the router, until it is told too. Text selection runs after, on what nobody took. pub(super) fn page_tool_input(ui: &mut egui::Ui, view: &mut DocView, frame: &mut PageFrame<'_>, slot: &PageSlot) -> PageInput { let PageEnv { added, @@ -324,13 +325,15 @@ fn tool_input( fn right_click(hits: &PageHits<'_>, under: &[crate::page_router::Hit], view: &mut DocView, p: Pos2) -> Option { let context = hits.context_at(view, p); // Add text takes every right-click on a page it can change, Edit text those on its own - // paragraphs and pictures, and a Prepare field tool the whole page, as before. - let editing_here = match hits.tool { + // paragraphs and pictures, the Link tool those on a link, and a Prepare field tool the whole + // page, as before: comments see none of them. + let tool_takes_it = match hits.tool { QuickTool::AddText => hits.can_modify || top(under.iter().cloned(), hits.tool).is_some(), QuickTool::EditText => top(under.iter().cloned(), hits.tool).is_some(), - _ => false, + QuickTool::Link => hits.can_modify && matches!(context, Some(Owner::Link { .. })), + _ => hits.field_tool.is_some(), }; - if !editing_here && hits.field_tool.is_none() { + if !tool_takes_it { let comment = match context { Some(Owner::Comment { index, .. }) => Some(index), _ => None, @@ -396,16 +399,16 @@ impl PageHits<'_> { /// What a right-click at `p` is on: what lies on top there, as for a press, but with every /// tool reaching comments and links, as a comment's menu shows whatever the tool. A highlight /// over a link is the highlight's ([`comments::context_hit`]). - /// The Link tool edits links, so its right-click on one is the link's whatever lies over it, as - /// its press there is. + /// The Link tool edits links and reaches no comment: on a document it can change, its + /// right-click is what its press there would take, when that is anything. fn context_at(&self, view: &DocView, p: Pos2) -> Option { - let hits = self.hits(view, p, true); if self.tool == QuickTool::Link - && let Some(link) = hits.iter().rev().find(|h| matches!(h.owner, Owner::Link { .. })) + && self.can_modify + && let Some(press) = top(self.hits(view, p, false), QuickTool::Link) { - return Some(link.owner); + return Some(press.owner); } - top(hits, QuickTool::Select).map(|h| h.owner) + top(self.hits(view, p, true), QuickTool::Select).map(|h| h.owner) } /// The hits at `p`, bottom up: links, comments; Edit text's pictures under its paragraphs (text @@ -413,7 +416,8 @@ impl PageHits<'_> { /// open paragraph's frame, the selected picture's grips over them all; the camera tool's corners; /// then form fields (Prepare's, being moved, over the comments; filled ones under added content /// and the comments that move). In crop mode, only the picture being cropped. - /// For a right-click (`context`), comments by [`comments::context_hit`]. + /// For a right-click (`context`), comments by [`comments::context_hit`], and no camera corner (it + /// has no menu). fn hits(&self, view: &DocView, p: Pos2, context: bool) -> Vec { let PageHits { page, @@ -475,7 +479,7 @@ impl PageHits<'_> { // together. let picks_up = matches!(tool, QuickTool::Select | QuickTool::AddText | QuickTool::EditText); let added = (editing_content && can_modify && picks_up).then(|| crate::content_ui::hit(xf, info, page, added, view, p)).flatten(); - let corner = (tool == QuickTool::EnhanceCamera && can_modify).then(|| crate::camera_ui::hit(xf, page, view, p)).flatten(); + let corner = (!context && tool == QuickTool::EnhanceCamera && can_modify).then(|| crate::camera_ui::hit(xf, page, view, p)).flatten(); let link = crate::link_ui::hit(xf, info, page, doc_links, tool, p); let comment = if context { comments::context_hit(pcx, p) } else { comments::hit(pcx, view, p, link.is_some()) }; [link, comment, pictures, paragraphs, added, objects, grips, corner, fields].into_iter().flatten().collect() diff --git a/crates/ui-egui/src/canvas/page_router.rs b/crates/ui-egui/src/canvas/page_router.rs index 447289ccd0..0b53038625 100644 --- a/crates/ui-egui/src/canvas/page_router.rs +++ b/crates/ui-egui/src/canvas/page_router.rs @@ -115,7 +115,8 @@ pub(crate) enum PagePress { /// The press began on this page, and the owner's hit won it: only its module acts on it. Mine(Owner), /// No button is down, or the press began on this page and nobody's hit won it: the active - /// tool's. + /// tool's. Also every page's while a press that began off the pages is held, and after the + /// press was dropped ([`PageRouter::cancel`]) until the button is up. Free, /// The press began on another page, or between the pages. Elsewhere, diff --git a/crates/ui-egui/src/tools/comments/input.rs b/crates/ui-egui/src/tools/comments/input.rs index 396aea8b13..52319276da 100644 --- a/crates/ui-egui/src/tools/comments/input.rs +++ b/crates/ui-egui/src/tools/comments/input.rs @@ -9,8 +9,9 @@ use crate::page_cursor::{Layer, Subject}; use crate::page_router::{CommentPart, Hit, Owner, PagePress}; /// Comment input on one page, before text selection runs. `press` is what this page has to do with -/// the press the pages hold: a press on a comment ([`hit`]) is that comment's, with any tool, and -/// one nobody holds is a commenting tool's. Returns `true` when the pointer gesture belongs to +/// the press the pages hold: a press on a comment where the tool reaches it ([`hit`]: the Select +/// tool, Fill & Sign over its marks, a shape tool over the selected shape's handles) is that +/// comment's, and one nobody holds is a commenting tool's. Returns `true` when the pointer gesture belongs to /// commenting (text selection must ignore it). pub(crate) fn page_input(ui: &egui::Ui, resp: &egui::Response, cx: &PageCx<'_>, press: PagePress, view: &mut DocView) -> bool { let (pointer, origin) = (cx.ptr.pos, cx.ptr.origin); diff --git a/crates/ui-egui/src/tools/link_ui.rs b/crates/ui-egui/src/tools/link_ui.rs index 9cd62d6969..659bad41e4 100644 --- a/crates/ui-egui/src/tools/link_ui.rs +++ b/crates/ui-egui/src/tools/link_ui.rs @@ -66,6 +66,10 @@ fn to_user(xf: &PageXform, info: &DocInfo, page: usize, p: Pos2) -> [f64; 2] { /// picked, and a drag from one draws a new link. The other tools find the links the renderer can /// follow, which have a target (`info.links`). The tip is the link's target. pub(crate) fn hit(xf: &PageXform, info: &DocInfo, page: usize, links: &[LinkItem], tool: QuickTool, p: Pos2) -> Option { + // A press off the page is no page's, so nothing there is a link's, the margin included. + if !xf.rect.contains(p) { + return None; + } let index = if tool == QuickTool::Link { links.iter().rfind(|l| l.page == page && screen_rect(xf, info, page, l.rect).expand(LINK_TOOL_MARGIN).contains(p))?.index } else { diff --git a/crates/ui-egui/tests/it/pointer.rs b/crates/ui-egui/tests/it/pointer.rs index 8a020cc4ea..04d98b9318 100644 --- a/crates/ui-egui/tests/it/pointer.rs +++ b/crates/ui-egui/tests/it/pointer.rs @@ -1356,3 +1356,88 @@ fn a_field_dragged_off_its_page_is_drawn_on_it() { let doc = s.session.get(s.views[0].id).unwrap(); assert!(doc.form.iter().any(|f| f.widgets.iter().any(|w| w.page == Some(0))), "a field on page 1"); } + +/// With the Link tool a right-click on a link is the link's and leaves comments as they are, as +/// before: the selected comment stays selected. +#[test] +fn a_right_click_with_the_link_tool_on_a_link_keeps_the_selected_comment() { + let mut h = open(note_over_link_pdf(), "150", false); + let on_note = at(&h, 150.0, 325.0); + mouse_click(&mut h, on_note); + assert_eq!(h.state().views[0].comments.selected, Some((0, 1)), "the note is selected"); + link_tool(&mut h); + // On the link, away from the note. + let on_link = at(&h, 70.0, 325.0); + right_click(&mut h, on_link); + let v = &h.state().views[0]; + assert_eq!((v.links.context, v.comments.selected), (Some((0, 0)), Some((0, 1))), "the link's menu; the note stays selected"); +} + +/// A 300×400 page with a rectangle comment round the camera tool's first corner (view 15, 20). +fn comment_at_camera_corner_pdf() -> Vec { + pdf(&[ + CATALOG, + PAGES, + "<< /Type /Page /Parent 2 0 R /Annots [4 0 R] >>", + "<< /Type /Annot /Subtype /Square /Rect [5 365 45 395] /C [1 0 0] /F 4 /T (Ana) >>", + ]) +} + +/// A camera corner has no menu: a right-click on a comment under it opens the comment's, as +/// before. +#[test] +fn a_right_click_on_a_comment_under_a_camera_corner_opens_the_comments_menu() { + let mut h = open(comment_at_camera_corner_pdf(), "150", false); + h.state_mut().quick_tool = QuickTool::EnhanceCamera; + h.run_steps(4); + let corner = at(&h, 15.0, 380.0); + right_click(&mut h, corner); + assert_eq!(h.state().views[0].comments.selected, Some((0, 0)), "the comment is selected for its menu"); +} + +/// A 300×400 page with a web link along its left edge, x 0..100, y 300..350. +fn edge_link_pdf() -> Vec { + pdf(&[ + CATALOG, + PAGES, + "<< /Type /Page /Parent 2 0 R /Annots [4 0 R] >>", + "<< /Type /Annot /Subtype /Link /Rect [0 300 100 350] /Border [0 0 0] /A << /S /URI /URI (https://example.org/) >> >>", + ]) +} + +/// The Link tool's margin round a link stops at the page's edge: beside the page, where a press +/// does nothing, no hand promises one. +#[test] +fn the_link_tools_hand_stops_at_the_pages_edge() { + let mut h = open(edge_link_pdf(), "150", false); + link_tool(&mut h); + let on_link = at(&h, 10.0, 325.0); + assert_eq!(cursor_at(&mut h, on_link), egui::CursorIcon::PointingHand, "on the link"); + let beside = at(&h, 0.0, 325.0) - egui::vec2(1.0, 0.0); + assert_ne!(cursor_at(&mut h, beside), egui::CursorIcon::PointingHand, "just off the page"); +} + +/// With Edit text a right-click on a paragraph is Edit text's, as before, though a comment lies +/// over it: the comment is not selected. +#[test] +fn a_right_click_with_edit_text_on_a_paragraph_under_a_comment_selects_no_comment() { + let mut h = open(comment_on_text_pdf(), "150", false); + h.state_mut().quick_tool = QuickTool::EditText; + h.run_steps(4); + let on_text = at(&h, 100.0, 303.0); + wait_for_cursor(&mut h, on_text, |c| c != egui::CursorIcon::Default); + right_click(&mut h, on_text); + assert_eq!(h.state().views[0].comments.selected, None); +} + +/// With a Prepare field tool a right-click is the tool's, as before: a comment under it is not +/// selected. +#[test] +fn a_right_click_with_a_field_tool_selects_no_comment() { + let mut h = open(comment_and_checkbox_pdf(), "150", false); + assert!(h.state_mut().execute("form.add.text")); + h.run_steps(4); + let on_comment = at(&h, 200.0, 125.0); + right_click(&mut h, on_comment); + assert_eq!(h.state().views[0].comments.selected, None); +}