diff --git a/crates/ui-egui/src/columns_dialog_tests.rs b/crates/ui-egui/src/columns_dialog_tests.rs index fa6b677f..7737031c 100644 --- a/crates/ui-egui/src/columns_dialog_tests.rs +++ b/crates/ui-egui/src/columns_dialog_tests.rs @@ -12,6 +12,24 @@ fn harness() -> Harness<'static, SlideApp> { let mut s = Session::with_new(); s.execute("shape.insert", &json!({"preset": "rect", "rect": [80.0, 80.0, 320.0, 160.0], "text": "Hello"})).unwrap(); let _ = s.execute("text.exit", &json!({})); + harness_of(s) +} + +/// Two text boxes with different columns, both selected: 2 columns 9 pt apart, and 3 columns +/// 36 pt apart. +fn two_boxes() -> Harness<'static, SlideApp> { + let mut s = Session::with_new(); + let mut ids = Vec::new(); + for (y, count, spacing) in [(80.0, 2, 9.0), (300.0, 3, 36.0)] { + ids.push(s.execute("shape.insert", &json!({"preset": "rect", "rect": [80.0, y, 320.0, 160.0], "text": "Hello"})).unwrap()["id"].clone()); + let _ = s.execute("text.exit", &json!({})); + s.execute("format.columns", &json!({"count": count, "spacing": spacing})).unwrap(); + } + s.execute("edit.select", &json!({"ids": ids})).unwrap(); + harness_of(s) +} + +fn harness_of(s: Session) -> Harness<'static, SlideApp> { let app = SlideApp::new(s, Services::default()); let mut h = Harness::builder().with_size(egui::vec2(1440.0, 900.0)).with_step_dt(1.0 / 60.0).build_ui_state( |ui, app: &mut SlideApp| { @@ -30,6 +48,47 @@ fn columns_of(h: &Harness<'_, SlideApp>) -> (Option, Option) { (body.columns, body.col_spacing) } +fn columns_of_each(h: &Harness<'_, SlideApp>) -> Vec<(Option, Option)> { + let st = h.state().session.active().unwrap(); + st.selected_shapes().iter().map(|sh| sh.text.as_ref().map(|t| (t.body.columns, t.body.col_spacing)).unwrap_or_default()).collect() +} + +/// Opens the dialog on `h`'s selection, types `fields` into it and presses OK. +fn ok_with(h: &mut Harness<'_, SlideApp>, fields: &[(&str, &str)]) { + h.state_mut().run("format.columns", json!({})).unwrap(); + h.run_steps(2); + let d = h.state_mut().dialog.as_mut().unwrap(); + for (key, value) in fields { + d.fields.insert((*key).into(), (*value).into()); + } + h.get_by_label("OK").click_accesskit(); + h.run_steps(2); + assert!(h.state().dialog.is_none(), "status {:?}", h.state().status); +} + +/// With text boxes whose columns differ, the dialog shows the first one's values, and a field +/// left as shown does not overwrite the others' own values. +#[test] +fn a_field_left_as_shown_keeps_each_text_box_s_own_value() { + let mut h = two_boxes(); + assert_eq!(columns_of_each(&h), [(Some(2), Some(9.0)), (Some(3), Some(36.0))]); + ok_with(&mut h, &[("count", "4")]); + assert_eq!(columns_of_each(&h), [(Some(4), Some(9.0)), (Some(4), Some(36.0))], "Number only"); + + let mut h = two_boxes(); + ok_with(&mut h, &[("spacing", "0.5")]); + assert_eq!(columns_of_each(&h), [(Some(2), Some(36.0)), (Some(3), Some(36.0))], "Spacing only"); +} + +#[test] +fn ok_with_nothing_changed_closes_without_an_edit() { + let mut h = two_boxes(); + let n = h.state().session.active().unwrap().history.undo.len(); + ok_with(&mut h, &[]); + assert_eq!(h.state().session.active().unwrap().history.undo.len(), n); + assert_eq!(columns_of_each(&h), [(Some(2), Some(9.0)), (Some(3), Some(36.0))]); +} + fn field(h: &Harness<'_, SlideApp>, key: &str) -> Option { h.state().dialog.as_ref().and_then(|d| d.fields.get(key).cloned()) } diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 3e994589..9ecbdd5c 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -2664,31 +2664,35 @@ fn grid_options(app: &mut SlideApp, ui: &mut Ui, d: &mut Dialog) -> bool { cancel } -/// The Columns dialog's fields as `format.columns` params: Number and Spacing (inches, sent in points). -/// A Spacing left as the dialog showed it sends the text box's own spacing, unrounded. -fn columns_value(d: &Dialog, shown_spacing: &str) -> Result { - let count = - field_of(d, "count").trim().parse::().ok().filter(|n| (1..=16).contains(n)).ok_or("Number must be a whole number from 1 to 16")?; - let typed = field_of(d, "spacing"); - if typed == shown_spacing - && let Some(pt) = d.params.get("spacing").and_then(Value::as_f64).filter(|v| v.is_finite()) - { - return Ok(json!({"count": count, "spacing": pt})); +/// The Columns dialog's changed fields as `format.columns` params: Number and Spacing (inches, +/// sent in points). A field left as the dialog showed it is left out, so each selected text box +/// keeps its own value; `None` when neither field changed. +fn columns_value(d: &Dialog, shown_count: &str, shown_spacing: &str) -> Result, String> { + let mut p = serde_json::Map::new(); + let count = field_of(d, "count"); + if count != shown_count { + let n = count.trim().parse::().ok().filter(|n| (1..=16).contains(n)).ok_or("Number must be a whole number from 1 to 16")?; + p.insert("count".into(), json!(n)); } - let spacing = typed - .trim() - .trim_end_matches("in") - .trim_end_matches('"') - .trim() - .parse::() - .ok() - .filter(|v| v.is_finite() && (0.0..=22.0).contains(v)) - .ok_or("Spacing must be from 0 to 22 inches")?; - Ok(json!({"count": count, "spacing": spacing * 72.0})) + let spacing = field_of(d, "spacing"); + if spacing != shown_spacing { + let inches = spacing + .trim() + .trim_end_matches("in") + .trim_end_matches('"') + .trim() + .parse::() + .ok() + .filter(|v| v.is_finite() && (0.0..=22.0).contains(v)) + .ok_or("Spacing must be from 0 to 22 inches")?; + p.insert("spacing".into(), json!(inches * 72.0)); + } + Ok((!p.is_empty()).then_some(Value::Object(p))) } /// Columns: the number of columns in the text box and the space between them. OK applies one -/// `format.columns` call; Cancel leaves the text box as it was. +/// `format.columns` call with the fields that changed, or none when neither did; Cancel leaves +/// the text box as it was. fn columns_dialog(app: &mut SlideApp, ui: &mut Ui, d: &mut Dialog) -> bool { let count = d.params.get("count").and_then(Value::as_u64).unwrap_or(1).to_string(); let spacing = num_text(d.params.get("spacing").and_then(Value::as_f64).unwrap_or(0.0) / 72.0); @@ -2700,8 +2704,12 @@ fn columns_dialog(app: &mut SlideApp, ui: &mut Ui, d: &mut Dialog) -> bool { } let (ok, cancel) = buttons(ui, "OK"); if ok { - match columns_value(d, &spacing).and_then(|p| app.run("format.columns", p)) { - Ok(_) => return true, + let applied = columns_value(d, &count, &spacing).and_then(|p| match p { + Some(p) => app.run("format.columns", p).map(|_| ()), + None => Ok(()), + }); + match applied { + Ok(()) => return true, Err(e) => { d.fields.insert("error".into(), e); return false;