From 84fe5e54b31ad62ea86feb360aaeaecfbda23b37 Mon Sep 17 00:00:00 2001 From: alehel Date: Mon, 2 Nov 2026 10:28:48 +0100 Subject: [PATCH] refactor(ui): tell comments and Fill & Sign whether the press is theirs Comments ran last in the page view's chain of `||`, so only when no earlier module had taken the frame, and asked the router themselves (`grabs`, `fill_grabs`, `draw_grabs`), with a hit test of their own for hover. A link, a field or added text taking the frame first hid a comment's gesture end, its Escape and the click that unselects it. Comments now run on every frame and are told the page's `PagePress`: - a press on a comment (its body, a handle or a point) goes to `select_input` with the part, with the Select tool, a shape tool (the selected shape's handles) or Fill & Sign (its marks); - the drawing tools act only on a press nobody holds (`Gestures::free`); - a click on anything but a comment leaves comments unselected. `grabs`, `fill_grabs`, `draw_grabs` and `cancelled_drag` (with the raw button read in `keys` it needed) are gone; `grabs_points` is `reaches_points`. Fill & Sign leaves the press to a mark when the press is someone's, or, with no button down, when a mark is the top hit under the pointer. The page's hover is that top hit (`PageInput.hover`), and the comments' hover outline follows it, so it no longer shows under a field or added text that would take a press. Behaviour change (the user's ruling, 2026-10-31): a click on a form field, or on added text, leaves the selected comment unselected, as a click on a link or the page already did. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/canvas.rs | 3 +- crates/ui-egui/src/canvas/page_input.rs | 37 ++++-- .../ui-egui/src/canvas/page_pointer/tests.rs | 7 +- crates/ui-egui/src/canvas/signature_drag.rs | 3 +- crates/ui-egui/src/tools/comments.rs | 2 +- crates/ui-egui/src/tools/comments/geometry.rs | 4 +- crates/ui-egui/src/tools/comments/input.rs | 115 ++++++++---------- crates/ui-egui/src/tools/comments/paint.rs | 28 +++-- crates/ui-egui/src/tools/comments/state.rs | 2 - crates/ui-egui/src/tools/comments/tests.rs | 36 ++++++ crates/ui-egui/tests/it/pointer.rs | 26 ++++ 11 files changed, 164 insertions(+), 99 deletions(-) diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 428f71c186..861833d25a 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -1496,7 +1496,8 @@ fn draw_page(ui: &mut egui::Ui, view: &mut DocView, frame: &mut PageFrame<'_>, i let slot = PageSlot { i, r, xf }; // A cancelled press takes nothing until the button is up. - let input = if view.press_cancelled { PageInput { consumed: true, area_tool: false } } else { page_tool_input(ui, view, frame, &slot) }; + let input = + if view.press_cancelled { PageInput { consumed: true, area_tool: false, hover: None } } else { page_tool_input(ui, view, frame, &slot) }; let painter = ui.painter(); let pcx = comments::PageCx { page: i, xf: &xf, ptr, info, tool, prefs, allowed, hidden: comments_hidden, redact_mark }; let preview_target = (matches!(tool, QuickTool::Select | QuickTool::Fill(_)) && !comments_hidden).then_some(view.comments.selected).flatten(); diff --git a/crates/ui-egui/src/canvas/page_input.rs b/crates/ui-egui/src/canvas/page_input.rs index 00edfe54cc..05a302926d 100644 --- a/crates/ui-egui/src/canvas/page_input.rs +++ b/crates/ui-egui/src/canvas/page_input.rs @@ -16,6 +16,8 @@ pub(super) struct PageInput { pub(super) consumed: bool, /// Redact or Highlight draws boxes off text. pub(super) area_tool: bool, + /// What a press under the pointer would take (the top hit there), for hover painting. + pub(super) hover: Option, } /// Common geometry and pointer state for a tool acting on one page. @@ -153,10 +155,13 @@ pub(super) fn page_tool_input(ui: &mut egui::Ui, view: &mut DocView, frame: &mut frame.out.stamp_placed = true; } } - // Over a mark already placed, the Fill & Sign tool picks it up (comments, below). + // A mark already placed takes its own press (comments, below), and with no button down the + // 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 && pointer.and_then(|p| top(hits.at(view, p), tool)).is_some()); if let QuickTool::Fill(ft) = tool && allowed - && !comments::fill_grabs(&pcx, view) + && !over_mark { match crate::fill_sign::page_input( cx, @@ -283,17 +288,23 @@ pub(super) fn page_tool_input(ui: &mut egui::Ui, view: &mut DocView, frame: &mut { view.links.context = Some((i, index)); } - let consumed = on_objects || on_edit_text || on_link || on_content || boxing || on_field || comments::page_input(ui, resp, &pcx, view); - // The cursor and the tip from what lies under the pointer, after the tools claimed theirs (the - // last claim on a layer stands). - if frame.env.unobstructed - && let Some(p) = pointer - { - for h in hits.at(view, p) { - view.cursor.claim_with_tip(h.layer, h.icon, h.tip); - } + // Comments, every frame: their own presses, a commenting tool's, and a click elsewhere that + // leaves them unselected. + let on_comment = comments::page_input(ui, resp, &pcx, press, view); + let consumed = on_objects || on_edit_text || on_link || on_content || boxing || on_field || on_comment; + let hover = pointer.filter(|_| frame.env.unobstructed).and_then(|p| claim_under(&hits, view, p)); + PageInput { consumed, area_tool, hover } +} + +/// The cursor and the tip from what lies under the pointer at `p`, after the tools claimed theirs +/// (the last claim on a layer stands). Returns the top of it, what a press there would take, for +/// hover painting. +fn claim_under(hits: &PageHits<'_>, view: &mut DocView, p: Pos2) -> Option { + let under = hits.at(view, p); + for h in &under { + view.cursor.claim_with_tip(h.layer, h.icon, h.tip.clone()); } - PageInput { consumed, area_tool } + top(under, hits.tool) } /// The link `owner` is, by its `/Annots` index. @@ -701,7 +712,7 @@ pub(super) fn page_overlays(ui: &mut egui::Ui, view: &mut DocView, frame: &mut P if input.area_tool { crate::redact_ui::paint(painter, i, view, pointer); } - comments::paint_page(ui, painter, &pcx, view); + comments::paint_page(ui, painter, &pcx, view, input.hover.as_ref()); if editing_content { crate::content_ui::paint_page(painter, &xf, i, info, added, view, pointer); } diff --git a/crates/ui-egui/src/canvas/page_pointer/tests.rs b/crates/ui-egui/src/canvas/page_pointer/tests.rs index 2e79b7481e..865f5051b0 100644 --- a/crates/ui-egui/src/canvas/page_pointer/tests.rs +++ b/crates/ui-egui/src/canvas/page_pointer/tests.rs @@ -217,14 +217,13 @@ const RAW_READS: [&str; 21] = [ /// Functions in the page modules that are not page input and read egui's pointer state /// themselves: (file, function, why). -const NOT_PAGE_INPUT: [(&str, &str, &str); 11] = [ +const NOT_PAGE_INPUT: [(&str, &str, &str); 10] = [ ("canvas/area.rs", "zoom_anchor", "a pinch zooms the whole view, and is read before the pages' widget exists"), ("canvas/area.rs", "drawing_edge_pointer", "scrolls the view at its edges, read before the pages' widget exists"), ("canvas/area.rs", "glide_interrupted", "a press anywhere stops the touchpad's glide"), ("canvas/area.rs", "presentation_click", "full screen's click to turn the page, read before the pages' widget exists"), ("canvas/bars.rs", "page_size_hint", "the page size hint sits in the view's corner, not on a page"), ("canvas/organize.rs", "organize_grid", "Organize Pages shows thumbnails, not pages"), - ("tools/comments/input.rs", "keys", "keyboard handling after the pages: a drag cancelled with Escape ends once no button is down"), ("tools/forms_ui.rs", "overlay", "the field editor is an Area of its own over the page"), ("tools/content_ui.rs", "editor", "the added-text editor is an Area of its own over the page"), ("tools/edit_text_ui.rs", "overlay", "the paragraph editor is an Area of its own over the page"), @@ -416,7 +415,7 @@ fn hit_test_calls(line: &str) -> Vec { /// The functions that call a hit test: (file, function, why). Only `PageHits::at` asks at the /// press point; the others ask where the pointer is. -const HIT_TESTERS: [(&str, &str, &str); 4] = [ +const HIT_TESTERS: [(&str, &str, &str); 3] = [ ( "canvas/page_input.rs", "hits", @@ -424,7 +423,6 @@ const HIT_TESTERS: [(&str, &str, &str); 4] = [ ), ("canvas/page_input.rs", "page_overlays", "a link's click counts only released over the link its press was on"), ("tools/forms_ui.rs", "page_input", "the field's tooltip under the pointer, and a click counts only released over the widget its press was on"), - ("tools/comments/input.rs", "grabs", "with no button down, whether a comment would take a press under the pointer, for hover painting"), ]; /// What ignores the press the pages hold until the button is up. @@ -493,7 +491,6 @@ fn page_modules_act_on_a_press_only_through_the_router() { ("tools/edit_text_ui.rs", "page_input"), ("tools/edit_text_ui/image.rs", "image_input"), ("tools/object_ui.rs", "input"), - ("tools/comments/input.rs", "select_input"), ] { let source = std::fs::read_to_string(src.join(file)).expect("page input module"); let body = source.lines().skip_while(|line| declares(line) != Some(function)).take_while(|line| *line != "}").collect::>().join("\n"); diff --git a/crates/ui-egui/src/canvas/signature_drag.rs b/crates/ui-egui/src/canvas/signature_drag.rs index c706b8a113..9f7a071ac7 100644 --- a/crates/ui-egui/src/canvas/signature_drag.rs +++ b/crates/ui-egui/src/canvas/signature_drag.rs @@ -402,8 +402,7 @@ mod tests { let doc = session.get(id).expect("open"); let index = doc.info.annotations.iter().find(|a| a.subtype == "Sound").expect("listed").index; let ctx = egui::Context::default(); - let mut cv = CommentView::default(); - cv.gesture = Some(Gesture::Move { page: 0, index, from: pos2(0.0, 0.0) }); + let mut cv = CommentView { gesture: Some(Gesture::Move { page: 0, index, from: pos2(0.0, 0.0) }), ..CommentView::default() }; let mut drag = SignatureDrag::default(); let key = Some((doc.edit_generation(), 0, index)); drag.prepare_move(&ctx, doc, &cv, None, false, 1.0); diff --git a/crates/ui-egui/src/tools/comments.rs b/crates/ui-egui/src/tools/comments.rs index d0c46d0453..83ee620ae1 100644 --- a/crates/ui-egui/src/tools/comments.rs +++ b/crates/ui-egui/src/tools/comments.rs @@ -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::{context, context_hit, describe, fill_grabs, hit, keys, nudge_keys, page_after_text, page_input}; +pub(crate) use input::{context, context_hit, describe, hit, keys, 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}; diff --git a/crates/ui-egui/src/tools/comments/geometry.rs b/crates/ui-egui/src/tools/comments/geometry.rs index 051cf09dba..ee67b92bb0 100644 --- a/crates/ui-egui/src/tools/comments/geometry.rs +++ b/crates/ui-egui/src/tools/comments/geometry.rs @@ -20,10 +20,10 @@ pub(super) fn reshapable(a: &Annotation) -> bool { && !a.intent.as_deref().is_some_and(|i| i.ends_with("Dimension")) } -/// Whether `tool` picks up a line's end and corner handles: the Select and Fill & Sign tools. +/// Whether `tool` reaches a line's end and corner handles: the Select and Fill & Sign tools. /// The drawing tools draw from where they are pressed, so a new line starts at the end of the /// one just drawn rather than dragging it. -pub(super) fn grabs_points(tool: QuickTool) -> bool { +pub(super) fn reaches_points(tool: QuickTool) -> bool { !matches!(tool, QuickTool::Comment(t) if t.draws() || t.clicks_points()) } diff --git a/crates/ui-egui/src/tools/comments/input.rs b/crates/ui-egui/src/tools/comments/input.rs index a12d971f5c..7ada11bc7f 100644 --- a/crates/ui-egui/src/tools/comments/input.rs +++ b/crates/ui-egui/src/tools/comments/input.rs @@ -1,43 +1,47 @@ use super::geometry::{ - CARET_SIZE, HANDLES, clamp_to, corner_ratio, drawn_box, grabs_points, handle_pos, is_markup, point_handle, reshapable, reshaped, resizable, + CARET_SIZE, HANDLES, clamp_to, corner_ratio, drawn_box, handle_pos, is_markup, point_handle, reaches_points, reshapable, reshaped, resizable, resized, snapped_caret, }; use super::paint::{drawn_shape, edit_anchor, erase, new_comment}; use super::state::{Composer, ComposerKind, Gesture, PageCx}; use super::{Annotation, CommentPrefs, CommentTool, DocInfo, DocView, Edit, Pos2, QuickTool, Rect, Shape, vec2}; use crate::page_cursor::{Layer, Subject}; -use crate::page_router::{CommentPart, Hit, Owner}; +use crate::page_router::{CommentPart, Hit, Owner, PagePress}; -/// Comment input on one page, before text selection runs. Returns `true` when the pointer -/// gesture belongs to commenting (text selection must ignore it). -pub(crate) fn page_input(ui: &egui::Ui, resp: &egui::Response, cx: &PageCx<'_>, view: &mut DocView) -> bool { +/// Comment input on one page, before text selection runs. `press` is what this page has to do with +/// the press the pages hold: a press on a comment ([`hit`]) is that comment's, with any tool, and +/// one nobody holds is a commenting tool's. Returns `true` when the pointer gesture belongs to +/// commenting (text selection must ignore it). +pub(crate) fn page_input(ui: &egui::Ui, resp: &egui::Response, cx: &PageCx<'_>, press: PagePress, view: &mut DocView) -> bool { let (pointer, origin) = (cx.ptr.pos, cx.ptr.origin); let page_rect = cx.xf.rect; - let pressed_here = origin.is_some_and(|o| page_rect.contains(o)); + let free = press == PagePress::Free; + let pressed_here = free && origin.is_some_and(|o| page_rect.contains(o)); let over_page = pointer.is_some_and(|p| page_rect.contains(p)); - let fill_grab = fill_grabs(cx, view); - let draw_grab = draw_grabs(cx, view); + let owned = match press { + PagePress::Mine(Owner::Comment { index, part }) => Some((index, part)), + _ => None, + }; let caret_text = if cx.tool == QuickTool::Comment(CommentTool::Caret) { view.page_text(cx.page) } else { None }; let cv = &mut view.comments; if cv.gesture.is_some() && ui.input_mut(|i| i.consume_key(egui::Modifiers::NONE, egui::Key::Escape)) { cv.gesture = None; - cv.cancelled_drag = true; return true; } - if cv.cancelled_drag { - return true; - } - let gestures = Gestures { ui, resp, cx, pointer, origin, page_rect, pressed_here, over_page }; + // 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 gestures = Gestures { ui, resp, cx, pointer, origin, page_rect, pressed_here, over_page, free }; match cx.tool { - QuickTool::Comment(_) if draw_grab => select_input(ui, resp, cx, view, pointer, origin), + // The selected shape's handles, and Fill & Sign's marks, take their own presses. + QuickTool::Comment(_) | QuickTool::Fill(_) if owned.is_some() || moving => select_input(ui, resp, cx, view, owned, pointer, origin), QuickTool::Comment(tool) if tool.draws() => gestures.draw_shape(view, tool), QuickTool::Comment(tool) if tool.clicks_points() => gestures.click_points(view, tool), QuickTool::Comment(CommentTool::Callout) => gestures.callout(view), QuickTool::Comment(CommentTool::Attach) => gestures.attach(view), QuickTool::Comment(CommentTool::Caret) => gestures.caret(view, caret_text.as_deref()), QuickTool::Comment(tool @ (CommentTool::Note | CommentTool::TextBox)) => gestures.note_or_box(view, tool), - QuickTool::Select => select_input(ui, resp, cx, view, pointer, origin), - QuickTool::Fill(_) if fill_grab => select_input(ui, resp, cx, view, pointer, origin), + QuickTool::Select => select_input(ui, resp, cx, view, owned, pointer, origin), _ => false, } } @@ -68,7 +72,7 @@ pub(crate) fn hit(cx: &PageCx<'_>, view: &DocView, p: Pos2, on_link: bool) -> Op }; let bodies = matches!(cx.tool, QuickTool::Select | QuickTool::Fill(_)); if let Some(a) = view.comments.selected.filter(|(page, _)| *page == cx.page).and_then(|(_, i)| cx.get(i)).filter(|a| pick(a) && cx.allowed) { - if grabs_points(cx.tool) + if reaches_points(cx.tool) && reshapable(a) && let Some(point) = point_handle(cx, a, p) { @@ -119,32 +123,6 @@ pub(crate) fn context(cx: &PageCx<'_>, p: Pos2, index: Option, view: &mut view.comments.selected = index.map(|i| (cx.page, i)); } -/// Whether the comments take the press on this page (or, with no button down, would take one -/// under the pointer), so the tools beneath leave it: a live move, resize or reshape, or a press -/// held by a comment that keeps its drag. -fn grabs(cx: &PageCx<'_>, view: &DocView) -> bool { - if matches!(view.comments.gesture, Some(Gesture::Move { page, .. } | Gesture::Resize { page, .. } | Gesture::Reshape { page, .. }) if page == cx.page) - { - return true; - } - if cx.ptr.any_down { - return matches!(view.router.owner(cx.page), Some(Owner::Comment { .. })) && view.router.keeps_drag(); - } - // Only the Select tool lets a link through a highlight, and it doesn't come here. - cx.ptr.pos.and_then(|p| hit(cx, view, p, false)).is_some_and(|h| !h.declines_drag) -} - -/// With a Fill & Sign tool, a Fill & Sign mark already on the page is picked up as with the Select -/// tool ([`hit`]), so the tool does not place a mark there. -pub(crate) fn fill_grabs(cx: &PageCx<'_>, view: &DocView) -> bool { - matches!(cx.tool, QuickTool::Fill(_)) && grabs(cx, view) -} - -/// With a shape tool, the selected comment's handles are picked up ([`hit`]). -fn draw_grabs(cx: &PageCx<'_>, view: &DocView) -> bool { - matches!(cx.tool, QuickTool::Comment(_)) && grabs(cx, view) -} - /// The tip for the comment under the pointer, with any tool, beside that tool's own cursor: Acrobat /// shows a comment's tooltip whatever tool is active. Not with Fill & Sign, whose placement stays /// clear; not while drawing freehand, where it would cover the next stroke (#429); and not while a @@ -180,13 +158,19 @@ pub(super) fn hover_tip(a: &Annotation, prefs: &CommentPrefs) -> Option Some(if body.is_empty() { who } else { format!("{who}\n{body}") }) } -/// The Select tool: select, move, resize, edit, delete, on the comment the press was on (its -/// owner, from [`hit`]). The cursor comes from [`hit`] too. -fn select_input(ui: &egui::Ui, resp: &egui::Response, cx: &PageCx<'_>, view: &mut DocView, pointer: Option, origin: Option) -> bool { - let owner = match view.router.owner(cx.page) { - Some(Owner::Comment { index, part }) => cx.get(index).map(|a| (a, part)), - _ => None, - }; +/// The Select tool: select, move, resize, edit, delete, on the comment the press is on and the +/// part of it (`owned`, from [`hit`]). The cursor comes from [`hit`] too. A click on anything else +/// (a link, a field, added text, the page) leaves comments unselected. +fn select_input( + ui: &egui::Ui, + resp: &egui::Response, + cx: &PageCx<'_>, + view: &mut DocView, + owned: Option<(usize, CommentPart)>, + pointer: Option, + origin: Option, +) -> bool { + let owner = owned.and_then(|(index, part)| cx.get(index).map(|a| (a, part))); let cv = &mut view.comments; let mut consumed = false; if resp.drag_started() @@ -262,8 +246,8 @@ fn select_input(ui: &egui::Ui, resp: &egui::Response, cx: &PageCx<'_>, view: &mu // (egui clears the press origin on release, so clicks are located by the pointer.) let on_page = pointer.is_some_and(|p| cx.xf.rect.contains(p)); if resp.clicked() && on_page { - // A click elsewhere (a link beneath text highlighting included: it follows the link) - // leaves comments unselected. + // A click elsewhere (a link beneath text highlighting included: it follows the link; a + // form field; added text) leaves comments unselected. match owner { Some((a, _)) => { cv.selected = Some((cx.page, a.index)); @@ -326,13 +310,21 @@ struct Gestures<'a> { pointer: Option, origin: Option, page_rect: Rect, + /// The press began on this page, and nobody holds it: the tool's. pressed_here: bool, over_page: bool, + /// Nobody holds the press, and it began on this page, or no button is down. + free: bool, } impl Gestures<'_> { + /// The tool's own click: one nobody holds. + fn clicked(&self) -> bool { + self.free && self.resp.clicked() + } + fn draw_shape(&self, view: &mut DocView, tool: CommentTool) -> bool { - let Self { ui, resp, cx, pointer, origin, page_rect, pressed_here, over_page } = *self; + let Self { ui, resp, cx, pointer, origin, page_rect, pressed_here, over_page, .. } = *self; let cv = &mut view.comments; if !cx.allowed { return false; @@ -380,7 +372,7 @@ impl Gestures<'_> { // it as a one-point ink stroke (a dot). The appearance uses round line caps, so a // single PDF point is visible and remains editable like any other drawing. if matches!(tool, CommentTool::Ink | CommentTool::FreehandHighlight) - && resp.clicked() + && self.clicked() && over_page && let Some(p) = pointer && let Some(shape) = drawn_shape(tool, &[cx.to_user(p)]) @@ -412,7 +404,7 @@ impl Gestures<'_> { cv.undo_point(); return true; } - if resp.clicked() + if self.clicked() && over_page && let Some(p) = pointer { @@ -492,7 +484,7 @@ impl Gestures<'_> { } fn attach(&self, view: &mut DocView) -> bool { - let Self { resp, cx, pointer, over_page, .. } = *self; + let Self { cx, pointer, over_page, .. } = *self; let cv = &mut view.comments; if !cx.allowed { return false; @@ -500,7 +492,7 @@ impl Gestures<'_> { if over_page { view.cursor.claim(Layer::ToolArea, egui::CursorIcon::Crosshair); } - if resp.clicked() + if self.clicked() && over_page && let Some(p) = pointer { @@ -511,7 +503,7 @@ impl Gestures<'_> { } fn caret(&self, view: &mut DocView, caret_text: Option<&pdfcraft_render::PageText>) -> bool { - let Self { resp, cx, pointer, over_page, .. } = *self; + let Self { cx, pointer, over_page, .. } = *self; let cv = &mut view.comments; if !cx.allowed { return false; @@ -519,7 +511,7 @@ impl Gestures<'_> { if over_page { view.cursor.claim(Layer::ToolArea, egui::CursorIcon::Text); } - if resp.clicked() + if self.clicked() && over_page && let Some(p) = pointer { @@ -573,7 +565,7 @@ impl Gestures<'_> { } } } - if resp.clicked() + if self.clicked() && over_page && let Some(p) = pointer { @@ -587,9 +579,6 @@ impl Gestures<'_> { /// Delete / Escape handling for comments (only while no text field has focus). pub(crate) fn keys(ctx: &egui::Context, view: &mut DocView, tool: &mut QuickTool, allowed: bool) { - if !ctx.input(|i| i.pointer.any_down()) { - view.comments.cancelled_drag = false; - } if ctx.egui_wants_keyboard_input() { return; } diff --git a/crates/ui-egui/src/tools/comments/paint.rs b/crates/ui-egui/src/tools/comments/paint.rs index 59b5ca773a..e784c332ae 100644 --- a/crates/ui-egui/src/tools/comments/paint.rs +++ b/crates/ui-egui/src/tools/comments/paint.rs @@ -1,12 +1,24 @@ -use super::geometry::{HANDLES, grabs_points, handle_pos, is_markup, reshapable, reshaped, resizable}; -use super::state::{Gesture, PageCx, SETTLE_SECS}; +use super::geometry::{HANDLES, handle_pos, is_markup, reaches_points, reshapable, reshaped, resizable}; +use super::state::{CommentView, Gesture, PageCx, SETTLE_SECS}; use super::{ Annotation, Color32, CommentTool, CornerRadius, DocInfo, DocView, Edit, LineEnding, NewAnnotation, Pos2, QuickTool, Rect, SELECT_BLUE, Shape, Stroke, color32, vec2, }; +use crate::page_router::{CommentPart, Hit, Owner}; -/// Paint comment selection, hover and gesture previews on a page. -pub(crate) fn paint_page(ui: &egui::Ui, painter: &egui::Painter, cx: &PageCx<'_>, view: &DocView) { +/// The comment outlined under the pointer: the one a press there would take (`hover`, the top hit), +/// with the Select tool or, over its own marks, Fill & Sign. Not the selected comment, nor while a +/// comment is moved, resized or drawn, nor one under a form field or added text that would take +/// the press. +pub(super) fn hover_outline<'c>(cx: &'c PageCx<'_>, cv: &CommentView, hover: Option<&Hit>) -> Option<&'c Annotation> { + let Some(Owner::Comment { index, part: CommentPart::Body }) = hover.map(|h| h.owner) else { return None }; + let tool_outlines = matches!(cx.tool, QuickTool::Select | QuickTool::Fill(_)); + (cv.gesture.is_none() && tool_outlines && cv.selected != Some((cx.page, index))).then(|| cx.get(index)).flatten() +} + +/// Paint comment selection, hover and gesture previews on a page. `hover` is the top hit under the +/// pointer. +pub(crate) fn paint_page(ui: &egui::Ui, painter: &egui::Painter, cx: &PageCx<'_>, view: &DocView, hover: Option<&Hit>) { let cv = &view.comments; let pointer = cx.ptr.pos; // A redaction mark shows its fill while the pointer is over it, as Acrobat's does, so the @@ -21,11 +33,7 @@ pub(crate) fn paint_page(ui: &egui::Ui, painter: &egui::Painter, cx: &PageCx<'_> painter.rect_filled(rect, CornerRadius::ZERO, fill); } } - if cv.gesture.is_none() - && let Some(a) = pointer.and_then(|p| cx.hit(p)) - && (cx.tool == QuickTool::Select || matches!(cx.tool, QuickTool::Fill(_)) && a.fill_sign) - && cv.selected != Some((cx.page, a.index)) - { + if let Some(a) = hover_outline(cx, cv, hover) { for r in cx.screen_rects(a) { painter.rect_stroke(r.expand(2.0), CornerRadius::same(2), Stroke::new(1.0, SELECT_BLUE.gamma_multiply(0.7)), egui::StrokeKind::Outside); } @@ -45,7 +53,7 @@ pub(crate) fn paint_page(ui: &egui::Ui, painter: &egui::Painter, cx: &PageCx<'_> return paint_gesture(painter, cx, view); } let shift = ui.input(|i| i.modifiers.shift); - if cx.allowed && grabs_points(cx.tool) && reshapable(a) { + if cx.allowed && reaches_points(cx.tool) && reshapable(a) { return paint_points(painter, cx, view, a, pointer, shift); } let r = cx.adjusted_rect(a, cv.gesture.as_ref(), pointer, shift, view.pending_edit.as_ref()).expand(2.0); diff --git a/crates/ui-egui/src/tools/comments/state.rs b/crates/ui-egui/src/tools/comments/state.rs index 6cac912210..07452f5b49 100644 --- a/crates/ui-egui/src/tools/comments/state.rs +++ b/crates/ui-egui/src/tools/comments/state.rs @@ -133,8 +133,6 @@ pub struct CommentView { /// Drawings just released: shown from their points until the page raster that includes the /// committed annotation arrives, so the stroke never vanishes in between (#1858). pub(crate) settling: Vec, - /// Escape ends egui's drag too; ignore its synthetic release until the mouse is up. - pub(super) cancelled_drag: bool, pub composer: Option, /// Reply being typed under the selected card. pub reply: String, diff --git a/crates/ui-egui/src/tools/comments/tests.rs b/crates/ui-egui/src/tools/comments/tests.rs index 6110e8a6fc..0f9edd6c22 100644 --- a/crates/ui-egui/src/tools/comments/tests.rs +++ b/crates/ui-egui/src/tools/comments/tests.rs @@ -499,3 +499,39 @@ fn text_boxes_fit_their_text() { assert_eq!(long[2], 300.0); assert!(long[3] - long[1] > 50.0); } + +/// The hover outline goes round the comment a press under the pointer would take (the top hit +/// there): not a highlight under a form field, which takes the press. +#[test] +fn the_hover_outline_goes_round_what_a_press_would_take() { + use super::paint::hover_outline; + use crate::page_cursor::Layer; + use crate::page_router::{CommentPart, Hit, Owner}; + let page = PageInfo { width: 200.0, height: 200.0, label: String::new(), crop: [0.0, 0.0, 200.0, 200.0], rotation: 0 }; + let xf = PageXform { rect: Rect::from_min_size(Pos2::ZERO, vec2(200.0, 200.0)), rot: 0, pw: 200.0, ph: 200.0 }; + let highlight = Annotation { rect: [50.0, 50.0, 150.0, 150.0], ..annotation("Highlight", None, false) }; + let info = DocInfo { pages: vec![page], annotations: vec![highlight], ..Default::default() }; + let prefs = CommentPrefs::default(); + // The pointer is over the highlight. + let ptr = PagePointer { pos: Some(pos2(100.0, 100.0)), ..PagePointer::default() }; + let cx = PageCx { + page: 0, + xf: &xf, + ptr: &ptr, + info: &info, + tool: QuickTool::Select, + prefs: &prefs, + allowed: true, + hidden: false, + redact_mark: ([0.0; 3], 1.0), + }; + let on = |owner, layer| Hit::new(owner, layer, egui::CursorIcon::PointingHand); + let comment = on(Owner::Comment { index: 0, part: CommentPart::Body }, Layer::Comment); + let field = on(Owner::Field { field: 0, widget: 0 }, Layer::Field); + let cv = CommentView::default(); + assert!(hover_outline(&cx, &cv, Some(&comment)).is_some(), "the highlight a press there selects"); + assert!(hover_outline(&cx, &cv, Some(&field)).is_none(), "not under a field that takes the press"); + assert!(hover_outline(&cx, &cv, None).is_none(), "nor where nothing would take it"); + let selected = CommentView { selected: Some((0, 0)), ..CommentView::default() }; + assert!(hover_outline(&cx, &selected, Some(&comment)).is_none(), "the selected comment has its own frame"); +} diff --git a/crates/ui-egui/tests/it/pointer.rs b/crates/ui-egui/tests/it/pointer.rs index c868dc8079..b63d9937fb 100644 --- a/crates/ui-egui/tests/it/pointer.rs +++ b/crates/ui-egui/tests/it/pointer.rs @@ -1129,3 +1129,29 @@ fn a_right_click_on_a_comment_with_a_drawing_tool_opens_its_menu() { right_click(&mut h, on_comment); assert_eq!(h.state().views[0].comments.selected, Some((0, 0)), "the comment is selected for its menu"); } + +/// The check box `agree` of [`checkbox_pdf`], with a rectangle comment at x 150..250, y 100..150. +fn comment_and_checkbox_pdf() -> Vec { + pdf(&[ + "<< /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [4 0 R] /DA (/ZaDb 0 Tf 0 g) /DR << /Font << /ZaDb 5 0 R >> >> >> >>", + PAGES, + "<< /Type /Page /Parent 2 0 R /Annots [4 0 R 6 0 R] >>", + "<< /Type /Annot /Subtype /Widget /FT /Btn /T (agree) /V /Off /AS /Off /Rect [100 200 120 220] /P 3 0 R /F 4 /MK << /CA (4) >> /DA (/ZaDb 11 Tf 0 g) >>", + "<< /Type /Font /Subtype /Type1 /BaseFont /ZapfDingbats >>", + "<< /Type /Annot /Subtype /Square /Rect [150 100 250 150] /C [1 0 0] /F 4 /T (Ana) >>", + ]) +} + +/// A click on anything but the selected comment leaves it unselected, a form field included, as +/// a click on a link or on the page does. A field's click used to keep it selected. +#[test] +fn a_click_on_a_form_field_deselects_the_selected_comment() { + let mut h = open(comment_and_checkbox_pdf(), "150", false); + let comment = at(&h, 200.0, 125.0); + mouse_click(&mut h, comment); + assert_eq!(h.state().views[0].comments.selected, Some((0, 1)), "the comment is selected"); + let check_box = at(&h, 110.0, 210.0); + mouse_click(&mut h, check_box); + assert_eq!(h.state().views[0].comments.selected, None, "a click on the check box leaves it unselected"); + assert_eq!(agree(&h).len(), 1, "and ticks the box"); +}