Fix #429: pen strokes stay unselected and show no author popup (#549)

Cause: apply_edit selected every new annotation after Edit::AddAnnotation,
so each stroke drawn with the Draw (ink) tool became selected and showed
its selection box over the page. With the pointer still resting on the
fresh stroke, the canvas hover popup also showed the comment's author,
covering where the next stroke goes.

Fix: a new Ink annotation made while the ink tool is active is left
unselected (other annotation types, and ink added any other way, are
still selected). The annotation hover popup is suppressed while the ink
or eraser tool is active, as it already was mid-gesture.

Test: kittest draws two pen strokes in a row and checks both exist,
nothing is selected, the pen stays on and no author popup appears; a
square drawn afterwards is still selected.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Brandon Thomas
2026-10-09 10:50:17 -04:00
committed by GitHub
co-authored by Claude Opus 5.5
parent 2aca0692fc
commit 6fce73f902
3 changed files with 36 additions and 2 deletions
+4 -2
View File
@@ -1805,8 +1805,10 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) {
}
}
}
// Annotation hover shows the comment, as Acrobat's popups do.
let gesturing = view.comments.gesture.is_some();
// Annotation hover shows the comment, as Acrobat's popups do; not while drawing
// freehand, where it would cover the next stroke (#429).
let freehand = matches!(tool, QuickTool::Comment(comments::CommentTool::Ink | comments::CommentTool::Eraser));
let gesturing = view.comments.gesture.is_some() || freehand;
for a in info.annotations.iter().filter(|a| a.page == i && a.in_reply_to.is_none() && !gesturing) {
let sr = xf.user_rect(info, i, a.rect);
if sr.contains(p) && hover_text.is_none() {
+8
View File
@@ -52,6 +52,14 @@ impl PdfCraftApp {
}
Edit::InsertBlankPage { at, .. } => view.select_pages(&[at.min(info.pages.len() - 1)]),
Edit::DeletePages { .. } => view.select_pages(&[]),
// A stroke drawn with the pen stays unselected, so the selection box and
// author popup don't sit over the next stroke (#429).
Edit::AddAnnotation(a)
if matches!(a.shape, pdfcraft_engine::Shape::Ink { .. })
&& self.quick_tool == crate::QuickTool::Comment(crate::comments::CommentTool::Ink) =>
{
view.comments.selected = None;
}
Edit::AddAnnotation(a) => {
// Select the new comment (appended last among the page's comments).
let newest = info.annotations.iter().filter(|x| x.page == a.page && x.in_reply_to.is_none()).map(|x| x.index).max();
+24
View File
@@ -171,6 +171,30 @@ fn shapes_and_ink_are_drawn_by_dragging() {
assert!((r[0] - 40.0).abs() < 3.0 && (r[1] - 40.0).abs() < 3.0 && (r[2] - 140.0).abs() < 3.0 && (r[3] - 100.0).abs() < 3.0, "{r:?}");
}
#[test]
fn pen_strokes_stay_unselected_so_writing_continues() {
let mut h = harness(|app| app.set_option("quick", "ink").unwrap());
// No Comments panel: the author's name should only show in a hover popup.
h.state_mut().set_option("panel", "none").unwrap();
h.run_steps(2);
drag_pt(&mut h, (40.0, 100.0), (140.0, 60.0));
assert_eq!(h.state().views[0].comments.selected, None, "the first stroke isn't selected");
// The pointer still rests on the stroke: no author popup covers it.
h.hover_at(at(&h, 140.0, 60.0));
h.run_steps(3);
assert!(h.query_by_label_contains("Tester").is_none(), "no author popup while drawing");
drag_pt(&mut h, (60.0, 100.0), (160.0, 60.0));
let c = comments(&h);
let kinds: Vec<&str> = c.iter().map(|a| a.subtype.as_str()).collect();
assert_eq!(kinds, ["Ink", "Ink"], "{c:?}");
assert_eq!(h.state().views[0].comments.selected, None, "nothing is selected after drawing");
assert_eq!(h.state().quick_tool, QuickTool::Comment(pdfcraft_ui_egui::comments::CommentTool::Ink), "the pen stays on");
// Other drawn shapes are still selected when made.
h.state_mut().set_option("quick", "square").unwrap();
drag_pt(&mut h, (180.0, 100.0), (260.0, 40.0));
assert_eq!(h.state().views[0].comments.selected, Some((0, 2)));
}
#[test]
fn sticky_note_composer_posts_and_returns_to_select() {
let mut h = harness(|app| app.set_option("quick", "note").unwrap());