Manage Rules: scroll the rule list and lay out only the visible rules (#3321)

The dialog laid out one row per rule with no height limit and ran
home.manageRules (copying and serialising every rule, and journaling a
step) on every frame. A sheet with thousands of rules made the window
as tall as the whole list and the application window go blank.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

crates/ui-egui/src/dialogs/cond_format_rules.rs

@@ -1,28 +1,40 @@
 use super::{Json, SheetApp, json};
 
+/// The list scrolls inside the dialog: a sheet read from a file can carry thousands of rules, and
+/// a list laid out in full made the window as tall as all of them (issue #3321).
+const LIST_HEIGHT: f32 = 360.0;
+
 pub(super) fn manage_rules(app: &mut SheetApp, ui: &mut egui::Ui) {
-    let list = app.session.run("home.manageRules", json!({})).unwrap_or(Json::Null);
-    let rules = list["rules"].as_array().cloned().unwrap_or_default();
-    if rules.is_empty() {
+    // Read the rules in place: running `home.manageRules` for them each frame copied and
+    // serialised every rule and journaled a step 60 times a second.
+    let count = app.session.active().and_then(|d| d.wb.active()).map_or(0, |sh| sh.cond_formats.len());
+    if count == 0 {
         ui.label("There are no conditional formatting rules on this sheet.");
+        return;
     }
-    for (i, r) in rules.iter().enumerate() {
-        ui.horizontal(|ui| {
-            let kind = r["rule"].as_object().and_then(|o| o.keys().next().cloned()).unwrap_or_default();
-            ui.label(format!(
-                "{}. {kind}  applies to {}",
-                i + 1,
-                r["ranges"].as_array().map(|a| a.iter().filter_map(|x| x.as_str()).collect::<Vec<_>>().join(",")).unwrap_or_default()
-            ));
-            if ui.small_button("▲").clicked() {
-                app.run_or_alert("home.manageRules", json!({"moveUp": i}));
-            }
-            if ui.small_button("▼").clicked() {
-                app.run_or_alert("home.manageRules", json!({"moveDown": i}));
-            }
-            if ui.small_button("Delete").clicked() {
-                app.run_or_alert("home.manageRules", json!({"delete": i}));
-            }
-        });
+    let mut action: Option<Json> = None;
+    let row_height = ui.spacing().interact_size.y;
+    egui::ScrollArea::vertical().max_height(LIST_HEIGHT).auto_shrink([false, true]).show_rows(ui, row_height, count, |ui, rows| {
+        let Some(sh) = app.session.active().and_then(|d| d.wb.active()) else { return };
+        for i in rows {
+            let Some(cf) = sh.cond_formats.get(i) else { break };
+            let kind = serde_json::to_value(&cf.rule).ok().and_then(|r| r.as_object().and_then(|o| o.keys().next().cloned())).unwrap_or_default();
+            let ranges = cf.ranges.iter().map(|r| r.a1()).collect::<Vec<_>>().join(",");
+            ui.horizontal(|ui| {
+                ui.label(format!("{}. {kind}  applies to {ranges}", i + 1));
+                if ui.small_button("▲").clicked() {
+                    action = Some(json!({"moveUp": i}));
+                }
+                if ui.small_button("▼").clicked() {
+                    action = Some(json!({"moveDown": i}));
+                }
+                if ui.small_button("Delete").clicked() {
+                    action = Some(json!({"delete": i}));
+                }
+            });
+        }
+    });
+    if let Some(params) = action {
+        app.run_or_alert("home.manageRules", params);
     }
 }

crates/ui-egui/tests/it/cf_rules_manager.rs

@@ -0,0 +1,64 @@
+//! Manage Rules on a sheet with thousands of conditional formats (issue #3321): the list scrolls
+//! inside the dialog, and only the visible rules are laid out, instead of one row per rule
+//! making the window as tall as the whole list and the application window go blank.
+
+use egui::vec2;
+use egui_kittest::kittest::Queryable;
+use gridcraft_engine::Session;
+use gridcraft_ui_egui::SheetApp;
+use serde_json::json;
+
+type Harness = egui_kittest::Harness<'static, SheetApp>;
+
+const RULES: usize = 3000;
+const SCREEN_H: f32 = 800.0;
+
+fn harness_with_rules(n: usize) -> Harness {
+    let mut session = Session::new();
+    session.new_workbook();
+    for i in 0..n {
+        let range = format!("A{}", i + 1);
+        session
+            .execute(
+                "home.conditionalFormat",
+                json!({"range": range, "rule": {"type": "cellIs", "operator": "greaterThan", "value": "5", "preset": "yellowFill"}}),
+            )
+            .unwrap();
+    }
+    let mut h = crate::common::builder().with_size(vec2(1200.0, SCREEN_H)).build_ui_state(
+        |ui, app: &mut SheetApp| {
+            app.logic(ui.ctx());
+            app.ui(ui);
+        },
+        crate::common::app(session),
+    );
+    h.run_steps(4);
+    h.state_mut().open_dialog("manageRules", json!({}));
+    h.run_steps(6);
+    h
+}
+
+#[test]
+fn many_rules_scroll_inside_a_dialog_that_fits_the_window() {
+    let mut h = harness_with_rules(RULES);
+    let deletes: Vec<_> = h.query_all_by_label("Delete").collect();
+    assert!(!deletes.is_empty(), "the first rules are listed");
+    // One more Delete sits in the ribbon.
+    assert!(deletes.len() < 100, "only the visible rules are laid out, got {}", deletes.len());
+    let lowest = deletes.iter().map(|d| d.rect().bottom()).fold(0.0, f32::max);
+    assert!(lowest <= SCREEN_H, "the list stays inside the window, lowest button at {lowest}");
+    drop(deletes);
+    // Deleting through the dialog removes the first rule and the dialog stays usable.
+    // (The ribbon has a Delete button of its own, outside the dialog's window.)
+    let window = h.get_by_label("Conditional Formatting Rules Manager").rect();
+    h.query_all_by_label("Delete").find(|d| window.contains_rect(d.rect())).unwrap().click();
+    h.run_steps(4);
+    let left = h.state().session.active().unwrap().wb.active().unwrap().cond_formats.len();
+    assert_eq!(left, RULES - 1);
+}
+
+#[test]
+fn an_empty_sheet_says_so() {
+    let h = harness_with_rules(0);
+    h.get_by_label("There are no conditional formatting rules on this sheet.");
+}

crates/ui-egui/tests/it/main.rs

@@ -15,6 +15,7 @@ mod borders;
 mod cell_editor_caret;
 mod cell_pictures;
 mod cf_rule_pick;
+mod cf_rules_manager;
 mod chart_data_table;
 mod chart_resize;
 mod chart_source_layering;