Merge #5658: M5: the text selection bar stays inside the pages' area (#5577)

# Conflicts:
#	crates/ui-egui/src/canvas/selection.rs

crates/ui-egui/src/canvas/selection.rs

@@ -76,9 +76,14 @@ impl DocView {
     /// Screen position of the centre of glyph `glyph` on `page`, if that page is on screen and its
     /// text layer is loaded (used by UI tests and automation to aim pointer input).
     pub fn glyph_screen_pos(&self, page: usize, glyph: usize) -> Option<Pos2> {
+        self.glyph_screen_rect(page, glyph).map(|r| r.center())
+    }
+
+    /// The screen rectangle of glyph `glyph` on `page`, when the page is on screen.
+    pub fn glyph_screen_rect(&self, page: usize, glyph: usize) -> Option<Rect> {
         let (_, xf) = self.screen_xforms.iter().find(|(p, _)| *p == page)?;
         let g = self.texts.get(&page)?.glyphs.get(glyph)?;
-        Some(xf.view_rect(g.rect).center())
+        Some(xf.view_rect(g.rect))
     }
 
     /// Selected text, if any (⌘C).
@@ -172,6 +177,9 @@ enum SelectionAction {
     Copy,
 }
 
+/// The selection bar's width before it has been laid out once (English labels).
+const SELECTION_BAR_GUESS: f32 = 520.0;
+
 /// Actions on an existing text selection, next to the selected words.
 pub(super) fn selection_actions(
     ui: &mut egui::Ui,
@@ -203,32 +211,33 @@ pub(super) fn selection_actions(
     if !viewport.intersects(rect) {
         return false;
     }
-    let x = rect.left().clamp(viewport.left() + 8.0, (viewport.right() - 520.0).max(viewport.left() + 8.0));
+    let id = egui::Id::unique(("text-selection-actions", view.view_key()));
+    // The bar stays inside the pages' area, by the width it had last frame (its labels make it
+    // wider in some languages), so it never covers a side panel; a first guess before it has one.
+    let width = egui::AreaState::load(ui.ctx(), id).and_then(|a| a.size).map_or(SELECTION_BAR_GUESS, |s| s.x);
+    let x = rect.left().clamp(viewport.left() + 8.0, (viewport.right() - width - 8.0).max(viewport.left() + 8.0));
     let y = if rect.bottom() + 48.0 < viewport.bottom() { rect.bottom() + 6.0 } else { (rect.top() - 44.0).max(viewport.top() + 6.0) };
     let mut chosen = None;
-    egui::Area::new(egui::Id::unique(("text-selection-actions", view.view_key()))).order(egui::Order::Foreground).fixed_pos(pos2(x, y)).show(
-        ui.ctx(),
-        |ui| {
-            egui::Frame::popup(ui.style()).show(ui, |ui| {
-                ui.horizontal(|ui| {
-                    for (label, action) in [
-                        (tl!("Add comment"), SelectionAction::Note),
-                        (tl!("Highlight"), SelectionAction::Markup(comments::CommentTool::Highlight)),
-                        (tl!("Underline"), SelectionAction::Markup(comments::CommentTool::Underline)),
-                        (tl!("Strikethrough"), SelectionAction::Markup(comments::CommentTool::StrikeOut)),
-                        (tl!("Replace text"), SelectionAction::Replace),
-                    ] {
-                        if ui.add_enabled(can_comment, egui::Button::new(label)).clicked() {
-                            chosen = Some(action);
-                        }
+    egui::Area::new(id).order(egui::Order::Foreground).fixed_pos(pos2(x, y)).show(ui.ctx(), |ui| {
+        egui::Frame::popup(ui.style()).show(ui, |ui| {
+            ui.horizontal(|ui| {
+                for (label, action) in [
+                    (tl!("Add comment"), SelectionAction::Note),
+                    (tl!("Highlight"), SelectionAction::Markup(comments::CommentTool::Highlight)),
+                    (tl!("Underline"), SelectionAction::Markup(comments::CommentTool::Underline)),
+                    (tl!("Strikethrough"), SelectionAction::Markup(comments::CommentTool::StrikeOut)),
+                    (tl!("Replace text"), SelectionAction::Replace),
+                ] {
+                    if ui.add_enabled(can_comment, egui::Button::new(label)).clicked() {
+                        chosen = Some(action);
                     }
-                    if ui.add_enabled(can_copy, egui::Button::new(tl!("Copy"))).clicked() {
-                        chosen = Some(SelectionAction::Copy);
-                    }
-                });
+                }
+                if ui.add_enabled(can_copy, egui::Button::new(tl!("Copy"))).clicked() {
+                    chosen = Some(SelectionAction::Copy);
+                }
             });
-        },
-    );
+        });
+    });
     let Some(action) = chosen else { return false };
     if let SelectionAction::Copy = action {
         // Copying closes the bar but keeps the words selected, for ⌘C or a second action.

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

@@ -3556,6 +3556,28 @@ fn an_upright_comment_on_a_turned_page_is_drawn_where_it_goes_before_release() {
     h.run_steps(3);
 }
 
+/// The selection bar stays inside the pages' area: its place was worked out for a 520 pt bar,
+/// so longer labels (German here) pushed it over the side panel next to a selection on the right.
+#[test]
+fn the_selection_bar_stays_inside_the_pages_area() {
+    let mut h = harness(|app| app.set_option("language", "de").unwrap());
+    // Narrower than the harness, as with a side panel open.
+    h.set_size(egui::vec2(1100.0, 900.0));
+    h.run_steps(4);
+    // The rightmost glyph of "The quick brown fox": the bar opens at its left.
+    let last = (0..64).filter_map(|i| h.state().views[0].glyph_screen_pos(0, i).map(|p| (i, p.x))).max_by(|a, b| a.1.total_cmp(&b.1)).unwrap().0;
+    h.state_mut().views[0].select_text(0, last, last);
+    h.run_steps(4);
+    let viewport = h.state().views[0].viewport_rect();
+    // The bar's whole area, frame included, not only its first and last buttons.
+    let id = egui::Id::unique(("text-selection-actions", h.state().views[0].id.0));
+    let bar = h.ctx.memory(|m| m.area_rect(id)).expect("the selection bar is shown");
+    // The bar opens at the selection's left edge: past the area's right edge, without the clamp.
+    let anchor = h.state().views[0].glyph_screen_rect(0, last).unwrap().left();
+    assert!(anchor + bar.width() > viewport.right(), "selection at {anchor}, bar {} wide, area {viewport:?}", bar.width());
+    assert!(viewport.contains_rect(bar), "bar {bar:?} inside {viewport:?}");
+}
+
 /// `n` pages with one comment each, "Card 1" to "Card n".
 fn one_comment_per_page(n: usize) -> Vec<u8> {
     let mut objs = vec!["<< /Type /Catalog /Pages 2 0 R >>".to_string()];