From 3ef94601d5e0d385a607ec055aaaa720ed259a80 Mon Sep 17 00:00:00 2001 From: Harrison Booth <73434116+Harrison-Booth@users.noreply.github.com> Date: Sat, 24 Oct 2026 15:47:17 -0500 Subject: [PATCH] Options: commit iteration fields only while iteration is on; accept the locale's decimal comma for Maximum Change Fields still holding unconfirmed text when the checkbox turns iteration off are no longer committed, and Maximum Change parses with the interface's decimal separator as well as a dot. --- crates/ui-egui/src/backstage.rs | 16 +++++++++------- crates/ui-egui/tests/it/file_menu.rs | 28 ++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/crates/ui-egui/src/backstage.rs b/crates/ui-egui/src/backstage.rs index ead3f81c4..3ceef246f 100644 --- a/crates/ui-egui/src/backstage.rs +++ b/crates/ui-egui/src/backstage.rs @@ -1001,13 +1001,14 @@ fn options(app: &mut SheetApp, ui: &mut Ui) { app.run_or_alert("formulas.calculationOptions", json!({"iterative": iterative})); } // Maximum Iterations and Maximum Change apply only while iteration is on, as in Excel. - ui.add_enabled_ui(calc.iterative, |ui| { + ui.add_enabled_ui(iterative, |ui| { ui.horizontal(|ui| { ui.add_space(24.0); egui::Grid::new("backstage_options_iteration").num_columns(2).spacing(vec2(12.0, 6.0)).show(ui, |ui| { ui.label(lang.tr("Maximum Iterations:")); let id = ui.make_persistent_id("backstage_max_iterations"); - if let Some(text) = crate::widgets::committed_text(ui, id, &calc.max_iterations.to_string(), Some(80.0)) { + // Text left in the field when iteration is switched off (it loses focus) is dropped. + if let Some(text) = crate::widgets::committed_text(ui, id, &calc.max_iterations.to_string(), Some(80.0)).filter(|_| iterative) { match parse_max_iterations(&text) { Some(n) if n != calc.max_iterations => { app.run_or_alert("formulas.calculationOptions", json!({"maxIterations": n})); @@ -1019,8 +1020,8 @@ fn options(app: &mut SheetApp, ui: &mut Ui) { ui.end_row(); ui.label(lang.tr("Maximum Change:")); let id = ui.make_persistent_id("backstage_max_change"); - if let Some(text) = crate::widgets::committed_text(ui, id, &calc.max_change.to_string(), Some(80.0)) { - match parse_max_change(&text) { + if let Some(text) = crate::widgets::committed_text(ui, id, &calc.max_change.to_string(), Some(80.0)).filter(|_| iterative) { + match parse_max_change(&text, lang.formula_locale()) { Some(d) if d != calc.max_change => { app.run_or_alert("formulas.calculationOptions", json!({"maxChange": d})); } @@ -1051,9 +1052,10 @@ fn parse_max_iterations(text: &str) -> Option { (1..=gridcraft_engine::model::CalcSettings::MAX_ITERATIONS).contains(&n).then_some(n) } -/// Maximum Change as typed: a finite number of 0 or more, or `None`. -fn parse_max_change(text: &str) -> Option { - let d = text.trim().parse::().ok()?; +/// Maximum Change as typed: a finite number of 0 or more, or `None`. The interface's decimal +/// separator is accepted as well as a dot. +fn parse_max_change(text: &str, locale: gridcraft_engine::formula::FormulaLocale) -> Option { + let d = text.trim().replace(locale.decimal_separator(), ".").parse::().ok()?; (d.is_finite() && d >= 0.0).then_some(d) } diff --git a/crates/ui-egui/tests/it/file_menu.rs b/crates/ui-egui/tests/it/file_menu.rs index 916185665..6b4cbc6a7 100644 --- a/crates/ui-egui/tests/it/file_menu.rs +++ b/crates/ui-egui/tests/it/file_menu.rs @@ -415,6 +415,34 @@ fn options_sets_maximum_iterations_and_change_only_while_iteration_is_on() { assert_eq!(calc(&h).max_change, 0.01); let change = field(&h, "Maximum Change:"); assert_eq!(h.get_all_by_role(Role::TextInput).find(|n| n.rect() == change).unwrap().value().as_deref(), Some("0.01")); + + // A value typed but not confirmed with Enter is dropped when the checkbox switches iteration off. + let iterations = field(&h, "Maximum Iterations:"); + h.input_mut().events.push(egui::Event::PointerMoved(iterations.center())); + h.run_steps(1); + h.get_all_by_role(Role::TextInput).find(|n| n.rect() == iterations).unwrap().click(); + h.run_steps(2); + h.key_press_modifiers(egui::Modifiers::COMMAND, egui::Key::A); + h.input_mut().events.push(egui::Event::Text("777".into())); + h.run_steps(2); + h.get_by_role_and_label(Role::CheckBox, "Enable iterative calculation").click(); + h.run_steps(3); + assert!(!calc(&h).iterative); + assert_eq!(calc(&h).max_iterations, 32767, "an unconfirmed entry is not committed once iteration is off"); + assert!(h.state().message.is_none()); + + // Spanish accepts a decimal comma in Maximum Change, and still a dot. + h.get_by_role_and_label(Role::CheckBox, "Enable iterative calculation").click(); + h.run_steps(3); + h.state_mut().run("app.language.set", serde_json::json!({"language": Language::Es.code()})).unwrap(); + h.state_mut().run("file.options", serde_json::json!({})).unwrap(); + h.run_steps(3); + let change = field(&h, "Cambio máximo:"); + retype(&mut h, change, "0,02"); + assert_eq!(calc(&h).max_change, 0.02); + retype(&mut h, change, "0.5"); + assert_eq!(calc(&h).max_change, 0.5); + assert!(h.state().message.is_none()); } /// Share › Export as Web Page (and the native File menu's Save as Web Page) asks for a name in its