From d6ba2413ba556894c67f89019f4da27f1cdd6950 Mon Sep 17 00:00:00 2001 From: mmmaarian03-crypto <253881360+mmmaarian03-crypto@users.noreply.github.com> Date: Thu, 22 Oct 2026 18:37:35 +0300 Subject: [PATCH 1/2] Comments: the composer hides while its comment is scrolled away (#4021) The floating comment card (new note, text box, edit comment) was an egui Area constrained to the whole window. When the page scrolled its comment under the mode bar, the card was pushed back into the window and sat over the mode bar and toolbar until the whole page left the screen. It now shows only while its anchor is inside the document view, and is kept inside that view. While the comment is out of sight the draft is kept, and the editor takes the focus again when it comes back. Test: tests/comments.rs the_composer_hides_while_its_comment_is_scrolled_away scrolls a sticky note's anchor above the document with its page still on screen; it fails without the change (the card sat at y=43, over the toolbar, with the document starting at y=134) and passes with it. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/comments.rs | 10 +++++++++- crates/ui-egui/tests/it/comments.rs | 28 ++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/comments.rs b/crates/ui-egui/src/comments.rs index d80f988b7..df2be3780 100644 --- a/crates/ui-egui/src/comments.rs +++ b/crates/ui-egui/src/comments.rs @@ -1564,6 +1564,7 @@ pub(crate) fn composer(ctx: &egui::Context, view: &mut DocView, info: &DocInfo, }; let cx = PageCx { page, xf: &xf, info, tool: QuickTool::Select, prefs, allowed: true, hidden: false, redact_mark: ([0.89, 0.13, 0.13], 1.0) }; let anchor = cx.to_screen(c.at); + let viewport = view.viewport_rect(); let t = Tokens::get(ctx); let mut post = false; let mut cancel = false; @@ -1576,8 +1577,15 @@ pub(crate) fn composer(ctx: &egui::Context, view: &mut DocView, info: &DocInfo, ComposerKind::Replace => "Replacement text", ComposerKind::Edit(_) => "Edit comment", }; + // Its comment scrolled out of the document: keep the draft until it is back, rather than + // pushing the card into the window over the mode bar and toolbar (#4021). + if !viewport.contains(anchor) { + c.focus = true; + return None; + } let pos = pos2(anchor.x + 12.0, anchor.y); - egui::Area::new(egui::Id::new(("comment-composer", view.id.0))).order(egui::Order::Foreground).fixed_pos(pos).show(ctx, |ui| { + let area = egui::Area::new(egui::Id::new(("comment-composer", view.id.0))).order(egui::Order::Foreground).constrain_to(viewport); + area.fixed_pos(pos).show(ctx, |ui| { egui::Frame::popup(ui.style()).inner_margin(egui::Margin::same(12)).corner_radius(CornerRadius::same(8)).show(ui, |ui| { ui.set_width(260.0); ui.horizontal(|ui| { diff --git a/crates/ui-egui/tests/it/comments.rs b/crates/ui-egui/tests/it/comments.rs index 6194c7516..336f7d215 100644 --- a/crates/ui-egui/tests/it/comments.rs +++ b/crates/ui-egui/tests/it/comments.rs @@ -675,6 +675,34 @@ fn sticky_note_composer_posts_and_returns_to_select() { assert_eq!(comments(&h).len(), 1); } +/// The composer leaves with its comment when the page scrolls it out of the document, and comes +/// back with the draft (#4021). It used to be pushed back into the window, over the mode bar and +/// the toolbar, while its comment was out of sight. +#[test] +fn the_composer_hides_while_its_comment_is_scrolled_away() { + let mut h = harness(|app| app.set_option("quick", "note").unwrap()); + click_pt(&mut h, (250.0, 180.0)); + field(&h, "Add a comment").type_text("Draft"); + h.run_steps(2); + let editors = |h: &Harness<'static, PdfCraftApp>| -> Vec { + h.query_all_by(|n| n.role() == egui::accesskit::Role::MultilineTextInput).map(|n| n.rect()).collect() + }; + let viewport = h.state().views[0].viewport_rect(); + assert!(editors(&h).iter().any(|r| viewport.contains(r.center())), "the composer opens in the document"); + // The page scrolled up: it still shows, the comment near its top doesn't. + h.state_mut().views[0].goto = Some((0, 0.4)); + h.run_steps(4); + assert!(h.state().views[0].page_screen_rect(0).is_some_and(|r| r.bottom() > viewport.top()), "page 1 still shows"); + assert!(at(&h, 250.0, 180.0).y < viewport.top(), "the comment is scrolled away"); + let shown = editors(&h); + assert!(shown.is_empty(), "the composer stays with its comment, not over the toolbar: {shown:?} (document {viewport:?})"); + // Back again, the draft is still there. + h.state_mut().views[0].goto = Some((0, 0.0)); + h.run_steps(4); + assert!(editors(&h).iter().any(|r| viewport.contains(r.center())), "the composer is back"); + assert_eq!(h.state().views[0].comments.composer.as_ref().map(|c| c.text.as_str()), Some("Draft")); +} + /// The rectangle of the comment whose text starts with `text`. fn rect_of(h: &Harness<'static, PdfCraftApp>, text: &str) -> [f32; 4] { let c = comments(h); From cdc0834f349e66732d1d6b6b3a7e5779b4b2a53a Mon Sep 17 00:00:00 2001 From: mmmaarian03-crypto <253881360+mmmaarian03-crypto@users.noreply.github.com> Date: Fri, 23 Oct 2026 08:42:54 +0300 Subject: [PATCH 2/2] Comments: the composer refocuses after its page was off screen (#4021) When the whole page left the screen, `composer` returned before it set `focus`, so the editor didn't take the keyboard again when the page came back (review on #4095). That path now asks for the focus too. Test `the_composer_takes_the_keyboard_back_after_its_page_was_off_screen` scrolls page 1 off the screen and back; it fails without the change. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/comments.rs | 5 ++++- crates/ui-egui/tests/it/comments.rs | 22 ++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/comments.rs b/crates/ui-egui/src/comments.rs index df2be3780..3cdbb953d 100644 --- a/crates/ui-egui/src/comments.rs +++ b/crates/ui-egui/src/comments.rs @@ -1559,7 +1559,10 @@ pub(crate) fn composer(ctx: &egui::Context, view: &mut DocView, info: &DocInfo, let c = view.comments.composer.as_ref()?; let page = c.page; let Some(xf) = view.page_xform(page) else { - // Scrolled away: keep the draft until the page is back. + // Scrolled away: keep the draft until the page is back, and give it the keyboard then. + if let Some(c) = view.comments.composer.as_mut() { + c.focus = true; + } return None; }; let cx = PageCx { page, xf: &xf, info, tool: QuickTool::Select, prefs, allowed: true, hidden: false, redact_mark: ([0.89, 0.13, 0.13], 1.0) }; diff --git a/crates/ui-egui/tests/it/comments.rs b/crates/ui-egui/tests/it/comments.rs index 336f7d215..d97c09fbf 100644 --- a/crates/ui-egui/tests/it/comments.rs +++ b/crates/ui-egui/tests/it/comments.rs @@ -703,6 +703,28 @@ fn the_composer_hides_while_its_comment_is_scrolled_away() { assert_eq!(h.state().views[0].comments.composer.as_ref().map(|c| c.text.as_str()), Some("Draft")); } +/// With its whole page scrolled off the screen the composer keeps its draft, and the editor takes +/// the keyboard again when the page comes back (#4021). +#[test] +fn the_composer_takes_the_keyboard_back_after_its_page_was_off_screen() { + let mut h = harness(|app| app.set_option("quick", "note").unwrap()); + click_pt(&mut h, (250.0, 180.0)); + field(&h, "Add a comment").type_text("Draft"); + h.run_steps(2); + let editor_focused = + |h: &Harness<'static, PdfCraftApp>| h.query_all_by(|n| n.role() == egui::accesskit::Role::MultilineTextInput).any(|n| n.is_focused()); + assert!(editor_focused(&h), "the new note's editor has the keyboard"); + let viewport = h.state().views[0].viewport_rect(); + h.state_mut().views[0].goto = Some((1, 0.6)); + h.run_steps(4); + assert!(h.state().views[0].page_screen_rect(0).is_none_or(|r| r.bottom() < viewport.top()), "page 1 is off the screen"); + assert!(!editor_focused(&h), "nothing to type into while the page is away"); + h.state_mut().views[0].goto = Some((0, 0.0)); + h.run_steps(4); + assert_eq!(h.state().views[0].comments.composer.as_ref().map(|c| c.text.as_str()), Some("Draft")); + assert!(editor_focused(&h), "the editor has the keyboard again"); +} + /// The rectangle of the comment whose text starts with `text`. fn rect_of(h: &Harness<'static, PdfCraftApp>, text: &str) -> [f32; 4] { let c = comments(h);