fix(comments): the note editor takes the keyboard once, so V and H pick tools again

After a highlight, its note editor in the Comments panel kept the
keyboard, so V and H were typed into the note (#2669, item 2).

- The editor asked for the keyboard on every frame when no other widget
  had it, so it took it back after every click elsewhere. It now takes
  it once, when it opens (CommentView::open_editor, EditorFocus::Take).
- Adding another comment left the editor open, and drawing is a drag,
  which egui doesn't treat as a click elsewhere. An empty editor on
  another comment now closes when a comment is added; one with text
  stays open with it but gives the keyboard back (EditorFocus::Release).

Part of #2669.

crates/ui-egui/src/comments.rs

@@ -496,6 +496,19 @@ pub struct CommentView {
     pub context_at: Option<(usize, [f64; 2])>,
     /// A one-shot tool just finished; the app returns to the Select tool unless pinned.
     pub tool_done: bool,
+    /// What the inline editor (`editing`) does with the keyboard the next time it is drawn.
+    pub(crate) editor_focus: Option<EditorFocus>,
+}
+
+/// A one-off change to the keyboard focus of the Comments panel's inline editor. The editor
+/// never asks for the keyboard on its own: when it did so on every frame, it took the keyboard
+/// back after every click elsewhere, and the tool keys (V, H) were typed into it (#2669).
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub(crate) enum EditorFocus {
+    /// The editor just opened: it takes the keyboard, once.
+    Take,
+    /// Another comment was added: the editor keeps its text but gives the keyboard back.
+    Release,
 }
 
 impl CommentView {
@@ -512,6 +525,25 @@ impl CommentView {
         }
         true
     }
+
+    /// Open the inline editor on comment (page, index) with `text`. It takes the keyboard once,
+    /// as it opens; after that the keyboard goes wherever the user clicks.
+    pub(crate) fn open_editor(&mut self, page: usize, index: usize, text: String) {
+        self.editing = Some((page, index, text));
+        self.editor_focus = Some(EditorFocus::Take);
+    }
+
+    /// A comment was just added. The editor open on another one must not keep the keyboard, or
+    /// the tool keys pressed next are typed into it (#2669): an empty editor closes, and one
+    /// holding text stays open with it, without the keyboard.
+    pub(crate) fn comment_added(&mut self) {
+        if self.editing.as_ref().is_some_and(|(_, _, text)| !text.is_empty()) {
+            self.editor_focus = Some(EditorFocus::Release);
+        } else {
+            self.editing = None;
+            self.editor_focus = None;
+        }
+    }
 }
 
 /// Comments panel order (Acrobat: Sort by page, author, date, type, colour).

crates/ui-egui/src/comments_panel.rs

