Menus: the main menu and its submenus stay inside the window and scroll when taller (#2217) (#2220)

Submenus of the main menu were not constrained to the window (constrain: false, so they sit
beside their row), so the Object submenu ran past the bottom edge with its lowest commands out of
reach. Menus and submenus are now held within the window's height (moved up, never above the top,
8 pt margins, still beside their row so menu-aim is unchanged), and a panel taller than that
scrolls with the wheel; the keyboard's focus scrolls into view.


Model-Effort: Opus 5.5 drove; Opus 5.5 briefed; Sonnet 5.5 wrote at medium

Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>

crates/ui-egui/src/lib.rs

@@ -180,6 +180,8 @@ mod menu_contrast_tests;
 #[cfg(test)]
 mod menu_input_tests;
 #[cfg(test)]
+mod menu_overflow_tests;
+#[cfg(test)]
 mod mirror_tests;
 #[cfg(test)]
 mod missing_fonts_tests;

crates/ui-egui/src/menu_overflow_tests.rs

@@ -0,0 +1,107 @@
+//! The main menu and its submenus stay inside the window (#2217): a submenu that would run past
+//! the bottom moves up to fit, and one taller than the window scrolls.
+
+use egui::{Event, Id, MouseWheelUnit, Rect, TouchPhase, pos2, vec2};
+use figcraft_engine::Session;
+use serde_json::json;
+
+use crate::layers_drag_tests::Ui;
+use crate::live_tests::{flat, text_center};
+use crate::menus::{HOVER_DELAY, main_menu_id};
+use crate::present_tests::id;
+
+/// A window `h` tall with the main menu open and the Object submenu showing.
+fn object_menu_open(s: Session, h: f32) -> Ui {
+    let mut u = Ui::new(s);
+    u.screen = vec2(1280.0, h);
+    u.app.ui.main_menu = true;
+    u.frame(vec![Event::PointerMoved(pos2(1000.0, 10.0))]);
+    u.frame(vec![]);
+    let m = u.ctx.memory(|m| m.area_rect(main_menu_id())).expect("main menu drawn");
+    // Frame margin, the Actions row and the space under it, then three rows down to Object.
+    let y = m.min.y + 8.0 + 30.0 + 6.0 + 28.0 * 3.0 + 14.0;
+    u.frame(vec![Event::PointerMoved(pos2(m.min.x + 60.0, y))]);
+    for _ in 0..((HOVER_DELAY + 0.3) * 60.0) as usize {
+        u.frame(vec![]);
+    }
+    u
+}
+
+fn object_panel(u: &Ui) -> Rect {
+    u.ctx.memory(|m| m.area_rect(Id::unique(("menu", "Object")))).expect("the Object submenu is drawn")
+}
+
+#[test]
+fn the_object_submenu_stays_inside_a_short_window() {
+    let u = object_menu_open(Session::new(), 600.0);
+    let panel = object_panel(&u);
+    let window = Rect::from_min_size(pos2(0.0, 0.0), vec2(1280.0, 600.0));
+    assert!(window.contains_rect(panel), "the Object submenu {panel:?} runs outside the window {window:?}");
+}
+
+#[test]
+fn a_menu_taller_than_the_window_scrolls_to_its_last_command() {
+    let mut s = Session::new();
+    let a = id(&s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 40, "height": 30})).unwrap());
+    let b = id(&s.execute("shape.rectangle", &json!({"x": 100, "y": 30, "width": 20, "height": 20})).unwrap());
+    s.execute("select.set", &json!({"ids": [b]})).unwrap();
+    let mut u = object_menu_open(s, 420.0);
+    let panel = object_panel(&u);
+    assert!(Rect::from_min_size(pos2(0.0, 0.0), vec2(1280.0, 420.0)).contains_rect(panel), "{panel:?} outside the window");
+    // Turn the wheel over the panel until the end of the list is in view.
+    for _ in 0..4 {
+        let wheel = Event::MouseWheel {
+            unit: MouseWheelUnit::Point,
+            source: egui::MouseWheelSource::Unknown,
+            delta: vec2(0.0, -1500.0),
+            phase: TouchPhase::Move,
+            modifiers: Default::default(),
+        };
+        u.frame(vec![Event::PointerMoved(panel.center()), wheel]);
+        for _ in 0..20 {
+            u.frame(vec![]);
+        }
+    }
+    let shapes = flat(&u.frame_output(vec![]).shapes);
+    let at = text_center(&shapes, "Move to page");
+    assert!(panel.contains(at), "the last commands are out of reach: at {at:?} outside {panel:?}");
+    // And a command above it, reached by scrolling back up, runs on click.
+    let wheel = Event::MouseWheel {
+        unit: MouseWheelUnit::Point,
+        source: egui::MouseWheelSource::Unknown,
+        delta: vec2(0.0, 1.0e5),
+        phase: TouchPhase::Move,
+        modifiers: Default::default(),
+    };
+    u.frame(vec![Event::PointerMoved(panel.center()), wheel]);
+    for _ in 0..20 {
+        u.frame(vec![]);
+    }
+    // Scroll down in small steps until "Send to back" shows inside the panel, then click it.
+    let mut hit = None;
+    for _ in 0..40 {
+        let shapes = flat(&u.frame_output(vec![]).shapes);
+        let at = text_center(&shapes, "Send to back");
+        if panel.contains(at) {
+            hit = Some(at);
+            break;
+        }
+        let wheel = Event::MouseWheel {
+            unit: MouseWheelUnit::Point,
+            source: egui::MouseWheelSource::Unknown,
+            delta: vec2(0.0, -120.0),
+            phase: TouchPhase::Move,
+            modifiers: Default::default(),
+        };
+        u.frame(vec![Event::PointerMoved(panel.center()), wheel]);
+        for _ in 0..10 {
+            u.frame(vec![]);
+        }
+    }
+    let at = hit.expect("Send to back scrolls into view");
+    crate::component_swap_default_tests::click(&mut u, at);
+    let doc = u.app.session.doc().unwrap();
+    let page = u.app.session.page().unwrap();
+    let kids = &doc.node(page).unwrap().children;
+    assert_eq!((kids.first().copied(), kids.last().copied()), (Some(b), Some(a)), "Send to back ran");
+}

