From afaa4bdeebe5f34ba04ccf1000dd08409e42b7fc Mon Sep 17 00:00:00 2001 From: pravdomil <2387356+pravdomil@users.noreply.github.com> Date: Sun, 11 Oct 2026 16:55:27 +0200 Subject: [PATCH] UI: leave no gap between the rows of a menu The app's item spacing left 4 px between two menu entries where neither highlighted and a click did nothing. Menus (context menus, the menu bar, menu buttons and their submenus) now use a menu style with no row gap and rows taller by that gap, so they keep their size. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/header.rs | 4 ++-- crates/ui-egui/src/menus.rs | 2 +- .../ui-egui/src/panels/camera_tracker_ui.rs | 2 +- crates/ui-egui/src/panels/effect_controls.rs | 4 ++-- crates/ui-egui/src/panels/essential.rs | 6 +++--- crates/ui-egui/src/panels/markers_ui.rs | 2 +- crates/ui-egui/src/panels/media_panels.rs | 2 +- crates/ui-egui/src/panels/project.rs | 6 +++--- crates/ui-egui/src/panels/render_queue.rs | 2 +- crates/ui-egui/src/panels/timeline.rs | 14 ++++++------- crates/ui-egui/src/panels/viewer.rs | 2 +- crates/ui-egui/src/widgets.rs | 21 +++++++++++++++++++ .../ui-egui/tests/ui_blank_context_menus.rs | 14 +++++++++++++ 13 files changed, 58 insertions(+), 23 deletions(-) diff --git a/crates/ui-egui/src/header.rs b/crates/ui-egui/src/header.rs index 1260c9c9..b3082235 100644 --- a/crates/ui-egui/src/header.rs +++ b/crates/ui-egui/src/header.rs @@ -101,7 +101,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { None }; let close_behavior = if held && resp.clicked() { egui::PopupCloseBehavior::IgnoreClicks } else { egui::PopupCloseBehavior::CloseOnClickOutside }; - egui::Popup::menu(&resp).open_memory(command).close_behavior(close_behavior).show(|ui| { + egui::Popup::menu(&resp).style(crate::widgets::menu_style).open_memory(command).close_behavior(close_behavior).show(|ui| { for tool in slot.iter() { let row = ui.selectable_label(app.ui.tool == *tool, tool.label()); app.auto.add(&format!("tools.select.{tool:?}"), row.rect, tool.label()); @@ -227,7 +227,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { let mresp = ui.interact(more, egui::Id::new("ws-more"), Sense::click()); p.text(more.center(), Align2::CENTER_CENTER, "»", Tokens::ui(15.0), if mresp.hovered() { t.text } else { t.text_dim }); app.auto.add("header.workspaces", more, "Workspaces"); - mresp.context_menu(|ui| ws_menu(app, ui)); + crate::widgets::context_menu(&mresp, |ui| ws_menu(app, ui)); if mresp.clicked() { widgets::open_popup(ui, egui::Id::new("ws-pop")); } diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 5c61b497..f9925bb7 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -1770,7 +1770,7 @@ pub fn menu_bar(app: &mut EffectcraftApp, ui: &mut egui::Ui) { let mut nav = crate::menu_keys::Nav::default(); ui.horizontal_centered(|ui| { ui.add_space(6.0); - egui::MenuBar::new().ui(ui, |ui| { + egui::MenuBar::new().config(egui::containers::menu::MenuConfig::new().style(crate::widgets::menu_style)).ui(ui, |ui| { for node in effectcraft_engine::menus::menu_bar() { if let MenuNode::Submenu { label, children } = node { let r = ui.menu_button(crate::i18n::label(app, "", label), |ui| { diff --git a/crates/ui-egui/src/panels/camera_tracker_ui.rs b/crates/ui-egui/src/panels/camera_tracker_ui.rs index 71cea155..2bccfae0 100644 --- a/crates/ui-egui/src/panels/camera_tracker_ui.rs +++ b/crates/ui-egui/src/panels/camera_tracker_ui.rs @@ -296,7 +296,7 @@ pub fn viewer_hook( ui.data_mut(|d| d.insert_temp(cid, pts)); } let base = json!({"layer": a.layer.0, "effect": a.uid}); - resp.context_menu(|ui| { + crate::widgets::context_menu(&resp, |ui| { let pts: Vec = ui.data(|d| d.get_temp(cid)).unwrap_or_default(); let with = |kind: Option<&str>, multiple: bool| { let mut q = base.clone(); diff --git a/crates/ui-egui/src/panels/effect_controls.rs b/crates/ui-egui/src/panels/effect_controls.rs index 501adf89..e3db9724 100644 --- a/crates/ui-egui/src/panels/effect_controls.rs +++ b/crates/ui-egui/src/panels/effect_controls.rs @@ -1164,7 +1164,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { if hresp.drag_started() { ctx.data_mut(|d| d.insert_temp(drag_id, g.uid)); } - hresp.context_menu(|ui| { + crate::widgets::context_menu(&hresp, |ui| { let lid = layer.id.0; let mut item = |ui: &mut egui::Ui, label: &str, enabled: bool, cmd: &str, params: serde_json::Value| { if ui.add_enabled(enabled, egui::Button::new(label)).clicked() { @@ -1246,7 +1246,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { let open = ui.input(|i| i.pointer.secondary_clicked()) && ui.rect_contains_pointer(body) && (!egui::Popup::is_any_open(&ctx) || egui::Popup::is_id_open(&ctx, menu_id)); - egui::Popup::context_menu(&bg).id(menu_id).open_memory(open.then_some(egui::SetOpenCommand::Bool(true))).show(|ui| { + egui::Popup::context_menu(&bg).style(crate::widgets::menu_style).id(menu_id).open_memory(open.then_some(egui::SetOpenCommand::Bool(true))).show(|ui| { ui.set_min_width(200.0); actions.extend(crate::menus::menu_contents(app, ui, "Effect")); }); diff --git a/crates/ui-egui/src/panels/essential.rs b/crates/ui-egui/src/panels/essential.rs index 048b0b58..2e99c7d3 100644 --- a/crates/ui-egui/src/panels/essential.rs +++ b/crates/ui-egui/src/panels/essential.rs @@ -129,7 +129,7 @@ fn body(app: &mut EffectcraftApp, ui: &mut egui::Ui) -> (Actions, Actions) { // ---- Bottom buttons. ui.horizontal_wrapped(|ui| { - let r = ui.menu_button(tr("Add Formatting"), |ui| { + let r = crate::widgets::menu_button(ui, tr("Add Formatting"), |ui| { if ui.button(tr("Add Group")).clicked() { actions.push(("essential.addGroup".into(), json!({"comp": cid.0}))); ui.close(); @@ -143,7 +143,7 @@ fn body(app: &mut EffectcraftApp, ui: &mut egui::Ui) -> (Actions, Actions) { // The selected property as a Font control (Source Text) or a uniform Scale slider. let has_sel = !app.session.state.selected_props.is_empty(); let r = ui.add_enabled_ui(has_sel, |ui| { - ui.menu_button(tr("Add Selected As"), |ui| { + crate::widgets::menu_button(ui, tr("Add Selected As"), |ui| { for (label, as_type) in [("Font Control", "font"), ("Uniform Scale", "scale")] { if ui.button(label).clicked() { actions.push(("essential.addProperty".into(), json!({"as": as_type}))); @@ -270,7 +270,7 @@ fn link_menu(app: &mut EffectcraftApp, ui: &mut egui::Ui, cid: ItemId, c: &EgCon _ => vec![], }; let has_sel = !app.session.state.selected_props.is_empty(); - let r = ui.menu_button(RichText::new("⋯").size(10.0), |ui| { + let r = crate::widgets::menu_button(ui, RichText::new("⋯").size(10.0), |ui| { if ui.button(tr("Add Mirror")).clicked() { actions.push(("essential.addMirror".into(), json!({"comp": cid.0, "control": c.id, "group": group}))); ui.close(); diff --git a/crates/ui-egui/src/panels/markers_ui.rs b/crates/ui-egui/src/panels/markers_ui.rs index c3631604..2b082312 100644 --- a/crates/ui-egui/src/panels/markers_ui.rs +++ b/crates/ui-egui/src/panels/markers_ui.rs @@ -337,7 +337,7 @@ fn marker(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painter, comp: ctx.data_mut(|d| d.remove::<(Option, usize, bool)>(drag_state_id())); app.session.history.merge_key = None; } - resp.context_menu(|ui| { + crate::widgets::context_menu(&resp, |ui| { let run = |ui: &mut egui::Ui, label: &str, cmd: &str, params: Value, app: &mut EffectcraftApp| { if ui.button(label).clicked() { if cmd == "dialog" { diff --git a/crates/ui-egui/src/panels/media_panels.rs b/crates/ui-egui/src/panels/media_panels.rs index cd6dca5d..4a0ab6ae 100644 --- a/crates/ui-egui/src/panels/media_panels.rs +++ b/crates/ui-egui/src/panels/media_panels.rs @@ -208,7 +208,7 @@ pub fn media_browser(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { if resp.drag_started() && !e.is_dir && e.kind.is_some() { egui::DragAndDrop::set_payload(&ctx, DragPayload::Files(vec![e.path.clone()])); } - resp.context_menu(|ui| { + crate::widgets::context_menu(&resp, |ui| { if !e.is_dir && ui.button(tr("Import")).clicked() { actions.push(("mediaBrowser.import", json!({"paths": [e.path]}))); ui.close(); diff --git a/crates/ui-egui/src/panels/project.rs b/crates/ui-egui/src/panels/project.rs index 21f84297..25aec1b8 100644 --- a/crates/ui-egui/src/panels/project.rs +++ b/crates/ui-egui/src/panels/project.rs @@ -578,7 +578,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { app.session.state.project_selection = vec![*id]; } let footage = matches!(it.kind, ItemKind::Footage(_)); - resp.context_menu(|ui| { + crate::widgets::context_menu(&resp, |ui| { if ui.button(crate::i18n::tr("Delete")).clicked() { actions.push(("project.delete".into(), json!({"items": targets(app, *id, selected)}))); ui.close(); @@ -743,7 +743,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { if empty.double_clicked() { actions.push(("file.import".into(), json!({}))); } - empty.context_menu(|ui| { + crate::widgets::context_menu(&empty, |ui| { for (key, label, command) in [("newComp", "New Composition…", "comp.new"), ("newFolder", "New Folder", "project.newFolder"), ("import", "Import File…", "file.import")] { @@ -809,7 +809,7 @@ fn label_index(l: Label) -> usize { /// The column header's context menu: show or hide the optional columns. fn column_menu(app: &mut EffectcraftApp, resp: &egui::Response) { - resp.context_menu(|ui| { + crate::widgets::context_menu(&resp, |ui| { ui.label(egui::RichText::new(crate::i18n::tr("Columns")).weak()); for (key, label, _) in COLUMNS { let mut on = app.ui.project_columns.iter().any(|c| c == key); diff --git a/crates/ui-egui/src/panels/render_queue.rs b/crates/ui-egui/src/panels/render_queue.rs index 502b8410..1e6953c1 100644 --- a/crates/ui-egui/src/panels/render_queue.rs +++ b/crates/ui-egui/src/panels/render_queue.rs @@ -559,7 +559,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { app.ui.focused = crate::dock::PanelKind::RenderQueue; } let item_id = it.id; - row_resp.context_menu(|ui| { + crate::widgets::context_menu(&row_resp, |ui| { if ui.button(tr("Duplicate")).clicked() { actions.push(("renderQueue.duplicate", json!({"item": item_id}))); ui.close(); diff --git a/crates/ui-egui/src/panels/timeline.rs b/crates/ui-egui/src/panels/timeline.rs index 61b2d22d..e3f80a11 100644 --- a/crates/ui-egui/src/panels/timeline.rs +++ b/crates/ui-egui/src/panels/timeline.rs @@ -1404,7 +1404,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { let bresp = ui.interact(bar, egui::Id::new("wa-bar"), Sense::click_and_drag()); app.auto.add("timeline.workArea.bar", bar, "Work area"); // Right-click: the work area's commands (After Effects). - bresp.context_menu(|ui| { + crate::widgets::context_menu(&bresp, |ui| { let mut chosen = None; for id in ["comp.trimToWorkArea", "edit.liftWorkArea", "edit.extractWorkArea"] { crate::menus::entry_for(app, ui, id, &mut chosen); @@ -1651,7 +1651,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { let under = hresp.interact_pointer_pos().and_then(|pt| column_at(&cw, pt.x)); ctx.data_mut(|d| d.insert_temp(egui::Id::new("tl-colhdr-under"), under.unwrap_or(""))); } - hresp.context_menu(|ui| { + crate::widgets::context_menu(&hresp, |ui| { let under: &str = ctx.data(|d| d.get_temp(egui::Id::new("tl-colhdr-under"))).unwrap_or(""); if !under.is_empty() && under != "name" && ui.button(crate::i18n::tr("Hide This")).clicked() { actions.push(("timeline.column".into(), json!({"column": under, "visible": false}))); @@ -2312,7 +2312,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { lp.text(lang_r.center(), Align2::CENTER_CENTER, "f", Tokens::semibold(10.0), if lang.hovered() { t.text } else { t.text_dim }); app.auto.add(&format!("timeline.prop.{uid}.exprLanguage"), lang_r, "Expression Language Menu"); let mut picked: Option<&'static str> = None; - egui::Popup::menu(&lang).show(|ui| { + egui::Popup::menu(&lang).style(crate::widgets::menu_style).show(|ui| { for (cat, items) in effectcraft_engine::commands::expr_tools::language_menu() { ui.menu_button(cat, |ui| { widgets::menu_scroll(ui, |ui| { @@ -2475,7 +2475,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { if name_resp.clicked() { actions.push(("prop.select".into(), json!({"layer": layer.id.0, "prop": uid, "add": ui.input(|i| i.modifiers.shift)}))); } - name_resp.context_menu(|ui| prop_menu(ui, &mut app.auto, cid, layer, prop, &mut actions)); + crate::widgets::context_menu(&name_resp, |ui| prop_menu(ui, &mut app.auto, cid, layer, prop, &mut actions)); // Value editors. let value = ectx.value(layer, prop); let vx = (cw.switches + 4.0).max(name_x + 120.0); @@ -2567,7 +2567,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { ui_actions.push(UiAct::SetTime(ct)); } } - kresp.context_menu(|ui| { + crate::widgets::context_menu(&kresp, |ui| { for (lbl, cmd, params) in [ ("Edit Value…", "keys.set", json!({"layer": layer.id.0, "prop": uid, "time": k.time.seconds()})), ("-", "", json!({})), @@ -2658,7 +2658,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { app.auto.add("timeline.empty", empty_rect, "Right-click to create a layer"); } for (response, area) in [(&outline_empty, "outline"), (&empty, "graph")] { - response.context_menu(|ui| { + crate::widgets::context_menu(&response, |ui| { let menu = ui.menu_button(crate::i18n::tr("New"), |ui| { for (key, label, command, params) in [ ("text", "Text", "layer.newText", json!({"text": "Text"})), @@ -3443,7 +3443,7 @@ fn prop_menu( } fn layer_context_menu(resp: &egui::Response, layer: &Layer, actions: &mut Vec<(String, serde_json::Value)>) { - resp.context_menu(|ui| layer_menu(ui, &[layer.id.0], actions)); + crate::widgets::context_menu(&resp, |ui| layer_menu(ui, &[layer.id.0], actions)); } /// Shared layer actions for Timeline rows and Composition context menus. diff --git a/crates/ui-egui/src/panels/viewer.rs b/crates/ui-egui/src/panels/viewer.rs index 5fe7697c..4a4fa0f1 100644 --- a/crates/ui-egui/src/panels/viewer.rs +++ b/crates/ui-egui/src/panels/viewer.rs @@ -1113,7 +1113,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { let context_layers: Vec = app.session.state.selected_layers.iter().filter(|id| comp.layer(**id).is_some_and(|l| !l.switches.locked)).map(|id| id.0).collect(); let mut context_actions = Vec::new(); - resp.context_menu(|ui| { + crate::widgets::context_menu(&resp, |ui| { let hits = ui.data(|d| d.get_temp::>(menu_hits_id)).unwrap_or_default(); if !hits.is_empty() { ui.menu_button(crate::i18n::tr("Select"), |ui| { diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index 18c6aedb..60c30786 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -579,6 +579,27 @@ pub fn menu_scroll(ui: &mut Ui, add: impl FnOnce(&mut Ui) -> R) -> R { egui::ScrollArea::vertical().max_height(max_h).min_scrolled_height(max_h).show(ui, add).inner } +/// egui's menu style with no gap between the rows: the gap the app's item spacing left between +/// two entries highlighted neither and swallowed a click there. The rows grow by that gap, so a +/// menu keeps its size. +pub fn menu_style(style: &mut egui::Style) { + egui::containers::menu::menu_style(style); + style.spacing.interact_size.y += style.spacing.item_spacing.y; + style.spacing.item_spacing.y = 0.0; +} + +/// `resp`'s context menu in the app's [`menu_style`] (its submenus inherit it). +pub fn context_menu(resp: &Response, add_contents: impl FnOnce(&mut Ui)) -> Option> { + egui::Popup::context_menu(resp).style(menu_style).show(add_contents) +} + +/// A button opening a menu in the app's [`menu_style`], for one outside the menu bar. +pub fn menu_button<'a, R>(ui: &mut Ui, atoms: impl egui::IntoAtoms<'a>, add_contents: impl FnOnce(&mut Ui) -> R) -> egui::InnerResponse> { + let config = egui::containers::menu::MenuConfig::new().style(menu_style); + let (response, inner) = egui::containers::menu::MenuButton::new(atoms).config(config).ui(ui, add_contents); + egui::InnerResponse::new(inner.map(|i| i.inner), response) +} + /// Whether a [`popup_menu`] is open (build its options only then: they can be long). pub fn popup_is_open(ui: &Ui, id: egui::Id) -> bool { ui.data(|d| d.get_temp(id.with("open")).unwrap_or(false)) diff --git a/crates/ui-egui/tests/ui_blank_context_menus.rs b/crates/ui-egui/tests/ui_blank_context_menus.rs index 561f50a0..de024c4b 100644 --- a/crates/ui-egui/tests/ui_blank_context_menus.rs +++ b/crates/ui-egui/tests/ui_blank_context_menus.rs @@ -195,3 +195,17 @@ fn timeline_property_names_have_the_property_menu() { choose(&mut h, scale, "editValue"); assert_eq!(h.state().dialog, Some(Dialog::Form)); } + +/// The rows of a context menu touch: a gap between two entries highlighted neither and +/// swallowed a click there. +#[test] +fn context_menu_rows_leave_no_gap_between_them() { + let mut h = harness(); + let p = rect(&h, "project.empty").center(); + right_click(&mut h, p); + let rows = ["newComp", "newFolder", "import"].map(|key| rect(&h, &format!("project.context.{key}"))); + for pair in rows.windows(2) { + assert!((pair[1].top() - pair[0].bottom()).abs() < 0.5, "gap between {:?} and {:?}", pair[0], pair[1]); + } + snapshot(&mut h, "project-rows"); +}