diff --git a/crates/ui-egui/src/canvas/selection.rs b/crates/ui-egui/src/canvas/selection.rs index c9bc8cd3e1..f50bbcfa70 100644 --- a/crates/ui-egui/src/canvas/selection.rs +++ b/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 { + 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 { 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. diff --git a/crates/ui-egui/tests/it/comments.rs b/crates/ui-egui/tests/it/comments.rs index 67ea715fc2..f1e185c273 100644 --- a/crates/ui-egui/tests/it/comments.rs +++ b/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 { let mut objs = vec!["<< /Type /Catalog /Pages 2 0 R >>".to_string()];