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 1/2] 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:?}"); +} From eea69b4ac68257b4cbc4ea8183154174b6e99c7b Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 16:59:17 +0000 Subject: [PATCH 2/2] M4: the selection-bar test anchors at the selection's left edge and checks the whole bar (#5658) Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01GbuSKAS2k5248LGFEapMUR --- crates/ui-egui/src/canvas/selection.rs | 7 ++++++- crates/ui-egui/tests/it/comments.rs | 17 +++++++---------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/crates/ui-egui/src/canvas/selection.rs b/crates/ui-egui/src/canvas/selection.rs index 85edeac495..894acd8d7a 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). diff --git a/crates/ui-egui/tests/it/comments.rs b/crates/ui-egui/tests/it/comments.rs index c098094bbe..26b7cefb42 100644 --- a/crates/ui-egui/tests/it/comments.rs +++ b/crates/ui-egui/tests/it/comments.rs @@ -3036,16 +3036,13 @@ fn the_selection_bar_stays_inside_the_pages_area() { 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:?}"); + // 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".