diff --git a/crates/ui-egui/src/menu_nav.rs b/crates/ui-egui/src/menu_nav.rs index 7e1f22185b..c58f0f70d4 100644 --- a/crates/ui-egui/src/menu_nav.rs +++ b/crates/ui-egui/src/menu_nav.rs @@ -51,6 +51,9 @@ pub struct Nav { pub(crate) views: Vec, /// The full height of each level's rows (taller than its view when it scrolls). pub(crate) contents: Vec, + /// Per level: the row its open submenu hangs from (level 0 has none). From the frame + /// before, since a submenu is drawn while its row is. + anchors: Vec>, /// Scroll the highlighted row of `level` into view when it is next drawn. reveal: bool, /// Highlight the first enabled row of this level once it has been drawn (a keyboard-opened @@ -183,6 +186,16 @@ impl Nav { self.first_at = Some(level + 1); } + /// Remember the row the submenu of `level` (1 = a top-level menu's submenu) hangs from. + pub fn set_anchor(&mut self, level: usize, row: Rect) { + if self.anchors.len() <= level { + self.anchors.resize(level + 1, None); + } + if let Some(a) = self.anchors.get_mut(level) { + *a = Some(row); + } + } + /// Handle this frame's navigation keys (after the menus were drawn). `tops` are the menu /// bar's popup ids; an activated command lands in `clicked`. pub fn keys(&mut self, ctx: &egui::Context, tops: &[Id], clicked: &mut Option) { @@ -252,6 +265,23 @@ fn pointer_on(ui: &Ui, rect: Rect) -> bool { ui.ctx().pointer_hover_pos().is_some_and(|p| rect.contains(p) && ui.ctx().layer_id_at(p) == Some(ui.layer_id())) } +/// The height a menu level's rows may take. Every level stays below the menu bar (#319): a +/// top-level menu hangs from it, and a submenu that is too tall for the window between its row +/// and the bottom, or (opening upward, as egui does when that fits) between the bar and its row, +/// scrolls instead of egui sliding it up over the menu bar, where it hid the menu titles. +/// `anchor` is the submenu's row (unknown on its first frame); `frame` the popup's margins. +pub fn level_room(screen: Rect, bar_bottom: Option, depth: usize, anchor: Option, frame: f32) -> f32 { + let top = bar_bottom.unwrap_or(screen.top()).max(screen.top()) + EDGE; + let bottom = screen.bottom() - EDGE; + let below_bar = bottom - top; + let room = match anchor.filter(|_| depth > 1) { + // Downward from the row, or upward from it, whichever leaves more room. + Some(row) => (bottom - row.top()).max(row.bottom() - top).min(below_bar), + None => below_bar, + } - frame; + if room.is_finite() { room.max(4.0 * ARROW) } else { 4.0 * ARROW } +} + /// Draw one level of a menu, bounded by the window: when its rows don't fit they scroll, with /// scroll arrows at the top and bottom. `depth` is 1 for a top-level menu. pub fn level(ui: &mut Ui, depth: usize, nav: &mut Nav, rows: impl FnOnce(&mut Ui, &mut Nav)) { @@ -261,11 +291,8 @@ pub fn level(ui: &mut Ui, depth: usize, nav: &mut Nav, rows: impl FnOnce(&mut Ui let screen = ctx.content_rect(); // The menu frame's margin and stroke around the rows. let frame = ui.spacing().menu_margin.sum().y + 2.0; - // A top-level menu hangs below the menu bar; a submenu may slide up to fit the window. - // (From the menu bar, not from where the popup is now: egui slides a popup that doesn't fit.) - let top = nav.bar_bottom.unwrap_or(screen.top()) + EDGE; - let room = if depth <= 1 { screen.bottom() - top - EDGE } else { screen.height() - 2.0 * EDGE } - frame; - let room = room.max(4.0 * ARROW); + let anchor = nav.anchors.get(level).copied().flatten(); + let room = level_room(screen, nav.bar_bottom, depth, anchor, frame); let key = ui.id().with(("pc-menu-level", depth)); // Rows' height from the last frame: does this level overflow? let content: Option = ctx.data(|d| d.get_temp(key)); diff --git a/crates/ui-egui/src/menu_nav_tests.rs b/crates/ui-egui/src/menu_nav_tests.rs index 097f5d8eac..e5698a276f 100644 --- a/crates/ui-egui/src/menu_nav_tests.rs +++ b/crates/ui-egui/src/menu_nav_tests.rs @@ -336,3 +336,77 @@ fn a_short_window_still_scrolls_the_file_menu_to_its_last_row() { } assert!(visible(&h, 0, last), "the wheel reaches File's last row"); } + +/// Every popup on screen (menus and submenus). +fn popups(h: &Harness<'static, PhotocraftApp>) -> Vec { + h.ctx + .memory(|m| m.areas().visible_layer_ids()) + .into_iter() + .filter(|l| l.order == egui::Order::Foreground) + .filter_map(|layer| h.ctx.memory(|m| m.area_rect(layer.id))) + .collect() +} + +/// No popup covers the menu bar or runs off the window, and every menu title stays visible. +fn check_bar_clear(h: &Harness<'static, PhotocraftApp>, what: &str) { + let bar = Nav::current(&h.ctx).bar_bottom.expect("the menu bar's bottom"); + let screen = h.ctx.content_rect(); + for r in popups(h) { + assert!(r.top() >= bar - 0.5, "{what}: popup {r:?} covers the menu bar (bottom {bar})"); + assert!(r.bottom() <= screen.bottom() + 0.5, "{what}: popup {r:?} runs off the window {screen:?}"); + } + for title in crate::menus::TOP_MENUS { + // The title is the topmost node of that name (a submenu row can share it). + let t = h.query_all_by_label(title).map(|n| n.rect()).min_by(|a, b| a.top().total_cmp(&b.top())).expect("menu title"); + assert!(popups(h).iter().all(|r| !r.intersects(t.shrink(1.0))), "{what}: {title} is covered"); + } +} + +#[test] +fn no_menu_or_submenu_covers_the_menu_bar() { + // #319: on a 1280 × 720 display (the reporter's, at 1× and 2×, less the title bar) tall menus, + // then tall submenus (Image › Adjustments), slid up over the menu bar and hid its titles. + let displays = [(1280.0, 703.0, 1.0), (2560.0, 1406.0, 2.0), (1366.0, 768.0, 1.0)]; + let mut checked = 0; + for display in displays { + for top in LONGEST { + let mut h = harness(display); + open(&mut h, top); + check_bar_clear(&h, &format!("{display:?} {top}")); + // Hover each submenu row in view, top to bottom: rows low in the menu open their + // submenus upward. + let view = Nav::current(&h.ctx).views.first().copied().unwrap_or(egui::Rect::NOTHING); + let subs: Vec = rows(&h, 0).iter().filter(|r| r.enabled && r.command.is_none() && view.contains_rect(r.rect)).map(|r| r.rect).collect(); + for (i, row) in subs.iter().enumerate() { + h.hover_at(row.center() + egui::vec2(-20.0, 0.0)); + h.run_steps(6); + if Nav::current(&h.ctx).rows.get(1).is_none_or(Vec::is_empty) { + continue; + } + checked += 1; + check_bar_clear(&h, &format!("{display:?} {top} submenu {i}")); + } + } + } + assert!(checked > 30, "checked {checked} submenus"); +} + +#[test] +fn level_room_keeps_every_level_below_the_bar() { + let screen = egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(1280.0, 703.0)); + let (bar, frame) = (Some(30.0), 14.0); + let below = 703.0 - EDGE - (30.0 + EDGE) - frame; + assert_eq!(level_room(screen, bar, 1, None, frame), below); + // A submenu from a row near the top opens downward with the room under it… + let row = egui::Rect::from_min_size(egui::pos2(100.0, 80.0), egui::vec2(200.0, 30.0)); + assert_eq!(level_room(screen, bar, 2, Some(row), frame), 703.0 - EDGE - 80.0 - frame); + // …and from a row near the bottom, upward to the bar. + let low = egui::Rect::from_min_size(egui::pos2(100.0, 600.0), egui::vec2(200.0, 30.0)); + assert_eq!(level_room(screen, bar, 2, Some(low), frame), 630.0 - (30.0 + EDGE) - frame); + // Never more than the space under the bar, never less than the scroll arrows need. + let odd = egui::Rect::from_min_size(egui::pos2(100.0, -50.0), egui::vec2(200.0, 30.0)); + assert!(level_room(screen, bar, 2, Some(odd), frame) <= below); + assert_eq!(level_room(egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(100.0, 40.0)), bar, 1, None, frame), 4.0 * ARROW); + assert_eq!(level_room(screen, Some(f32::NAN), 1, None, frame), level_room(screen, None, 1, None, frame)); + assert_eq!(level_room(egui::Rect::NOTHING, bar, 2, Some(row), frame), 4.0 * ARROW); +} diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 8e06771232..be3fe7983f 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -855,7 +855,12 @@ fn render_level_rows(ui: &mut egui::Ui, items: &[&MenuItem], depth: usize, click let enabled = any_enabled || !child.is_empty(); ui.add_enabled_ui(enabled, |ui| { nav.row(ui, depth - 1, enabled, None, |ui, nav| { - (ui.menu_button(crate::i18n::tr(lang, name), |ui| render_level(ui, &child, depth + 1, clicked, nav)).response, ()) + let r = ui.menu_button(crate::i18n::tr(lang, name), |ui| render_level(ui, &child, depth + 1, clicked, nav)); + if r.inner.is_some() { + // Where the submenu hangs from, to keep it below the menu bar (#319). + nav.set_anchor(depth, r.response.rect); + } + (r.response, ()) }); }); last_was_sep = false;