Merge PR #1110: Size the list validation dropdown to up to 8 rows (#1089)

crates/ui-egui/src/grid.rs

@@ -3173,6 +3173,18 @@ pub(crate) fn paint_overlays(
     let _ = (wb, si);
 }
 
+/// Rows the list-validation dropdown shows before it scrolls (Excel shows 8).
+const LIST_PICKER_MAX_ROWS: usize = 8;
+const LIST_PICKER_ROW_H: f32 = 18.0;
+/// Popup frame around the rows: 2 px inner margin and the window stroke on each side, rounded up.
+const LIST_PICKER_CHROME: f32 = 8.0;
+
+/// Popup height for `n` items: every item up to `LIST_PICKER_MAX_ROWS`, then a scrollbar.
+fn list_picker_height(n: usize, row_h: f32, gap: f32) -> f32 {
+    let rows = n.clamp(1, LIST_PICKER_MAX_ROWS) as f32;
+    rows * row_h + (rows - 1.0) * gap
+}
+
 fn list_picker(app: &mut SheetApp, ui: &mut egui::Ui, geo: &Geo, was_open: bool) {
     let Some(c) = app.grid.list_picker else { return };
     let Some(d) = app.session.active() else { return };
@@ -3185,14 +3197,24 @@ fn list_picker(app: &mut SheetApp, ui: &mut egui::Ui, geo: &Geo, was_open: bool)
     };
     let items = gridcraft_engine::cmd::data::list_items(&wb, si, &dv);
     let r = geo.cell_rect(sh, c);
-    let (at, pivot) = (geo.mirror_pos(r.left_bottom()), if geo.rtl { Align2::RIGHT_TOP } else { Align2::LEFT_TOP });
+    // Fit the popup to its rows (up to LIST_PICKER_MAX_ROWS); without a minimum the scroll area collapses to its 64 px default.
+    let height = list_picker_height(items.len(), LIST_PICKER_ROW_H, ui.spacing().item_spacing.y);
+    // Open below the cell, or above it when the rows would run past the bottom of the window (as Excel does).
+    let screen = ui.ctx().content_rect();
+    let above = r.bottom() + height + LIST_PICKER_CHROME > screen.bottom() && r.top() - height - LIST_PICKER_CHROME >= screen.top();
+    let (at, pivot) = match (above, geo.rtl) {
+        (false, false) => (geo.mirror_pos(r.left_bottom()), Align2::LEFT_TOP),
+        (false, true) => (geo.mirror_pos(r.left_bottom()), Align2::RIGHT_TOP),
+        (true, false) => (geo.mirror_pos(r.left_top()), Align2::LEFT_BOTTOM),
+        (true, true) => (geo.mirror_pos(r.left_top()), Align2::RIGHT_BOTTOM),
+    };
     let mut close = false;
     let response = egui::Area::new(egui::Id::new("dv_list")).fixed_pos(at).pivot(pivot).order(egui::Order::Foreground).show(ui.ctx(), |ui| {
         egui::Frame::popup(ui.style()).inner_margin(2.0).show(ui, |ui| {
             ui.set_min_width(r.width().max(100.0));
-            egui::ScrollArea::vertical().max_height(200.0).show(ui, |ui| {
+            egui::ScrollArea::vertical().max_height(height).min_scrolled_height(height).show(ui, |ui| {
                 for it in &items {
-                    if ui.add(egui::Button::new(it.as_str()).frame(false).min_size(vec2(r.width().max(100.0), 18.0))).clicked() {
+                    if ui.add(egui::Button::new(it.as_str()).frame(false).min_size(vec2(r.width().max(100.0), LIST_PICKER_ROW_H))).clicked() {
                         app.run_or_alert("cell.set", json!({"cell": c.a1(), "input": it}));
                         close = true;
                     }
@@ -3451,6 +3473,14 @@ mod tests {
     }
     use super::*;
 
+    #[test]
+    fn list_picker_fits_up_to_eight_rows_then_scrolls() {
+        assert_eq!(list_picker_height(5, 18.0, 3.0), 5.0 * 18.0 + 4.0 * 3.0);
+        assert_eq!(list_picker_height(8, 18.0, 3.0), 8.0 * 18.0 + 7.0 * 3.0);
+        assert_eq!(list_picker_height(30, 18.0, 3.0), list_picker_height(8, 18.0, 3.0));
+        assert_eq!(list_picker_height(0, 18.0, 3.0), 18.0);
+    }
+
     #[test]
     fn wheel_zoom_steps_along_the_ladder_and_stops_at_100() {
         for (from, zoom_in, to) in [

crates/ui-egui/tests/popup_dismissal.rs

@@ -174,3 +174,42 @@ fn worksheet_popups_close_on_escape_or_window_focus_loss() {
         }
     }
 }
+
+/// Gives `cell` a list validation of `count` items, opens its dropdown and returns the popup's rectangle.
+fn open_list_of(h: &mut Harness, cell: CellRef, count: usize) -> egui::Rect {
+    let items: Vec<String> = (1..=count).map(|i| format!("Item {i}")).collect();
+    let formula1 = format!("\"{}\"", items.join(","));
+    h.state_mut().session.execute("data.validation", json!({"range": cell.a1(), "type": "list", "formula1": formula1})).unwrap();
+    h.state_mut().run("selection.set", json!({"cell": cell.a1()})).unwrap();
+    h.run_steps(2);
+    let geometry = geo(h);
+    let sh = h.state().session.active().unwrap().wb.active().unwrap();
+    let arrow = grid::validation_arrow(sh, &geometry, cell).unwrap().center();
+    click(h, arrow, PointerButton::Primary);
+    assert!(is_open(h, Menu::Validation));
+    h.ctx.memory(|m| m.area_rect(egui::Id::new("dv_list"))).unwrap()
+}
+
+#[test]
+fn validation_dropdown_fits_up_to_eight_items_then_scrolls() {
+    // Each item row is 18 px high; the popup used to collapse to egui's 64 px scroll minimum (about three items).
+    for (count, min_h, max_h) in [(5, 5.0 * 18.0, 12.0 * 18.0), (12, 8.0 * 18.0, 12.0 * 18.0)] {
+        let mut h = harness();
+        let popup = open_list_of(&mut h, CellRef::parse("E5").unwrap(), count);
+        assert!(popup.height() >= min_h && popup.height() < max_h, "{count} items: popup is {} px high", popup.height());
+    }
+}
+
+#[test]
+fn validation_dropdown_opens_above_a_cell_near_the_bottom_of_the_window() {
+    let mut h = harness();
+    let bottom = h.state().grid.rect.unwrap().bottom();
+    let geometry = geo(&h);
+    let sh = h.state().session.active().unwrap().wb.active().unwrap();
+    // The last row of column E that is fully visible above the bottom of the grid.
+    let visible = |row: u32| CellRef::parse(&format!("E{row}")).filter(|&c| geometry.cell_rect(sh, c).bottom() <= bottom);
+    let cell = (1..200).map_while(visible).last().unwrap();
+    let popup = open_list_of(&mut h, cell, 8);
+    let r = geo(&h).cell_rect(h.state().session.active().unwrap().wb.active().unwrap(), cell);
+    assert!(popup.bottom() <= r.top() + 0.5, "popup {popup:?} covers the cell {r:?}");
+}