Fix #189: in-window menus and submenus stay below the menu bar

egui keeps a popup inside the window, so a menu taller than the room
under the bar (Photo, on a short window) was slid up over the bar: the
titles vanished under it, and the pointer left on the title rested on
the first row and opened its submenu by itself. Every level now gets
the room it has below the bar (a submenu: from its row down, or up to
the bar when egui opens it upward) and scrolls inside it, with wheel,
drag and ▲/▼ strips that scroll while the pointer rests on them, as in
PhotoCraft's menu bar (storytold/photocraft#337). Menu levels and
submenu rows are registered for the control channel and tests.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
ragtux 2026-10-08 17:50:18 -05:00
parent c395789026
commit 2d47ba3207
4 changed files with 291 additions and 6 deletions

View File

@ -15,6 +15,7 @@ pub mod icons;
pub mod import;
pub mod lightroom_import;
pub mod links;
pub mod menu_level;
pub mod menubar;
pub mod menus;
pub mod merge;
@ -44,6 +45,8 @@ mod tests_masking;
#[cfg(test)]
mod tests_masking_layout;
#[cfg(test)]
mod tests_menubar;
#[cfg(test)]
mod tests_offline;
#[cfg(test)]
mod tests_panels;

View File

@ -0,0 +1,177 @@
//! Menus bounded by the window (#189). egui keeps a popup inside the window: a menu or submenu
//! taller than the room below the menu bar was slid up over the bar, which hid the menu titles,
//! and the pointer that had rested on the title then rested on the menu's first row and opened
//! its submenu by itself. Here every level of a menu gets only the room it has below the bar (a
//! submenu: from its row down, or up to the bar when egui opens it upward) and scrolls inside
//! it: wheel, dragging, and ▲ / ▼ strips that scroll while the pointer rests on them, like
//! native menus on a small display. The same approach as PhotoCraft's menu bar (its #319).
use egui::{Id, Rect, Sense, Ui, pos2, vec2};
/// Height of a scroll-arrow strip.
pub const ARROW: f32 = 16.0;
/// Gap kept between a menu and the window edge.
const EDGE: f32 = 6.0;
/// How fast resting on a scroll arrow scrolls, in points per second.
const SPEED: f32 = 600.0;
fn anchor_id(depth: usize) -> Id {
Id::new(("lc-menu-anchor", depth))
}
/// Remember the row the submenu at `depth` (2 = a top-level menu's submenu) hangs from. It is
/// read on the next frame: a submenu is drawn while its row is, before the row's rect is known.
pub fn set_anchor(ctx: &egui::Context, depth: usize, row: Rect) {
let pass = ctx.cumulative_pass_nr();
ctx.data_mut(|d| d.insert_temp(anchor_id(depth), (row, pass)));
}
/// The row the submenu at `depth` hung from on the previous frame (none when it has just opened,
/// or when a stored row belongs to a submenu that has closed since).
fn anchor(ctx: &egui::Context, depth: usize) -> Option<Rect> {
anchor_at(ctx.data(|d| d.get_temp(anchor_id(depth))), ctx.cumulative_pass_nr())
}
/// The anchor `stored` as `(row, pass it was set on)`, if that was the pass before `pass`.
fn anchor_at(stored: Option<(Rect, u64)>, pass: u64) -> Option<Rect> {
stored.filter(|(_, at)| at.saturating_add(1) == pass).map(|(row, _)| row)
}
/// The height a menu level's rows may take. Every level stays below the menu bar: 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 bar. `anchor` is the submenu's row (unknown on its
/// first frame), `rows` the level's rows' height when known (the popup shrinks to it), and
/// `frame` the popup's margins.
pub fn level_room(screen: Rect, bar_bottom: Option<f32>, depth: usize, anchor: Option<Rect>, rows: Option<f32>, 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) {
Some(row) => {
// Downward from the row, or upward from it, whichever leaves more room. egui opens a
// submenu upward only when the downward one doesn't fit the window (it hangs from the
// row's top less half the frame margin).
let (down, up) = (bottom - row.top(), row.bottom() - top);
let opens_up = |h: f32| row.top() - (frame - 2.0) / 2.0 + h + frame > screen.bottom();
let want = rows.unwrap_or(f32::INFINITY);
if want > down && up > down && opens_up(want.min(up)) { up } else { down }.min(below_bar)
}
None => below_bar,
} - frame;
if room.is_finite() { room.max(4.0 * ARROW) } else { 4.0 * ARROW }
}
/// Is the pointer over `rect` on this ui's layer (not under a submenu drawn on top)?
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()))
}
/// Draw one level of a menu, bounded by the window: when its rows don't fit the room below the
/// menu bar they scroll, with scroll arrows at the top and bottom. `depth` is 1 for a top-level
/// menu; `bar_bottom` is where the menu bar ends. The level's rect is registered as
/// `menu-level:<depth>` for the control channel and tests.
pub fn level(ui: &mut Ui, depth: usize, bar_bottom: Option<f32>, rows: impl FnOnce(&mut Ui)) {
let ctx = ui.ctx().clone();
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;
let key = ui.id().with(("lc-menu-level", depth));
// Rows' height from the last frame: does this level overflow?
let content: Option<f32> = ctx.data(|d| d.get_temp(key));
let room = level_room(screen, bar_bottom, depth, anchor(&ctx, depth), content, frame);
let over = content.is_some_and(|h| h > room + 0.5);
let up = over.then(|| ui.allocate_exact_size(vec2(0.0, ARROW), Sense::hover()).0);
let height = if over { room - 2.0 * ARROW } else { room };
// The popup's `Ui` is only as tall as the popup was last frame (egui's default area size on
// the first), and a scroll area never grows past that. Ask for the room the window has;
// auto-shrink then fits the area to its rows, so a menu scrolls only when they don't fit.
let out = egui::ScrollArea::vertical()
.id_salt(("menu-level", depth))
.max_height(height)
.min_scrolled_height(height)
.scroll_source(egui::containers::scroll_area::ScrollSource::ALL)
.show(ui, rows);
let down = over.then(|| ui.allocate_exact_size(vec2(0.0, ARROW), Sense::hover()).0);
ctx.data_mut(|d| d.insert_temp(key, out.content_size.y));
crate::widgets::register(&ctx, format!("menu-level:{depth}"), ui.min_rect());
let (Some(up), Some(down)) = (up, down) else { return };
let t = crate::theme::Tokens::get(&ctx);
let x = ui.min_rect().x_range();
let max = (out.content_size.y - out.inner_rect.height()).max(0.0);
let offset = out.state.offset.y;
let dt = ctx.input(|i| i.stable_dt).clamp(0.0, 0.1);
let mut target = offset;
for (strip, dir, live) in [(up, -1.0, offset > 0.5), (down, 1.0, offset < max - 0.5)] {
let strip = Rect::from_x_y_ranges(x, strip.y_range());
let c = strip.center();
let (h, w) = (4.5, 6.0);
let tri = if dir < 0.0 {
vec![pos2(c.x - w, c.y + h * 0.5), pos2(c.x + w, c.y + h * 0.5), pos2(c.x, c.y - h * 0.5)]
} else {
vec![pos2(c.x - w, c.y - h * 0.5), pos2(c.x + w, c.y - h * 0.5), pos2(c.x, c.y + h * 0.5)]
};
ui.painter().add(egui::Shape::convex_polygon(tri, if live { t.text } else { t.text_disabled }, egui::Stroke::NONE));
let id = key.with(dir < 0.0);
let r = ui.interact(strip, id, Sense::hover());
r.widget_info(|| egui::WidgetInfo::labeled(egui::WidgetType::Button, live, if dir < 0.0 { "Scroll menu up" } else { "Scroll menu down" }));
if live && pointer_on(ui, strip) {
target = (offset + dir * SPEED * dt.max(1.0 / 120.0)).clamp(0.0, max);
ctx.request_repaint();
}
// The wheel scrolls over the arrows too.
if pointer_on(ui, strip) {
let wheel = ctx.input(|i| i.smooth_scroll_delta.y);
if wheel != 0.0 {
target = (target - wheel).clamp(0.0, max);
}
}
}
if target != offset {
let mut st = out.state;
st.offset.y = target;
st.store(&ctx, out.id);
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn level_room_keeps_every_level_below_the_bar() {
let screen = Rect::from_min_size(egui::Pos2::ZERO, vec2(1280.0, 703.0));
let (bar, frame) = (Some(42.0), 14.0);
let below = 703.0 - EDGE - (42.0 + EDGE) - frame;
assert_eq!(level_room(screen, bar, 1, None, None, frame), below);
// A submenu from a row near the top opens downward with the room under it…
let row = Rect::from_min_size(pos2(100.0, 80.0), vec2(200.0, 30.0));
assert_eq!(level_room(screen, bar, 2, Some(row), None, frame), 703.0 - EDGE - 80.0 - frame);
// …and from a row near the bottom, upward to the bar.
let low = Rect::from_min_size(pos2(100.0, 600.0), vec2(200.0, 30.0));
assert_eq!(level_room(screen, bar, 2, Some(low), None, frame), 630.0 - (42.0 + EDGE) - frame);
// A short submenu from a low row still fits downward: it keeps that room.
assert_eq!(level_room(screen, bar, 2, Some(low), Some(50.0), frame), 703.0 - EDGE - 600.0 - frame);
// Never more than the space under the bar, never less than the scroll arrows need.
let odd = Rect::from_min_size(pos2(100.0, -50.0), vec2(200.0, 30.0));
assert!(level_room(screen, bar, 2, Some(odd), None, frame) <= below);
assert_eq!(level_room(Rect::from_min_size(egui::Pos2::ZERO, vec2(100.0, 40.0)), bar, 1, None, None, frame), 4.0 * ARROW);
assert_eq!(level_room(screen, Some(f32::NAN), 1, None, None, frame), level_room(screen, None, 1, None, None, frame));
assert_eq!(level_room(Rect::NOTHING, bar, 2, Some(row), None, frame), 4.0 * ARROW);
// A top-level menu ignores a stale anchor.
assert_eq!(level_room(screen, bar, 1, Some(low), None, frame), below);
}
#[test]
fn an_anchor_is_read_on_the_next_frame_only() {
let row = Rect::from_min_size(pos2(10.0, 20.0), vec2(200.0, 24.0));
// set on pass 7: the submenu's room is measured from it on pass 8…
assert_eq!(anchor_at(Some((row, 7)), 8), Some(row));
// …not on the pass it was set (the row is drawn after its submenu), nor once the
// submenu has closed and stopped refreshing it
assert_eq!(anchor_at(Some((row, 7)), 7), None);
assert_eq!(anchor_at(Some((row, 7)), 9), None);
assert_eq!(anchor_at(None, 8), None);
assert_eq!(anchor_at(Some((row, u64::MAX)), 0), None);
}
}

View File

@ -746,22 +746,27 @@ pub fn show_in_window(app: &mut LightcraftApp, ui: &mut egui::Ui, max_width: f32
let mut clicked: Option<(String, Value)> = None;
let start = ui.cursor().left();
let mac = ui.ctx().os() == egui::os::OperatingSystem::Mac;
// Menus hang below the whole top bar and never grow past the window (#189): a tall menu
// scrolls instead of egui sliding it up over the titles.
let bar_bottom = Some(ui.max_rect().bottom());
if total <= max_width {
let saved = ui.spacing().item_spacing.x;
ui.spacing_mut().item_spacing.x = TITLE_GAP;
for (title, items) in &bar {
let r = ui.add(egui::Button::new(egui::RichText::new(crate::i18n::tr(title)).font(font.clone()).color(t.text_label)).frame(false));
crate::widgets::register(ui.ctx(), format!("menu:{title}"), r.rect);
egui::Popup::menu(&r).show(|ui| nodes_ui(ui, items, mac, &mut clicked));
egui::Popup::menu(&r).show(|ui| crate::menu_level::level(ui, 1, bar_bottom, |ui| nodes_ui(ui, items, mac, &mut clicked, 1, bar_bottom)));
}
ui.spacing_mut().item_spacing.x = saved;
} else {
let r = ui.add(egui::Button::new(egui::RichText::new(crate::i18n::tr("Menu")).font(font.clone()).color(t.text_label)).frame(false));
crate::widgets::register(ui.ctx(), "menu:all", r.rect);
egui::Popup::menu(&r).show(|ui| {
for (title, items) in &bar {
ui.menu_button(crate::i18n::tr(title), |ui| nodes_ui(ui, items, mac, &mut clicked));
}
crate::menu_level::level(ui, 1, bar_bottom, |ui| {
for (title, items) in &bar {
submenu(ui, title, crate::i18n::tr(title).to_string(), 1, bar_bottom, |ui| nodes_ui(ui, items, mac, &mut clicked, 2, bar_bottom));
}
});
});
}
if let Some((id, params)) = clicked {
@ -776,7 +781,19 @@ pub fn show_in_window(app: &mut LightcraftApp, ui: &mut egui::Ui, max_width: f32
ui.cursor().left() - start
}
fn nodes_ui(ui: &mut egui::Ui, nodes: &[MenuNode], mac: bool, clicked: &mut Option<(String, Value)>) {
/// A submenu row showing `text` at `depth`, whose rows `children` draws one level deeper,
/// bounded like every level. The row is registered as `menusub:<label>` (the untranslated
/// label), and while its submenu is open it is the anchor the next frame's room is measured from.
fn submenu(ui: &mut egui::Ui, label: &str, text: String, depth: usize, bar_bottom: Option<f32>, children: impl FnOnce(&mut egui::Ui)) {
let r = ui.menu_button(text, |ui| crate::menu_level::level(ui, depth + 1, bar_bottom, children));
crate::widgets::register(ui.ctx(), format!("menusub:{label}"), r.response.rect);
if r.inner.is_some() {
crate::menu_level::set_anchor(ui.ctx(), depth + 1, r.response.rect);
}
}
/// `depth` is 1 for a top-level menu's rows; `bar_bottom` is where the menu bar ends.
fn nodes_ui(ui: &mut egui::Ui, nodes: &[MenuNode], mac: bool, clicked: &mut Option<(String, Value)>, depth: usize, bar_bottom: Option<f32>) {
ui.set_min_width(220.0);
for n in nodes {
match n {
@ -784,7 +801,9 @@ fn nodes_ui(ui: &mut egui::Ui, nodes: &[MenuNode], mac: bool, clicked: &mut Opti
ui.separator();
}
MenuNode::Submenu { label, children } => {
ui.menu_button(format!(" {}", crate::i18n::tr(label)), |ui| nodes_ui(ui, children, mac, clicked));
submenu(ui, label, format!(" {}", crate::i18n::tr(label)), depth, bar_bottom, |ui| {
nodes_ui(ui, children, mac, clicked, depth + 1, bar_bottom)
});
}
MenuNode::Item { id, params, label, shortcut, enabled, checked } => {
// a gutter for check marks, like native menus

View File

@ -0,0 +1,86 @@
//! #189: the in-window menu bar's menus and submenus stay below the bar. On a short window a
//! tall menu (Photo) used to be slid up over the bar by egui, hiding the menu titles, and the
//! pointer left on the title then opened the first row's submenu by itself.
use crate::headless::Headless;
use crate::{LightcraftApp, Services};
use serde_json::json;
use std::time::Duration;
const T: Duration = Duration::from_secs(5);
const SETTLE: Duration = Duration::from_secs(10);
fn app(size: [f32; 2]) -> Headless {
let app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), Services { png: None, ..Default::default() });
let mut h = Headless::new(app, size, 1.0);
h.settle(SETTLE);
h
}
fn widget(h: &Headless, id: &str) -> Option<egui::Rect> {
h.app.widgets.iter().find(|(w, _)| w == id).map(|(_, r)| *r)
}
fn widgets_with(h: &Headless, prefix: &str) -> Vec<(String, egui::Rect)> {
h.app.widgets.iter().filter(|(w, _)| w.starts_with(prefix)).cloned().collect()
}
/// The open level `id` hangs below every menu title and ends inside the window.
fn check_bar_clear(h: &Headless, id: &str, what: &str) {
let level = widget(h, id).unwrap_or_else(|| panic!("{what}: no {id} on screen"));
let titles = widgets_with(h, "menu:");
assert!(!titles.is_empty(), "{what}: no menu titles");
let bar_bottom = titles.iter().map(|(_, r)| r.bottom()).fold(f32::MIN, f32::max);
assert!(level.top() >= bar_bottom - 0.5, "{what}: {id} {level:?} starts above the bar's bottom {bar_bottom}");
assert!(level.bottom() <= h.size.y + 0.5, "{what}: {id} {level:?} runs past the window's bottom {}", h.size.y);
for (title, r) in &titles {
assert!(!level.intersects(*r), "{what}: {id} {level:?} covers {title} {r:?}");
}
}
#[test]
fn no_menu_or_submenu_covers_the_menu_bar() {
// The reporter's 1280 × 720 display less the title bar, and a shorter window where even a
// scrolled menu's submenus must open upward.
let sizes = [[1280.0, 703.0], [1160.0, 560.0]];
let mut submenus = 0;
for size in sizes {
for top in ["Photo", "File", "View"] {
let what = format!("{size:?} {top}");
let mut h = app(size);
assert!(widget(&h, "menu-level:1").is_none(), "{what}: a menu is open before any click");
let r = h.request("ui.clickWidget", json!({"id": format!("menu:{top}")}), T);
assert_eq!(r["ok"], true, "{what}: {r}");
h.settle(SETTLE);
check_bar_clear(&h, "menu-level:1", &what);
// Hover each submenu row in view, top to bottom: rows low in the menu open upward.
let level = widget(&h, "menu-level:1").unwrap_or_else(|| panic!("{what}: menu"));
let rows: Vec<String> = widgets_with(&h, "menusub:").into_iter().filter(|(_, r)| level.contains_rect(*r)).map(|(id, _)| id).collect();
for row in rows {
let r = h.request("ui.hoverWidget", json!({"id": row}), T);
assert_eq!(r["ok"], true, "{what}: {r}");
h.settle(SETTLE);
if widget(&h, "menu-level:2").is_none() {
continue;
}
submenus += 1;
check_bar_clear(&h, "menu-level:2", &format!("{what} › {row}"));
}
}
}
assert!(submenus >= 6, "opened {submenus} submenus");
}
#[test]
fn a_tall_menu_scrolls_instead_of_growing_past_the_window() {
let mut h = app([1160.0, 420.0]);
let r = h.request("ui.clickWidget", json!({"id": "menu:Photo"}), T);
assert_eq!(r["ok"], true, "{r}");
h.settle(SETTLE);
check_bar_clear(&h, "menu-level:1", "420 pt window");
let level = widget(&h, "menu-level:1").expect("Photo menu");
// Not every row fits: some are scrolled out of view, none drawn outside the menu.
let rows = widgets_with(&h, "menusub:");
assert!(!rows.is_empty(), "no submenu rows registered");
assert!(rows.iter().any(|(_, r)| !level.contains_rect(*r)), "every row fits a 420 pt window: {rows:?}");
}