From 549391a50da435aef273cbcba938f53cfb03725f Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Tue, 10 Nov 2026 19:20:56 -0800 Subject: [PATCH] Menus: a scrolling list keeps each label on one line A popup measures its list on the first frame, when the scroll area does not yet show its bar. When the bar then appeared, its column took its width from the items, and the widest label wrapped onto two lines: Distort & Transform in the Add New Effect menu, and Black and White Logo in the Image Trace presets, in short windows. menu_scroll now lays the items out at least as wide as on the popup's first frame, so the bar's column widens the popup instead. That width is kept until the popup closes and is never wider than the window has room for. The items are laid out in the same child Ui on every frame, so their ids do not change when the width does. --- crates/ui-egui/src/panels/appearance/tests.rs | 17 ++- .../ui-egui/src/panels/image_trace/tests.rs | 18 +++- .../ui-egui/src/panels/tests_long_popups.rs | 100 +++++++++++++++++- crates/ui-egui/src/widgets/dropdowns.rs | 34 ++++-- 4 files changed, 158 insertions(+), 11 deletions(-) diff --git a/crates/ui-egui/src/panels/appearance/tests.rs b/crates/ui-egui/src/panels/appearance/tests.rs index d3050ad5e..27bae8fcf 100644 --- a/crates/ui-egui/src/panels/appearance/tests.rs +++ b/crates/ui-egui/src/panels/appearance/tests.rs @@ -1,5 +1,6 @@ use super::super::tests_long_popups::{ - PopupWindow, assert_last_row_scrolls_into_view, assert_popup_stays_put, assert_short_list_does_not_scroll, fx_menu_ends, + PopupWindow, assert_last_row_scrolls_into_view, assert_popup_stays_put, assert_rows_stay_on_one_line, assert_short_list_does_not_scroll, + fx_menu_ends, }; use super::*; @@ -65,6 +66,20 @@ fn a_short_swatch_picker_does_not_scroll() { assert_eq!(window.clickable(TILE), tiles, "a list that fits does not scroll"); } +/// The Add New Effect popup keeps each group's name on one line when its list scrolls: the scroll +/// bar's column widens the popup instead of narrowing the rows. +#[test] +fn the_add_effect_popup_keeps_its_rows_on_one_line_when_it_scrolls() { + let mut groups: Vec = vec![]; + for effect in vectorcraft_effects::effect_catalog() { + let group = effect.menu.get(1).copied().unwrap_or("Other").to_string(); + if !groups.contains(&group) { + groups.push(group); + } + } + assert_rows_stay_on_one_line(&mut app_with_a_selected_rectangle(), draw_bottom_bar, fx_button, &groups, "Distort & Transform", 120.0); +} + /// The Add New Effect button: the third of the bottom bar's 24 pt buttons. fn fx_button(window: &PopupWindow) -> egui::Pos2 { window.clickable(vec2(24.0, 24.0))[2].center() diff --git a/crates/ui-egui/src/panels/image_trace/tests.rs b/crates/ui-egui/src/panels/image_trace/tests.rs index 7e797cead..1586943c0 100644 --- a/crates/ui-egui/src/panels/image_trace/tests.rs +++ b/crates/ui-egui/src/panels/image_trace/tests.rs @@ -1,4 +1,6 @@ -use super::super::tests_long_popups::{PopupWindow, assert_last_row_scrolls_into_view, assert_short_list_does_not_scroll}; +use super::super::tests_long_popups::{ + PopupWindow, assert_last_row_scrolls_into_view, assert_rows_stay_on_one_line, assert_short_list_does_not_scroll, +}; use super::*; use crate::panels::library_panel; use egui::{Event, PointerButton, Pos2, Rect, vec2}; @@ -66,6 +68,20 @@ fn a_long_tracing_preset_list_scrolls_inside_a_short_window() { assert_last_row_scrolls_into_view(&mut app, 200.0, draw, tracing_presets_arrow, &first, "Saved 29"); } +/// The list of presets keeps each name on one line when it scrolls: the scroll bar's column widens +/// the list instead of narrowing the rows. +#[test] +fn a_long_tracing_preset_list_keeps_its_rows_on_one_line() { + let mut app = VectorcraftApp::new(vectorcraft_engine::Session::new(), Default::default()); + let draw = |app: &mut VectorcraftApp, ui: &mut Ui| trace_button(app, ui, TRACE_BUTTON_W); + for n in 0..30 { + app.session.execute("imageTrace.presets.save", &json!({"name": format!("Saved {n}"), "preset": "Default"})).unwrap(); + } + let presets = app.session.execute("imageTrace.presets", &json!({})).unwrap(); + let names: Vec = presets["presets"].as_array().unwrap().iter().map(|p| p["name"].as_str().unwrap().to_string()).collect(); + assert_rows_stay_on_one_line(&mut app, draw, tracing_presets_arrow, &names, "Black and White Logo", 120.0); +} + #[test] fn a_large_image_traces_and_the_status_bar_says_it_is_slower() { let mut app = VectorcraftApp::new(vectorcraft_engine::Session::new(), Default::default()); diff --git a/crates/ui-egui/src/panels/tests_long_popups.rs b/crates/ui-egui/src/panels/tests_long_popups.rs index f12e91ab7..c562d3540 100644 --- a/crates/ui-egui/src/panels/tests_long_popups.rs +++ b/crates/ui-egui/src/panels/tests_long_popups.rs @@ -21,6 +21,7 @@ type Draw<'a> = Box; pub(super) struct PopupWindow<'a> { ctx: egui::Context, time: f64, + width: f32, height: f32, draw: Draw<'a>, texts: Vec<(String, Rect)>, @@ -30,7 +31,7 @@ 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, height, draw: Box::new(draw), texts: vec![] }; + let mut window = Self { ctx, time: 0.0, width: WIDTH, height, draw: Box::new(draw), texts: vec![] }; window.frames(app, 2); window } @@ -38,7 +39,7 @@ impl<'a> PopupWindow<'a> { /// One frame with `events`. fn frame(&mut self, app: &mut VectorcraftApp, events: Vec) { self.time += 1.0 / 60.0; - let screen_rect = Some(Rect::from_min_size(Pos2::ZERO, vec2(WIDTH, self.height))); + let screen_rect = Some(Rect::from_min_size(Pos2::ZERO, vec2(self.width, self.height))); let input = egui::RawInput { time: Some(self.time), screen_rect, events, ..Default::default() }; let draw = &mut self.draw; let mut out = self.ctx.run_ui(input, |ui| draw(app, ui)); @@ -46,6 +47,12 @@ impl<'a> PopupWindow<'a> { self.texts = crate::tests_removeanchors::shapes_text(&out.shapes.iter().map(|c| c.shape.clone()).collect::>()); } + /// Makes the window `width` pt wide and draws the frames that takes. + pub(super) fn resize(&mut self, app: &mut VectorcraftApp, width: f32) { + self.width = width; + self.frames(app, OPEN_FRAMES); + } + fn frames(&mut self, app: &mut VectorcraftApp, count: usize) { for _ in 0..count { self.frame(app, vec![]); @@ -58,6 +65,14 @@ impl<'a> PopupWindow<'a> { self.texts.iter().rfind(|(text, _)| text.trim() == wanted).map(|(_, rect)| *rect) } + /// The rect of the popup drawn on top: the last shown area in the foreground. + pub(super) fn popup(&self) -> Option { + self.ctx.memory(|m| { + let top = m.layer_ids().filter(|layer| layer.order == egui::Order::Foreground && m.areas().is_visible(layer)).last()?; + m.area_rect(top.id) + }) + } + /// The rects of the last frame's clickable widgets `size` across, in the order they were drawn. pub(super) fn clickable(&self, size: egui::Vec2) -> Vec { self.ctx.viewport(|vp| { @@ -81,9 +96,14 @@ impl<'a> PopupWindow<'a> { /// Turns the wheel far down with the pointer at `over`, and lets the scrolling finish. pub(super) fn wheel_down(&mut self, app: &mut VectorcraftApp, over: Pos2) { + self.wheel(app, over, 5000.0); + } + + /// Turns the wheel `down` pt down with the pointer at `over`, and lets the scrolling finish. + pub(super) fn wheel(&mut self, app: &mut VectorcraftApp, over: Pos2, down: f32) { let wheel = Event::MouseWheel { unit: egui::MouseWheelUnit::Point, - delta: vec2(0.0, -5000.0), + delta: vec2(0.0, -down), phase: egui::TouchPhase::Move, modifiers: Default::default(), source: egui::MouseWheelSource::Unknown, @@ -189,6 +209,80 @@ pub(super) fn assert_last_row_scrolls_into_view( assert!(row.bottom() <= height, "{last} at {row:?} is inside the {height} pt window"); } +/// In windows 200, 400 and 900 pt tall, the popup of the button `pick` finds draws each of its rows +/// `rows` that are in view on one line, as tall as the row with the shortest name, and left of the +/// 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. +pub(super) fn assert_rows_stay_on_one_line( + app: &mut VectorcraftApp, + mut draw: impl FnMut(&mut VectorcraftApp, &mut Ui), + pick: impl Fn(&PopupWindow) -> Pos2, + rows: &[String], + widest: &str, + narrow: f32, +) { + let bar = crate::widgets::list_scroll_style().allocated_width(); + let in_view = + |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")); + for (row, rect) in drawn { + assert!( + rect.left() >= popup.left() && rect.right() <= popup.right() - bar, + "{row} at {rect:?} is drawn left of the scroll bar's column in the popup at {popup:?} in the {name} window" + ); + } + popup + }; + for height in [200.0, 400.0, TALL] { + let mut window = PopupWindow::new(app, height, |app, ui| draw(app, ui)); + let button = pick(&window); + window.click(app, button); + for _ in 0..rows.len() { + if window.text(widest).is_some() { + break; + } + let drawn = in_view(&window); + let Some((_, middle)) = drawn.get(drawn.len() / 2) else { break }; + window.wheel(app, middle.center(), 20.0); + } + let opened = window.text(widest).unwrap_or_else(|| panic!("the popup's row {widest} is in view in the {height} pt window")); + let drawn = in_view(&window); + assert!(drawn.len() >= 3, "the popup's rows are in view in the {height} pt window: {drawn:?}"); + let Some((shortest, line)) = drawn.iter().min_by_key(|(row, _)| row.chars().count()) else { panic!("no rows in view") }; + let line = line.height(); + for (row, rect) in &drawn { + assert!( + rect.height() <= line + 0.5, + "{row} at {rect:?} is drawn on one line, as tall as {shortest} ({line} pt), in the {height} pt window" + ); + } + for (row, rect) in &drawn { + assert!(rect.width() <= opened.width() + 0.5, "{widest} is drawn as wide as {row} at {rect:?} or wider in the {height} pt window"); + } + let popup = assert_inside(&window, &drawn, &format!("{height} pt")); + // The popup's size settles within a frame of opening; after that it stays the same. + window.frames(app, 30); + let settled = window.popup(); + assert!( + settled.is_some_and(|settled| (settled.width() - popup.width()).abs() <= 0.5 && settled.min == popup.min), + "the popup at {settled:?} stays where it opened, at {popup:?}, in the {height} pt window" + ); + window.frames(app, 300); + assert_eq!(window.text(widest), Some(opened), "{widest} keeps its place and size while the popup stays open in the {height} pt window"); + assert_eq!(window.popup(), settled, "the popup keeps its place and size while it stays open in the {height} pt window"); + if height == 200.0 { + window.resize(app, narrow); + let drawn = in_view(&window); + assert!(!drawn.is_empty(), "the popup's rows are in view in the {narrow} pt wide window"); + let popup = assert_inside(&window, &drawn, &format!("{narrow} pt wide")); + assert!(popup.left() >= 0.0, "the popup at {popup:?} starts inside the {narrow} pt wide window"); + } + } +} + /// In a tall window the popup of the button `pick` finds shows its rows `first` to `last` without /// scrolling, and the wheel moves neither. pub(super) fn assert_short_list_does_not_scroll( diff --git a/crates/ui-egui/src/widgets/dropdowns.rs b/crates/ui-egui/src/widgets/dropdowns.rs index 28ed7c6fb..44903d661 100644 --- a/crates/ui-egui/src/widgets/dropdowns.rs +++ b/crates/ui-egui/src/widgets/dropdowns.rs @@ -379,29 +379,51 @@ fn popup_anchor(ui: &Ui) -> Option<(Rect, bool)> { /// The body of a menu or popup list: as tall as its items up to the bottom bars of the window it /// opens in, and scrolling only past that, so long menus (Window, Effect, a panel's menu) stay /// 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). +/// 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. /// /// 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. pub fn menu_scroll(ui: &mut Ui, add_contents: impl FnOnce(&mut Ui) -> R) -> R { - let ctx = ui.ctx(); - let tallest = (ctx.content_rect().height() - egui::Frame::popup(ui.style()).total_margin().sum().y).max(0.0); + let ctx = ui.ctx().clone(); + let margins = egui::Frame::popup(ui.style()).total_margin().sum(); + let tallest = (ctx.content_rect().height() - margins.y).max(0.0); let top = ui.next_widget_position().y; // The rows above the list (the popup's margin, a search field) move with the popup. let popup_top = ui.stack().iter().last().map_or(top, |root| root.max_rect.top()); let ahead = top - popup_top; - let (floor, window_top) = (menu_floor(ctx), ctx.content_rect().top()); + let (floor, window_top) = (menu_floor(&ctx), ctx.content_rect().top()); let room = if let Some((anchor, submenu)) = popup_anchor(ui) { let (down_from, up_to) = if submenu { (anchor.top(), anchor.bottom()) } else { (anchor.bottom(), anchor.top()) }; menu_room_either(floor - down_from - ahead, up_to - window_top - ahead, tallest) } else { 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. + 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); ui.scope(|ui| { - ui.spacing_mut().scroll = list_scroll_style(); + ui.spacing_mut().scroll = scroll; // The minimum lets the list grow past a popup's default 400 pt size; it still shrinks to // its items when they need less. - egui::ScrollArea::vertical().max_height(room).min_scrolled_height(room).show(ui, add_contents).inner + let area = egui::ScrollArea::vertical().max_height(room).min_scrolled_height(room); + 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 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 width = first.unwrap_or(out.content_size.x); + ctx.data_mut(|d| d.insert_temp(key, (frame, width))); + out.inner }) .inner }