From 831d916d3d7dc9b0dbd9beef06e54ebe290680df Mon Sep 17 00:00:00 2001 From: bflatastic Date: Tue, 6 Oct 2026 00:03:11 +0700 Subject: [PATCH] Fix #84: the toolbar and the panel icon column scroll in short windows In a window too short for them, the toolbar (tools, Fill/Stroke, colour chips, drawing modes, Screen Mode, Edit Toolbar) and the dock's panel icon column scroll with the mouse wheel, or with the scroll bar that shows while the pointer is over them, instead of being cut off at the hint bar. Root cause: both were drawn straight into their side panels with no scroll area, and the toolbar's only fallback (two columns) runs out below about 760 pt. They now draw inside widgets::strip_scroll, a vertical ScrollArea next to the menus' widgets::menu_scroll. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/dock.rs | 56 ++++++++-- crates/ui-egui/src/toolbar.rs | 189 +++++++++++++++++++++------------- crates/ui-egui/src/widgets.rs | 7 ++ 3 files changed, 168 insertions(+), 84 deletions(-) diff --git a/crates/ui-egui/src/dock.rs b/crates/ui-egui/src/dock.rs index 67ef44ce..f6232ed8 100644 --- a/crates/ui-egui/src/dock.rs +++ b/crates/ui-egui/src/dock.rs @@ -62,19 +62,22 @@ pub fn show(app: &mut VectorcraftApp, ui: &mut Ui) { .frame(egui::Frame::NONE.fill(t.panel).inner_margin(egui::Margin::symmetric(4, 6)).stroke(Stroke::new(1.5, t.border))) .show(ui, |ui| { ui.spacing_mut().item_spacing.y = 2.0; - for (gi, group) in ICON_PANEL_GROUPS.iter().enumerate() { - if gi > 0 { - let (r, _) = ui.allocate_exact_size(vec2(ui.available_width(), 7.0), Sense::hover()); - ui.painter().line_segment([r.left_center() + vec2(4.0, 0.0), r.right_center() - vec2(4.0, 0.0)], Stroke::new(1.0, t.divider)); - } - for id in group.iter() { - let Some((_, label, icon)) = ICON_PANELS.iter().find(|p| p.0 == *id) else { continue }; - let open = app.ui.open_panel.as_deref() == Some(*id); - if widgets::icon_button(ui, icon, label, open, 30.0).clicked() { - app.ui.open_panel = if open { None } else { Some(id.to_string()) }; + // The icons scroll in a window too short for them. + widgets::strip_scroll(ui, "icon_column", |ui| { + for (gi, group) in ICON_PANEL_GROUPS.iter().enumerate() { + if gi > 0 { + let (r, _) = ui.allocate_exact_size(vec2(ui.available_width(), 7.0), Sense::hover()); + ui.painter().line_segment([r.left_center() + vec2(4.0, 0.0), r.right_center() - vec2(4.0, 0.0)], Stroke::new(1.0, t.divider)); + } + for id in group.iter() { + let Some((_, label, icon)) = ICON_PANELS.iter().find(|p| p.0 == *id) else { continue }; + let open = app.ui.open_panel.as_deref() == Some(*id); + if widgets::icon_button(ui, icon, label, open, 30.0).clicked() { + app.ui.open_panel = if open { None } else { Some(id.to_string()) }; + } } } - } + }); }); } @@ -115,3 +118,34 @@ pub fn floating_panel(app: &mut VectorcraftApp, ctx: &egui::Context) { app.ui.open_panel = None; } } + +#[cfg(test)] +mod tests { + use serde_json::json; + use vectorcraft_engine::Session; + + use super::*; + use crate::toolbar::tests::{wheel, widget_rects}; + + #[test] + fn the_icon_column_scrolls_in_a_short_window() { + let mut app = VectorcraftApp::new(Session::new(), Default::default()); + app.run("file.new", json!({"width": 300, "height": 300})).unwrap(); + let ctx = egui::Context::default(); + theme::install_fonts(&ctx); + let mut frame = |time: f64, events| { + let screen = egui::Rect::from_min_size(egui::Pos2::ZERO, vec2(900.0, 400.0)); + let input = egui::RawInput { time: Some(time), events, screen_rect: Some(screen), ..Default::default() }; + ctx.run_ui(input, |ui| show(&mut app, ui)).textures_delta.clear(); + widget_rects(&ctx, vec2(30.0, 30.0)) + }; + let icons = frame(0.0, vec![]); + assert!(icons.last().unwrap().bottom() > 400.0, "the last panel icon starts below the window"); + frame(0.1, wheel(icons[0].center(), 2000.0)); + let mut icons = vec![]; + for k in 2..40 { + icons = frame(f64::from(k) * 0.1, vec![]); + } + assert!(icons.last().unwrap().bottom() <= 400.0, "scrolled into view: {:?}", icons.last()); + } +} diff --git a/crates/ui-egui/src/toolbar.rs b/crates/ui-egui/src/toolbar.rs index 2fcd264c..cab91cd4 100644 --- a/crates/ui-egui/src/toolbar.rs +++ b/crates/ui-egui/src/toolbar.rs @@ -106,77 +106,84 @@ pub fn show(app: &mut VectorcraftApp, ui: &mut Ui) { Stroke::new(0.4, t.text_disabled), ); } - let active = app.session.tool_id(); - let mut open_flyout: Option<(Vec<&'static str>, egui::Rect)> = None; - let mut i = 0; - while i < all.len() { - if let Some(cat) = all[i].0 { - let (r, _) = ui.allocate_exact_size(vec2(ui.available_width(), 18.0), Sense::hover()); - let label = if cols == 1 && cat.len() > 6 { format!("{}...", &cat[..4]) } else { cat.to_string() }; - ui.painter().text(r.center() + vec2(0.0, 2.0), egui::Align2::CENTER_CENTER, label, egui::FontId::proportional(11.0), t.text); - } - // One row = `cols` slots (a category label always starts a new row). - let mut row = vec![i]; - while row.len() < cols && i + row.len() < all.len() && all[i + row.len()].0.is_none() { - row.push(i + row.len()); - } - ui.horizontal(|ui| { - ui.spacing_mut().item_spacing.x = 0.0; - if cols == 1 { - ui.add_space((WIDTH - 36.0) / 2.0); - } else { - ui.add_space(2.0); + // The tools and the controls under them scroll in a window too short for them. + widgets::strip_scroll(ui, "toolbar", |ui| { + let active = app.session.tool_id(); + let mut open_flyout: Option<(Vec<&'static str>, egui::Rect)> = None; + let mut i = 0; + while i < all.len() { + if let Some(cat) = all[i].0 { + let (r, _) = ui.allocate_exact_size(vec2(ui.available_width(), 18.0), Sense::hover()); + let label = if cols == 1 && cat.len() > 6 { format!("{}...", &cat[..4]) } else { cat.to_string() }; + ui.painter().text(r.center() + vec2(0.0, 2.0), egui::Align2::CENTER_CENTER, label, egui::FontId::proportional(11.0), t.text); } - for &k in &row { - let slot = &all[k].1; - let shown_id = if slot.contains(&active) { - active.to_string() + // One row = `cols` slots (a category label always starts a new row). + let mut row = vec![i]; + while row.len() < cols && i + row.len() < all.len() && all[i + row.len()].0.is_none() { + row.push(i + row.len()); + } + ui.horizontal(|ui| { + ui.spacing_mut().item_spacing.x = 0.0; + if cols == 1 { + ui.add_space((WIDTH - 36.0) / 2.0); } else { - app.ui.slot_tool.get(slot[0]).cloned().unwrap_or_else(|| slot[0].to_string()) - }; - let Some(shown) = tool_info(&shown_id).or_else(|| tool_info(slot[0])) else { continue }; - let is_active = slot.contains(&active); - let (rect, resp) = ui.allocate_exact_size(vec2(36.0, PITCH - 1.0), Sense::click_and_drag()); - let well = egui::Rect::from_center_size(rect.center(), vec2(35.5, 27.5)); - if is_active { - ui.painter().rect_filled(well, CornerRadius::same(1), t.tool_active); - } else if resp.hovered() { - ui.painter().rect_filled(well, CornerRadius::same(1), t.hover); + ui.add_space(2.0); } - let ir = egui::Rect::from_center_size(rect.center(), vec2(18.0, 18.0)); - icons::paint(ui, icons::tool_icon(shown.icon), ir, if is_active { t.text_strong } else { t.icon }); - if slot.len() > 1 { - let c = rect.center() + vec2(12.5, 10.0); - ui.painter().add(egui::Shape::convex_polygon(vec![c, c + vec2(-3.5, 0.0), c + vec2(0.0, -3.5)], t.icon, Stroke::NONE)); + for &k in &row { + let slot = &all[k].1; + let shown_id = if slot.contains(&active) { + active.to_string() + } else { + app.ui.slot_tool.get(slot[0]).cloned().unwrap_or_else(|| slot[0].to_string()) + }; + let Some(shown) = tool_info(&shown_id).or_else(|| tool_info(slot[0])) else { continue }; + let is_active = slot.contains(&active); + let (rect, resp) = ui.allocate_exact_size(vec2(36.0, PITCH - 1.0), Sense::click_and_drag()); + let well = egui::Rect::from_center_size(rect.center(), vec2(35.5, 27.5)); + if is_active { + ui.painter().rect_filled(well, CornerRadius::same(1), t.tool_active); + } else if resp.hovered() { + ui.painter().rect_filled(well, CornerRadius::same(1), t.hover); + } + let ir = egui::Rect::from_center_size(rect.center(), vec2(18.0, 18.0)); + icons::paint(ui, icons::tool_icon(shown.icon), ir, if is_active { t.text_strong } else { t.icon }); + if slot.len() > 1 { + let c = rect.center() + vec2(12.5, 10.0); + ui.painter().add(egui::Shape::convex_polygon( + vec![c, c + vec2(-3.5, 0.0), c + vec2(0.0, -3.5)], + t.icon, + Stroke::NONE, + )); + } + let long_press = resp.is_pointer_button_down_on() + && ui.input(|inp| inp.pointer.press_start_time().is_some_and(|s| inp.time - s > 0.35)); + let alt = ui.input(|inp| inp.modifiers.alt); + if (resp.secondary_clicked() || long_press) && slot.len() > 1 { + open_flyout = Some((slot.clone(), rect)); + } else if resp.clicked() && alt && slot.len() > 1 { + let idx = slot.iter().position(|x| *x == shown.id).unwrap_or(0); + app.select_tool(slot[(idx + 1) % slot.len()]); + } else if resp.double_clicked() { + app.select_tool(shown.id); + app.run("tool.options", json!({ "tool": shown.id })).ok(); + } else if resp.clicked() { + app.select_tool(shown.id); + } + resp.on_hover_text(tip(shown)); } - let long_press = - resp.is_pointer_button_down_on() && ui.input(|inp| inp.pointer.press_start_time().is_some_and(|s| inp.time - s > 0.35)); - let alt = ui.input(|inp| inp.modifiers.alt); - if (resp.secondary_clicked() || long_press) && slot.len() > 1 { - open_flyout = Some((slot.clone(), rect)); - } else if resp.clicked() && alt && slot.len() > 1 { - let idx = slot.iter().position(|x| *x == shown.id).unwrap_or(0); - app.select_tool(slot[(idx + 1) % slot.len()]); - } else if resp.double_clicked() { - app.select_tool(shown.id); - app.run("tool.options", json!({ "tool": shown.id })).ok(); - } else if resp.clicked() { - app.select_tool(shown.id); - } - resp.on_hover_text(tip(shown)); - } - }); - i += row.len(); - } - if let Some((slot, rect)) = open_flyout { - app.ui.flyout = Some(0); - ui.data_mut(|d| { - d.insert_temp(egui::Id::new("flyout-anchor"), rect); - d.insert_temp(egui::Id::new("flyout-tools"), slot.iter().map(|s| s.to_string()).collect::>()); - }); - } - ui.add_space(8.0); - bottom_controls(app, ui, &t); + }); + i += row.len(); + } + if let Some((slot, rect)) = open_flyout { + app.ui.flyout = Some(0); + ui.data_mut(|d| { + d.insert_temp(egui::Id::new("flyout-anchor"), rect); + d.insert_temp(egui::Id::new("flyout-tools"), slot.iter().map(|s| s.to_string()).collect::>()); + }); + } + ui.add_space(8.0); + bottom_controls(app, ui, &t); + }); }); flyout(app, ui.ctx()); } @@ -312,7 +319,7 @@ fn flyout(app: &mut VectorcraftApp, ctx: &egui::Context) { } #[cfg(test)] -mod tests { +pub(crate) mod tests { use egui::{Event, PointerButton, Pos2}; use vectorcraft_engine::Session; @@ -320,22 +327,58 @@ mod tests { /// One headless frame of the toolbar; returns the tool buttons' rects, top to bottom. fn frame(app: &mut VectorcraftApp, ctx: &egui::Context, time: f64, events: Vec) -> Vec { + frame_in(app, ctx, time, events, 1200.0) + } + + /// [`frame`] in a window `height` points tall. + fn frame_in(app: &mut VectorcraftApp, ctx: &egui::Context, time: f64, events: Vec, height: f32) -> Vec { let input = egui::RawInput { time: Some(time), events, - screen_rect: Some(egui::Rect::from_min_size(Pos2::ZERO, vec2(400.0, 1200.0))), + screen_rect: Some(egui::Rect::from_min_size(Pos2::ZERO, vec2(400.0, height))), ..Default::default() }; let mut out = ctx.run_ui(input, |ui| show(app, ui)); out.textures_delta.clear(); - let mut r: Vec = ctx.viewport(|vp| { - let size = vec2(36.0, PITCH - 1.0); - vp.prev_pass.widgets.layers().flat_map(|(_, w)| w.iter()).filter(|w| w.rect.size() == size).map(|w| w.rect).collect() - }); + widget_rects(ctx, vec2(36.0, PITCH - 1.0)) + } + + /// The rects of the last frame's widgets of `size`, top to bottom. + pub(crate) fn widget_rects(ctx: &egui::Context, size: egui::Vec2) -> Vec { + let mut r: Vec = + ctx.viewport(|vp| vp.prev_pass.widgets.layers().flat_map(|(_, w)| w.iter()).filter(|w| w.rect.size() == size).map(|w| w.rect).collect()); r.sort_by(|a, b| a.top().total_cmp(&b.top())); r } + /// A mouse wheel turn over `at`, scrolling the content up by `dy` points. + pub(crate) fn wheel(at: Pos2, dy: f32) -> Vec { + let unit = egui::MouseWheelUnit::Point; + vec![Event::PointerMoved(at), Event::MouseWheel { unit, delta: vec2(0.0, -dy), phase: egui::TouchPhase::Move, modifiers: Default::default() }] + } + + #[test] + fn the_toolbar_scrolls_in_a_short_window() { + let mut app = VectorcraftApp::new(Session::new(), Default::default()); + app.run("file.new", json!({"width": 300, "height": 300})).unwrap(); + let ctx = egui::Context::default(); + // 500 pt: even two columns don't fit, so Edit Toolbar (the last button) starts below the window. + let tools = frame_in(&mut app, &ctx, 0.0, vec![], 500.0); + let edit_toolbar = || widget_rects(&ctx, vec2(26.0, 26.0)).last().copied().unwrap(); + assert!(edit_toolbar().bottom() > 500.0); + frame_in(&mut app, &ctx, 0.1, wheel(tools[0].center(), 2000.0), 500.0); + for k in 2..40 { + frame_in(&mut app, &ctx, f64::from(k) * 0.1, vec![], 500.0); + } + assert!(edit_toolbar().bottom() <= 500.0, "scrolled into view: {:?}", edit_toolbar()); + // Back up to the top. + frame_in(&mut app, &ctx, 4.0, wheel(tools[0].center(), -2000.0), 500.0); + for k in 41..80 { + frame_in(&mut app, &ctx, f64::from(k) * 0.1, vec![], 500.0); + } + assert_eq!(frame_in(&mut app, &ctx, 8.0, vec![], 500.0)[0], tools[0]); + } + /// A double-click at `at`, `time` seconds in (a second apart from the last). fn double_click(app: &mut VectorcraftApp, ctx: &egui::Context, time: f64, at: Pos2) { let b = |pressed| Event::PointerButton { pos: at, button: PointerButton::Primary, pressed, modifiers: Default::default() }; diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index 3b2512ba..c1fd7ea2 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -458,6 +458,13 @@ pub fn menu_scroll(ui: &mut Ui, add_contents: impl FnOnce(&mut Ui) -> R) -> R egui::ScrollArea::vertical().max_height(room).min_scrolled_height(room).show(ui, add_contents).inner } +/// 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. +pub fn strip_scroll(ui: &mut Ui, id: &str, add_contents: impl FnOnce(&mut Ui) -> R) -> R { + egui::ScrollArea::vertical().id_salt(id).auto_shrink([false, true]).show(ui, add_contents).inner +} + /// Whether the blend-mode list draws a separator above `BlendMode::ALL[i]`: where a /// [`BlendMode::group`] starts. pub fn blend_separator_before(i: usize) -> bool {