M5: selecting from anywhere but a page press forgets the group member a press picked

CommentView remembered the group member a press on the page picked, so
Delete and the arrow keys act on the member clicked, as Acrobat desktop
does. The memory outlived the selection it came from: after choosing the
group's card in the Comments panel, a deletion, an undo or a selection
made by a tool, Delete still took the member clicked earlier.

Every selection now goes through CommentView::select(), which clears the
remembered member; pick() (a press on the page) stays the only place that
sets it. The panel's cards, deletion and undo, the context menus, the
drawing tools, the measuring panel and the control channel use select().

Test: choosing_the_groups_card_makes_delete_act_on_the_card_not_the_member_clicked
(ui-egui). With select() leaving the member in place it fails: the Circle
clicked earlier goes instead of the Square the card shows
(left: ["Square"], right: ["Circle"]).

Review on #5517.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv

crates/ui-egui/src/app/control.rs

@@ -355,7 +355,7 @@ impl PdfCraftApp {
                 let (p, i) = value.split_once(':').ok_or("comment: PAGE:INDEX")?;
                 let (p, i): (usize, usize) =
                     (p.trim().parse().map_err(|_| "comment: PAGE:INDEX")?, i.trim().parse().map_err(|_| "comment: PAGE:INDEX")?);
-                v.comments.selected = Some((p.saturating_sub(1), i.saturating_sub(1)));
+                v.comments.select(Some((p.saturating_sub(1), i.saturating_sub(1))));
                 v.comments.reveal = true;
             }
             // `--combine-view list`: Combine files as thumbnails (grid) or as the table (list).

crates/ui-egui/src/editing.rs

@@ -55,12 +55,12 @@ fn select_after_edit(
             if matches!(a.shape, pdfcraft_engine::Shape::Ink { .. })
                 && quick_tool == crate::QuickTool::Comment(crate::comments::CommentTool::Ink) =>
         {
-            view.comments.selected = None;
+            view.comments.select(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.is_reply()).map(|x| x.index).max();
-            view.comments.selected = newest.map(|n| (a.page, n));
+            view.comments.select(newest.map(|n| (a.page, n)));
             view.comments.reveal = true;
             // A new highlight opens its note for typing straight away, as in Acrobat,
             // unless a note typed into another card is still unsaved.
@@ -647,7 +647,7 @@ impl PdfCraftApp {
                     .session
                     .get(view.id)
                     .and_then(|d| d.info.annotations.iter().filter(|a| a.page == page && !a.is_reply()).map(|a| a.index).max());
-                view.comments.selected = newest.map(|index| (page, index));
+                view.comments.select(newest.map(|index| (page, index)));
                 view.comments.reveal = true;
                 self.quick_tool = crate::QuickTool::Select;
             }

crates/ui-egui/src/panels/comments_panel.rs

@@ -497,7 +497,7 @@ fn card(ui: &mut egui::Ui, view: &mut DocView, context: CardContext<'_>) -> Opti
         .on_hover_cursor(egui::CursorIcon::PointingHand)
         .on_hover_text(crate::i18n::fmt(tl!("{kind} — click to show it on the page"), &[("kind", tl!(kind_label(a)))]));
     if click.clicked() {
-        view.comments.selected = Some(key);
+        view.comments.select(Some(key));
         view.comments.reply.clear();
         *nav = Some(Nav::Flash(a.page, a.shown_rect(rotation)));
     }
@@ -548,7 +548,7 @@ fn card(ui: &mut egui::Ui, view: &mut DocView, context: CardContext<'_>) -> Opti
             ui.separator();
             // A locked reply can't be deleted, so it keeps the comment it answers too.
             if ui.add_enabled(doc.annotation_deletable(a.page, a.index), egui::Button::new(tl!("Delete"))).clicked() {
-                view.comments.selected = None;
+                view.comments.select(None);
                 edit = Some(Edit::DeleteAnnotation { page: a.page, index: a.index });
                 ui.close();
             }

crates/ui-egui/src/tools/comments/input.rs

@@ -34,7 +34,7 @@ pub(crate) fn page_input(ui: &egui::Ui, resp: &egui::Response, cx: &PageCx<'_>,
         // On a group subordinate, its group (the primary), so the menu offers the comment's actions.
         match cx.click_hit(p) {
             Some(a) => cv.pick(cx, a),
-            None => cv.selected = None,
+            None => cv.select(None),
         }
     }
     let gestures = Gestures { ui, resp, cx, pointer, origin, page_rect, pressed_here, over_page };
@@ -275,7 +275,7 @@ fn select_input(ui: &egui::Ui, resp: &egui::Response, cx: &PageCx<'_>, view: &mu
                 cv.reveal = true;
                 consumed = true;
             }
-            None => cv.selected = None,
+            None => cv.select(None),
         }
     }
     if resp.double_clicked()
@@ -353,7 +353,7 @@ impl Gestures<'_> {
             && let Some(o) = origin
         {
             cv.gesture = Some(Gesture::Draw { page: cx.page, tool, points: vec![cx.to_user(o)] });
-            cv.selected = None;
+            cv.select(None);
         }
         if let Some(Gesture::Draw { page, tool, points }) = cv.gesture.as_mut()
             && *page == cx.page
@@ -393,7 +393,7 @@ impl Gestures<'_> {
             && let Some(p) = pointer
             && let Some(shape) = drawn_shape(tool, &[cx.to_user(p)])
         {
-            cv.selected = None;
+            cv.select(None);
             let now = ui.input(|i| i.time);
             cv.settle(now, cx.page, tool, &[cx.to_user(p)]);
             view.pending_edit = Some(new_comment(cx, tool, shape, String::new()));
@@ -444,7 +444,7 @@ impl Gestures<'_> {
                 }
                 _ => {
                     cv.gesture = Some(Gesture::Draw { page: cx.page, tool, points: vec![u] });
-                    cv.selected = None;
+                    cv.select(None);
                 }
             }
         }
