diff --git a/crates/ui-egui/src/comments.rs b/crates/ui-egui/src/comments.rs index 442bf9111..75039ff6e 100644 --- a/crates/ui-egui/src/comments.rs +++ b/crates/ui-egui/src/comments.rs @@ -1571,11 +1571,15 @@ 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) }; 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; @@ -1588,8 +1592,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 36bd049dd..4477f502b 100644 --- a/crates/ui-egui/tests/it/comments.rs +++ b/crates/ui-egui/tests/it/comments.rs @@ -707,6 +707,56 @@ fn a_sticky_note_on_a_turned_page_is_picked_where_it_is_drawn() { assert_eq!(h.state().views[0].comments.selected, Some((0, 0))); } +/// 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")); +} + +/// 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);