From e1936cbfd0c181eebf2fefef4a4e49bb277833b0 Mon Sep 17 00:00:00 2001 From: moppeeer <339764117+moppeeer@users.noreply.github.com> Date: Sat, 10 Oct 2026 16:18:48 +0200 Subject: [PATCH] fix(ui): Highlight swatch list rows and keep panels clear of scroll bars Swatch lists (Character panel, Properties, task bar) shaded the row under the pointer with the theme's hover grey, a few levels from the popup's own background, so it looked as if nothing changed. The row under the pointer is now white on blue, like a menu; the current swatch keeps its shading. The lists also grow to fit their longest name, and their rows stop short of the scroll bar that floats over the list's edge, so names like C=15 M=100 Y=100 K=0 are no longer cut by it. The four copies of the list are one shared function now. The Glyphs panel counted its columns from the panel's whole width, so the last column ran under the floating scroll bar and was cut off; the columns now leave the bar room. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/panels/glyphs.rs | 49 +++++++- crates/ui-egui/src/panels/mod.rs | 155 +++++++++++++++++++++--- crates/ui-egui/src/panels/properties.rs | 11 +- crates/ui-egui/src/taskbar.rs | 22 +--- 4 files changed, 186 insertions(+), 51 deletions(-) diff --git a/crates/ui-egui/src/panels/glyphs.rs b/crates/ui-egui/src/panels/glyphs.rs index 3fb9d00eb..63f02e9ac 100644 --- a/crates/ui-egui/src/panels/glyphs.rs +++ b/crates/ui-egui/src/panels/glyphs.rs @@ -71,7 +71,10 @@ pub fn show(app: &mut DesignApp, ui: &mut egui::Ui) { } }); } - let cols = ((ui.available_width() / CELL).floor() as u32).max(1); + // The scroll bar floats over the grid's right edge: leave it room. + let scroll = &ui.spacing().scroll; + let bar = scroll.bar_inner_margin + scroll.bar_width + scroll.bar_outer_margin; + let cols = (((ui.available_width() - bar) / CELL).floor() as u32).max(1); let ppp = ui.ctx().pixels_per_point(); let cell_px = (CELL * ppp).round() as u32; let ink = t.text_strong; @@ -138,3 +141,47 @@ pub fn show(app: &mut DesignApp, ui: &mut egui::Ui) { ); ui.data_mut(|d| d.insert_temp(id, st)); } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn the_grid_leaves_room_for_the_scroll_bar() { + let mut app = DesignApp::new(designcraft_engine::Session::new(), crate::Services::default()); + app.run("file.new", json!({})).unwrap(); + let ctx = egui::Context::default(); + crate::theme::install_fonts(&ctx, ""); + crate::theme::apply(&ctx, &Tokens::for_brightness(crate::theme::Brightness::MediumDark)); + let scroll = ctx.global_style().spacing.scroll; + let bar = scroll.bar_inner_margin + scroll.bar_width + scroll.bar_outer_margin; + for width in [236.0, 256.0, 300.0, 333.0] { + let mut panel = egui::Rect::NOTHING; + let mut grid = None; + for _ in 0..2 { + // The app raises the texture limit for the grid (see `test_window`). + let raw = egui::RawInput { + screen_rect: Some(egui::Rect::from_min_size(egui::Pos2::ZERO, vec2(800.0, 700.0))), + max_texture_side: Some(8192), + ..Default::default() + }; + let mut out = ctx.run_ui(raw, |ui| { + ui.allocate_ui(vec2(width, 650.0), |ui| { + ui.set_width(width); + panel = ui.max_rect(); + show(&mut app, ui); + }); + }); + out.textures_delta.clear(); + // The grid is the one textured mesh that isn't text. + grid = out.shapes.iter().find_map(|s| match &s.shape { + egui::Shape::Mesh(m) if m.texture_id != egui::TextureId::default() => Some(m.calc_bounds()), + _ => None, + }); + } + let grid = grid.expect("the glyph grid is drawn"); + assert!(grid.right() <= panel.right() - bar + 0.5, "{width}: the grid {grid:?} runs under the scroll bar of {panel:?}"); + assert!(grid.right() > panel.right() - bar - CELL, "{width}: no column is left out ({grid:?})"); + } + } +} diff --git a/crates/ui-egui/src/panels/mod.rs b/crates/ui-egui/src/panels/mod.rs index 581f6747e..9100ffe9b 100644 --- a/crates/ui-egui/src/panels/mod.rs +++ b/crates/ui-egui/src/panels/mod.rs @@ -158,29 +158,67 @@ const SWATCH_MENU_ROW_H: f32 = 22.0; /// One swatch-menu row: a chip and the swatch name; the whole row is clickable (the chip included). /// `name == current` highlights it. Returns true when the row was clicked. -pub fn swatch_menu_row(ui: &mut egui::Ui, doc: &designcraft_doc::Document, name: &str, current: &str) -> bool { +/// Where a swatch row's name starts, after its chip. +const SWATCH_MENU_TEXT_X: f32 = 28.0; + +fn swatch_menu_font() -> egui::FontId { + egui::FontId::proportional(12.5) +} + +/// One swatch in a swatch list, `width` wide: the swatch under the pointer is highlighted (white +/// on blue, like a menu), the current one shaded. +pub fn swatch_menu_row(ui: &mut egui::Ui, doc: &designcraft_doc::Document, name: &str, current: &str, width: f32) -> bool { let t = Tokens::get(ui.ctx()); - let (row, resp) = ui.allocate_exact_size(vec2(ui.available_width(), SWATCH_MENU_ROW_H), egui::Sense::click()); + let (row, resp) = ui.allocate_exact_size(vec2(width, SWATCH_MENU_ROW_H), egui::Sense::click()); resp.widget_info(|| egui::WidgetInfo::selected(egui::WidgetType::SelectableLabel, true, name == current, name)); - if name == current { - ui.painter().rect_filled(row, 0.0, t.row_selected); - } else if resp.hovered() { - ui.painter().rect_filled(row, 0.0, t.hover); + let hovered = resp.hovered(); + if hovered { + ui.painter().rect_filled(row, 2.0, t.accent_strong); + } else if name == current { + ui.painter().rect_filled(row, 2.0, t.row_selected); } let (c, g) = crate::widgets::swatch_colors(doc, name, 1.0); let chip = egui::Rect::from_min_size(row.min + vec2(4.0, 3.0), vec2(16.0, 16.0)); crate::widgets::paint_chip(ui.painter(), chip, c, g); + // A name too long for the list is cut at the row's end. + let text = ui.painter().with_clip_rect(ui.clip_rect().intersect(row.shrink2(vec2(2.0, 0.0)))); crate::rtl::paint( - ui.painter(), - row.min + vec2(28.0, SWATCH_MENU_ROW_H / 2.0), + &text, + row.min + vec2(SWATCH_MENU_TEXT_X, SWATCH_MENU_ROW_H / 2.0), egui::Align2::LEFT_CENTER, name, - egui::FontId::proportional(12.5), - t.text, + swatch_menu_font(), + if hovered { egui::Color32::WHITE } else { t.text }, ); resp.clicked() } +/// The list in a swatch popup → the swatch picked. It is as wide as its longest name (within +/// limits), and its rows stop short of the scroll bar that floats over the list's edge. +pub fn swatch_list(ui: &mut egui::Ui, doc: &designcraft_doc::Document, current: &str) -> Option { + let font = swatch_menu_font(); + let widest = doc + .swatches + .iter() + .map(|sw| ui.painter().layout_no_wrap(sw.name.clone(), font.clone(), egui::Color32::PLACEHOLDER).size().x) + .fold(0.0, f32::max); + let scroll = &ui.spacing().scroll; + let bar = scroll.bar_inner_margin + scroll.bar_width + scroll.bar_outer_margin; + let width = (SWATCH_MENU_TEXT_X + widest + 10.0 + bar).clamp(200.0, 360.0); + ui.set_min_width(width); + ui.set_max_width(width); + let mut picked = None; + egui::ScrollArea::vertical().max_height(320.0).show(ui, |ui| { + for sw in &doc.swatches { + if swatch_menu_row(ui, doc, &sw.name, current, width - bar) { + picked = Some(sw.name.clone()); + ui.close(); + } + } + }); + picked +} + /// A swatch dropdown showing a chip and name; `on_pick` gets the chosen swatch name. pub fn swatch_picker(app: &mut DesignApp, ui: &mut egui::Ui, id: &str, current: Option, on_pick: impl FnOnce(&mut DesignApp, String)) { let Some(doc) = app.session.active().map(|d| d.doc.clone()) else { return }; @@ -202,16 +240,7 @@ pub fn swatch_picker(app: &mut DesignApp, ui: &mut egui::Ui, id: &str, current: Tokens::get(ui.ctx()).icon, ); egui::Popup::menu(&resp).show(|ui| { - ui.set_min_width(200.0); - ui.set_max_width(280.0); - egui::ScrollArea::vertical().max_height(320.0).show(ui, |ui| { - for sw in &doc.swatches { - if swatch_menu_row(ui, &doc, &sw.name, &cur) { - picked = Some(sw.name.clone()); - ui.close(); - } - } - }); + picked = swatch_list(ui, &doc, &cur); }); }); if let Some(p) = picked { @@ -698,3 +727,89 @@ mod font_menu_ui_tests { } } } + +#[cfg(test)] +mod swatch_list_tests { + use super::*; + + fn rects(shapes: &[egui::epaint::ClippedShape]) -> Vec<(egui::Rect, egui::Color32)> { + fn walk(s: &egui::Shape, out: &mut Vec<(egui::Rect, egui::Color32)>) { + match s { + egui::Shape::Rect(r) => out.push((r.rect, r.fill)), + egui::Shape::Vec(v) => v.iter().for_each(|s| walk(s, out)), + _ => {} + } + } + let mut out = vec![]; + shapes.iter().for_each(|s| walk(&s.shape, &mut out)); + out + } + + fn texts(shapes: &[egui::epaint::ClippedShape]) -> Vec<(String, egui::Rect)> { + fn walk(s: &egui::Shape, out: &mut Vec<(String, egui::Rect)>) { + match s { + egui::Shape::Text(t) => out.push((t.galley.job.text.clone(), t.visual_bounding_rect())), + egui::Shape::Vec(v) => v.iter().for_each(|s| walk(s, out)), + _ => {} + } + } + let mut out = vec![]; + shapes.iter().for_each(|s| walk(&s.shape, &mut out)); + out + } + + #[test] + fn the_swatch_under_the_pointer_is_highlighted_and_names_clear_the_scroll_bar() { + let mut app = DesignApp::new(designcraft_engine::Session::new(), crate::Services::default()); + app.run("file.new", serde_json::json!({})).unwrap(); + // A long name, as process swatches have. + app.run("swatch.create", serde_json::json!({"name": "C=15 M=100 Y=100 K=0 Long Name", "color": {"c": 15, "m": 100, "y": 100, "k": 0}})) + .unwrap(); + let long = "C=15 M=100 Y=100 K=0 Long Name"; + let ctx = egui::Context::default(); + crate::theme::install_fonts(&ctx, ""); + let tokens = Tokens::for_brightness(crate::theme::Brightness::MediumDark); + crate::theme::apply(&ctx, &tokens); + let mut time = 0.0; + let mut frame = |app: &mut DesignApp, events: Vec| { + time += 0.1; + let raw = egui::RawInput { + screen_rect: Some(egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(800.0, 700.0))), + time: Some(time), + events, + ..Default::default() + }; + let mut out = ctx.run_ui(raw, |ui| swatch_picker(app, ui, "list", Some("[Black]".into()), |_, _| {})); + out.textures_delta.clear(); + out.shapes + }; + frame(&mut app, vec![]); + let chip = egui::pos2(17.0, 17.0); + for pressed in [true, false] { + frame( + &mut app, + vec![ + egui::Event::PointerMoved(chip), + egui::Event::PointerButton { pos: chip, button: egui::PointerButton::Primary, pressed, modifiers: Default::default() }, + ], + ); + } + let shapes = frame(&mut app, vec![]); + let (_, name) = texts(&shapes).into_iter().find(|(t, _)| t == "[Registration]").expect("the list is open"); + let pointer = name.center(); + frame(&mut app, vec![egui::Event::PointerMoved(pointer)]); + let shapes = frame(&mut app, vec![]); + let all = rects(&shapes); + let (row, _) = *all.iter().find(|(r, c)| *c == tokens.accent_strong && r.contains(pointer)).expect("the hovered row is highlighted"); + let (list, _) = *all + .iter() + .filter(|(r, c)| *c == tokens.panel && r.contains_rect(row)) + .min_by(|a, b| a.0.area().total_cmp(&b.0.area())) + .expect("the popup"); + let scroll = ctx.global_style().spacing.scroll; + assert!(row.right() + scroll.bar_width <= list.right(), "rows {row:?} stop short of the scroll bar in {list:?}"); + let (_, long_rect) = texts(&shapes).into_iter().find(|(t, _)| t == long).unwrap(); + assert!(long_rect.right() <= row.right(), "the longest name {long_rect:?} fits its row {row:?}"); + assert!(texts(&shapes).iter().any(|(t, r)| t == "[Black]" && r.right() <= row.right())); + } +} diff --git a/crates/ui-egui/src/panels/properties.rs b/crates/ui-egui/src/panels/properties.rs index d9c74e40d..03c2c87a6 100644 --- a/crates/ui-egui/src/panels/properties.rs +++ b/crates/ui-egui/src/panels/properties.rs @@ -725,16 +725,7 @@ fn appearance_section(app: &mut DesignApp, ui: &mut Ui, i: &SelInfo) { fn swatch_menu(app: &mut DesignApp, resp: &egui::Response, doc: &designcraft_doc::Document, cur: &str, on_pick: impl FnOnce(&mut DesignApp, String)) { let mut picked = None; egui::Popup::menu(resp).show(|ui| { - ui.set_min_width(200.0); - ui.set_max_width(280.0); - egui::ScrollArea::vertical().max_height(320.0).show(ui, |ui| { - for sw in &doc.swatches { - if super::swatch_menu_row(ui, doc, &sw.name, cur) { - picked = Some(sw.name.clone()); - ui.close(); - } - } - }); + picked = super::swatch_list(ui, doc, cur); }); if let Some(p) = picked { on_pick(app, p); diff --git a/crates/ui-egui/src/taskbar.rs b/crates/ui-egui/src/taskbar.rs index cf66e9d5f..eaf5e7cce 100644 --- a/crates/ui-egui/src/taskbar.rs +++ b/crates/ui-egui/src/taskbar.rs @@ -168,16 +168,7 @@ fn text_controls(app: &mut DesignApp, ui: &mut Ui, in_text: bool) { resp.widget_info(|| egui::WidgetInfo::labeled(egui::WidgetType::Button, true, "Task Bar fill")); let mut picked = None; egui::Popup::menu(&resp).show(|ui| { - ui.set_min_width(180.0); - ui.set_max_width(280.0); - egui::ScrollArea::vertical().max_height(300.0).show(ui, |ui| { - for s in &doc.swatches { - if panels::swatch_menu_row(ui, &doc, &s.name, &sw) { - picked = Some(s.name.clone()); - ui.close(); - } - } - }); + picked = panels::swatch_list(ui, &doc, &sw); }); if let Some(p) = picked { let _ = app.run("type.char", json!({"attrs": {"fill": p}})); @@ -241,16 +232,7 @@ fn object_controls(app: &mut DesignApp, ui: &mut Ui) { }); let mut picked = None; egui::Popup::menu(&resp).show(|ui| { - ui.set_min_width(180.0); - ui.set_max_width(280.0); - egui::ScrollArea::vertical().max_height(300.0).show(ui, |ui| { - for s in &doc.swatches { - if panels::swatch_menu_row(ui, &doc, &s.name, &sw) { - picked = Some(s.name.clone()); - ui.close(); - } - } - }); + picked = panels::swatch_list(ui, &doc, &sw); }); if let Some(p) = picked { let _ = app.run(cmd, json!({"swatch": p}));