@@ -476,7 +476,7 @@ impl Gestures<'_> {
             && let Some(o) = origin
         {
             cv.gesture = Some(Gesture::Draw { page: cx.page, tool: CommentTool::Callout, points: vec![cx.to_user(o)] });
-            cv.selected = None;
+            cv.select(None);
         }
         if let Some(Gesture::Draw { page, points, .. }) = cv.gesture.as_mut()
             && *page == cx.page
@@ -513,7 +513,7 @@ impl Gestures<'_> {
             && let Some(p) = pointer
         {
             cv.attach_at = Some((cx.page, cx.to_user(p)));
-            cv.selected = None;
+            cv.select(None);
         }
         true
     }
@@ -535,7 +535,7 @@ impl Gestures<'_> {
             let snapped = cx.info.pages.get(cx.page).zip(caret_text).and_then(|(page, t)| snapped_caret(page, t, vx, vy));
             let (at, size) = snapped.unwrap_or_else(|| (cx.to_user(p), CARET_SIZE));
             cv.composer = Some(Composer { page: cx.page, at, kind: ComposerKind::Caret { size }, text: String::new(), focus: true });
-            cv.selected = None;
+            cv.select(None);
         }
         true
     }
@@ -557,7 +557,7 @@ impl Gestures<'_> {
                 && let Some(o) = origin
             {
                 cv.gesture = Some(Gesture::Draw { page: cx.page, tool, points: vec![cx.to_user(o)] });
-                cv.selected = None;
+                cv.select(None);
             }
             if let Some(Gesture::Draw { page, points, .. }) = cv.gesture.as_mut()
                 && *page == cx.page
@@ -587,7 +587,7 @@ impl Gestures<'_> {
         {
             let kind = if tool == CommentTool::Note { ComposerKind::Note } else { ComposerKind::TextBox { size: None } };
             cv.composer = Some(Composer { page: cx.page, at: cx.to_user(p), kind, text: String::new(), focus: true });
-            cv.selected = None;
+            cv.select(None);
         }
         true
     }
@@ -613,7 +613,7 @@ pub(crate) fn keys(ctx: &egui::Context, view: &mut DocView, tool: &mut QuickTool
         && let Some((page, index)) = cv.target()
         && ctx.input(|i| i.key_pressed(egui::Key::Delete) || i.key_pressed(egui::Key::Backspace))
     {
-        cv.selected = None;
+        cv.select(None);
         view.pending_edit = Some(Edit::DeleteAnnotation { page, index });
     }
 }

crates/ui-egui/src/tools/comments/menu.rs

