From 726fbfe3271294d9b9790fe3d8c24dfdd930d1fe Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 14 Nov 2026 18:48:39 -0800 Subject: [PATCH] Menus: long popup rows are shortened with an ellipsis so the popup stays inside a narrow window In a window narrower than a popup list's rows (Add New Effect, Tracing Presets and the other lists built on menu_scroll), the list kept its rows at full width and the popup ran past the window's right edge. The list is now no wider than the window allows; a row that does not fit ends in an ellipsis and shows its full text in a tooltip, dimmed rows included. Screen readers and clicks still use the row's full text. Menus that ask for a minimum width (the menu bar's 230 pt, submenus and context menus) take the window's width when it is less, through menu_min_width. The long-popup tests now check that the popup ends inside the window, and a new test covers a shortened row's tooltip, screen-reader name and click. --- crates/ui-egui/src/chrome/control_bar.rs | 2 +- crates/ui-egui/src/custom_toolbars.rs | 2 +- crates/ui-egui/src/menus/bar.rs | 4 +- crates/ui-egui/src/menus/context.rs | 4 +- crates/ui-egui/src/panels.rs | 2 +- crates/ui-egui/src/panels/color_guide.rs | 2 +- crates/ui-egui/src/panels/gradient.rs | 2 +- .../ui-egui/src/panels/tests_long_popups.rs | 80 +++++++++++++++++- crates/ui-egui/src/widgets.rs | 2 +- crates/ui-egui/src/widgets/dropdowns.rs | 84 +++++++++++++++++-- 10 files changed, 165 insertions(+), 19 deletions(-) diff --git a/crates/ui-egui/src/chrome/control_bar.rs b/crates/ui-egui/src/chrome/control_bar.rs index fc06926247..cd31acd8c9 100644 --- a/crates/ui-egui/src/chrome/control_bar.rs +++ b/crates/ui-egui/src/chrome/control_bar.rs @@ -573,7 +573,7 @@ fn dock(app: &mut VectorcraftApp, edge: &str) { fn panel_menu(app: &mut VectorcraftApp, ui: &mut Ui, rect: egui::Rect) { let resp = crate::panels::menu_button(ui, "control", rect); egui::Popup::menu(&resp).show(|ui| { - ui.set_min_width(160.0); + widgets::menu_min_width(ui, 160.0); widgets::menu_scroll(ui, |ui| { for group in CONTROL_BAR_GROUPS { if widgets::menu_item(ui, group_label(group), true, group_on(app, group)) { diff --git a/crates/ui-egui/src/custom_toolbars.rs b/crates/ui-egui/src/custom_toolbars.rs index 31f83c8bba..f8c3033a5e 100644 --- a/crates/ui-egui/src/custom_toolbars.rs +++ b/crates/ui-egui/src/custom_toolbars.rs @@ -1377,7 +1377,7 @@ pub(crate) fn add_menu_id(name: &str) -> egui::Id { /// bottom bars, as other long menus do ([`widgets::menu_scroll`]). fn add_menu(button: &egui::Response, t: &Tokens, bar: &CustomToolbar, actions: &mut Vec) { egui::Popup::menu(button).id(add_menu_id(&bar.name)).show(|ui| { - ui.set_min_width(200.0); + widgets::menu_min_width(ui, 200.0); widgets::menu_scroll(ui, |ui| { let held: Vec<&'static str> = bar.slots.iter().flatten().filter_map(|id| tool_info(id).map(|t| t.id)).collect(); let heading = |ui: &mut egui::Ui, text: &str| ui.label(egui::RichText::new(text).size(12.0).color(t.text_strong)); diff --git a/crates/ui-egui/src/menus/bar.rs b/crates/ui-egui/src/menus/bar.rs index 575f11f74f..85588620e8 100644 --- a/crates/ui-egui/src/menus/bar.rs +++ b/crates/ui-egui/src/menus/bar.rs @@ -117,7 +117,7 @@ fn switch_on_hover(ctx: &egui::Context, titles: &[egui::Response]) -> bool { /// it scrolls when it is taller than the window. pub(super) fn menu_body(app: &VectorcraftApp, ui: &mut egui::Ui, items: &[Item], clicked: &mut Option<(String, Value)>) { widgets::menu_scroll(ui, |ui| { - ui.set_min_width(230.0); + widgets::menu_min_width(ui, 230.0); render_items(app, ui, items, true, clicked); }); } @@ -137,7 +137,7 @@ pub(super) fn render_items(app: &VectorcraftApp, ui: &mut egui::Ui, items: &[Ite Entry::Sub(label, children) => { ui.menu_button(label, |ui| { widgets::menu_scroll(ui, |ui| { - ui.set_min_width(200.0); + widgets::menu_min_width(ui, 200.0); render_items(app, ui, children, checks || only_toggles(app, children), clicked); }); }); diff --git a/crates/ui-egui/src/menus/context.rs b/crates/ui-egui/src/menus/context.rs index bc11e703e0..68b2a93574 100644 --- a/crates/ui-egui/src/menus/context.rs +++ b/crates/ui-egui/src/menus/context.rs @@ -272,7 +272,7 @@ fn prune(app: &VectorcraftApp, items: Vec) -> Vec { /// The canvas context menu's popup; the item clicked goes in `clicked`, for [`invoke`]. pub fn context_menu_body(app: &VectorcraftApp, ui: &mut egui::Ui, clicked: &mut Option<(String, Value)>) { widgets::menu_scroll(ui, |ui| { - ui.set_min_width(200.0); + widgets::menu_min_width(ui, 200.0); // Its toggles already say what they do (Show Rulers, Hide Guides): no check-mark gutter, // except in a submenu of toggles alone (Spelling › Auto Spell Check). render_items(app, ui, &context_items(app), false, clicked); @@ -289,7 +289,7 @@ pub fn ruler_unit_items() -> Vec { /// `clicked`, for [`invoke`]. pub fn ruler_menu_body(app: &VectorcraftApp, ui: &mut egui::Ui, clicked: &mut Option<(String, Value)>) { widgets::menu_scroll(ui, |ui| { - ui.set_min_width(160.0); + widgets::menu_min_width(ui, 160.0); render_items(app, ui, &ruler_unit_items(), true, clicked); }); } diff --git a/crates/ui-egui/src/panels.rs b/crates/ui-egui/src/panels.rs index bbe91da5a6..8c5b257327 100644 --- a/crates/ui-egui/src/panels.rs +++ b/crates/ui-egui/src/panels.rs @@ -265,7 +265,7 @@ pub fn panel_menu(app: &mut VectorcraftApp, ui: &mut Ui, id: &str, rect: Rect) { let resp = menu_button(ui, id, rect); egui::Popup::menu(&resp).show(|ui| { crate::widgets::menu_scroll(ui, |ui| { - ui.set_min_width(220.0); + crate::widgets::menu_min_width(ui, 220.0); if panel_menu_items(app, ui, id) { ui.separator(); } diff --git a/crates/ui-egui/src/panels/color_guide.rs b/crates/ui-egui/src/panels/color_guide.rs index 3ef2fc75e4..510b87cde5 100644 --- a/crates/ui-egui/src/panels/color_guide.rs +++ b/crates/ui-egui/src/panels/color_guide.rs @@ -99,7 +99,7 @@ pub(crate) fn set_limit(app: &mut VectorcraftApp, p: &Value) -> Result { height: f32, draw: Draw<'a>, texts: Vec<(String, Rect)>, + /// The full texts of the last frame's texts drawn ending in an ellipsis. + shortened: Vec, + /// The last frame's screen-reader nodes, when the window's context has them turned on. + nodes: Vec, } impl<'a> PopupWindow<'a> { pub(super) fn new(app: &mut VectorcraftApp, height: f32, draw: impl FnMut(&mut VectorcraftApp, &mut Ui) + 'a) -> Self { let ctx = egui::Context::default(); crate::theme::install_fonts(&ctx); - let mut window = Self { ctx, time: 0.0, width: WIDTH, height, draw: Box::new(draw), texts: vec![] }; + let mut window = Self { ctx, time: 0.0, width: WIDTH, height, draw: Box::new(draw), texts: vec![], shortened: vec![], nodes: vec![] }; window.frames(app, 2); window } @@ -45,6 +49,15 @@ impl<'a> PopupWindow<'a> { let mut out = self.ctx.run_ui(input, |ui| draw(app, ui)); out.textures_delta.clear(); self.texts = crate::tests_removeanchors::shapes_text(&out.shapes.iter().map(|c| c.shape.clone()).collect::>()); + self.shortened = out + .shapes + .iter() + .filter_map(|c| match &c.shape { + egui::Shape::Text(text) if text.galley.elided => Some(text.galley.text().to_string()), + _ => None, + }) + .collect(); + self.nodes = out.platform_output.accesskit_update.map(|u| u.nodes.into_iter().map(|(_, n)| n).collect()).unwrap_or_default(); } /// Makes the window `width` pt wide and draws the frames that takes. @@ -214,7 +227,7 @@ pub(super) fn assert_last_row_scrolls_into_view( /// scroll bar's column, whether or not its list scrolls. The row `widest` is drawn the widest; the /// wheel brings it into view when the popup opens without it. The popup and that row keep their /// place and size while the popup stays open. After the 200 pt window becomes `narrow` pt wide, -/// the popup and its rows in view stay inside the window. +/// the popup, its scroll bar and its rows in view stay inside the window. pub(super) fn assert_rows_stay_on_one_line( app: &mut VectorcraftApp, mut draw: impl FnMut(&mut VectorcraftApp, &mut Ui), @@ -228,6 +241,7 @@ pub(super) fn assert_rows_stay_on_one_line( |window: &PopupWindow| -> Vec<(String, Rect)> { rows.iter().filter_map(|row| window.text(row).map(|rect| (row.clone(), rect))).collect() }; let assert_inside = |window: &PopupWindow, drawn: &[(String, Rect)], name: &str| { let popup = window.popup().unwrap_or_else(|| panic!("the popup is open in the {name} window")); + assert!(popup.right() <= window.width + 0.5, "the popup at {popup:?} ends inside the {name} window, at x = {}", window.width); for (row, rect) in drawn { assert!( rect.left() >= popup.left() && rect.right() <= popup.right() - bar, @@ -333,6 +347,68 @@ fn app() -> VectorcraftApp { VectorcraftApp::new(vectorcraft_engine::Session::new(), Default::default()) } +/// A row's name too long for the narrow window in [`a_shortened_menu_row_keeps_its_full_text`]. +const LONG_ROW: &str = "A row whose name is far too long for a narrow window"; +/// A dimmed row's name too long for the same window. +const DIMMED_ROW: &str = "A dimmed row whose name is also far too long"; + +/// In a window too narrow for a menu's row, the menu stays inside the window and the row ends in +/// an ellipsis. Screen readers get the row's full text, a click still chooses it, and resting the +/// pointer on it shows the full text in a tooltip inside the window, as it does on a dimmed row. A +/// row that fits is drawn in full and shows no tooltip. +#[test] +fn a_shortened_menu_row_keeps_its_full_text() { + let clicked = std::cell::Cell::new(false); + let draw = |_: &mut VectorcraftApp, ui: &mut Ui| { + let button = ui.button("Open"); + egui::Popup::menu(&button).show(|ui| { + crate::widgets::menu_scroll(ui, |ui| { + // More than the window has, as the menu bar's menus ask for. + crate::widgets::menu_min_width(ui, 230.0); + if ui.button(LONG_ROW).clicked() { + clicked.set(true); + } + let _ = ui.button("Short"); + let _ = ui.add_enabled(false, egui::Button::new(DIMMED_ROW)); + }) + }); + }; + let mut app = app(); + let mut window = PopupWindow::new(&mut app, 300.0, draw); + window.ctx.enable_accesskit(); + window.resize(&mut app, 160.0); + let at = window.text("Open").expect("the button").center(); + window.click(&mut app, at); + let popup = window.popup().expect("the menu is open"); + assert!(popup.left() >= 0.0 && popup.right() <= 160.5, "the menu at {popup:?} is inside the 160 pt wide window"); + let row = window.text(LONG_ROW).expect("the long row"); + assert!(row.right() <= popup.right(), "the long row at {row:?} ends inside the menu at {popup:?}"); + assert!(window.shortened.iter().any(|text| text == LONG_ROW), "the long row ends in an ellipsis: {:?}", window.shortened); + assert!(!window.shortened.iter().any(|text| text == "Short"), "the short row is drawn in full"); + assert!(window.nodes.iter().any(|node| node.label() == Some(LONG_ROW)), "screen readers get the long row's full text"); + let drawn = |window: &PopupWindow, wanted: &str| window.texts.iter().filter(|(text, _)| text == wanted).count(); + assert_eq!(drawn(&window, LONG_ROW), 1, "no tooltip shows before the pointer rests on the row"); + + window.frame(&mut app, vec![Event::PointerMoved(window.text("Short").expect("the short row").center())]); + window.frames(&mut app, 60); + assert_eq!(drawn(&window, "Short"), 1, "the short row shows no tooltip"); + + window.frame(&mut app, vec![Event::PointerMoved(row.center())]); + window.frames(&mut app, 60); + assert_eq!(drawn(&window, LONG_ROW), 2, "resting the pointer on the long row shows its full text in a tooltip"); + let tip = window.texts.iter().filter(|(text, _)| text == LONG_ROW).map(|(_, rect)| *rect).find(|rect| *rect != row); + assert!(tip.is_some_and(|tip| tip.left() >= 0.0 && tip.right() <= 160.5), "the tooltip at {tip:?} is inside the window"); + + assert!(window.shortened.iter().any(|text| text == DIMMED_ROW), "the dimmed row ends in an ellipsis"); + let dimmed = window.text(DIMMED_ROW).expect("the dimmed row"); + window.frame(&mut app, vec![Event::PointerMoved(dimmed.center())]); + window.frames(&mut app, 60); + assert_eq!(drawn(&window, DIMMED_ROW), 2, "resting the pointer on the dimmed row shows its full text in a tooltip"); + + window.click(&mut app, row.center()); + assert!(clicked.get(), "a click on the shortened row chooses it"); +} + /// A long menu opened from a button `y` pt down a window stays at one place on every frame, and so /// does its submenu, whether it opens below the button, above it, or in a window shorter than it. #[test] diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index 36145224d5..0738144ade 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -45,7 +45,7 @@ pub use drag::{ }; #[cfg(test)] use dropdowns::combo_button_id; -pub(crate) use dropdowns::{combo, combo_menu, combo_menu_field_named, combo_named, field_focused_in, popover_is_open}; +pub(crate) use dropdowns::{combo, combo_menu, combo_menu_field_named, combo_named, field_focused_in, menu_min_width, popover_is_open}; pub use dropdowns::{ dropdown, dropdown_icon_names, dropdown_named, dropdown_names, dropdown_names_named, dropdown_with, dropdown_with_named, list_scroll_style, menu_room, menu_scroll, panel_scroll, popover, set_menu_floor, strip_scroll, diff --git a/crates/ui-egui/src/widgets/dropdowns.rs b/crates/ui-egui/src/widgets/dropdowns.rs index 1539be8700..7f088f25ef 100644 --- a/crates/ui-egui/src/widgets/dropdowns.rs +++ b/crates/ui-egui/src/widgets/dropdowns.rs @@ -443,7 +443,9 @@ fn popup_anchor(ui: &Ui) -> Option<(Rect, bool)> { /// reachable on small windows and in a floating panel's short OS window. Its scroll bar has a column /// of its own, so it never covers the shortcuts or the submenu arrows (#2086, #5347). When the list /// scrolls, that column widens the popup instead of narrowing the items, so their labels stay on one -/// line. +/// line. In a window too narrow for the items, the list is as wide as the window allows, and a row +/// that does not fit ends in an ellipsis and shows its full text in a tooltip. Screen readers and +/// keyboard selection still use the full text. /// /// The list's height comes from the room below and above the button, row or point the popup opens /// from. A popup with an id of its own measures from its own top instead. @@ -463,14 +465,13 @@ pub fn menu_scroll(ui: &mut Ui, add_contents: impl FnOnce(&mut Ui) -> R) -> R menu_room(floor, top, tallest) }; // The items' width on the first frame of this showing, when the scroll area does not yet show - // its bar. Later frames lay the items out at least that wide, and the bar's column is added to - // the popup's width. The width is kept until the popup closes. + // its bar. Later frames lay the items out that wide, up to the widest the window allows, and + // the bar's column is added to the popup's width. The width is kept until the popup closes. let key = ui.next_auto_id().with("menu-scroll-width"); let frame = ctx.cumulative_frame_nr(); let first = ctx.data(|d| d.get_temp::<(u64, f32)>(key)).filter(|(shown, _)| shown + 1 >= frame).map(|(_, width)| width); let scroll = list_scroll_style(); - // The widest the items can be with the popup's margins and the bar's column inside the window. - let widest = (ctx.content_rect().width() - margins.x - scroll.allocated_width()).max(0.0); + let widest = menu_list_widest(ui); ui.scope(|ui| { ui.spacing_mut().scroll = scroll; // The minimum lets the list grow past a popup's default 400 pt size; it still shrinks to @@ -479,9 +480,20 @@ pub fn menu_scroll(ui: &mut Ui, add_contents: impl FnOnce(&mut Ui) -> R) -> R let out = area.show(ui, |ui| { // Always the same child, so the items' ids stay the same on the frame the width changes. let max = ui.max_rect(); - let width = first.map_or(max.width(), |first| first.min(widest).max(max.width())); + let width = first.map_or(max.width(), |first| first.max(max.width()).min(widest)); let rect = egui::Rect::from_min_size(max.min, egui::vec2(width, max.height())); - ui.scope_builder(egui::UiBuilder::new().max_rect(rect).layout(*ui.layout()), add_contents).inner + let painted = ctx.graphics(|g| g.get(ui.layer_id()).map_or(0, |list| list.all_entries().len())); + let inner = ui + .scope_builder(egui::UiBuilder::new().max_rect(rect).layout(*ui.layout()), |ui| { + // The first frame measures the items at their full width. + if first.is_some() { + ui.style_mut().wrap_mode = Some(egui::TextWrapMode::Truncate); + } + add_contents(ui) + }) + .inner; + show_shortened_rows(ui, painted); + inner }); let width = first.unwrap_or(out.content_size.x); ctx.data_mut(|d| d.insert_temp(key, (frame, width))); @@ -490,6 +502,64 @@ pub fn menu_scroll(ui: &mut Ui, add_contents: impl FnOnce(&mut Ui) -> R) -> R .inner } +/// The widest the items of a [`menu_scroll`] list can be with the popup's margins and the scroll +/// bar's column inside the window. +fn menu_list_widest(ui: &Ui) -> f32 { + let margins = egui::Frame::popup(ui.style()).total_margin().sum(); + (ui.ctx().content_rect().width() - margins.x - list_scroll_style().allocated_width()).max(0.0) +} + +/// Makes a menu at least `width` wide, or as wide as the window allows the items of a +/// [`menu_scroll`] list when that is less, so the menu stays inside a narrow window. +pub(crate) fn menu_min_width(ui: &mut Ui, width: f32) { + ui.set_min_width(width.min(menu_list_widest(ui))); +} + +/// For each text that `ui`'s layer drew after its first `painted` shapes and that ends in an +/// ellipsis, the clickable row it is drawn in shows the full text in a tooltip that wraps to fit +/// inside the window. +fn show_shortened_rows(ui: &Ui, painted: usize) { + let ctx = ui.ctx(); + let layer = ui.layer_id(); + let shortened: Vec<(egui::Pos2, egui::text::LayoutJob)> = ctx.graphics(|g| { + let Some(list) = g.get(layer) else { return vec![] }; + list.all_entries() + .skip(painted) + .filter_map(|clipped| match &clipped.shape { + egui::Shape::Text(text) if text.galley.elided => { + let job = egui::text::LayoutJob { + sections: text.galley.job.sections.clone(), + text: text.galley.job.text.clone(), + language: text.galley.job.language.clone(), + ..Default::default() + }; + Some((Rect::from_min_size(text.pos, text.galley.size()).center(), job)) + } + _ => None, + }) + .collect() + }); + if shortened.is_empty() { + return; + } + let rows: Vec<(egui::Id, Rect)> = + ctx.viewport(|vp| vp.this_pass.widgets.get_layer(layer).filter(|w| w.sense.senses_click()).map(|w| (w.id, w.rect)).collect()); + for (at, job) in shortened { + let Some(response) = rows.iter().rfind(|(_, rect)| rect.contains(at)).and_then(|(id, _)| ctx.read_response(*id)) else { continue }; + let tip = |ui: &mut Ui| { + let margins = ui.spacing().menu_margin.sum().x + 2.0 * ui.spacing().item_spacing.x; + ui.set_max_width(ui.spacing().tooltip_width.min(ui.ctx().content_rect().width() - margins).max(0.0)); + ui.add(egui::Label::new(job).wrap()); + }; + // A dimmed row shows its full text too. + if response.enabled() { + response.on_hover_ui(tip); + } else { + response.on_disabled_hover_ui(tip); + } + } +} + /// A column of tool or panel buttons (the toolbar, the dock's icon column) that scrolls when the /// window is too short for it: with the mouse wheel, or the scroll bar shown while the pointer is /// over it.