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 }