Merge pull request #2056 from ZIDAN44/fix/2043-variable-picker-height

Styles: the style and variable pickers grow back to their full height (#2043)

crates/ui-egui/src/lib.rs

@@ -253,6 +253,8 @@ mod ui_params_tests;
 #[cfg(test)]
 mod ui_scale_tests;
 #[cfg(test)]
+mod variable_picker_tests;
+#[cfg(test)]
 mod variables_export_ui_tests;
 #[cfg(test)]
 mod variables_rename_tests;

crates/ui-egui/src/panels/variable_ui.rs

@@ -423,6 +423,9 @@ pub fn picker(app: &mut FigApp, ctx: &egui::Context) {
     let mut chosen: Option<Pick> = None;
     let mut close = false;
     let area = egui::Area::new(Id::unique("variable_picker_area")).order(egui::Order::Foreground).fixed_pos(p.at + vec2(0.0, 4.0)).show(ctx, |ui| {
+        // The whole screen's height, not last frame's, so the list grows back after a search (#2043);
+        // egui keeps the area on the screen.
+        ui.set_max_height(crate::popover::max_height(ctx.content_rect()));
         egui::Frame::NONE.fill(t.menu).corner_radius(CornerRadius::same(10)).inner_margin(egui::Margin::same(8)).show(ui, |ui| {
             ui.set_width(240.0);
             ui.horizontal(|ui| {

crates/ui-egui/src/popover.rs

@@ -177,6 +177,7 @@ impl Popover {
                 ui.set_invisible();
                 ctx.request_repaint();
             }
+            ui.set_max_height(max_height(screen));
             // The panel is painted into a place kept before the content: the shadow and the fill
             // go under it, though its size is only known once the slot is laid out.
             let bg = ui.painter().add(egui::Shape::Noop);
@@ -276,6 +277,16 @@ fn open_pos(anchor: Pos2, size: Vec2, screen: Rect) -> Pos2 {
     pos2(anchor.x - size.x, y)
 }
 
+/// The height a popover's content is offered: the screen's, less [`EDGE`] above and below.
+///
+/// egui offers an area the size it had last frame, so content that fills what it is offered (a
+/// scrolling list) could only ever shrink: a search that narrowed a picker's list left it a few
+/// rows high after the search was cleared (#2043). The list takes what it needs of this, and the
+/// placement keeps the panel on the screen.
+pub(crate) fn max_height(screen: Rect) -> f32 {
+    (screen.height() - 2.0 * EDGE).max(HEADER_H)
+}
+
 /// Keeps the panel inside the screen: whole when it fits, its top-left corner when it doesn't.
 fn clamp(pos: Pos2, size: Vec2, screen: Rect) -> Pos2 {
     let (x0, y0) = (screen.min.x + EDGE, screen.min.y + EDGE);

crates/ui-egui/src/variable_picker_tests.rs

@@ -0,0 +1,67 @@
+//! The variable picker ("Select variable" on every bindable field, aliases, styles and prototype
+//! actions) drawn headless: its list follows its rows while it is open — a search that narrows it
+//! shrinks the picker, clearing the search grows it back — and it stays on the screen (#2043: egui
+//! offers an area the size it had last frame, so the list used to stay a few rows high).
+
+use egui::{Id, Pos2, RawInput, Rect, pos2, vec2};
+use figcraft_doc::NodeId;
+use figcraft_engine::Session;
+use serde_json::json;
+
+use crate::layers_drag_tests::Ui;
+
+/// A selected rectangle and 30 number variables.
+fn scene() -> (Ui, NodeId) {
+    let mut s = Session::new();
+    let r = NodeId(s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 100, "height": 100})).unwrap()["id"].as_u64().unwrap());
+    s.execute("variable.createCollection", &json!({"name": "Sizes"})).unwrap();
+    for i in 0..30 {
+        s.execute("variable.create", &json!({"collection": "Sizes", "name": format!("w-{}", 100 + 10 * i), "type": "number", "value": i})).unwrap();
+    }
+    s.execute("select.set", &json!({"ids": [r.0]})).unwrap();
+    (Ui::new(s), r)
+}
+
+/// Opens the picker for `r`'s stroke weight under `at`.
+fn open(u: &mut Ui, r: NodeId, at: Pos2) {
+    let screen = RawInput { screen_rect: Some(Rect::from_min_size(Pos2::ZERO, vec2(1440.0, 900.0))), ..Default::default() };
+    let _ = u.ctx.run_ui(screen, |ui| crate::variable_ui::open(ui, &[r], "strokeWeight", figcraft_doc::VariableType::Number, at));
+}
+
+/// The picker's rect after a few frames.
+fn settled(u: &mut Ui) -> Rect {
+    for _ in 0..5 {
+        u.frame(vec![]);
+    }
+    crate::variable_ui::picker_rect(&u.ctx).expect("the variable picker shows")
+}
+
+fn search(u: &mut Ui, q: &str) {
+    u.ctx.data_mut(|d| d.insert_temp(Id::unique("variable_picker_query"), q.to_string()));
+}
+
+#[test]
+fn the_variable_list_grows_back_after_a_search_narrows_it() {
+    let (mut u, r) = scene();
+    open(&mut u, r, pos2(900.0, 200.0));
+    let open = settled(&mut u);
+    assert!(open.height() > 300.0, "the picker lists the variables: {open:?}");
+    search(&mut u, "w-100");
+    let narrowed = settled(&mut u);
+    assert!(narrowed.height() < open.height() - 100.0, "a search for one variable shrinks it: {open:?} → {narrowed:?}");
+    search(&mut u, "");
+    let again = settled(&mut u);
+    assert!((again.height() - open.height()).abs() < 1.0, "clearing the search grows it back: {open:?} → {again:?}");
+}
+
+#[test]
+fn the_variable_picker_opened_near_the_bottom_shows_its_whole_list_on_the_screen() {
+    let (mut u, r) = scene();
+    open(&mut u, r, pos2(900.0, 200.0));
+    let full = settled(&mut u);
+    let (mut u, r) = scene();
+    open(&mut u, r, pos2(900.0, 880.0));
+    let low = settled(&mut u);
+    assert!((low.height() - full.height()).abs() < 1.0, "as tall as higher up: {full:?} vs {low:?}");
+    assert!(low.max.y <= 900.0 && low.min.y >= 0.0, "on the screen: {low:?}");
+}

crates/ui-egui/src/widgets/style_picker/tests.rs

@@ -194,6 +194,68 @@ fn a_click_in_the_colour_picker_keeps_the_style_editor_open() {
     );
 }
 
+/// #2043: the list follows its rows while the picker is open. A search that narrows it shrinks
+/// the popover, and clearing the search grows it back to the full list (egui offers an area the
+/// size it had last frame, so the list used to stay a few rows high).
+#[test]
+fn the_style_list_grows_back_after_a_search_narrows_it() {
+    use crate::layers_drag_tests::Ui;
+    let mut s = Session::new();
+    let r = s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 10, "height": 10})).unwrap()["id"].clone();
+    for i in 0..30 {
+        let name = format!("Blue/{}", 100 + 10 * i);
+        s.execute("style.create", &json!({"type": "paint", "name": name, "value": [{"type": "solid", "color": "#0D99FF"}]})).unwrap();
+    }
+    s.execute("select.set", &json!({"ids": [r]})).unwrap();
+    let mut u = Ui::new(s);
+    open_for_selection(&mut u.app, StyleSlot::Fill, None);
+    let height = |u: &mut Ui| {
+        for _ in 0..5 {
+            u.frame(vec![]);
+        }
+        u.app.ui.style_picker_popover.as_ref().expect("the style picker shows").rect.height()
+    };
+    let open = height(&mut u);
+    assert!(open > 300.0, "the picker lists the styles: {open}");
+    u.app.ui.style_picker.as_mut().expect("open").query = "Blue/100".into();
+    let narrowed = height(&mut u);
+    assert!(narrowed < open - 100.0, "a search for one style shrinks it: {open} → {narrowed}");
+    u.app.ui.style_picker.as_mut().expect("open").query.clear();
+    let again = height(&mut u);
+    assert!((again - open).abs() < 1.0, "clearing the search grows it back: {open} → {again}");
+}
+
+/// #2043: egui keeps an area's size after it closes, so a picker opened once on a short list (the
+/// effect styles, none yet) opened again on the colour styles a few rows high.
+#[test]
+fn the_style_list_opened_after_a_short_one_shows_every_style_it_fits() {
+    use crate::layers_drag_tests::Ui;
+    let mut s = Session::new();
+    let r = s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 10, "height": 10})).unwrap()["id"].clone();
+    for i in 0..30 {
+        let name = format!("Blue/{}", 100 + 10 * i);
+        s.execute("style.create", &json!({"type": "paint", "name": name, "value": [{"type": "solid", "color": "#0D99FF"}]})).unwrap();
+    }
+    s.execute("select.set", &json!({"ids": [r]})).unwrap();
+    let mut u = Ui::new(s);
+    let height = |u: &mut Ui, slot: StyleSlot| {
+        open_for_selection(&mut u.app, slot, None);
+        for _ in 0..5 {
+            u.frame(vec![]);
+        }
+        let h = u.app.ui.style_picker_popover.as_ref().expect("the style picker shows").rect.height();
+        u.app.ui.style_picker = None;
+        u.app.ui.style_picker_popover = None;
+        u.frame(vec![]);
+        h
+    };
+    let first = height(&mut u, StyleSlot::Fill);
+    let short = height(&mut u, StyleSlot::Effect);
+    assert!(short < first - 100.0, "no effect styles: {short}");
+    let again = height(&mut u, StyleSlot::Fill);
+    assert!((again - first).abs() < 1.0, "the colour styles as tall as the first time: {first} → {again}");
+}
+
 #[test]
 fn local_style_drops_move_into_the_folder_above() {
     let st = |id: &str, name: &str| Style {