From d3df1bcea213f5c37fb710c9626f8437c49dc9b1 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Mon, 5 Oct 2026 00:53:11 +0700 Subject: [PATCH] M14.1: dialogs and menus size to their content The shared dialog frame's button row took all the height left in the window (a right-to-left layout fills the available height), and every dialog shared one window id, so a dialog never shrank and inherited the size of the one before it: the save prompt showed a tall empty band around its buttons. The row is now as tall as the buttons and each dialog kind has its own window. Menu items forced themselves to the popup's full available width, so every menu opened at egui's maximum menu width. The menu layout already justifies items, so popups are now as wide as their widest label and shortcut (at least 230 pt). Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/dialogs/mod.rs | 8 ++++-- crates/ui-egui/src/dialogs/tests.rs | 23 +++++++++++++++++ crates/ui-egui/src/menus.rs | 39 ++++++++++++++++++++++++----- 3 files changed, 62 insertions(+), 8 deletions(-) diff --git a/crates/ui-egui/src/dialogs/mod.rs b/crates/ui-egui/src/dialogs/mod.rs index 51e9077e..cb08277d 100644 --- a/crates/ui-egui/src/dialogs/mod.rs +++ b/crates/ui-egui/src/dialogs/mod.rs @@ -195,7 +195,8 @@ pub fn show(app: &mut VectorcraftApp, ctx: &egui::Context) { }); let heading = (spec.heading)(&d); egui::Window::new(heading.as_str()) - .id(egui::Id::new("dialog")) + // One window per kind, so a dialog never inherits another dialog's size. + .id(egui::Id::new(("dialog", d.kind.as_str()))) .order(egui::Order::Foreground) .collapsible(false) .resizable(false) @@ -211,7 +212,10 @@ pub fn show(app: &mut VectorcraftApp, ctx: &egui::Context) { ui.add_space(12.0); cancel = (spec.body)(app, ui, &mut d); ui.add_space(16.0); - ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { + // The button row is as tall as the buttons: a right-to-left layout would otherwise take + // all the height left in the window, so the window could never shrink to its content. + let row = egui::vec2(ui.available_width(), ui.spacing().interact_size.y); + ui.allocate_ui_with_layout(row, egui::Layout::right_to_left(egui::Align::Center), |ui| { if let Some(label) = spec.ok && widgets::primary_button(ui, label).clicked() { diff --git a/crates/ui-egui/src/dialogs/tests.rs b/crates/ui-egui/src/dialogs/tests.rs index 0b459ef8..6b59910f 100644 --- a/crates/ui-egui/src/dialogs/tests.rs +++ b/crates/ui-egui/src/dialogs/tests.rs @@ -15,6 +15,7 @@ fn app() -> VectorcraftApp { fn frame(app: &mut VectorcraftApp, input: egui::RawInput) { let ctx = egui::Context::default(); theme::install_fonts(&ctx); + theme::apply(&ctx, Default::default()); let mut out = ctx.run_ui(input, |ui| show(app, ui.ctx())); out.textures_delta.clear(); } @@ -176,3 +177,25 @@ fn effect_dialog_applies_to_its_appearance_item() { assert_eq!(n.appearance.items[1].effects().len(), 1, "the effect goes to the stroke"); assert!(n.appearance.effects.is_empty() && n.appearance.items[0].effects().is_empty()); } + +#[test] +fn dialogs_are_as_tall_as_their_content() { + // The save prompt, shown after a taller dialog, is as tall as its text and buttons: no empty + // band around the buttons, nothing inherited from the other dialog. + let mut app = app(); + let ctx = egui::Context::default(); + theme::install_fonts(&ctx); + theme::apply(&ctx, Default::default()); + let screen = egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(1600.0, 1000.0)); + let height = |app: &mut VectorcraftApp, kind: &str, fields: serde_json::Value| { + open(app, kind, fields); + for _ in 0..30 { + let input = egui::RawInput { screen_rect: Some(screen), ..Default::default() }; + ctx.run_ui(input, |ui| show(app, ui.ctx())).textures_delta.clear(); + } + ctx.memory(|m| m.area_rect(egui::Id::new(("dialog", kind)))).unwrap().height() + }; + let tall = height(&mut app, "command", json!({"__command": "object.move", "a": 1, "b": 2, "c": 3, "d": 4, "e": 5, "f": 6, "g": 7, "h": 8})); + let prompt = height(&mut app, crate::unsaved::KIND, json!({"index": 0, "name": "Untitled-1", "then": "close"})); + assert!(prompt < 180.0 && prompt < tall, "save prompt {prompt} pt tall (the dialog before it: {tall} pt)"); +} diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index ef51a5cc..149c0d61 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -1338,10 +1338,7 @@ pub fn menu_bar(app: &mut VectorcraftApp, ui: &mut egui::Ui) { } else { egui::RichText::new(*title).size(13.0).color(t.text) }; - ui.menu_button(text, |ui| { - ui.set_min_width(230.0); - render_items(app, ui, items, &mut clicked); - }); + ui.menu_button(text, |ui| menu_body(app, ui, items, &mut clicked)); } }); if let Some((id, p)) = clicked { @@ -1349,6 +1346,12 @@ pub fn menu_bar(app: &mut VectorcraftApp, ui: &mut egui::Ui) { } } +/// A top-level menu's popup: as wide as its widest item (label plus shortcut), at least 230 pt. +fn menu_body(app: &VectorcraftApp, ui: &mut egui::Ui, items: &[Item], clicked: &mut Option<(String, Value)>) { + ui.set_min_width(230.0); + render_items(app, ui, items, clicked); +} + fn render_items(app: &VectorcraftApp, ui: &mut egui::Ui, items: &[Item], clicked: &mut Option<(String, Value)>) { let t = Tokens::get(ui.ctx()); for it in items { @@ -1367,7 +1370,7 @@ fn render_items(app: &VectorcraftApp, ui: &mut egui::Ui, items: &[Item], clicked } Item::Todo(label, sc) => { ui.add_enabled_ui(false, |ui| { - ui.add(egui::Button::new(*label).shortcut_text(pretty_shortcut(sc)).min_size(egui::vec2(ui.available_width(), 0.0))); + ui.add(egui::Button::new(*label).shortcut_text(pretty_shortcut(sc))); }) .response .on_disabled_hover_text("Coming soon — tracked in the parity plan"); @@ -1386,7 +1389,7 @@ fn render_items(app: &VectorcraftApp, ui: &mut egui::Ui, items: &[Item], clicked Some(false) => format!(" {label}"), None => label, }; - let r = ui.add_enabled(en, egui::Button::new(text).shortcut_text(sc).min_size(egui::vec2(ui.available_width(), 0.0))); + let r = ui.add_enabled(en, egui::Button::new(text).shortcut_text(sc)); if r.clicked() { *clicked = Some(click_target(label_of(it), id, p)); ui.close(); @@ -1847,4 +1850,28 @@ mod tests { menu_entries(&app).into_iter().filter(|e| e.command.as_deref() == Some("effect.expandAppearance")).map(|e| e.path).collect(); assert_eq!(paths, [vec!["Object".to_string()]]); } + + #[test] + fn menus_are_as_wide_as_their_items() { + // Items size the popup to the widest label and shortcut instead of the widest a menu may be. + let app = VectorcraftApp::new(vectorcraft_engine::Session::new(), crate::Services::default()); + let ctx = egui::Context::default(); + crate::theme::install_fonts(&ctx); + let screen = egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(1600.0, 1000.0)); + let tree = menu_tree(); + let (_, file) = tree.iter().find(|(t, _)| *t == "File").unwrap(); + let id = egui::Id::new("test-menu"); + for _ in 0..3 { + let input = egui::RawInput { screen_rect: Some(screen), ..Default::default() }; + let mut out = ctx.run_ui(input, |ui| { + egui::Area::new(id).show(ui.ctx(), |ui| { + egui::containers::menu::menu_style(ui.style_mut()); + ui.with_layout(egui::Layout::top_down_justified(egui::Align::Min), |ui| menu_body(&app, ui, file, &mut None)); + }); + }); + out.textures_delta.clear(); + } + let w = ctx.memory(|m| m.area_rect(id)).unwrap().width(); + assert!((230.0..340.0).contains(&w), "File menu is {w} pt wide"); + } }