diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 95855e56a..e83db2fa1 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/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::>().join(","))) + .map(|doc| { + ( + doc.wb.active().map(|sh| sh.name.clone()).unwrap_or_default(), + doc.selection.ranges.iter().map(RangeRef::a1).collect::>().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); } } diff --git a/crates/ui-egui/src/grid.rs b/crates/ui-egui/src/grid.rs index 157f608fe..87ac5787e 100644 --- a/crates/ui-egui/src/grid.rs +++ b/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() { diff --git a/crates/ui-egui/tests/it/cf_rule_pick.rs b/crates/ui-egui/tests/it/cf_rule_pick.rs index 642a91a18..a523c9c6d 100644 --- a/crates/ui-egui/tests/it/cf_rule_pick.rs +++ b/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::>(), ["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::>(); + 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::>(), ["F8:G9"]); +}