From bc55ca1463830fe680b8f7e62b1be29e98e681fb Mon Sep 17 00:00:00 2001 From: Trevor Brown Date: Fri, 6 Nov 2026 20:59:35 -0500 Subject: [PATCH] Local variables: the bulk menu scrolls when it is taller than the window (#2002) The Move-to-group and Scope rows now sit in a vertical ScrollArea capped at 60% of the screen height, so a collection with many groups no longer pushes rows past the window. The test harness gains a settable screen size. Co-Authored-By: Claude Fable 5.1 --- crates/ui-egui/src/dialogs/variables_panel.rs | 70 ++++++++++--------- crates/ui-egui/src/layers_drag_tests.rs | 6 +- crates/ui-egui/src/variables_select_tests.rs | 58 +++++++++++++++ 3 files changed, 100 insertions(+), 34 deletions(-) diff --git a/crates/ui-egui/src/dialogs/variables_panel.rs b/crates/ui-egui/src/dialogs/variables_panel.rs index d0a0cc6bc..1a2a79151 100644 --- a/crates/ui-egui/src/dialogs/variables_panel.rs +++ b/crates/ui-egui/src/dialogs/variables_panel.rs @@ -119,41 +119,47 @@ fn bulk_menu(app: &mut FigApp, ui: &mut Ui, coll_groups: &[String], vars: &[&Var bulk_run(app, "Delete variables", vars, |v| Some(("variable.delete", json!({"variable": v.id})))); ui.close(); } - let moves = std::iter::once(None).chain(coll_groups.iter().map(|g| Some(g.as_str()))); - for g in moves { - let label = g.map_or_else(|| tr("Move out of any group").to_string(), |g| trf("Move to group {group}", &[("group", &g)])); - if menu_item(ui, &label, "") { - bulk_run(app, "Move variables", vars, |v| { - let short = v.name.rsplit('/').next().unwrap_or(&v.name); - let name = g.map_or_else(|| short.to_string(), |g| format!("{g}/{short}")); - (name != v.name).then(|| ("variable.update", json!({"variable": v.id, "name": name}))) - }); - ui.close(); - } - } - let mut scopes: Vec<(&'static str, &'static str)> = Vec::new(); - for v in vars { - for s in scopes_for(v.kind) { - if !scopes.contains(s) { - scopes.push(*s); + // Many groups make the menu taller than the window: the rows after Delete scroll. + let max_h = ui.ctx().content_rect().height() * 0.6; + egui::ScrollArea::vertical().id_salt("variables_bulk_menu_scroll").max_height(max_h).show(ui, |ui| { + let moves = std::iter::once(None).chain(coll_groups.iter().map(|g| Some(g.as_str()))); + for g in moves { + let label = g.map_or_else(|| tr("Move out of any group").to_string(), |g| trf("Move to group {group}", &[("group", &g)])); + if menu_item(ui, &label, "") { + bulk_run(app, "Move variables", vars, |v| { + let short = v.name.rsplit('/').next().unwrap_or(&v.name); + let name = g.map_or_else(|| short.to_string(), |g| format!("{g}/{short}")); + (name != v.name).then(|| ("variable.update", json!({"variable": v.id, "name": name}))) + }); + ui.close(); } } - } - let set = |app: &mut FigApp, label: &str, value: Option<&'static str>, ui: &mut Ui| { - if menu_item(ui, label, "") { - bulk_run(app, "Change scope", vars, |v| match value { - None => Some(("variable.update", json!({"variable": v.id, "scopes": Value::Null}))), - Some("") => Some(("variable.update", json!({"variable": v.id, "scopes": []}))), - Some(s) => scopes_for(v.kind).iter().any(|(id, _)| *id == s).then(|| ("variable.update", json!({"variable": v.id, "scopes": [s]}))), - }); - ui.close(); + let mut scopes: Vec<(&'static str, &'static str)> = Vec::new(); + for v in vars { + for s in scopes_for(v.kind) { + if !scopes.contains(s) { + scopes.push(*s); + } + } } - }; - set(app, tr("Scope: all supported properties"), None, ui); - set(app, tr("Scope: hidden from pickers"), Some(""), ui); - for (id, label) in scopes { - set(app, &trf("Scope: only {scope}", &[("scope", &tr(label))]), Some(id), ui); - } + let set = |app: &mut FigApp, label: &str, value: Option<&'static str>, ui: &mut Ui| { + if menu_item(ui, label, "") { + bulk_run(app, "Change scope", vars, |v| match value { + None => Some(("variable.update", json!({"variable": v.id, "scopes": Value::Null}))), + Some("") => Some(("variable.update", json!({"variable": v.id, "scopes": []}))), + Some(s) => { + scopes_for(v.kind).iter().any(|(id, _)| *id == s).then(|| ("variable.update", json!({"variable": v.id, "scopes": [s]}))) + } + }); + ui.close(); + } + }; + set(app, tr("Scope: all supported properties"), None, ui); + set(app, tr("Scope: hidden from pickers"), Some(""), ui); + for (id, label) in scopes { + set(app, &trf("Scope: only {scope}", &[("scope", &tr(label))]), Some(id), ui); + } + }); } impl Editor { diff --git a/crates/ui-egui/src/layers_drag_tests.rs b/crates/ui-egui/src/layers_drag_tests.rs index 978a3b236..f1d644778 100644 --- a/crates/ui-egui/src/layers_drag_tests.rs +++ b/crates/ui-egui/src/layers_drag_tests.rs @@ -13,11 +13,13 @@ pub(crate) struct Ui { pub(crate) ctx: egui::Context, pub(crate) app: FigApp, time: f64, + /// The test screen's size (1440 x 900 unless a test shrinks it). + pub(crate) screen: egui::Vec2, } impl Ui { pub(crate) fn new(s: Session) -> Self { - let mut u = Self { ctx: egui::Context::default(), app: FigApp::new(s, Box::new(NoServices)), time: 0.0 }; + let mut u = Self { ctx: egui::Context::default(), app: FigApp::new(s, Box::new(NoServices)), time: 0.0, screen: vec2(1440.0, 900.0) }; for _ in 0..3 { u.frame(vec![]); } @@ -44,7 +46,7 @@ impl Ui { pub(crate) fn frame_raw(&mut self, raw: RawInput) -> egui::FullOutput { self.time += 1.0 / 60.0; self.app.clock.set_test_now(Some(self.time)); - let input = RawInput { screen_rect: Some(Rect::from_min_size(Pos2::ZERO, vec2(1440.0, 900.0))), time: Some(self.time), ..raw }; + let input = RawInput { screen_rect: Some(Rect::from_min_size(Pos2::ZERO, self.screen)), time: Some(self.time), ..raw }; let app = &mut self.app; let mut out = self.ctx.run_ui(input, |ui| app.ui(ui)); out.textures_delta.clear(); diff --git a/crates/ui-egui/src/variables_select_tests.rs b/crates/ui-egui/src/variables_select_tests.rs index 55062db96..b391df326 100644 --- a/crates/ui-egui/src/variables_select_tests.rs +++ b/crates/ui-egui/src/variables_select_tests.rs @@ -192,3 +192,61 @@ fn a_row_outside_the_selection_keeps_the_single_variable_menu() { let menu = flat(&u.frame_output(vec![]).shapes); assert!(has_text(&menu, "Delete variable") && !has_text(&menu, "Delete 2 variables")); } + +/// The bounding rect of every popup layer (the menu's area). +fn popup_rect(u: &Ui) -> egui::Rect { + u.ctx.memory(|m| { + let rects: Vec = + m.areas().visible_layer_ids().into_iter().filter(|l| l.order == egui::Order::Foreground).filter_map(|l| m.area_rect(l.id)).collect(); + assert!(!rects.is_empty(), "no popup open"); + rects.into_iter().reduce(|a, b| a.union(b)).unwrap() + }) +} + +#[test] +fn the_bulk_menu_scrolls_when_forty_groups_make_it_taller_than_the_window() { + let mut u = window(); + for g in 0..40 { + u.app + .session + .execute("variable.create", &json!({"collection": "Theme", "name": format!("g{g:02}/leaf"), "type": "number", "value": 0})) + .unwrap(); + } + u.screen = egui::vec2(1000.0, 500.0); + for _ in 0..5 { + u.frame(vec![]); + } + click_row(&mut u, "bravo", Modifiers::NONE); + click_row(&mut u, "charlie", SHIFT); + let p = row(&mut u, "charlie"); + click(&mut u, p, PointerButton::Secondary, Modifiers::NONE); + let menu = flat(&u.frame_output(vec![]).shapes); + assert!(has_text(&menu, "Delete 2 variables")); + let screen = egui::Rect::from_min_size(Pos2::ZERO, u.screen); + let area = popup_rect(&u); + assert!(screen.contains_rect(area), "the menu area {area:?} runs past the screen {screen:?}"); + let last = "Move to group g39"; + let screen_h = u.screen.y; + let visible = |shapes: &[egui::Shape]| crate::live_tests::texts(shapes).iter().any(|(t, p, s)| t == last && p.y + s.y <= screen_h); + assert!(!visible(&menu), "the last group row should start out of view"); + let at = Pos2::new(area.center().x, area.center().y); + let mut shapes = menu; + for _ in 0..30 { + u.frame(vec![ + Event::PointerMoved(at), + Event::MouseWheel { + unit: egui::MouseWheelUnit::Point, + source: egui::MouseWheelSource::Unknown, + phase: egui::TouchPhase::Move, + delta: egui::vec2(0.0, -400.0), + modifiers: Modifiers::NONE, + }, + ]); + shapes = flat(&u.frame_output(vec![Event::PointerMoved(at)]).shapes); + if visible(&shapes) { + break; + } + } + assert!(visible(&shapes), "scrolling the menu never brought {last:?} into view"); + assert!(screen.contains_rect(popup_rect(&u))); +}