From b3509dc6299d0739aa4bd47a0cbee6e776811b27 Mon Sep 17 00:00:00 2001 From: alehel Date: Tue, 3 Nov 2026 08:13:32 +0100 Subject: [PATCH] fix(ui): a comment's move, resize or reshape ends once its button is up A comment's gesture ended only on egui's `drag_stopped()`. When the pages missed its release (the button let go where they didn't see it), the gesture stayed live: it kept the page's frames from Fill & Sign and the shape tools, and the next drag that stopped on the page committed it, taking the comment there. It now ends, as the other page gestures do, on `drag_stopped()` or once the primary button is up, where the pointer is. Two tests planted a live move with the button up to check that a live move keeps its page's presses (`main`'s `active_comment_gesture_keeps_highlighted_link_click` and 4a's `a_fill_and_sign_click_places_nothing_while_a_mark_is_moved`). That state now ends at once, so both hold the press through the planted move instead: the intent stands, and each still fails with the live-move guard taken out. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/tools/comments/input.rs | 4 ++- crates/ui-egui/tests/it/document_links.rs | 10 ++++++- crates/ui-egui/tests/it/pointer.rs | 33 ++++++++++++++++++++-- 3 files changed, 42 insertions(+), 5 deletions(-) diff --git a/crates/ui-egui/src/tools/comments/input.rs b/crates/ui-egui/src/tools/comments/input.rs index ac3c2a6878..11f6e0549b 100644 --- a/crates/ui-egui/src/tools/comments/input.rs +++ b/crates/ui-egui/src/tools/comments/input.rs @@ -229,7 +229,9 @@ fn select_input( if matches!(cv.gesture, Some(Gesture::Move { page, .. } | Gesture::Resize { page, .. } | Gesture::Reshape { page, .. }) if page == cx.page) { consumed = true; } - if resp.drag_stopped() + // A move, resize or reshape ends where its drag stops, or once the button is up when the pages + // missed its release. + if (resp.drag_stopped() || !cx.ptr.primary_down) && let Some(p) = pointer { match cv.gesture.clone() { diff --git a/crates/ui-egui/tests/it/document_links.rs b/crates/ui-egui/tests/it/document_links.rs index 8405446df2..eb2bfa7afe 100644 --- a/crates/ui-egui/tests/it/document_links.rs +++ b/crates/ui-egui/tests/it/document_links.rs @@ -717,14 +717,22 @@ fn form_widget_keeps_priority_over_highlighted_link() { assert_eq!(h.state().last_opened_url, None); } +/// A comment's gesture keeps the page's input until its button is up: a press held through it and +/// released on a highlighted link follows no link. (A gesture left over once the button is up +/// ends: `a_comment_move_whose_release_the_pages_missed_ends_once_the_button_is_up`.) #[test] fn active_comment_gesture_keeps_highlighted_link_click() { let _gpu = gpu(); let mut h = overlapping_annotation_harness(overlapping_annotation_fixture("Highlight", false)); let r = h.state().views[0].page_screen_rect(0).unwrap(); let at = egui::pos2(r.min.x + r.width() * 0.5, r.min.y + r.height() * (1.0 - 325.0 / 400.0)); + h.hover_at(at); + h.run_steps(2); + h.drag_at(at); + h.run_steps(1); h.state_mut().views[0].comments.gesture = Some(pdfcraft_ui_egui::comments::Gesture::Move { page: 0, index: 1, from: at }); - click_at(&mut h, at); + h.drop_at(at); + h.run_steps(4); assert_eq!(h.state().pending_link, None); assert_eq!(h.state().last_opened_url, None); } diff --git a/crates/ui-egui/tests/it/pointer.rs b/crates/ui-egui/tests/it/pointer.rs index b163f5aee8..93ad9fbf80 100644 --- a/crates/ui-egui/tests/it/pointer.rs +++ b/crates/ui-egui/tests/it/pointer.rs @@ -1569,6 +1569,27 @@ fn a_right_click_selects_its_comment_though_a_gesture_was_left_over() { assert_eq!(h.state().views[0].comments.selected, Some((0, 1)), "the note under the right-click is selected"); } +/// A comment's move ends once its button is up, though the pages missed its release: a drag elsewhere +/// later moves nothing. The move waited for the next drag's end on the page, and took the comment +/// there. +#[test] +fn a_comment_move_whose_release_the_pages_missed_ends_once_the_button_is_up() { + let mut h = open(stacked_comments_pdf(), "150", false); + let rect = |h: &Harness<'_, PdfCraftApp>| { + let s = h.state(); + s.session.get(s.views[0].id).unwrap().info.annotations.iter().find(|a| a.index == 0).map(|a| a.rect) + }; + let before = rect(&h); + let bottom = at(&h, 60.0, 160.0); + h.state_mut().views[0].comments.gesture = Some(pdfcraft_ui_egui::comments::Gesture::Move { page: 0, index: 0, from: bottom }); + h.hover_at(bottom); + h.run_steps(2); + assert!(h.state().views[0].comments.gesture.is_none(), "the move is over"); + let blank = at(&h, 250.0, 60.0); + mouse_drag(&mut h, blank, egui::vec2(30.0, 0.0)); + assert_eq!(rect(&h), before, "nothing moved the comment"); +} + /// A click that began off the page (in the gutter beside it) is no page's: the selected comment /// stays selected, as the Link tool's and Prepare's selections do. It used to unselect it. #[test] @@ -1763,8 +1784,9 @@ fn a_note_tool_click_on_added_text_starts_no_note() { 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. +/// With Fill & Sign, a mark being moved on the page keeps the page's presses, as before: a press held +/// through its move and released places no new mark. (A move left over once the button is up ends: +/// `a_comment_move_whose_release_the_pages_missed_ends_once_the_button_is_up`.) #[test] fn a_fill_and_sign_click_places_nothing_while_a_mark_is_moved() { let mut h = open(comment_on_text_pdf(), "150", false); @@ -1776,8 +1798,13 @@ fn a_fill_and_sign_click_places_nothing_while_a_mark_is_moved() { }; let before = marks(&h); let blank = at(&h, 150.0, 100.0); + h.hover_at(blank); + h.run_steps(1); + h.drag_at(blank); + h.run_steps(1); 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); + h.drop_at(blank); + h.run_steps(4); assert_eq!(marks(&h), before, "no mark is placed"); }