From f28406f6807db4f623c2cfed660a035336045bba Mon Sep 17 00:00:00 2001 From: Lcoding15 <125505497+Lcoding15@users.noreply.github.com> Date: Sun, 1 Nov 2026 15:07:32 +0100 Subject: [PATCH] M5: the text selection bar stays inside the pages' area (#5577) The bar's place was clamped for a 520 pt bar, so with longer labels (German, about 900 pt) a selection on the right pushed it over the side panel. It is now clamped by the width it was laid out at last frame, with 520 pt only as the first guess. The body of the Area is re-indented by rustfmt because its first line got shorter. --- crates/ui-egui/src/canvas/selection.rs | 48 ++++++++++++++------------ crates/ui-egui/tests/it/comments.rs | 25 ++++++++++++++ 2 files changed, 51 insertions(+), 22 deletions(-) diff --git a/crates/ui-egui/src/canvas/selection.rs b/crates/ui-egui/src/canvas/selection.rs index c2b0545df6..85edeac495 100644 --- a/crates/ui-egui/src/canvas/selection.rs +++ b/crates/ui-egui/src/canvas/selection.rs @@ -156,6 +156,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, @@ -187,32 +190,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.id.0)); + // 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.id.0))).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 664f61733e..1d52d50954 100644 --- a/crates/ui-egui/tests/it/comments.rs +++ b/crates/ui-egui/tests/it/comments.rs @@ -3022,3 +3022,28 @@ fn an_upright_comment_on_a_turned_page_is_drawn_where_it_goes_before_release() { h.drop_at(from + delta); 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(); + let (first, copy) = (h.get_by_label("Kommentar hinzufügen").rect(), h.get_by_label("Kopieren").rect()); + let glyph = h.state().views[0].glyph_screen_pos(0, last).unwrap(); + // The setup puts a bar opening at the glyph past the area's right edge. + assert!( + glyph.x + (copy.right() - first.left()) > viewport.right(), + "glyph at {}, bar {} wide, area {viewport:?}", + glyph.x, + copy.right() - first.left() + ); + assert!(first.left() >= viewport.left() && copy.right() <= viewport.right(), "bar {first:?}..{copy:?} inside {viewport:?}"); +}