@@ -96,7 +96,7 @@ pub(crate) fn context_menu(
             // subordinate, Delete takes just it, as Acrobat's does.
             let (dpage, dindex) = view.comments.target().unwrap_or((page, index));
             if ui.add_enabled(allowed && deletable(dpage, dindex), egui::Button::new(tl!("Delete"))).clicked() {
-                view.comments.selected = None;
+                view.comments.select(None);
                 action = Some(CanvasAction::Edit(Box::new(Edit::DeleteAnnotation { page: dpage, index: dindex })));
                 ui.close();
             }
@@ -127,7 +127,7 @@ pub(crate) fn context_menu(
     if !info.annotations.is_empty() {
         ui.separator();
         if ui.add_enabled(can_modify, egui::Button::new(tl!("Flatten comments"))).clicked() {
-            view.comments.selected = None;
+            view.comments.select(None);
             action = Some(CanvasAction::Edit(Box::new(Edit::Flatten { comments: true, fields: false })));
             ui.close();
         }

crates/ui-egui/src/tools/comments/state.rs

@@ -199,6 +199,14 @@ impl CommentView {
         }
     }
 
+    /// Select `key`, or nothing, from anywhere but a press on the page ([`Self::pick`]): the group
+    /// member a press picked goes, so Delete and the arrow keys act on what is selected now, not
+    /// on a member remembered from before (after the Comments panel, a deletion or undo).
+    pub(crate) fn select(&mut self, key: Option<(usize, usize)>) {
+        self.selected = key;
+        self.member = None;
+    }
+
     /// Select what a press on this page's `a` selects ([`PageCx::selects`]), remembering `a` when
     /// it is a group subordinate.
     pub(crate) fn pick(&mut self, cx: &PageCx<'_>, a: &Annotation) {
@@ -245,7 +253,7 @@ impl CommentView {
     /// were on, where it now is (deleting a reply keeps its comment selected), and close when it
     /// went too.
     pub(crate) fn annotation_deleted(&mut self, before: &[Annotation], after: &[Annotation], gone: (usize, usize)) {
-        self.selected = self.selected.and_then(|k| after_delete(before, after, gone, k));
+        self.select(self.selected.and_then(|k| after_delete(before, after, gone, k)));
         self.editing = self.editing.take().and_then(|(p, i, text)| after_delete(before, after, gone, (p, i)).map(|(p, i)| (p, i, text)));
         if self.editing.is_none() {
             self.editor_focus = None;
@@ -257,7 +265,7 @@ impl CommentView {
     /// editor close, its unsaved text with it, rather than Save writing that text over whatever
     /// took its place.
     pub(crate) fn history_stepped(&mut self) {
-        self.selected = None;
+        self.select(None);
         self.editing = None;
         self.editor_focus = None;
     }

crates/ui-egui/src/tools/measure_ui.rs

@@ -562,7 +562,7 @@ fn panel_body(app: &mut PdfCraftApp, ui: &mut egui::Ui, _t: &Tokens) {
     }
     if let Some((page, annotation, reading, scale)) = select {
         app.views[index].current = page;
-        app.views[index].comments.selected = Some((page, annotation));
+        app.views[index].comments.select(Some((page, annotation)));
         app.views[index].measure.reading = Some(reading);
         app.views[index].measure.scale = Some(scale);
         app.quick_tool = QuickTool::Select;

crates/ui-egui/tests/it/comments.rs

@@ -3108,6 +3108,29 @@ fn delete_deletes_the_group_member_clicked() {
     assert_eq!(shapes(&h), [("Circle".to_string(), true)], "the Square goes, the Circle stays as a comment of its own");
 }
 
+/// #5517 review: the member a click on the page picked is forgotten once the selection is made
+/// anywhere else. After clicking the Circle, choosing the group's card in the Comments panel
+/// selects the comment the card shows, the Square, so Delete deletes the Square, not the Circle
+/// the page click remembered.
+#[test]
+fn choosing_the_groups_card_makes_delete_act_on_the_card_not_the_member_clicked() {
+    let shapes = |h: &Harness<'static, PdfCraftApp>| -> Vec<String> {
+        let mut out: Vec<String> = comments(h).iter().filter(|a| a.subtype != "Text").map(|a| a.subtype.clone()).collect();
+        out.sort();
+        out
+    };
+    let mut h = harness_with_bytes(GROUP_FIXTURE.to_vec(), |app| app.set_option("panel", "comments").unwrap());
+    click_pt(&mut h, (220.0, 150.0));
+    assert_eq!(h.state().views[0].comments.selected, Some((0, 0)), "the Circle's click selects the group");
+    // The selected card's header row, with its author.
+    h.get_by_label("Alice").click();
+    h.run_steps(3);
+    assert_eq!(h.state().views[0].comments.selected, Some((0, 0)), "the card selects the same group");
+    h.key_press(egui::Key::Delete);
+    h.run_steps(3);
+    assert_eq!(shapes(&h), ["Circle".to_string()], "the card's comment, the Square, goes; the Circle stays");
+}
+
 /// The Square's and the Circle's `/Rect` in [`GROUP_FIXTURE`] (or one built like it).
 fn group_rects(h: &Harness<'static, PdfCraftApp>) -> (Vec<f32>, Vec<f32>) {
     let all = comments(h);