diff --git a/crates/ui-egui/src/canvas/page_input.rs b/crates/ui-egui/src/canvas/page_input.rs index eda260451d..248b5a767f 100644 --- a/crates/ui-egui/src/canvas/page_input.rs +++ b/crates/ui-egui/src/canvas/page_input.rs @@ -99,7 +99,7 @@ pub(super) fn page_tool_input(ui: &mut egui::Ui, view: &mut DocView, frame: &mut // What lies under the pointer before the tools act, asked once: for a click's release, a // right-click, and Fill & Sign's preview. let wants_under = resp.clicked() || resp.secondary_clicked() || matches!(tool, QuickTool::Fill(_)) && !ptr.any_down; - let under = pointer.filter(|_| wants_under).map(|p| hits.at(view, p)).unwrap_or_default(); + let under = pointer.filter(|p| wants_under && xf.rect.contains(*p)).map(|p| hits.at(view, p)).unwrap_or_default(); let released_over = match press { PagePress::Mine(owner) if resp.clicked() => under.iter().any(|h| h.owner == owner), _ => false, @@ -227,7 +227,8 @@ fn link_of(owner: Option) -> Option { /// The active tool's own input on page `i`: the one gate for presses nobody holds. Measure, the /// stamps, Fill & Sign, Sign area, zoom and snapshot, the camera's quadrilateral and crop act only /// on a press that began on this page and that no hit won (`press` is Free), and keep their cursor -/// and gestures. `over_mark`: a Fill & Sign mark takes the press, or would take one under the +/// and gestures. The camera also drags a corner whose press it holds, and crop's double-click +/// opens Set Page Boxes wherever it is. `over_mark`: a Fill & Sign mark takes the press, or would take one under the /// pointer. fn tool_input( ui: &egui::Ui, @@ -323,7 +324,7 @@ fn tool_input( /// selected; links take theirs from what this returns. /// `under`: the hits at `p` before the tools act. fn right_click(hits: &PageHits<'_>, under: &[crate::page_router::Hit], view: &mut DocView, p: Pos2) -> Option { - let context = hits.context_at(view, p); + let context = hits.context_at(view, p, under); // Add text takes every right-click on a page it can change, Edit text those on its own // 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. @@ -333,7 +334,10 @@ fn right_click(hits: &PageHits<'_>, under: &[crate::page_router::Hit], view: &mu QuickTool::Link => hits.can_modify && matches!(context, Some(Owner::Link { .. })), _ => hits.field_tool.is_some(), }; - if !tool_takes_it { + // A comment being moved, resized or reshaped keeps its selection until its release. + let comment_dragged = + matches!(view.comments.gesture, Some(comments::Gesture::Move { .. } | comments::Gesture::Resize { .. } | comments::Gesture::Reshape { .. })); + if !tool_takes_it && !comment_dragged { let comment = match context { Some(Owner::Comment { index, .. }) => Some(index), _ => None, @@ -400,11 +404,12 @@ impl PageHits<'_> { /// 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 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 { + /// right-click is what its press there would take (`under`, the hits at `p`), when that is + /// anything. + fn context_at(&self, view: &DocView, p: Pos2, under: &[crate::page_router::Hit]) -> Option { if self.tool == QuickTool::Link && self.can_modify - && let Some(press) = top(self.hits(view, p, false), QuickTool::Link) + && let Some(press) = top(under.iter().cloned(), QuickTool::Link) { return Some(press.owner); } diff --git a/crates/ui-egui/src/canvas/page_pointer/tests.rs b/crates/ui-egui/src/canvas/page_pointer/tests.rs index dd37e44373..5919fdae80 100644 --- a/crates/ui-egui/src/canvas/page_pointer/tests.rs +++ b/crates/ui-egui/src/canvas/page_pointer/tests.rs @@ -407,7 +407,7 @@ const ROUTER_WRITES: [&str; 4] = ["router.begin(", "router.press(", "router.sett const ROUTER_WRITERS: [(&str, &str, &str); 3] = [ ("canvas/page_input.rs", "page_tool_input", "the hits at the press point decide its owner, once per press"), ("canvas.rs", "draw_pages", "the press is over once the button is up and its release seen, and a new one drops the last owner"), - ("canvas.rs", "cancel_pointer_gestures", "every pointer gesture's cancel: Escape, a dialog, automation's cancel"), + ("canvas.rs", "cancel_pointer_gestures", "every pointer gesture's cancel: Escape, automation's cancel"), ]; /// Ways to make a `Hit`. @@ -430,7 +430,9 @@ fn hit_test_calls(line: &str) -> Vec { .collect() } -/// The functions that call a hit test: (file, function, why). `PageHits::hits` is the one: the hits +/// The functions that call a hit test: (file, function, why). Hit tests are found by name (a +/// function named `hit` or `…_hit`, called by path, bare or as a method); a point test by another +/// name has a list of its own (`COMMENT_POINT_TESTERS`). `PageHits::hits` is the one: the hits /// at the press point decide its owner, and those under the pointer give its cursor, its tip, hover /// painting and a right-click. const HIT_TESTERS: [(&str, &str, &str); 1] = [( @@ -439,6 +441,16 @@ const HIT_TESTERS: [(&str, &str, &str); 1] = [( "`PageHits::hits`: every module's hits at a point, for the pointer's cursor and tip, a right-click and, once, the press's owner", )]; +/// The comments' point test, which their hit tests are built on: which comment lies under a point. +const COMMENT_POINT_TESTS: [&str; 1] = [".comment_at("]; + +/// The functions that ask which comment lies under a point: (file, function, why). +const COMMENT_POINT_TESTERS: [(&str, &str, &str); 3] = [ + ("tools/comments/input.rs", "hit", "the comments' hit test"), + ("tools/comments/input.rs", "context_hit", "the comments' hit test for a right-click"), + ("tools/comments/paint.rs", "paint_page", "a redaction mark shows its fill under the pointer with every tool, as comments' tips show"), +]; + /// What reads who holds the press the pages hold. const ROUTER_READS: [&str; 3] = ["router.press_for(", "router.owner(", "router.keeps_drag("]; @@ -521,6 +533,14 @@ fn page_modules_act_on_a_press_only_through_the_router() { Add the function to ROUTER_READERS in src/canvas/page_pointer/tests.rs, with why.", "no longer reads who holds the press: remove it from ROUTER_READERS.", )); + problems.extend(misuses( + &COMMENT_POINT_TESTS, + &COMMENT_POINT_TESTERS, + "asks which comment lies under a point. The comments' hit tests answer that for a press and a right-click: \ + act on what the page view tells you, rather than testing the point again. A new place that must? Add the \ + function to COMMENT_POINT_TESTERS in src/canvas/page_pointer/tests.rs, with why.", + "no longer asks which comment lies under a point: remove it from COMMENT_POINT_TESTERS.", + )); problems.extend(misuses( &PRESS_IGNORES, &PRESS_IGNORERS, diff --git a/crates/ui-egui/src/canvas/page_router.rs b/crates/ui-egui/src/canvas/page_router.rs index 0b53038625..be83411072 100644 --- a/crates/ui-egui/src/canvas/page_router.rs +++ b/crates/ui-egui/src/canvas/page_router.rs @@ -112,7 +112,8 @@ pub(crate) fn top(hits: impl IntoIterator, tool: QuickTool) -> Optio /// What one page has to do with the press the pages hold. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(crate) enum PagePress { - /// The press began on this page, and the owner's hit won it: only its module acts on it. + /// The press began on this page, and the owner's hit won it: its module acts on it. Others + /// only let go of their own state (a click on anything but the selected comment unselects it). Mine(Owner), /// No button is down, or the press began on this page and nobody's hit won it: the active /// tool's. Also every page's while a press that began off the pages is held, and after the @@ -135,7 +136,6 @@ impl PagePress { /// A press the pages hold, and who it belongs to. #[derive(Clone, Debug, PartialEq)] pub(crate) struct Capture { - pub page: usize, pub owner: Owner, pub declines_drag: bool, } @@ -159,7 +159,7 @@ impl PageRouter { /// A press on `page`: `hits` at the press point decide who it belongs to. A press nobody's /// hit claims is held by nobody (the page's text, or a tool that acts anywhere on the page). pub(crate) fn press(&mut self, page: usize, hits: impl IntoIterator, tool: QuickTool) { - self.capture = top(hits, tool).map(|h| Capture { page, owner: h.owner, declines_drag: h.declines_drag }); + self.capture = top(hits, tool).map(|h| Capture { owner: h.owner, declines_drag: h.declines_drag }); self.began = Some(Some(page)); } @@ -167,7 +167,7 @@ impl PageRouter { pub(crate) fn press_for(&self, page: usize) -> PagePress { match (self.began, &self.capture) { (None, _) => PagePress::Free, - (Some(Some(p)), Some(c)) if p == page => PagePress::Mine(c.owner), + (Some(Some(p)), Some(capture)) if p == page => PagePress::Mine(capture.owner), (Some(Some(p)), None) if p == page => PagePress::Free, _ => PagePress::Elsewhere, } @@ -175,7 +175,7 @@ impl PageRouter { /// Who holds the press on `page`. pub(crate) fn owner(&self, page: usize) -> Option { - self.capture.as_ref().filter(|c| c.page == page).map(|c| c.owner) + self.press_for(page).owner() } /// Whether the press is held by someone who keeps its drag: then the page's text is not @@ -192,7 +192,7 @@ impl PageRouter { } } - /// Drops the press (Escape, a dialog, automation's cancel): every page is the active tool's again + /// Drops the press (Escape, automation's cancel): every page is the active tool's again /// ([`PagePress::Free`]). Page input skips a press Escape cancelled until the button is up. pub(crate) fn cancel(&mut self) { self.capture = None; diff --git a/crates/ui-egui/src/canvas/tests.rs b/crates/ui-egui/src/canvas/tests.rs index 02aeeacd09..c0a797fa67 100644 --- a/crates/ui-egui/src/canvas/tests.rs +++ b/crates/ui-egui/src/canvas/tests.rs @@ -2038,7 +2038,7 @@ fn image_handles_account_for_document_and_view_rotation() { } } -/// Cancelling the pages' pointer gestures (Escape, a dialog, automation's cancel) lets go of a +/// Cancelling the pages' pointer gestures (Escape, automation's cancel) lets go of a /// camera corner being dragged: the quadrilateral stays, and no corner follows the pointer. #[test] fn cancelling_pointer_gestures_lets_go_of_a_camera_corner() { diff --git a/crates/ui-egui/src/tools/link_ui.rs b/crates/ui-egui/src/tools/link_ui.rs index 659bad41e4..069564afff 100644 --- a/crates/ui-egui/src/tools/link_ui.rs +++ b/crates/ui-egui/src/tools/link_ui.rs @@ -132,7 +132,8 @@ pub(crate) fn page_input(cx: PageToolContext<'_>, free: bool, owner: Option Vec { } /// 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. +/// does nothing, no hand promises one (as before the margin moved into the one link test). #[test] fn the_link_tools_hand_stops_at_the_pages_edge() { let mut h = open(edge_link_pdf(), "150", false); @@ -1441,3 +1442,64 @@ fn a_right_click_with_a_field_tool_selects_no_comment() { right_click(&mut h, on_comment); assert_eq!(h.state().views[0].comments.selected, None); } + +/// With the Link tool a click that began off the page (in the gutter beside it) is no page's: the +/// selected link stays selected. It used to unselect it. +#[test] +fn a_click_that_began_off_the_page_keeps_the_selected_link() { + let mut h = open(link_pdf(), "150", false); + link_tool(&mut h); + let on_link = at(&h, 150.0, 325.0); + mouse_click(&mut h, on_link); + assert_eq!(h.state().views[0].links.selected, Some((0, 0)), "the link is selected"); + let left = page(&h).min.x; + slipping_click(&mut h, egui::pos2(left - 2.0, on_link.y), egui::pos2(left + 3.0, on_link.y)); + assert_eq!(h.state().views[0].links.selected, Some((0, 0)), "a click that began off the page leaves it selected"); +} + +/// A right-click while a comment is moved leaves the moved comment selected: the move ends on +/// it. The right-click used to select the comment under it mid-drag. +#[test] +fn a_right_click_during_a_comment_move_keeps_the_moved_comment_selected() { + let mut h = open(stacked_comments_pdf(), "150", false); + // "Bottom note" alone, then "Top note" alone. + let (bottom, top) = (at(&h, 60.0, 160.0), at(&h, 190.0, 290.0)); + h.hover_at(bottom); + h.run_steps(1); + h.drag_at(bottom); + h.run_steps(1); + h.hover_at(bottom + egui::vec2(0.0, 20.0)); + h.run_steps(1); + for pressed in [true, false] { + h.event(egui::Event::PointerButton { pos: top, button: egui::PointerButton::Secondary, pressed, modifiers: egui::Modifiers::NONE }); + } + h.run_steps(1); + let end = bottom + egui::vec2(0.0, 40.0); + h.hover_at(end); + h.run_steps(1); + h.drop_at(end); + h.run_steps(3); + assert_eq!(h.state().views[0].comments.selected, Some((0, 0)), "the moved comment stays selected"); +} + +/// A click on added text leaves the selected comment unselected, as a click on a form field does +/// (ruling 1). It used to keep it selected. +#[test] +fn a_click_on_added_text_deselects_the_selected_comment() { + let mut h = open(comment_on_text_pdf(), "150", false); + assert!(h.state_mut().apply_edit(pdfcraft_engine::Edit::AddText { + page: 0, + text: pdfcraft_engine::AddedText { rect: [40.0, 195.0, 140.0, 210.0], text: "Added".into(), ..Default::default() }, + })); + // The Edit panel open, with the Select tool: added text is picked up. + assert!(h.state_mut().execute("edit.text")); + h.run_steps(3); + h.state_mut().quick_tool = QuickTool::Select; + h.run_steps(3); + let on_comment = at(&h, 150.0, 315.0); + mouse_click(&mut h, on_comment); + assert_eq!(h.state().views[0].comments.selected, Some((0, 0)), "the comment is selected"); + let on_added = at(&h, 90.0, 202.0); + mouse_click(&mut h, on_added); + assert_eq!(h.state().views[0].comments.selected, None, "a click on the added text leaves it unselected"); +}