@@ -323,8 +323,10 @@ fn card(
                 ui.set_width(ui.available_width());
                 if editing && let Some((_, _, text)) = view.comments.editing.as_mut() {
                     let r = ui.add(crate::widgets::block(text).desired_rows(2).desired_width(f32::INFINITY).id_salt(("comment-edit", key)));
-                    if !r.has_focus() && !ui.memory(|m| m.focused().is_some()) {
-                        r.request_focus();
+                    match view.comments.editor_focus.take() {
+                        Some(crate::comments::EditorFocus::Take) => r.request_focus(),
+                        Some(crate::comments::EditorFocus::Release) => r.surrender_focus(),
+                        None => {}
                     }
                     let submit = crate::comments::submit_key(ui, &r, false);
                     ui.horizontal(|ui| {
@@ -433,7 +435,7 @@ fn card(
         *nav = Some(Nav::Flash(a.page, a.rect));
     }
     if click.double_clicked() && allowed {
-        view.comments.editing = Some((a.page, a.index, a.contents.clone().unwrap_or_default()));
+        view.comments.open_editor(a.page, a.index, a.contents.clone().unwrap_or_default());
     }
     let menu_id = ui.id().with(("card-menu", key));
     if open_menu {
@@ -448,7 +450,7 @@ fn card(
         .show(|ui| {
             ui.set_min_width(160.0);
             if ui.button(tl_ctx!("comment menu", "Edit")).clicked() {
-                view.comments.editing = Some((a.page, a.index, a.contents.clone().unwrap_or_default()));
+                view.comments.open_editor(a.page, a.index, a.contents.clone().unwrap_or_default());
                 ui.close();
             }
             ui.menu_button(tl!("Set status"), |ui| {

crates/ui-egui/src/editing.rs

@@ -116,6 +116,9 @@ impl PdfCraftApp {
                 } else {
                     view.document_changed(info);
                 }
+                if matches!(edit, Edit::AddAnnotation(_)) {
+                    view.comments.comment_added();
+                }
                 // Keep the pages the user acted on selected, where they now are.
                 match edit {
                     Edit::MovePages { pages, to } => {
@@ -142,9 +145,9 @@ impl PdfCraftApp {
                         // unless a note typed into another card is still unsaved.
                         if let (pdfcraft_engine::Shape::TextMarkup { kind: pdfcraft_engine::Markup::Highlight, .. }, Some(n)) = (&a.shape, newest)
                             && a.contents.is_empty()
-                            && view.comments.editing.as_ref().is_none_or(|(_, _, text)| text.is_empty())
+                            && view.comments.editing.is_none()
                         {
-                            view.comments.editing = Some((a.page, n, String::new()));
+                            view.comments.open_editor(a.page, n, String::new());
                         }
                     }
                     Edit::DeleteAnnotation { .. } => view.comments.selected = None,

crates/ui-egui/tests/comments.rs

@@ -231,6 +231,99 @@ fn a_new_highlight_opens_its_note_for_typing() {
     assert_eq!(h.state().views[0].comments.editing, Some((0, 1, "typed".into())));
 }
 
+/// The Comments panel's editor for a comment's note (the reply field is the other multiline one).
+fn note_editor<'a>(h: &'a Harness<'static, PdfCraftApp>) -> egui_kittest::Node<'a> {
+    h.query_all_by(|n| n.role() == egui::accesskit::Role::MultilineTextInput && n.placeholder() != Some("Add a reply")).next().expect("note editor")
+}
+
+/// The text in the open note editor.
+fn note(h: &Harness<'static, PdfCraftApp>) -> Option<String> {
+    h.state().views[0].comments.editing.as_ref().map(|(_, _, text)| text.clone())
+}
+
+/// A letter key as a keyboard sends it: the key goes down, types its letter and comes up.
+fn type_key(h: &mut Harness<'static, PdfCraftApp>, key: egui::Key, letter: &str) {
+    h.key_down(key);
+    h.event(egui::Event::Text(letter.into()));
+    h.key_up(key);
+    h.run_steps(2);
+}
+
+/// Highlight glyphs `from..to` of page 1's text with the highlighter.
+fn highlight_text(h: &mut Harness<'static, PdfCraftApp>, from: usize, to: usize) {
+    let (a, b) = {
+        let v = &h.state().views[0];
+        (v.glyph_screen_pos(0, from).expect("text layer"), v.glyph_screen_pos(0, to).expect("glyph"))
+    };
+    drag(h, a, b);
+    h.run_steps(2);
+}
+
+#[test]
+fn clicking_the_page_gives_the_keyboard_back_from_a_highlight_note() {
+    // #2669: the note editor took the keyboard back on every frame, so after a click on the
+    // page V and H were typed into the note instead of picking a tool.
+    let mut h = harness(|app| {
+        app.set_option("quick", "highlight").unwrap();
+        app.set_option("panel", "comments").unwrap();
+    });
+    highlight_text(&mut h, 4, 14);
+    assert!(note_editor(&h).is_focused(), "a new highlight's note takes the keyboard straight away");
+    h.event(egui::Event::Text("Note".into()));
+    h.run_steps(2);
+    assert_eq!(note(&h).as_deref(), Some("Note"), "typing goes into the note");
+    // A click on an empty part of the page gives the keyboard back to the page.
+    click_pt(&mut h, (250.0, 20.0));
+    type_key(&mut h, egui::Key::V, "v");
+    assert_eq!(h.state().quick_tool, QuickTool::Select, "V picks the Select tool");
+    assert_eq!(note(&h).as_deref(), Some("Note"), "V is not typed into the note");
+    type_key(&mut h, egui::Key::H, "h");
+    assert_eq!(h.state().quick_tool, QuickTool::Hand, "H picks the Hand tool");
+    assert_eq!(note(&h).as_deref(), Some("Note"), "nor is H");
+    // Clicking back into the note types into it again, and the letters are text there.
+    note_editor(&h).click();
+    h.run_steps(2);
+    type_key(&mut h, egui::Key::V, "v");
+    assert_eq!(note(&h).as_deref(), Some("Notev"), "the note takes keys while the user types in it");
+    assert_eq!(h.state().quick_tool, QuickTool::Hand, "typed letters don't pick tools");
+}
+
+#[test]
+fn drawing_another_comment_takes_the_keyboard_from_a_highlight_note() {
+    // #2669: a highlight's note kept the keyboard while rectangles and ovals were drawn (a drag
+    // is not a click, so the editor never let go), and V and H were typed into it.
+    use pdfcraft_ui_egui::comments::CommentTool;
+    let mut h = harness(|app| {
+        app.set_option("quick", "highlight").unwrap();
+        app.set_option("panel", "comments").unwrap();
+    });
+    highlight_text(&mut h, 4, 14);
+    assert!(note_editor(&h).is_focused());
+    // The tools are picked without a click, as a shortcut would: the drawing alone must take
+    // the keyboard back. An empty note closes.
+    h.state_mut().quick_tool = QuickTool::Comment(CommentTool::Rectangle);
+    drag_pt(&mut h, (40.0, 100.0), (140.0, 40.0));
+    assert_eq!(comments(&h).len(), 2);
+    assert_eq!(h.state().views[0].comments.editing, None, "the empty note closed");
+    type_key(&mut h, egui::Key::H, "h");
+    assert_eq!(h.state().quick_tool, QuickTool::Hand, "H picks the Hand tool");
+    // A note with text in it stays open with its text, but lets the keyboard go.
+    h.state_mut().quick_tool = QuickTool::Comment(CommentTool::Highlight);
+    drag_pt(&mut h, (160.0, 100.0), (260.0, 40.0));
+    h.run_steps(2);
+    assert_eq!(h.state().views[0].comments.editing, Some((0, 2, String::new())), "the new area highlight's note opens");
+    assert!(note_editor(&h).is_focused());
+    h.event(egui::Event::Text("Note".into()));
+    h.run_steps(2);
+    h.state_mut().quick_tool = QuickTool::Comment(CommentTool::Rectangle);
+    drag_pt(&mut h, (40.0, 30.0), (260.0, 8.0));
+    assert_eq!(comments(&h).len(), 4);
+    assert_eq!(h.state().views[0].comments.editing, Some((0, 2, "Note".into())), "a note with text stays open");
+    type_key(&mut h, egui::Key::H, "h");
+    assert_eq!(h.state().quick_tool, QuickTool::Hand, "H picks the Hand tool");
+    assert_eq!(note(&h).as_deref(), Some("Note"), "H is not typed into the note");
+}
+
 #[test]
 fn ctrl_enter_posts_a_highlight_comment_and_shift_enter_adds_a_line() {
     let mut h = harness(|app| {