Conditional formatting: New Rule remembers its sheet by name, so moving or inserting sheets while it is open keeps the rule there (#2348)

crates/ui-egui/src/dialogs.rs

@@ -54,8 +54,9 @@ pub struct Dialog {
 #[derive(Clone, Debug)]
 struct RefPick {
     key: &'static str,
-    /// The sheet the dialog was opened on: references to another sheet carry its name.
-    sheet: usize,
+    /// The sheet the dialog was opened on, by name: its index changes when sheets are inserted or
+    /// moved while the dialog is open. References to another sheet carry that sheet's name.
+    sheet: String,
     /// Caret (char index), mirrored from the text box while it has the keyboard.
     caret: usize,
     /// The reference last inserted (byte span), replaced by the next click or the drag.
@@ -109,10 +110,10 @@ impl Dialog {
     /// point mode. An empty box starts with `=`. After an operand there is no place for a
     /// reference, so nothing changes. `active_sheet` names the sheet shown when it is not the
     /// dialog's own.
-    pub fn pick_reference(&mut self, cell: CellRef, active_sheet: (usize, &str), extend: bool) {
+    pub fn pick_reference(&mut self, cell: CellRef, active_sheet: &str, extend: bool) {
         let Some(pick) = self.pick.as_mut() else { return };
         let text = self.values.get(pick.key).and_then(Json::as_str).unwrap_or("").to_string();
-        let mut ed = crate::editor::EditState::new(pick.sheet, cell, text, true, false);
+        let mut ed = crate::editor::EditState::new(0, cell, text, true, false);
         ed.caret = pick.caret.min(ed.text.chars().count());
         ed.point = pick.point;
         if ed.text.trim().is_empty() {
@@ -127,8 +128,11 @@ impl Dialog {
             pick.anchor = cell;
         }
         let range = abs(&RangeRef::new(pick.anchor, cell));
-        let reference =
-            if active_sheet.0 == pick.sheet { range } else { format!("{}!{range}", gridcraft_engine::formula::quote_sheet(active_sheet.1)) };
+        let reference = if active_sheet.eq_ignore_ascii_case(&pick.sheet) {
+            range
+        } else {
+            format!("{}!{range}", gridcraft_engine::formula::quote_sheet(active_sheet))
+        };
         ed.insert_ref(&reference);
         pick.caret = ed.caret;
         pick.point = ed.point;
@@ -755,7 +759,12 @@ impl Dialog {
                     let (sheet, applies_to) = app
                         .session
                         .active()
-                        .map(|doc| (doc.wb.active_sheet, doc.selection.ranges.iter().map(RangeRef::a1).collect::<Vec<_>>().join(",")))
+                        .map(|doc| {
+                            (
+                                doc.wb.active().map(|sh| sh.name.clone()).unwrap_or_default(),
+                                doc.selection.ranges.iter().map(RangeRef::a1).collect::<Vec<_>>().join(","),
+                            )
+                        })
                         .unwrap_or_default();
                     d.pick = Some(RefPick { key: "formula", sheet, caret: 0, point: None, anchor: CellRef::new(0, 0), refocus: false, applies_to });
                 }
@@ -1184,8 +1193,8 @@ pub(crate) fn build_params(d: &Dialog) -> Json {
                 rule.insert(k.clone(), val.clone());
             }
             v = json!({"rule": Json::Object(rule)});
-            if let Some(pick) = d.pick.as_ref().filter(|pick| !pick.applies_to.is_empty()) {
-                v["sheet"] = json!(pick.sheet);
+            if let Some(pick) = d.pick.as_ref().filter(|pick| !pick.applies_to.is_empty() && !pick.sheet.is_empty()) {
+                v["sheet"] = json!(pick.sheet.as_str());
                 v["range"] = json!(pick.applies_to);
             }
         }

crates/ui-egui/src/grid.rs

@@ -2339,7 +2339,7 @@ fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Response, geo: &
         // A dialog's formula box in point mode (New Formatting Rule) takes the reference; the
         // selection, which the dialog acts on, stays.
         if let Some(d) = app.dialog.as_mut().filter(|d| d.picks_references()) {
-            d.pick_reference(c, (wb.active_sheet, &sh.name), false);
+            d.pick_reference(c, &sh.name, false);
             app.grid.drag = Drag::Select;
             app.grid.drag_select = true;
             if resp.drag_started()
@@ -2713,7 +2713,7 @@ fn select_to_pointer(app: &mut SheetApp, geo: &Geo, sh: &Sheet, wb: &Workbook, p
         geo.col_at(sh, p.x.clamp(geo.cells.left() + 1.0, geo.cells.right() - 1.0)),
     );
     if let Some(d) = app.dialog.as_mut().filter(|d| d.picks_references()) {
-        d.pick_reference(cell, (wb.active_sheet, &sh.name), true);
+        d.pick_reference(cell, &sh.name, true);
     } else if let Some(ed) = app.editor.as_mut()
         && ed.point.is_some()
     {

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

@@ -175,3 +175,23 @@ fn pointing_on_another_sheet_keeps_the_rule_on_the_sheet_it_was_opened_for() {
     let cf = wb.sheet(0).unwrap().cond_formats.last().expect("the rule is on the sheet the dialog was opened on");
     assert_eq!(cf.ranges.iter().map(|r| r.a1()).collect::<Vec<_>>(), ["F8:G9"]);
 }
+
+#[test]
+fn the_rule_follows_its_sheet_when_sheets_move_while_the_dialog_is_open() {
+    let mut h = harness();
+    type_text(&mut h, "=");
+    click(&mut h, "B2");
+    h.state_mut().session.execute("sheet.move", json!({"sheet": 0, "to": 1})).unwrap();
+    h.run_steps(3);
+    let names = |h: &Harness| h.state().session.active().unwrap().wb.sheets.iter().map(|s| s.name.clone()).collect::<Vec<_>>();
+    assert_eq!(names(&h), ["Sheet2", "Sheet1"], "Sheet1 now sits at index 1");
+    h.key_press(Key::Enter);
+    h.run_steps(4);
+    assert!(h.state().dialog.is_none(), "Enter confirmed the rule");
+    let wb = &h.state().session.active().unwrap().wb;
+    let sheet1 = wb.sheets.iter().find(|s| s.name == "Sheet1").unwrap();
+    let sheet2 = wb.sheets.iter().find(|s| s.name == "Sheet2").unwrap();
+    assert!(sheet2.cond_formats.is_empty(), "nothing lands on the sheet that took index 0");
+    let cf = sheet1.cond_formats.last().expect("the rule stays on Sheet1");
+    assert_eq!(cf.ranges.iter().map(|r| r.a1()).collect::<Vec<_>>(), ["F8:G9"]);
+}