crates/ui-egui/src/menus.rs

@@ -382,40 +382,49 @@ pub fn popups(app: &mut FigApp, ctx: &egui::Context) {
     // Main menu.
     if app.ui.main_menu {
         let mut close = false;
-        let area = egui::Area::new(main_menu_id()).fixed_pos(pos2(8.0, crate::tabs::HEIGHT + 48.0)).order(egui::Order::Foreground).show(ctx, |ui| {
-            menu_frame(ui, &t, At { root: main_menu_id(), path: String::new() }, |ui| {
-                // Quick actions search row.
-                let (r, resp) = ui.allocate_exact_size(vec2(ui.available_width(), 30.0), Sense::click());
-                plain_row_hover(ui, &resp);
-                a11y::menu_item(&resp, tr("Actions…"), crate::shortcuts::shortcut("app.actions").unwrap_or(""), None);
-                ui.painter().rect_filled(r.shrink(2.0), CornerRadius::same(5), Color32::from_white_alpha(18));
-                icons::paint(ui.painter(), Rect::from_center_size(pos2(r.min.x + 16.0, r.center().y), vec2(14.0, 14.0)), Icon::Search, t.menu_dim);
-                ui.painter().text(pos2(r.min.x + 32.0, r.center().y), Align2::LEFT_CENTER, tr("Actions…"), regular(12.0), t.menu_dim);
-                ui.painter().text(
-                    pos2(r.max.x - 10.0, r.center().y),
-                    Align2::RIGHT_CENTER,
-                    pretty_shortcut(crate::shortcuts::shortcut("app.actions").unwrap_or("")),
-                    regular(12.0),
-                    t.menu_dim,
-                );
-                if resp.clicked() {
-                    set_palette(&mut app.ui, true);
-                    close = true;
-                }
-                ui.add_space(6.0);
-                for top in TOP_MENUS {
-                    if submenu_at(
-                        ui,
-                        SubmenuOptions { area: Id::unique(("menu", *top)), key: top, label: tr(top), gap: 12.0, constrain: false, tokens: &t },
-                        |ui| menu_level(app, ui, top).then_some(()),
-                    )
-                    .is_some()
-                    {
+        let area = egui::Area::new(main_menu_id())
+            .fixed_pos(pos2(8.0, crate::tabs::HEIGHT + 48.0))
+            .order(egui::Order::Foreground)
+            .constrain_to(tree::vertical_bounds(ctx))
+            .show(ctx, |ui| {
+                menu_frame(ui, &t, At { root: main_menu_id(), path: String::new() }, |ui| {
+                    // Quick actions search row.
+                    let (r, resp) = ui.allocate_exact_size(vec2(ui.available_width(), 30.0), Sense::click());
+                    plain_row_hover(ui, &resp);
+                    a11y::menu_item(&resp, tr("Actions…"), crate::shortcuts::shortcut("app.actions").unwrap_or(""), None);
+                    ui.painter().rect_filled(r.shrink(2.0), CornerRadius::same(5), Color32::from_white_alpha(18));
+                    icons::paint(
+                        ui.painter(),
+                        Rect::from_center_size(pos2(r.min.x + 16.0, r.center().y), vec2(14.0, 14.0)),
+                        Icon::Search,
+                        t.menu_dim,
+                    );
+                    ui.painter().text(pos2(r.min.x + 32.0, r.center().y), Align2::LEFT_CENTER, tr("Actions…"), regular(12.0), t.menu_dim);
+                    ui.painter().text(
+                        pos2(r.max.x - 10.0, r.center().y),
+                        Align2::RIGHT_CENTER,
+                        pretty_shortcut(crate::shortcuts::shortcut("app.actions").unwrap_or("")),
+                        regular(12.0),
+                        t.menu_dim,
+                    );
+                    if resp.clicked() {
+                        set_palette(&mut app.ui, true);
                         close = true;
                     }
-                }
+                    ui.add_space(6.0);
+                    for top in TOP_MENUS {
+                        if submenu_at(
+                            ui,
+                            SubmenuOptions { area: Id::unique(("menu", *top)), key: top, label: tr(top), gap: 12.0, constrain: false, tokens: &t },
+                            |ui| menu_level(app, ui, top).then_some(()),
+                        )
+                        .is_some()
+                        {
+                            close = true;
+                        }
+                    }
+                });
             });
-        });
         let clicked_outside = ctx.input(|i| i.pointer.any_pressed()) && !over_menu(ctx) || ctx.input(|i| i.key_pressed(egui::Key::Escape));
         if close || (clicked_outside && !area.response.hovered()) {
             app.ui.main_menu = false;

crates/ui-egui/src/menus/tree.rs

@@ -289,6 +289,7 @@ pub(super) fn submenu_at<R>(ui: &mut Ui, options: SubmenuOptions<'_>, add: impl
         .fixed_pos(pos)
         .order(egui::Order::Foreground)
         .constrain(constrain || cramped)
+        .constrain_to(if constrain || cramped { screen } else { vertical_bounds(ui.ctx()) })
         .show(ui.ctx(), |ui| menu_frame(ui, t, child, add));
     let panel = shown.response.rect;
     with_level(ui.ctx(), &at, |l| l.panel = Some(panel));
@@ -316,12 +317,64 @@ pub(super) fn menu_frame<R>(ui: &mut Ui, t: &Tokens, at: At, add: impl FnOnce(&m
             ui.set_width(w);
             ui.spacing_mut().item_spacing = vec2(0.0, 0.0);
             let key = at_key(ui);
-            ui.data_mut(|d| d.insert_temp(key, at));
-            add(ui)
+            ui.data_mut(|d| d.insert_temp(key, at.clone()));
+            // Taller than the window (as measured last frame): its rows scroll. Otherwise they
+            // draw straight into the panel, so a menu that fits behaves as it always did.
+            let max_h = (ui.ctx().content_rect().height() - 2.0 * EDGE_MARGIN - 16.0).max(MENU_ROW_H);
+            let height_key = at_key(ui).with("content_h");
+            let last_h = ui.data(|d| d.get_temp::<f32>(height_key)).unwrap_or(0.0);
+            if last_h <= max_h {
+                let top = ui.cursor().min.y;
+                let r = add(ui);
+                let h = ui.min_rect().max.y - top;
+                ui.data_mut(|d| d.insert_temp(height_key, h));
+                if h > max_h {
+                    ui.ctx().request_repaint();
+                }
+                return r;
+            }
+            // The scroll area's ui has its own scope, so it gets the level and width state of this
+            // one, and hands back the width its rows asked for.
+            let outer_w = width_key(ui);
+            let asked = ui.data(|d| d.get_temp::<f32>(outer_w));
+            let shown = egui::ScrollArea::vertical()
+                .max_height(max_h)
+                .auto_shrink([false, true])
+                .scroll_source(egui::scroll_area::ScrollSource::MOUSE_WHEEL | egui::scroll_area::ScrollSource::SCROLL_BAR)
+                .show(ui, |ui| {
+                    let (inner_at, inner_w) = (at_key(ui), width_key(ui));
+                    ui.data_mut(|d| {
+                        d.insert_temp(inner_at, at);
+                        if let Some(a) = asked {
+                            d.insert_temp(inner_w, a);
+                        }
+                    });
+                    let r = add(ui);
+                    (r, ui.data(|d| d.get_temp::<f32>(inner_w)))
+                });
+            ui.data_mut(|d| d.insert_temp(height_key, shown.content_size.y));
+            let (r, wanted) = shown.inner;
+            if let Some(wanted) = wanted.filter(|w| asked.is_none_or(|a| *w > a)) {
+                ui.data_mut(|d| d.insert_temp(outer_w, wanted));
+            }
+            r
         })
         .inner
 }
 
+/// The gap a menu keeps from the window's top and bottom edges.
+pub(super) const EDGE_MARGIN: f32 = 8.0;
+
+/// A menu row's height.
+const MENU_ROW_H: f32 = 28.0;
+
+/// Where a menu may sit: anywhere across, within the window's height less the margins. A menu that
+/// would run past the bottom moves up (never above the top); one taller than that scrolls.
+pub(super) fn vertical_bounds(ctx: &egui::Context) -> Rect {
+    let s = ctx.content_rect();
+    Rect::from_min_max(pos2(-1.0e5, s.min.y + EDGE_MARGIN), pos2(1.0e5, s.max.y - EDGE_MARGIN))
+}
+
 /// A menu's narrowest width.
 const MENU_W: f32 = 240.0;