diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 6775702384..95767e2c06 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -1207,6 +1207,19 @@ struct PageFrame<'a> { current_height: f32, pressed_page: Option, calc_hover: Option, + /// A page took this frame's right-click. + right_clicked: bool, +} + +impl PageFrame<'_> { + /// Where this frame's right-click is, on the page at `rect`, for the first page it is on: pages + /// that touch (the gaps hidden) can share a row of pixels, and a right-click there is one + /// page's, as a press is. + fn right_click_on(&mut self, rect: Rect) -> Option { + let p = self.pointer.filter(|p| !self.right_clicked && self.resp.secondary_clicked() && rect.contains(*p))?; + self.right_clicked = true; + Some(p) + } } /// Visible heights (points) closer than this are equal: one page size can meet the viewport a @@ -1502,7 +1515,7 @@ fn draw_page(ui: &mut egui::Ui, view: &mut DocView, frame: &mut PageFrame<'_>, i // A cancelled press takes nothing until the button is up, though the page's menu still opens // where a right-click is. let input = if view.press_cancelled { - if let Some(p) = pointer.filter(|p| frame.resp.secondary_clicked() && xf.rect.contains(*p)) { + if let Some(p) = frame.right_click_on(xf.rect) { comments::anchor(&pcx, p, view); } PageInput { consumed: true, area_tool: false, hover: None, released_over: false } @@ -1683,6 +1696,7 @@ fn draw_pages( current_height: f32::INFINITY, pressed_page: None, calc_hover: None, + right_clicked: false, }; for &i in visible_pages { draw_page(ui, view, &mut frame, i); diff --git a/crates/ui-egui/src/canvas/page_input.rs b/crates/ui-egui/src/canvas/page_input.rs index 39b39e11b0..0a5d9b24e5 100644 --- a/crates/ui-egui/src/canvas/page_input.rs +++ b/crates/ui-egui/src/canvas/page_input.rs @@ -109,7 +109,7 @@ pub(super) fn page_tool_input(ui: &mut egui::Ui, view: &mut DocView, frame: &mut // 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. let press_held = ptr.primary_down && !resp.long_touched(); - let context = pointer.filter(|p| resp.secondary_clicked() && xf.rect.contains(*p)).and_then(|p| right_click(&hits, &under, view, p, press_held)); + 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 = @@ -332,7 +332,8 @@ fn tool_input( /// ([`PageHits::context_at`]), which the menu then acts on. A comment there is selected, and none /// elsewhere; a Prepare field is selected; links take theirs from what this returns. /// `under`: the hits at `p` before the tools act. `press_held`: the primary button is held, and the -/// selections its press acts on are left alone until its release. +/// comment and Prepare selections its press acts on are left alone until its release; links still +/// take theirs, as before. fn right_click(hits: &PageHits<'_>, under: &[crate::page_router::Hit], view: &mut DocView, p: Pos2, press_held: bool) -> Option { let context = hits.context_at(view, p, under); // Delete page, Paste and Copy Image act where the menu opened, whoever takes the right-click. diff --git a/crates/ui-egui/src/canvas/page_pointer/tests.rs b/crates/ui-egui/src/canvas/page_pointer/tests.rs index 4936c81367..e0ddc358fe 100644 --- a/crates/ui-egui/src/canvas/page_pointer/tests.rs +++ b/crates/ui-egui/src/canvas/page_pointer/tests.rs @@ -293,7 +293,8 @@ fn outside_test_modules(source: &str) -> Vec<(usize, &str)> { } /// Each name `find` finds in a line of the page modules' code (their tests left out), as (file, -/// the top-level function or `impl` method it is in, line, name). +/// the top-level function or `impl` method it is in, line, name). A line rustfmt continued (a +/// method chain split as `view` / `.router` / `.owner(i)`) is read with the one it continues. fn page_module_matches(find: impl Fn(&str) -> Vec) -> Vec<(&'static str, Option, usize, String)> { let src = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src"); let mut found = Vec::new(); @@ -301,6 +302,7 @@ fn page_module_matches(find: impl Fn(&str) -> Vec) -> Vec<(&'static str, let source = std::fs::read_to_string(src.join(file)).expect("page module").replace("\r\n", "\n"); // The top-level function, or method of an `impl`, that each line is in. let (mut function, mut in_impl) = (None, false); + let mut lines: Vec<(usize, Option<&str>, String)> = Vec::new(); for (n, line) in outside_test_modules(&source) { if line.starts_with("impl") { (function, in_impl) = (None, true); @@ -312,7 +314,13 @@ fn page_module_matches(find: impl Fn(&str) -> Vec) -> Vec<(&'static str, if line.trim_start().starts_with("//") { continue; } - for name in find(line) { + match lines.last_mut() { + Some((_, _, text)) if line.trim_start().starts_with('.') => text.push_str(line.trim_start()), + _ => lines.push((n, function, line.to_owned())), + } + } + for (n, function, text) in lines { + for name in find(&text) { found.push((file, function.map(str::to_owned), n, name)); } } @@ -320,6 +328,41 @@ fn page_module_matches(find: impl Fn(&str) -> Vec) -> Vec<(&'static str, found } +/// Each hit test a page module imports under a name that is not a hit test's (`use …::hit as +/// under;`), which the checks by name would not see. +fn hit_test_aliases() -> Vec { + let src = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src"); + let mut found = Vec::new(); + for file in PAGE_MODULES { + let source = std::fs::read_to_string(src.join(file)).expect("page module"); + // Each `use` statement, from its first line to its `;`, wherever it is. + let mut statement: Option = None; + for line in source.lines().map(str::trim) { + if statement.is_none() && ["use ", "pub use ", "pub(crate) use ", "pub(super) use "].iter().any(|start| line.starts_with(start)) { + statement = Some(String::new()); + } + let Some(text) = statement.as_mut() else { continue }; + text.push_str(line); + text.push(' '); + if !line.ends_with(';') { + continue; + } + for part in text.split([',', '{', '}', ';']) { + let Some((path, alias)) = part.split_once(" as ") else { continue }; + let (name, alias) = (path.trim().rsplit("::").next().unwrap_or_default(), alias.trim()); + if is_hit_test(name) && !is_hit_test(alias) { + found.push(format!( + "src/{file}: `{name} as {alias}` imports a hit test under another name, which the checks by name miss: \ + name it `hit` or `…_hit`." + )); + } + } + statement = None; + } + } + found +} + /// Each use of one of `names` in the page modules' code (their tests left out). fn page_module_uses(names: &[&str]) -> Vec<(&'static str, Option, usize, String)> { page_module_matches(|line| names.iter().filter(|name| uses(line, name)).map(|name| (*name).to_owned()).collect()) @@ -615,6 +658,7 @@ fn page_modules_act_on_a_press_only_through_the_router() { src/canvas/page_pointer/tests.rs, with why.", "calls no hit test any more: remove it from HIT_TESTERS.", )); + problems.extend(hit_test_aliases()); // An entry no page module calls any more no longer guards anything. let called = page_module_uses(&PAGE_HIT_CALLS); problems.extend( diff --git a/crates/ui-egui/src/canvas/page_router.rs b/crates/ui-egui/src/canvas/page_router.rs index 0863642aee..e3b88d1a2c 100644 --- a/crates/ui-egui/src/canvas/page_router.rs +++ b/crates/ui-egui/src/canvas/page_router.rs @@ -118,7 +118,8 @@ pub(crate) enum PagePress { /// 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 the page view does not hold is down (one that began /// over a panel), and after the press was dropped ([`PageRouter::cancel`]) until the button is - /// up. + /// up (`Elsewhere` instead when Escape came in the press's own frame; page input skips a + /// cancelled press either way). Free, /// The press began on another page, or off the pages (between them, or beside one). Elsewhere, diff --git a/crates/ui-egui/tests/it/pointer.rs b/crates/ui-egui/tests/it/pointer.rs index 1fc7c51dd3..205a5b2663 100644 --- a/crates/ui-egui/tests/it/pointer.rs +++ b/crates/ui-egui/tests/it/pointer.rs @@ -1607,3 +1607,86 @@ fn a_right_click_with_edit_text_on_added_text_unselects_the_comment() { right_click(&mut h, on_added); assert_eq!(h.state().views[0].comments.selected, None); } + +/// Two pages with their gaps hidden, after `tool` set the tool up, and a point on the row of pixels +/// where they touch, which lies on both. +fn touching_pages(tool: impl FnOnce(&mut PdfCraftApp)) -> (Harness<'static, PdfCraftApp>, egui::Pos2) { + let mut h = open(two_pages_pdf(), "100", false); + h.state_mut().views[0].show_page_gaps = false; + tool(h.state_mut()); + h.run_steps(4); + 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 seam = egui::pos2(one.min.x + 100.0, one.max.y); + assert!(one.contains(seam) && two.contains(seam), "the pages touch: {one:?} {two:?}"); + (h, seam) +} + +/// The pages the document's stamps are on. +fn stamp_pages(h: &Harness<'_, PdfCraftApp>) -> Vec { + let s = h.state(); + s.session.get(s.views[0].id).unwrap().info.annotations.iter().filter(|a| a.subtype == "Stamp").map(|a| a.page).collect() +} + +/// Where pages touch (the gaps hidden), a press on the row they share is the first page's: a stamp +/// clicked there lands on page 1. The second page used to take the press over. +#[test] +fn a_stamp_clicked_where_two_pages_touch_lands_on_the_first() { + let (mut h, seam) = touching_pages(|app| app.quick_tool = QuickTool::Stamp(StampKind::DynApproved)); + mouse_click(&mut h, seam); + assert_eq!(stamp_pages(&h), vec![0]); +} + +/// A click pressed on one page and released on the next (the gaps hidden, within egui's click +/// distance) places nothing: the press is the first page's, and the pointer is off it. `main` placed +/// the stamp on the second page. +#[test] +fn a_stamp_click_pressed_on_one_page_and_released_on_the_next_places_nothing() { + let (mut h, seam) = touching_pages(|app| app.quick_tool = QuickTool::Stamp(StampKind::DynApproved)); + slipping_click(&mut h, seam - egui::vec2(0.0, 2.0), seam + egui::vec2(0.0, 2.0)); + assert_eq!(stamp_pages(&h), Vec::::new()); +} + +/// Where pages touch (the gaps hidden), a right-click on the row they share is the first page's: +/// the menu is anchored there. Both pages used to take it, and the second's anchor stood. +#[test] +fn a_right_click_where_two_pages_touch_is_the_first_pages() { + let (mut h, seam) = touching_pages(|_| {}); + right_click(&mut h, seam); + assert_eq!(h.state().views[0].comments.context_at.map(|(page, _)| page), Some(0)); +} + +/// With Prepare open, a click that began off the page (in the gutter beside it) is no page's: the +/// selected field stays selected, as the Link tool's and comments' selections do. +#[test] +fn with_prepare_a_click_that_began_off_the_page_keeps_the_selected_field() { + 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 left = page(&h).min.x; + slipping_click(&mut h, egui::pos2(left - 2.0, on_box.y), egui::pos2(left + 3.0, on_box.y)); + assert!(h.state().views[0].prepare.selected.is_some(), "a click that began off the page leaves it selected"); +} + +/// 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] +fn a_note_tool_click_on_added_text_starts_no_note() { + 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() }, + })); + assert!(h.state_mut().execute("edit.text")); + h.run_steps(3); + h.state_mut().quick_tool = QuickTool::Comment(CommentTool::Note); + h.run_steps(3); + let on_added = at(&h, 90.0, 202.0); + mouse_click(&mut h, on_added); + assert!(h.state().views[0].comments.composer.is_none(), "no note is started"); +}