Merge PR #4095: Comments: the composer hides while its comment is scrolled away (#4021)

Conflicts:
- crates/ui-egui/tests/it/comments.rs: kept both PRs' new tests (adjacent additions).
This commit is contained in:
aiongg
2026-10-24 06:22:46 +00:00
2 changed files with 63 additions and 2 deletions
+13 -2
View File
@@ -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| {
+50
View File
@@ -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<egui::Rect> {
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);