mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-11-09 12:32:52 +00:00
fix(ui): a right-button drag on the pages starts nothing a left one does
The pages' presses are the primary button's (`PagePointer.edges`, the router's `began`), so a right-button drag was a press nobody held, and every tool that started on egui's `drag_started()`, which counts every button, started on it: the Rectangle tool drew a rectangle, Prepare's selection rectangle selected the fields in it, the Link tool opened Link Properties for a new link, the camera moved its quadrilateral to the page, and the Select tool selected text. On `main` too. `PagePointer.drag_started` is a primary drag of a press the pages hold, started this frame. The page modules read it instead of egui's (comments' shapes, callout and text box, Prepare, the Link tool, the camera, the text layer, image pick, and the Edit text group, whose owned presses were primary already); zoom, crop, Sign area and Redact already asked for the primary button. The guard refuses a bare `.drag_started()` in a page module (`NOT_PAGE_DRAGS` lists the widgets of their own: Organize's thumbnails, the signature pad, the Scan tab), and the router's `drag_started` is `drag_began`, so the rule reads cleanly. Tests, one per family, each seen failing with the fix reverted: `a_right_button_drag_draws_no_rectangle`, `…_selects_no_field`, `…_draws_no_link`, `…_leaves_the_camera_quadrilateral`, `…_selects_no_text`; the guard rule against a planted `#[cfg(any())]` call. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
13 files changed
+125
-28
No files matched your search
@@ -3,7 +3,7 @@
|
||||
//! other image as a lossless PNG (with its transparency). The clipboard gets the file, as when a
|
||||
//! file is copied in Finder or Explorer; Copy Image copies the pixels instead.
|
||||
|
||||
use egui::{Color32, CornerRadius, Pos2, Stroke};
|
||||
use egui::{Color32, CornerRadius, Stroke};
|
||||
|
||||
use crate::PdfCraftApp;
|
||||
use crate::canvas::{DocView, PageXform};
|
||||
@@ -30,11 +30,11 @@ pub(crate) fn page_input(
|
||||
page: usize,
|
||||
info: &DocInfo,
|
||||
doc: &pdfcraft_engine::Document,
|
||||
pointer: Option<Pos2>,
|
||||
ptr: &crate::page_pointer::PagePointer,
|
||||
view: &mut DocView,
|
||||
) {
|
||||
let Some(p) = pointer.filter(|p| xf.rect.contains(*p)) else { return };
|
||||
if resp.drag_started() || resp.double_clicked() || resp.triple_clicked() {
|
||||
let Some(p) = ptr.pos.filter(|p| xf.rect.contains(*p)) else { return };
|
||||
if ptr.drag_started || resp.double_clicked() || resp.triple_clicked() {
|
||||
view.picked_image = None;
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -115,8 +115,8 @@ pub(super) fn page_tool_input(ui: &mut egui::Ui, view: &mut DocView, frame: &mut
|
||||
}
|
||||
}
|
||||
// A drag that the press's owner declines is nobody's once it starts.
|
||||
if resp.drag_started_by(egui::PointerButton::Primary) {
|
||||
view.router.drag_started();
|
||||
if ptr.drag_started {
|
||||
view.router.drag_began();
|
||||
}
|
||||
// What this page has to do with the press: its owner's, the active tool's, or another page's.
|
||||
let press = view.router.press_for(i);
|
||||
@@ -663,7 +663,7 @@ fn update_selection_on_press(ui: &egui::Ui, view: &mut DocView, text: &PageText,
|
||||
view.selection = Some(Selection::run(gesture.page, first, last));
|
||||
}
|
||||
}
|
||||
if gesture.resp.drag_started() && press_here && gesture.text_drag {
|
||||
if gesture.ptr.drag_started && press_here && gesture.text_drag {
|
||||
view.actions_dismissed = None;
|
||||
view.selection_drag = true;
|
||||
let origin = gesture.ptr.origin.unwrap_or(gesture.pointer);
|
||||
@@ -778,7 +778,7 @@ pub(super) fn page_overlays(ui: &mut egui::Ui, view: &mut DocView, frame: &mut P
|
||||
// A press nobody holds picks the picture under it: a link's click follows the link only (ruling
|
||||
// 5).
|
||||
if tool == QuickTool::Select && input.press == PagePress::Free && !input.consumed {
|
||||
crate::image_pick::page_input(resp, &xf, i, info, doc, pointer, view);
|
||||
crate::image_pick::page_input(resp, &xf, i, info, doc, ptr, view);
|
||||
}
|
||||
crate::image_pick::drop_stale(doc, view);
|
||||
crate::image_pick::paint(painter, &xf, i, info, view);
|
||||
|
||||
@@ -49,6 +49,9 @@ pub(crate) struct PagePointer {
|
||||
/// The modifier keys held at this frame's first primary press, or else now: a press with Shift
|
||||
/// or ⌘ adds to a selection.
|
||||
pub modifiers: Modifiers,
|
||||
/// A primary press the pages hold turned into a drag this frame. egui's own `drag_started()`
|
||||
/// counts every button, so a right-button drag would start what a left-button drag does.
|
||||
pub drag_started: bool,
|
||||
}
|
||||
|
||||
impl PagePointer {
|
||||
@@ -79,9 +82,10 @@ impl PagePointer {
|
||||
resp.dragged() || resp.drag_stopped() || (!modal && (resp.interact_pointer_pos().is_some() || press_origin.is_some_and(|p| on_top(&p))));
|
||||
let pos = at.filter(|p| held || on_top(p));
|
||||
if !held {
|
||||
return Self { pos, origin: None, edges: Vec::new(), released: None, primary_down, any_down, modifiers };
|
||||
return Self { pos, origin: None, edges: Vec::new(), released: None, primary_down, any_down, modifiers, drag_started: false };
|
||||
}
|
||||
Self { pos, origin: press_origin, edges, released, primary_down, any_down, modifiers }
|
||||
let drag_started = resp.drag_started_by(PointerButton::Primary);
|
||||
Self { pos, origin: press_origin, edges, released, primary_down, any_down, modifiers, drag_started }
|
||||
}
|
||||
|
||||
/// Where the pointer is, when it is inside `r`.
|
||||
|
||||
@@ -271,6 +271,18 @@ fn declares(line: &str) -> Option<&str> {
|
||||
rest.find(|c: char| !(c.is_alphanumeric() || c == '_')).map(|end| &rest[..end])
|
||||
}
|
||||
|
||||
/// egui's drag start, which counts every button: a right-button drag would start what a left one
|
||||
/// does. Page modules read `PagePointer::drag_started`, the primary button's.
|
||||
const ANY_BUTTON_DRAG: [&str; 1] = [".drag_started()"];
|
||||
|
||||
/// Functions in the page modules that start a drag of a widget of their own, not of the pages:
|
||||
/// (file, function, why).
|
||||
const NOT_PAGE_DRAGS: [(&str, &str, &str); 3] = [
|
||||
("canvas/organize.rs", "drag_pages", "Organize Pages drags thumbnails, not pages"),
|
||||
("tools/fill_sign.rs", "signature_pad", "the signature dialog's drawing pad"),
|
||||
("tools/fill_sign/scan.rs", "scan_tab", "the signature dialog's Scan tab"),
|
||||
];
|
||||
|
||||
/// Ways to set egui's cursor.
|
||||
const CURSOR_WRITES: [&str; 4] = ["set_cursor_icon(", ".cursor_icon = ", "on_hover_cursor(", "on_hover_and_drag_cursor("];
|
||||
|
||||
@@ -462,6 +474,14 @@ fn page_modules_read_the_pointer_only_through_page_pointer() {
|
||||
}
|
||||
}
|
||||
}
|
||||
problems.extend(misuses(
|
||||
&ANY_BUTTON_DRAG,
|
||||
&NOT_PAGE_DRAGS,
|
||||
"starts a drag with any button: a right-button drag would start what a left one does. Read \
|
||||
`PagePointer::drag_started` (the primary button's) instead. A widget of its own, not the pages? Add the \
|
||||
function to NOT_PAGE_DRAGS in src/canvas/page_pointer/tests.rs, with why.",
|
||||
"starts no drag of its own any more: remove it from NOT_PAGE_DRAGS.",
|
||||
));
|
||||
assert!(problems.is_empty(), "{}", problems.join("\n"));
|
||||
}
|
||||
|
||||
@@ -486,12 +506,12 @@ fn page_modules_claim_the_cursor_and_never_set_it() {
|
||||
const ROUTER_WRITES: [&str; 10] = [
|
||||
"router.begin(",
|
||||
"router.press(",
|
||||
"router.drag_started(",
|
||||
"router.drag_began(",
|
||||
"router.settle(",
|
||||
"router.cancel(",
|
||||
"PageRouter::begin(",
|
||||
"PageRouter::press(",
|
||||
"PageRouter::drag_started(",
|
||||
"PageRouter::drag_began(",
|
||||
"PageRouter::settle(",
|
||||
"PageRouter::cancel(",
|
||||
];
|
||||
|
||||
@@ -219,7 +219,7 @@ impl PageRouter {
|
||||
/// The press turned into a drag: an owner that declines its drag lets it go, and from then on
|
||||
/// the press is nobody's (a drag across a highlight selects the text under it; with the Link
|
||||
/// tool, a drag from a link draws a new one). Its click stayed the owner's.
|
||||
pub(in crate::canvas) fn drag_started(&mut self) {
|
||||
pub(in crate::canvas) fn drag_began(&mut self) {
|
||||
if self.capture.as_ref().is_some_and(|c| c.declines_drag) {
|
||||
self.capture = None;
|
||||
}
|
||||
@@ -327,11 +327,11 @@ mod tests {
|
||||
let mut r = PageRouter::default();
|
||||
r.begin();
|
||||
r.press(1, [Hit { declines_drag: true, ..hit(Layer::Comment, 0) }], QuickTool::Select);
|
||||
r.drag_started();
|
||||
r.drag_began();
|
||||
assert_eq!((r.press_for(1), r.press_for(0)), (PagePress::Free, PagePress::Elsewhere));
|
||||
r.begin();
|
||||
r.press(1, [hit(Layer::Comment, 2)], QuickTool::Select);
|
||||
r.drag_started();
|
||||
r.drag_began();
|
||||
assert_eq!(r.press_for(1), PagePress::Mine(hit(Layer::Comment, 2).owner));
|
||||
}
|
||||
}
|
||||
@@ -102,14 +102,14 @@ pub(crate) fn page_input(resp: &egui::Response, xf: &PageXform, ptr: &PagePointe
|
||||
// began, a click where it came up (egui has cleared the press's origin by its release, and the
|
||||
// pointer may have moved on in the same frame).
|
||||
let at = if resp.clicked() { ptr.released } else { origin };
|
||||
if (resp.drag_started() || resp.clicked())
|
||||
if (ptr.drag_started || resp.clicked())
|
||||
&& press == PagePress::Free
|
||||
&& at.is_some_and(|p| xf.rect.contains(p))
|
||||
&& view.camera.is_none_or(|q| q.page != page)
|
||||
{
|
||||
view.camera = Some(CameraQuad::inset(page, xf.pw, xf.ph));
|
||||
}
|
||||
if resp.drag_started()
|
||||
if ptr.drag_started
|
||||
&& let PagePress::Mine(Owner::CameraCorner { corner }) = press
|
||||
&& let Some(q) = view.camera.as_mut().filter(|q| q.page == page)
|
||||
{
|
||||
|
||||
@@ -194,7 +194,7 @@ fn select_input(
|
||||
};
|
||||
let cv = &mut view.comments;
|
||||
let mut consumed = false;
|
||||
if resp.drag_started()
|
||||
if cx.ptr.drag_started
|
||||
&& let (Some((a, part)), Some(o)) = (owner, origin)
|
||||
{
|
||||
match part {
|
||||
@@ -356,7 +356,7 @@ impl Gestures<'_> {
|
||||
if over_page {
|
||||
view.cursor.claim(Layer::ToolArea, egui::CursorIcon::Crosshair);
|
||||
}
|
||||
if resp.drag_started()
|
||||
if cx.ptr.drag_started
|
||||
&& pressed_here
|
||||
&& let Some(o) = origin
|
||||
{
|
||||
@@ -479,7 +479,7 @@ impl Gestures<'_> {
|
||||
if over_page {
|
||||
view.cursor.claim(Layer::ToolArea, egui::CursorIcon::Crosshair);
|
||||
}
|
||||
if resp.drag_started()
|
||||
if cx.ptr.drag_started
|
||||
&& pressed_here
|
||||
&& let Some(o) = origin
|
||||
{
|
||||
@@ -560,7 +560,7 @@ impl Gestures<'_> {
|
||||
// A text box also takes a drag, as in Acrobat: the box is drawn at that size and its
|
||||
// text wraps inside it (#2669).
|
||||
if tool == CommentTool::TextBox {
|
||||
if resp.drag_started()
|
||||
if cx.ptr.drag_started
|
||||
&& pressed_here
|
||||
&& let Some(o) = origin
|
||||
{
|
||||
|
||||
@@ -485,7 +485,7 @@ pub(crate) fn page_input(cx: PageToolContext<'_>, ccx: ContentCx<'_>, view: &mut
|
||||
}
|
||||
// A drag on what the press was on: a corner resizes the selected item, the item itself moves,
|
||||
// with the tools that pick added content up.
|
||||
if resp.drag_started() {
|
||||
if ptr.drag_started {
|
||||
let origin = ptr.origin.unwrap_or(p);
|
||||
match owned.filter(|_| picks_up) {
|
||||
Some((_, Some((left, top)))) => {
|
||||
|
||||
@@ -770,7 +770,7 @@ pub(crate) fn page_input(cx: PageToolContext<'_>, lines: &[pdfcraft_engine::Text
|
||||
PagePress::Mine(Owner::Paragraph { block, part }) => Some((block, part)),
|
||||
_ => None,
|
||||
};
|
||||
if resp.drag_started()
|
||||
if ptr.drag_started
|
||||
&& active.is_none()
|
||||
&& let Some(o) = ptr.origin
|
||||
&& let Some((block, part)) = owner
|
||||
|
||||
@@ -280,7 +280,7 @@ fn crop_input(cx: &ImageCx<'_>, view: &mut DocView, i: usize, image: &pdfcraft_e
|
||||
|
||||
// A drag takes what its press was on ([`crop_hit`]).
|
||||
if gate
|
||||
&& resp.drag_started()
|
||||
&& ptr.drag_started
|
||||
&& let Some(o) = ptr.origin
|
||||
&& let PagePress::Mine(Owner::Picture { part: PicturePart::Crop(part), .. }) = press
|
||||
{
|
||||
@@ -768,7 +768,7 @@ pub(crate) fn image_input(cx: &ImageCx<'_>, images: &[pdfcraft_engine::PageImage
|
||||
// A drag of the selected picture takes what its press was on ([`body_hit`], [`handle_hit`]):
|
||||
// the rotate knob turns it, a grip resizes it, the picture itself moves.
|
||||
if gate
|
||||
&& resp.drag_started()
|
||||
&& ptr.drag_started
|
||||
&& let Some(o) = ptr.origin
|
||||
&& let PagePress::Mine(Owner::Picture { index, part }) = press
|
||||
&& index == i
|
||||
|
||||
@@ -115,10 +115,10 @@ pub(crate) fn page_input(cx: PageToolContext<'_>, free: bool, owner: Option<usiz
|
||||
}
|
||||
return true;
|
||||
}
|
||||
// A drag that nobody holds (a link's hit lets its drag go, `PageRouter::drag_started`) draws a
|
||||
// A drag that nobody holds (a link's hit lets its drag go, `PageRouter::drag_began`) draws a
|
||||
// new link from where its press began on this page. egui may see the drag only once the pointer
|
||||
// is past the page's edge.
|
||||
if resp.drag_started()
|
||||
if ptr.drag_started
|
||||
&& free
|
||||
&& let Some(origin) = ptr.origin.filter(|o| xf.rect.contains(*o))
|
||||
{
|
||||
|
||||
@@ -390,7 +390,7 @@ pub(crate) fn page_input(
|
||||
// A drag starts from where its press began on this page: egui may see it only once the pointer
|
||||
// is past the page's edge.
|
||||
if allowed
|
||||
&& resp.drag_started()
|
||||
&& ptr.drag_started
|
||||
&& (held || free)
|
||||
&& let Some(origin) = ptr.origin.filter(|o| xf.rect.contains(*o))
|
||||
{
|
||||
|
||||
@@ -1438,6 +1438,79 @@ fn a_right_click_on_a_comment_under_a_camera_corner_opens_the_comments_menu() {
|
||||
assert_eq!(h.state().views[0].comments.selected, Some((0, 0)), "the comment is selected for its menu");
|
||||
}
|
||||
|
||||
/// A right-button drag from `a` to `b`, in frames of their own.
|
||||
fn right_drag(h: &mut Harness<'_, PdfCraftApp>, a: egui::Pos2, b: egui::Pos2) {
|
||||
let button = |pos, pressed| egui::Event::PointerButton { pos, button: egui::PointerButton::Secondary, pressed, modifiers: egui::Modifiers::NONE };
|
||||
h.hover_at(a);
|
||||
h.run_steps(1);
|
||||
h.event(button(a, true));
|
||||
h.run_steps(1);
|
||||
for k in 1..=4 {
|
||||
h.hover_at(a + (b - a) * (k as f32 / 4.0));
|
||||
h.run_steps(1);
|
||||
}
|
||||
h.event(button(b, false));
|
||||
h.run_steps(3);
|
||||
}
|
||||
|
||||
// A right-button drag starts nothing a left-button drag does. egui's drag start counts every
|
||||
// button, and each of these started on it.
|
||||
|
||||
/// With the Rectangle tool, a right-button drag draws no rectangle.
|
||||
#[test]
|
||||
fn a_right_button_drag_draws_no_rectangle() {
|
||||
let mut h = open(text_pdf(), "150", false);
|
||||
h.state_mut().quick_tool = QuickTool::Comment(CommentTool::Rectangle);
|
||||
h.run_steps(4);
|
||||
let (a, b) = (at(&h, 60.0, 150.0), at(&h, 200.0, 80.0));
|
||||
right_drag(&mut h, a, b);
|
||||
let s = h.state();
|
||||
assert!(s.session.get(s.views[0].id).unwrap().info.annotations.is_empty(), "no rectangle is drawn");
|
||||
}
|
||||
|
||||
/// With Prepare open, a right-button drag around a field selects it by no selection rectangle.
|
||||
#[test]
|
||||
fn a_right_button_drag_selects_no_field() {
|
||||
let mut h = open(comment_and_checkbox_pdf(), "150", false);
|
||||
assert!(h.state_mut().execute("form.prepare"));
|
||||
h.run_steps(4);
|
||||
let (a, b) = (at(&h, 60.0, 250.0), at(&h, 140.0, 180.0));
|
||||
right_drag(&mut h, a, b);
|
||||
assert!(h.state().views[0].prepare.selected.is_none(), "no field is selected");
|
||||
}
|
||||
|
||||
/// With the Link tool, a right-button drag draws no link.
|
||||
#[test]
|
||||
fn a_right_button_drag_draws_no_link() {
|
||||
let mut h = open(text_pdf(), "150", false);
|
||||
link_tool(&mut h);
|
||||
let (a, b) = (at(&h, 60.0, 150.0), at(&h, 200.0, 80.0));
|
||||
right_drag(&mut h, a, b);
|
||||
assert!(h.state().link_draft.is_none() && h.state().dialog != Some(Dialog::LinkProps), "no link is drawn");
|
||||
}
|
||||
|
||||
/// With the camera, a right-button drag on another page leaves the quadrilateral where it is.
|
||||
#[test]
|
||||
fn a_right_button_drag_leaves_the_camera_quadrilateral() {
|
||||
let mut h = open(two_pages_pdf(), "100", false);
|
||||
h.state_mut().quick_tool = QuickTool::EnhanceCamera;
|
||||
h.run_steps(4);
|
||||
assert_eq!(h.state().views[0].camera.as_ref().map(|q| q.page), Some(0));
|
||||
let two = h.state().views[0].page_screen_rect(1).expect("page 2 on screen");
|
||||
right_drag(&mut h, two.min + egui::vec2(40.0, 40.0), two.min + egui::vec2(120.0, 100.0));
|
||||
assert_eq!(h.state().views[0].camera.as_ref().map(|q| q.page), Some(0), "the quadrilateral stays on page 1");
|
||||
}
|
||||
|
||||
/// With the Select tool, a right-button drag across text selects none.
|
||||
#[test]
|
||||
fn a_right_button_drag_selects_no_text() {
|
||||
let mut h = open(text_pdf(), "150", false);
|
||||
let (a, b) = (at(&h, 45.0, 303.0), at(&h, 200.0, 303.0));
|
||||
assert_eq!(wait_for_cursor(&mut h, a, |c| c == egui::CursorIcon::Text), egui::CursorIcon::Text, "the page's text has arrived");
|
||||
right_drag(&mut h, a, b);
|
||||
assert!(h.state().views[0].selected_text().is_none(), "no text is selected");
|
||||
}
|
||||
|
||||
/// A 300×400 page with a web link along its left edge, x 0..100, y 300..350.
|
||||
fn edge_link_pdf() -> Vec<u8> {
|
||||
pdf(&[
|
||||
|
||||
Reference in new issue
Block a user