From e6d5297a86cf16140b50048d3b10a3d5f5dac2e0 Mon Sep 17 00:00:00 2001 From: Egon 5000 <210198019+elZanco@users.noreply.github.com> Date: Fri, 16 Oct 2026 13:16:36 +0200 Subject: [PATCH] UI: the menu bar browses from the press until the release A title opens on the press and the menus follow the cursor while the button is held: hovering a category opens its submenu and releasing over an entry picks it. An outside press or a focus loss closes the open menu again. --- crates/ui-egui/src/menu_keys.rs | 39 ++++++++- crates/ui-egui/src/menus.rs | 134 ++++++++++++++++++++++++++++--- crates/ui-egui/tests/ui_menus.rs | 128 +++++++++++++++++++++++++++++ 3 files changed, 289 insertions(+), 12 deletions(-) diff --git a/crates/ui-egui/src/menu_keys.rs b/crates/ui-egui/src/menu_keys.rs index 397385309..9477e142b 100644 --- a/crates/ui-egui/src/menu_keys.rs +++ b/crates/ui-egui/src/menu_keys.rs @@ -10,7 +10,7 @@ //! shortcuts and the panels don't see them (Space must not start a preview). use egui::containers::menu::{MenuState, SubMenu, find_menu_root}; -use egui::{Context, Event, Id, Key, Modifiers, Popup, Pos2, Response, Ui}; +use egui::{Context, Event, Id, Key, Modifiers, Popup, Pos2, Rect, Response, Ui}; /// An entry of an open menu as drawn this frame. #[derive(Clone, Copy)] @@ -39,6 +39,12 @@ pub struct Nav { tops: Vec<(Id, Id)>, /// Open menus by depth: the top-level menu, then each open submenu. levels: Vec, + /// The titles and open menus drawn this frame: a press outside them is an outside press, + /// which closes the menu (native behaviour; the press itself still reaches what's under it). + rects: Vec, + /// A title press opened (or kept) the menu this frame: releasing the button over an entry + /// chooses it (After Effects' press-and-browse). + browsing: bool, } /// What to highlight once a menu opened from the keyboard has been drawn: the first entry of the @@ -73,6 +79,37 @@ impl Nav { self.tops.push((button.id, Popup::default_response_id(button))); } + /// The rect `rect` (a menu title or an open menu) holds, so a press there doesn't close. + pub fn cover(&mut self, rect: Rect) { + self.rects.push(rect); + } + + /// Whether `pos` is inside a title or menu drawn this frame. + pub fn covers(&self, pos: Pos2) -> bool { + self.rects.iter().any(|r| r.contains(pos)) + } + + /// A title press is in progress (its button still held): see [`Nav::browsing`]. + pub fn set_browsing(&mut self) { + self.browsing = true; + } + + /// Whether the menu bar is mid press-and-browse (a title was pressed this pointer-down and + /// hasn't been released yet): releasing over an entry then chooses it. + pub fn browsing(&self) -> bool { + self.browsing + } + + /// The popup of the open top-level menu (only one can be open), if any. + pub fn open_popup(&self, ctx: &Context) -> Option { + self.tops.iter().find(|(_, p)| Popup::is_id_open(ctx, *p)).map(|(_, p)| *p) + } + + /// The position in the bar of the open top-level menu, if any. + pub fn open_index(&self, ctx: &Context) -> Option { + self.tops.iter().position(|(_, p)| Popup::is_id_open(ctx, *p)) + } + /// An entry drawn at `depth` (0: a top-level menu) in the menu `ui`. Hovering an entry moves /// the keyboard highlight to it, and a highlighted entry scrolls into view. pub fn entry(&mut self, ui: &Ui, depth: usize, r: &Response, submenu: bool) { diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index b1eba4f0c..dbdd32e39 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -1974,32 +1974,118 @@ fn handle_shortcuts_impl(app: &mut EffectcraftApp, ctx: &egui::Context, tab_only } } +/// The id under which [`menu_bar`] remembers which of its menus was open at the end of the last +/// frame (its title's open look, and the hover that switches between the titles). +fn open_menu_id() -> egui::Id { + egui::Id::new("menu-bar-open") +} + +/// The id under which [`menu_bar`] remembers a title press still in progress (press-and-browse). +fn browse_menu_id() -> egui::Id { + egui::Id::new("menu-bar-browse") +} + /// Draw the in-window menu bar (the engine's AE menu tree). +/// +/// The titles open on the press, not the release (native menu bars): while one is held the menus +/// can be browsed by moving onto them, and releasing over an entry chooses it. Hovering another +/// title switches the open menu to it, and the open one closes on a press outside it or when the +/// window loses focus. The pointer leaving the window — onto the OS title bar, directly above the +/// bar — closes nothing, however long: the platform forwards none of the title bar's clicks to +/// egui (only the cursor leaving), so there is no click to close on and a graze must not drop the +/// menu. The closing press itself still reaches the widget under it, as a click does in After +/// Effects. pub fn menu_bar(app: &mut EffectcraftApp, ui: &mut egui::Ui) { let ctx = ui.ctx().clone(); let mut clicked: Option<(String, Value)> = None; let mut nav = crate::menu_keys::Nav::default(); + // The open menu as of the last frame: the titles are drawn before this frame can open or + // close anything, and what happened last frame is what they must react to. + let open_prev: Option = ctx.data(|d| d.get_temp(open_menu_id())).flatten(); + if ctx.data(|d| d.get_temp(browse_menu_id())).unwrap_or(false) { + nav.set_browsing(); + } + let mut drawn = 0; ui.horizontal_centered(|ui| { ui.add_space(6.0); 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| { + let MenuNode::Submenu { label, children } = node else { continue }; + // The open title takes the open button's look while its menu is up (as in AE). + let inactive = ui.style().visuals.widgets.inactive; + if open_prev == Some(drawn) { + ui.style_mut().visuals.widgets.inactive = ui.style().visuals.widgets.open; + } + let mut response = ui.add(egui::Button::new(crate::i18n::label(app, "", label))); + ui.style_mut().visuals.widgets.inactive = inactive; + // A press on a title opens its menu at once (a second press on the open one closes + // it), and while it is held the menus below are browsable. The keyboard and the + // accessibility activation report `clicked` without the primary button (the press + // then happened somewhere else), and toggle the menu. Hovering another title + // switches to it once the pointer has moved: merely resting there must not take a + // keyboard-driven menu over. + let moved = ctx.input(|i| i.pointer.delta() != egui::Vec2::ZERO); + // With the button down elsewhere nothing is hovered, so the title under the + // pointer takes the highlight by hand: the hover-switch to it shows, one frame + // later like egui's own hover (the open title keeps its open look). + if moved && response.contains_pointer() && open_prev.is_some_and(|p| p != drawn) { + response = response.highlight(); + } + let pressed = ctx.input(|i| i.pointer.button_pressed(egui::PointerButton::Primary)) && response.hovered(); + let cmd = if pressed { + nav.set_browsing(); + Some(egui::SetOpenCommand::Bool(open_prev != Some(drawn))) + } else if response.clicked() && !response.clicked_by(egui::PointerButton::Primary) { + Some(egui::SetOpenCommand::Toggle) + } else if moved && response.enabled() && response.contains_pointer() && open_prev.is_some_and(|p| p != drawn) { + Some(egui::SetOpenCommand::Bool(true)) + } else { + None + }; + let inner = egui::Popup::menu(&response) + .close_behavior(egui::PopupCloseBehavior::IgnoreClicks) + .style(crate::widgets::menu_style) + .open_memory(cmd) + .show(|ui| { ui.set_min_width(if label == "Effect" { 200.0 } else { 280.0 }); crate::widgets::menu_scroll(ui, |ui| menu_nodes(app, ui, children, &mut clicked, &mut nav, 0)); }); - nav.top(&r.response); - app.auto.add(&format!("menu.{label}"), r.response.rect, label); + nav.top(&response); + // The titles and the open menus: a press inside them is not an outside press. + nav.cover(response.rect); + if let Some(inner) = &inner { + nav.cover(inner.response.rect); } + app.auto.add(&format!("menu.{label}"), response.rect, label); + drawn += 1; } }); }); crate::menu_keys::handle(&ctx, &nav); + // egui leaves the closing to us (`IgnoreClicks`), so the menus close here: at once on a press + // outside them (the press itself still reaches the widget below) and when the window loses + // focus. The pointer leaving the window closes nothing: moving onto the OS title bar fires + // the cursor-left event and the platform forwards none of its clicks to egui, so there is no + // click to close on — and a graze or a stay must not drop the menu, as in native apps. + if let Some(top) = nav.open_popup(&ctx) { + let focus_lost = ctx.input(|i| i.events.iter().any(|e| matches!(e, egui::Event::WindowFocused(false)))); + let outside_press = ctx.input(|i| i.pointer.any_pressed() && i.pointer.interact_pos().is_some_and(|p| !nav.covers(p))); + if focus_lost || outside_press { + egui::Popup::close_id(&ctx, top); + } + } if let Some((id, params)) = clicked && let Err(e) = invoke(app, &ctx, &id, if params.is_null() { json!({}) } else { params }) { app.ui.status = e; } + // What the next frame starts from: the open title, and a title press still in progress. + let open = nav.open_index(&ctx); + let browsing = open.is_some() && nav.browsing() && ctx.input(|i| i.pointer.primary_down()); + ctx.data_mut(|d| { + d.insert_temp(open_menu_id(), open); + d.insert_temp(browse_menu_id(), browsing); + }); } /// The entries of top-level menu `name` (the Effect menu is the Effect Controls panel's context @@ -2051,9 +2137,10 @@ fn menu_nodes( Some(t) => r.on_hover_text(t), None => r, }; + let r = browse_hover(nav, r); app.auto.add(&format!("menu.{name}.{i}"), r.rect, &e.label); nav.entry(ui, depth, &r, false); - if r.clicked() { + if r.clicked() || browse_pick(nav, ui, &r) { *clicked = Some((e.command.clone(), e.params.clone())); ui.close(); } @@ -2065,19 +2152,27 @@ fn menu_nodes( // every other submenu goes through the menu catalog. let shown = crate::i18n::effect_category(children, label) .unwrap_or_else(|| crate::i18n::submenu(app, label, effectcraft_engine::menus::submenu_label(&app.session, label, &dyn_ctx(&ws, &[])))); - let r = ui.menu_button((gutter(false), shown.as_str()), |ui| { + let browsing = nav.browsing(); + let egui::InnerResponse { inner, response } = ui.menu_button((gutter(false), shown.as_str()), |ui| { ui.set_min_width(if children.len() > 30 { 200.0 } else { 240.0 }); crate::widgets::menu_scroll(ui, |ui| menu_nodes(app, ui, children, clicked, nav, depth + 1)); }); - nav.entry(ui, depth, &r.response, true); - if r.inner.is_some() { - nav.opened(depth, &r.response); + let response = browse_hover(nav, response); + nav.entry(ui, depth, &response, true); + if inner.is_some() { + // `ui.menu_button` drops the popup's own response on the floor: read it back, + // so that a press inside this submenu doesn't count as an outside press. + let sub = egui::containers::menu::SubMenu::id_from_widget_id(response.id); + if let Some(resp) = ui.ctx().read_response(sub) { + nav.cover(resp.rect); + } + nav.opened(depth, &response); } } MenuNode::Item(e) => { - let r = menu_entry(app, ui, e); + let r = browse_hover(nav, menu_entry(app, ui, e)); nav.entry(ui, depth, &r, false); - if r.clicked() { + if r.clicked() || browse_pick(nav, ui, &r) { *clicked = Some((e.command.clone(), e.params.clone())); ui.close(); } @@ -2086,6 +2181,23 @@ fn menu_nodes( } } +/// The row `r` highlighted while the bar is mid press-and-browse: with a button down egui reports +/// nothing as hovered — which would leave browsing with no hover feedback at all — so the row +/// under the pointer takes the highlight by hand. It reads as the hover look one frame later, +/// exactly egui's own hover latency; disabled rows ignore it (`Noninteractive`), as in native +/// menus. Context menus use their own [`crate::menu_keys::Nav`], where the bar never browses. +fn browse_hover(nav: &crate::menu_keys::Nav, r: egui::Response) -> egui::Response { + if nav.browsing() && r.contains_pointer() { r.highlight() } else { r } +} + +/// Whether a press-and-browse (a menu-bar title still held down) releases over entry `r`, which +/// then chooses it like a click does. egui reports neither for it: the release is far from the +/// press, so there is no click, and nothing but the pressed widget is hovered while a button is +/// down, so the row has to be tested by the pointer being over it. +fn browse_pick(nav: &crate::menu_keys::Nav, ui: &egui::Ui, r: &egui::Response) -> bool { + nav.browsing() && r.enabled() && r.contains_pointer() && ui.input(|i| i.pointer.button_released(egui::PointerButton::Primary)) +} + /// The menu bar's submenu at `path` (`["File", "Interpret Footage"]`) in a panel's context menu, /// with the menu bar's labels, shortcuts and enabled state; a chosen entry goes to `clicked`. pub(crate) fn submenu_at(app: &mut EffectcraftApp, ui: &mut egui::Ui, path: &[&str], clicked: &mut Option<(String, Value)>) { diff --git a/crates/ui-egui/tests/ui_menus.rs b/crates/ui-egui/tests/ui_menus.rs index ca4c8cabc..a59599dfd 100644 --- a/crates/ui-egui/tests/ui_menus.rs +++ b/crates/ui-egui/tests/ui_menus.rs @@ -29,6 +29,25 @@ fn click_at(h: &mut Harness<'_, EffectcraftApp>, p: Pos2) { h.run_steps(2); } +/// Press the primary button at `p` and keep it down. +fn press(h: &mut Harness<'_, EffectcraftApp>, p: Pos2) { + h.input_mut().events.push(Event::PointerMoved(p)); + h.input_mut().events.push(Event::PointerButton { pos: p, button: egui::PointerButton::Primary, pressed: true, modifiers: Default::default() }); + h.run_steps(2); +} + +/// Move the pointer to `p` (press or no press, whichever is held). +fn move_to(h: &mut Harness<'_, EffectcraftApp>, p: Pos2) { + h.input_mut().events.push(Event::PointerMoved(p)); + h.run_steps(2); +} + +/// Release the primary button at `p`. +fn release(h: &mut Harness<'_, EffectcraftApp>, p: Pos2) { + h.input_mut().events.push(Event::PointerButton { pos: p, button: egui::PointerButton::Primary, pressed: false, modifiers: Default::default() }); + h.run_steps(2); +} + /// Press and release `key` (one frame each), then let the UI settle. fn key(h: &mut Harness<'_, EffectcraftApp>, key: Key) { h.input_mut().events.push(Event::Key { key, physical_key: None, pressed: true, repeat: false, modifiers: Modifiers::NONE }); @@ -244,3 +263,112 @@ fn space_previews_after_opening_the_demo_from_home() { key(&mut h, Key::Space); assert!(h.state().playback.playing, "Space previews the demo (#1059)"); } + +/// The titles open on the press, not the release, so the menus can be browsed with the button +/// still held down: moving onto an entry and releasing there chooses it (as in After Effects). +#[test] +fn a_menu_opens_on_press_and_browsing_releases_choose_an_entry() { + let mut h = harness(); + let edit = rect(&h, "menu.Edit"); + press(&mut h, edit.center()); + assert!(shown(&h, "Keyboard Shortcuts"), "the Edit menu opens on the press, before any release"); + let item = h.query_by_label_contains("Keyboard Shortcuts").expect("the Edit menu is open").rect(); + move_to(&mut h, item.center()); + release(&mut h, item.center()); + assert_eq!(h.state().dialog, Some(effectcraft_ui_egui::Dialog::Shortcuts), "the release over the entry chose it"); + assert!(!shown(&h, "Quick Apply"), "and closed the menu"); +} + +/// While a menu is open, hovering another title switches to that menu (native menu bars). +#[test] +fn hovering_another_title_while_a_menu_is_open_switches_to_it() { + let mut h = harness(); + let edit = rect(&h, "menu.Edit"); + click_at(&mut h, edit.center()); + assert!(shown(&h, "Keyboard Shortcuts"), "the Edit menu is open"); + let file = rect(&h, "menu.File"); + move_to(&mut h, file.center()); + assert!(shown(&h, "Open Project"), "hovering File opened the File menu"); + assert!(!shown(&h, "Keyboard Shortcuts"), "and closed Edit"); +} + +/// The press on the title of the open menu closes it again, and the click's release — a plain +/// click on the title — leaves it closed. +#[test] +fn pressing_the_open_title_again_closes_its_menu() { + let mut h = harness(); + let file = rect(&h, "menu.File"); + click_at(&mut h, file.center()); + assert!(shown(&h, "Open Project"), "the File menu is open"); + click_at(&mut h, file.center()); + assert!(!shown(&h, "Open Project"), "the second press closed it"); +} + +/// Press-and-hold browse (AE's gesture) reaches into a submenu: while the button is still held, +/// moving onto the submenu row opens it, and the release inside the submenu picks the entry. +#[test] +fn hold_browse_opens_a_submenu_and_the_release_inside_picks_an_entry() { + let mut h = harness(); + // Applying an effect needs a selected layer, as in the app after clicking one in the timeline. + let first = { + let cid = h.state().session.active_comp_id().expect("the demo opens a comp"); + let comp = h.state().session.project.comp(cid).expect("the demo comp exists"); + comp.layers.iter().find(|l| l.effects().is_some()).expect("the demo comp has a layer that takes effects").id + }; + h.state_mut().session.state.selected_layers = vec![first]; + let effect = rect(&h, "menu.Effect"); + press(&mut h, effect.center()); + assert!(shown(&h, "Effect Controls"), "the Effect menu opens on the press"); + // The categories come from the effect registry; the first alphabetically is "3D Channel". + let cat = h.query_by_label_contains("3D Channel").expect("the category row is shown").rect(); + move_to(&mut h, cat.center()); + assert!(shown(&h, "Cryptomatte"), "moving onto a category while still holding opened its submenu"); + let entry = h.query_by_label_contains("Cryptomatte").expect("the submenu is open").rect(); + move_to(&mut h, entry.center()); + release(&mut h, entry.center()); + assert!(!shown(&h, "Effect Controls"), "the release closed the menus"); + assert!(h.state().session.state.last_effect.is_some(), "and the release chose the entry: it applied its effect"); +} + +/// The open menus close on a press outside them and when the window loses focus. The pointer +/// leaving the window — onto the OS title bar, whose clicks are never forwarded to egui — closes +/// nothing, however long it stays away: there is no click to close on, and a graze or a stay +/// must not drop the menu. The closing press itself is not swallowed: it reaches the widget under +/// it, as a click does in After Effects. +#[test] +fn outside_presses_and_focus_loss_close_the_menus() { + let mut h = harness(); + let file = rect(&h, "menu.File"); + click_at(&mut h, file.center()); + assert!(shown(&h, "Open Project")); + // A press in the empty part of the bar, outside every title and menu. + click_at(&mut h, pos2(1100.0, 12.0)); + assert!(!shown(&h, "Open Project"), "a press outside the menus closes them"); + // The press reaches the header's workspace tab behind the open menu: the menu closes and the + // tab still acts (the menus don't swallow input). + click_at(&mut h, file.center()); + assert!(shown(&h, "Open Project")); + let tab = rect(&h, "header.workspace.Review"); + click_at(&mut h, tab.center()); + assert_eq!(h.state().ui.workspace, "Review", "the press reached the tab behind the open menu"); + assert!(!shown(&h, "Open Project"), "and closed the menu"); + // Moving onto the OS title bar fires the cursor-left event — its clicks are never forwarded + // to egui — which egui reports as `PointerGone`. It must not close the menus, however long + // the pointer stays away. + click_at(&mut h, file.center()); + assert!(shown(&h, "Open Project")); + h.input_mut().events.push(Event::PointerGone); + h.run_steps(1); + assert!(shown(&h, "Open Project"), "leaving the window alone doesn't drop the menus at once"); + let now = h.ctx.input(|i| i.time); + h.input_mut().time = Some(now + 60.0); + h.run_steps(2); + assert!(shown(&h, "Open Project"), "staying away doesn't close them either"); + h.input_mut().events.push(Event::PointerMoved(file.center())); + h.run_steps(2); + assert!(shown(&h, "Open Project"), "the pointer coming back keeps them open too"); + // Losing focus (alt-tabbing away, or clicking another window). + h.input_mut().events.push(Event::WindowFocused(false)); + h.run_steps(2); + assert!(!shown(&h, "Open Project"), "losing focus closes the menus"); +}