fix(ui): a mark being moved keeps Fill & Sign's presses; the guard sees calls by path

Found in review:
- **Fill & Sign during a mark's move.** `main` placed no Fill & Sign mark while a comment's move,
  resize or reshape was live on the page, whoever held the press. That guard was lost: with a
  move left over (its release never seen), the next click placed a new mark. A live gesture on
  the page now counts as over a mark again (`comments::moving_on`, which the comments' own
  dispatch uses too).
- **The guard** missed the router's and the page view's hits called by path
  (`PageRouter::owner(&view.router, i)`), and its doc said the `.at(` it matches was confined to
  `page_input.rs`.
- `measure_ui::Clicks` derived traits nothing uses.

Tests: a Fill & Sign click while a mark's move is live places nothing, seen failing without the
fix. A planted router read by path fails the guard.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
alehelandClaude Opus 5.5 committed 2026-11-02 20:39:51 +01:00
1 parent 6756ab0ed4
commit 6752b36807
6 files changed
+50 -17

No files matched your search

+7 -6
View File
@@ -110,10 +110,11 @@ pub(super) fn page_tool_input(ui: &mut egui::Ui, view: &mut DocView, frame: &mut
// right-click with the finger still down, opens its menu.
let press_held = ptr.primary_down && !resp.long_touched();
let context = frame.right_click_on(xf.rect).and_then(|p| right_click(&hits, &under, view, p, press_held));
// A mark already placed takes its own press (comments, below), and with no button down the
// Fill & Sign tool's preview hides over one, where a press would pick it up.
let over_mark =
matches!(tool, QuickTool::Fill(_)) && (matches!(press, PagePress::Mine(_)) || !ptr.any_down && top(under.iter().cloned(), tool).is_some());
// 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 over one,
// where a press would pick it up.
let over_mark = matches!(tool, QuickTool::Fill(_))
&& (matches!(press, PagePress::Mine(_)) || comments::moving_on(view, i) || !ptr.any_down && top(under.iter().cloned(), tool).is_some());
tool_input(ui, view, frame, slot, &pcx, press, over_mark);
let on_field = if preparing {
let o = crate::prepare::page_input(cx, form, field_tool, prepare_allowed, press, view);
@@ -237,8 +238,8 @@ fn link_of(owner: Option<Owner>) -> Option<usize> {
/// on a press that began on this page and that no hit won (`press` is Free), and keep their cursor
/// 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`: with Fill & Sign, a hit holds the press (a
/// mark's, or a Prepare field's), or with no button down a mark lies under the pointer, where a
/// press would pick it up.
/// mark's, or a Prepare field's), a mark is being moved on this page, or with no button down a mark
/// lies under the pointer, where a press would pick it up.
fn tool_input(
ui: &egui::Ui,
view: &mut DocView,
@@ -454,7 +454,16 @@ fn page_modules_claim_the_cursor_and_never_set_it() {
}
/// What decides, keeps or drops the press the pages hold.
const ROUTER_WRITES: [&str; 4] = ["router.begin(", "router.press(", "router.settle(", "router.cancel("];
const ROUTER_WRITES: [&str; 8] = [
"router.begin(",
"router.press(",
"router.settle(",
"router.cancel(",
"PageRouter::begin(",
"PageRouter::press(",
"PageRouter::settle(",
"PageRouter::cancel(",
];
/// The functions that decide, keep or drop the press the pages hold: (file, function, why).
const ROUTER_WRITERS: [(&str, &str, &str); 3] = [
@@ -498,12 +507,12 @@ const HIT_TESTERS: [(&str, &str, &str); 1] = [(
/// The page view's asks for every module's hits at a point (`PageHits::hits`, and `at` and
/// `context_at`, which wrap it).
const PAGE_HITS_ASKS: [&str; 3] = [".hits(", ".at(", ".context_at("];
const PAGE_HITS_ASKS: [&str; 6] = [".hits(", ".at(", ".context_at(", "PageHits::hits(", "PageHits::at(", "PageHits::context_at("];
/// The functions that ask the page view's hits at a point: (file, function, why). `PageHits` is
/// private to `page_input.rs`, so this keeps a new function there from asking by these names; a
/// call by path (`PageHits::at(&hits, …)`) and a second ask inside a listed function are for
/// review to catch.
/// private to `page_input.rs`, so this keeps a new function there from asking; a second ask inside
/// a listed function is for review to catch. The method names are matched in every page module,
/// so an `at` of another type there would need them narrowed.
const PAGE_HITS_ASKERS: [(&str, &str, &str); 5] = [
("canvas/page_input.rs", "at", "`PageHits::at`: the hits at a point, as a press there would take them"),
("canvas/page_input.rs", "context_at", "`PageHits::context_at`: what a right-click at a point is on"),
@@ -528,7 +537,8 @@ const COMMENT_POINT_TESTERS: [(&str, &str, &str); 3] = [
];
/// What reads who holds the press the pages hold.
const ROUTER_READS: [&str; 3] = ["router.press_for(", "router.owner(", "router.keeps_drag("];
const ROUTER_READS: [&str; 6] =
["router.press_for(", "router.owner(", "router.keeps_drag(", "PageRouter::press_for(", "PageRouter::owner(", "PageRouter::keeps_drag("];
/// The functions that read who holds the press: (file, function, why). The page view asks once per
/// page and tells each module what the press is to its page (`PagePress`); the Edit text group, the
+1 -1
View File
@@ -37,7 +37,7 @@ pub(crate) use geometry::{corner_ratio, page_rotation, resized};
pub use geometry::{drawn_text_box_rect, text_box_rect};
#[cfg(test)]
use input::hover_tip;
pub(crate) use input::{anchor, context, context_hit, describe, hit, keys, nudge_keys, page_after_text, page_input};
pub(crate) use input::{anchor, context, context_hit, describe, hit, keys, moving_on, nudge_keys, page_after_text, page_input};
pub use menu::{CanvasAction, fill_picker, recent_custom_colours, remember_custom_colour, status_badge, swatch_grid};
#[cfg(test)]
use menu::{RECENT_CUSTOM, same_colour};
+7 -3
View File
@@ -31,9 +31,7 @@ pub(crate) fn page_input(ui: &egui::Ui, resp: &egui::Response, cx: &PageCx<'_>,
cv.gesture = None;
return true;
}
// A comment being moved, resized or reshaped on this page keeps the frame until it ends.
let moving =
matches!(cv.gesture, Some(Gesture::Move { page, .. } | Gesture::Resize { page, .. } | Gesture::Reshape { page, .. }) if page == cx.page);
let moving = moving_on(view, cx.page);
let gestures = Gestures { ui, resp, cx, pointer, origin, page_rect, pressed_here, over_page, free };
match cx.tool {
// The selected shape's handles, and Fill & Sign's marks, take their own presses.
@@ -51,6 +49,12 @@ pub(crate) fn page_input(ui: &egui::Ui, resp: &egui::Response, cx: &PageCx<'_>,
}
}
/// A comment (a Fill & Sign mark included) is being moved, resized or reshaped on `page`: it keeps
/// the page's frames until its gesture ends.
pub(crate) fn moving_on(view: &DocView, page: usize) -> bool {
matches!(view.comments.gesture, Some(Gesture::Move { page: p, .. } | Gesture::Resize { page: p, .. } | Gesture::Reshape { page: p, .. }) if p == page)
}
/// What of the comments lies under `p`, for the cursor, the tip and the press: the one hit test
/// of this module. With the Select tool, the selected comment's point handles, then its resize
/// handles, then the topmost comment. With a Fill & Sign tool, the same over Fill & Sign's marks
+1 -1
View File
@@ -203,7 +203,7 @@ fn snap_to_content(
}
/// The Measure tools' own clicks this frame: those of a press nobody holds.
#[derive(Clone, Copy, Debug, Default)]
#[derive(Clone, Copy)]
pub(crate) struct Clicks {
pub single: bool,
pub double: bool,
+18
View File
@@ -1690,3 +1690,21 @@ fn a_note_tool_click_on_added_text_starts_no_note() {
mouse_click(&mut h, on_added);
assert!(h.state().views[0].comments.composer.is_none(), "no note is started");
}
/// With Fill & Sign, a mark being moved on the page keeps the page's presses, as before: a click while
/// its move is still live (its release never seen) places no new mark.
#[test]
fn a_fill_and_sign_click_places_nothing_while_a_mark_is_moved() {
let mut h = open(comment_on_text_pdf(), "150", false);
h.state_mut().quick_tool = QuickTool::Fill(FillTool::Check);
h.run_steps(4);
let marks = |h: &Harness<'_, PdfCraftApp>| {
let s = h.state();
s.session.get(s.views[0].id).unwrap().info.annotations.len()
};
let before = marks(&h);
let blank = at(&h, 150.0, 100.0);
h.state_mut().views[0].comments.gesture = Some(pdfcraft_ui_egui::comments::Gesture::Move { page: 0, index: 0, from: blank });
mouse_click(&mut h, blank);
assert_eq!(marks(&h), before, "no mark is placed");
}