mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-11-06 18:52:36 +00:00
fix(ui): a right-click between the pages anchors no page's menu; the guard finds router readers
Found in review: - **A right-click off the pages.** Between the pages or beside one, no page took the right-click, but the page menu still opened over the page view, on the last anchor: Delete page, Paste and Copy Image acted on a page right-clicked long before. This was so on `main` too. The anchor is now cleared on every right-click before the pages set it, so off the pages the menu acts on the current page. - **The guard** took a file as a page module only by marks a router read doesn't need. A file that reads `.router.` is one too, so its router reads are checked. - **Docs.** egui ends a press's drag when any button is released, not when a second goes down. `link_ui::hit` takes the tip from the link it found rather than looking it up again. Tests: a right-click between the pages clears the anchor, seen failing without the fix. A planted router read in an unlisted file fails the guard. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
6752b36807
commit
568ea94f48
5 files changed
+33
-11
No files matched your search
@@ -1645,6 +1645,9 @@ fn draw_pages(
|
||||
if resp.secondary_clicked() {
|
||||
// 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.
|
||||
view.comments.context_at = None;
|
||||
let mut over_image = false;
|
||||
if tool == QuickTool::EditText {
|
||||
let pointer_pos = ptr.pos;
|
||||
|
||||
@@ -105,9 +105,9 @@ pub(super) fn page_tool_input(ui: &mut egui::Ui, view: &mut DocView, frame: &mut
|
||||
_ => false,
|
||||
};
|
||||
// A right-click while the primary button is held (a comment moved, a box drawn) leaves the
|
||||
// selections that press acts on alone; egui ends the press's own interaction when another button
|
||||
// goes down, so the button is what tells. A touch's long press, which egui reports as a
|
||||
// right-click with the finger still down, opens its menu.
|
||||
// selections that press acts on alone. egui ends the press's own interaction when any button is
|
||||
// released, the right-click's own frame included, so the button is what tells. A touch's long
|
||||
// press, which egui reports as a 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), as does one being moved on this
|
||||
|
||||
@@ -191,10 +191,10 @@ const PAGE_MODULES: [&str; 37] = [
|
||||
];
|
||||
|
||||
/// What a file that acts on the pages uses: the pointer, a tool's or the comments' page context,
|
||||
/// what a press is to a page, or the router's hits and owners. Such a file is a page module. One
|
||||
/// that acts on the pages through the page view's response and a bare point alone uses none of
|
||||
/// them (`canvas/image_pick.rs`): PAGE_MODULES lists it by hand.
|
||||
const PAGE_MODULE_MARKS: [&str; 5] = ["PagePointer", "PageToolContext", "PageCx", "PagePress", "page_router::"];
|
||||
/// what a press is to a page, the router's hits and owners, or the router itself. Such a file is a
|
||||
/// page module. One that acts on the pages through the page view's response and a bare point alone
|
||||
/// uses none of them (`canvas/image_pick.rs`): PAGE_MODULES lists it by hand.
|
||||
const PAGE_MODULE_MARKS: [&str; 6] = ["PagePointer", "PageToolContext", "PageCx", "PagePress", "page_router::", ".router."];
|
||||
|
||||
/// The router's own files, which define what page modules use.
|
||||
const ROUTER_FILES: [&str; 3] = ["canvas/page_pointer.rs", "canvas/page_router.rs", "canvas/page_cursor.rs"];
|
||||
|
||||
@@ -70,12 +70,14 @@ pub(crate) fn hit(xf: &PageXform, info: &DocInfo, page: usize, links: &[LinkItem
|
||||
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
|
||||
let (index, link) = if tool == QuickTool::Link {
|
||||
let index = links.iter().rfind(|l| l.page == page && screen_rect(xf, info, page, l.rect).expand(LINK_TOOL_MARGIN).contains(p))?.index;
|
||||
(index, info.links.iter().find(|l| l.page == page && l.index == index))
|
||||
} else {
|
||||
info.links.iter().rfind(|l| l.page == page && xf.user_rect(info, page, l.rect).contains(p))?.index
|
||||
let link = info.links.iter().rfind(|l| l.page == page && xf.user_rect(info, page, l.rect).contains(p))?;
|
||||
(link.index, Some(link))
|
||||
};
|
||||
let tip = info.links.iter().find(|l| l.page == page && l.index == index).map(|l| target_label(info, &l.target));
|
||||
let tip = link.map(|l| target_label(info, &l.target));
|
||||
let declines_drag = tool == QuickTool::Link;
|
||||
Some(Hit { tip, declines_drag, ..Hit::new(Owner::Link { index }, Layer::Link, egui::CursorIcon::PointingHand) })
|
||||
}
|
||||
|
||||
@@ -1708,3 +1708,20 @@ fn a_fill_and_sign_click_places_nothing_while_a_mark_is_moved() {
|
||||
mouse_click(&mut h, blank);
|
||||
assert_eq!(marks(&h), before, "no mark is placed");
|
||||
}
|
||||
|
||||
/// A right-click between the pages anchors no page's menu, so Delete page and Paste act on the
|
||||
/// current page. The menu used to keep the anchor of the last right-click on a page.
|
||||
#[test]
|
||||
fn a_right_click_between_the_pages_anchors_no_pages_menu() {
|
||||
let mut h = open(two_pages_pdf(), "100", false);
|
||||
h.run_steps(2);
|
||||
h.state_mut().views[0].comments.context_at = Some((0, [10.0, 10.0]));
|
||||
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.context_at, None);
|
||||
}
|
||||
Reference in new issue
Block a user