Merge pull request #5127 from dotCooCoo/fix/vars-p8

Variables: Edit Data Set keeps an empty text value when OK changes nothing

crates/ui-egui/src/dialogs.rs

@@ -443,11 +443,15 @@ pub fn accepts_field(d: &Dialog, field: &str) -> bool {
 }
 
 /// Set `field` of `d` to `value` for `ui.dialog.set`, as the dialog's own control for it does: a
-/// Pattern Brush Options corner tile set this way is saved by OK, as one chosen from its menu is.
+/// Pattern Brush Options corner tile set this way is saved by OK, as one chosen from its menu is,
+/// and an Edit Data Set field set this way is no longer the row's empty text value.
 pub fn set_field(d: &mut Dialog, field: String, value: Value) {
     if d.kind == brush_options::KIND {
         brush_art_options::note_set(d, &field);
     }
+    if d.kind == variables::KIND {
+        variables::note_set(d, &field);
+    }
     d.fields.insert(field, value);
 }
 

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

@@ -86,6 +86,13 @@ pub fn open(app: &mut VectorcraftApp, id: &str, preselect: Option<&str>) -> bool
             let Some(current) = vars.dataset(&name) else { return true };
             fields.insert("name".into(), json!(name));
             value_fields(&vars, Some(&current.values), &mut fields);
+            // An empty text value is a value, but its field looks like a field with no value:
+            // mark it so OK keeps it until the field is edited.
+            for (var, value) in &current.values {
+                if matches!(value, DataValue::Text(t) if t.is_empty()) {
+                    fields.insert(empty_marker(&value_key(var)), json!(true));
+                }
+            }
         }
         "dataset.select" => {
             fields.insert("name".into(), json!(""));
@@ -142,6 +149,12 @@ fn value_key(name: &str) -> String {
     format!("value:{name}")
 }
 
+/// The key that marks the field `key` as holding the edited row's empty text value. Editing the
+/// field removes the mark.
+fn empty_marker(key: &str) -> String {
+    format!("__empty:{key}")
+}
+
 fn body(app: &mut VectorcraftApp, ui: &mut egui::Ui, d: &mut Dialog) -> bool {
     let t = Tokens::get(ui.ctx());
     let Some(st) = app.session.active() else {
@@ -227,11 +240,23 @@ fn text(ui: &mut egui::Ui, d: &mut Dialog, key: &str, width: f32) -> egui::Respo
     let mut s = d.str(key);
     let r = ui.add(egui::TextEdit::singleline(&mut s).align(egui::Align2::LEFT_CENTER).desired_width(width));
     if r.changed() {
-        d.fields.insert(key.into(), json!(s));
+        set_text(d, key, s);
     }
     r
 }
 
+/// Write a typed `s` into `key`.
+fn set_text(d: &mut Dialog, key: &str, s: String) {
+    note_set(d, key);
+    d.fields.insert(key.into(), json!(s));
+}
+
+/// Records that `field` was typed in or set by `ui.dialog.set`: it no longer holds the row's
+/// empty text value, so emptying it unsets the variable.
+pub(super) fn note_set(d: &mut Dialog, field: &str) {
+    d.fields.remove(&empty_marker(field));
+}
+
 /// A checkbox for `key`, written back as it is toggled, which screen readers call `name`.
 fn checkbox(ui: &mut egui::Ui, d: &mut Dialog, key: &str, name: &str) {
     let b = d.bool(key);
@@ -256,7 +281,8 @@ fn dropdown(ui: &mut egui::Ui, d: &mut Dialog, key: &str, options: &[&str], widt
 }
 
 /// The `values` object the dialog's fields mean: a text, image or graph variable whose field is
-/// empty carries no value in this row, and a visibility checkbox is always a value — its false,
+/// empty carries no value in this row, except a text variable whose value in the edited row was
+/// already the empty text, which keeps it. A visibility checkbox is always a value — its false,
 /// too.
 fn values_of(d: &Dialog, vars: &Variables) -> Value {
     let mut out = serde_json::Map::new();
@@ -265,7 +291,7 @@ fn values_of(d: &Dialog, vars: &Variables) -> Value {
         match v.kind {
             VariableKind::Text | VariableKind::Image | VariableKind::Graph => {
                 let text = d.str(&key);
-                if !text.is_empty() {
+                if !text.is_empty() || (v.kind == VariableKind::Text && d.bool(&empty_marker(&key))) {
                     out.insert(v.name.clone(), json!(text));
                 }
             }
@@ -282,7 +308,7 @@ fn text_area(ui: &mut egui::Ui, d: &mut Dialog, key: &str, width: f32) {
     let mut s = d.str(key);
     let r = ui.add(egui::TextEdit::multiline(&mut s).desired_width(width).desired_rows(4));
     if r.changed() {
-        d.fields.insert(key.into(), json!(s));
+        set_text(d, key, s);
     }
 }
 
@@ -352,6 +378,51 @@ mod tests {
         assert_eq!(sets["datasets"][0]["values"]["Badge"], json!({"visible": false}));
     }
 
+    #[test]
+    fn an_untouched_edit_keeps_an_empty_text_value() {
+        let mut app = app_with_variables();
+        let id = app.run("text.create", json!({"x": 10, "y": 10, "text": "x"})).unwrap()["id"].as_u64().unwrap();
+        app.run("variable.bind", json!({"variable": "Title", "ids": [id]})).unwrap();
+        app.run("dataset.new", json!({"name": "Row 1", "values": {"Title": "", "Badge": true}})).unwrap();
+        app.run("dataset.new", json!({"name": "Row 2", "values": {"Title": "Bob", "Badge": true}})).unwrap();
+        app.run("dataset.select", json!({"name": "Row 1"})).unwrap();
+        open(&mut app, "dataset.set", None);
+        assert_eq!(app.ui.dialog.as_ref().unwrap().str(&value_key("Title")), "");
+        crate::dialogs::confirm(&mut app).unwrap();
+        let sets = app.run("dataset.list", json!({})).unwrap();
+        assert_eq!(sets["datasets"][0]["values"]["Title"], json!({"text": ""}), "OK with nothing changed keeps the empty text");
+        app.run("dataset.select", json!({"name": "Row 2"})).unwrap();
+        app.run("dataset.select", json!({"name": "Row 1"})).unwrap();
+        assert_eq!(app.run("text.getRange", json!({"id": id})).unwrap()["text"], "", "the row still empties the text");
+        // A field the user empties on a row that had a value still leaves the variable unset.
+        app.run("dataset.select", json!({"name": "Row 2"})).unwrap();
+        open(&mut app, "dataset.set", None);
+        app.ui.dialog.as_mut().unwrap().fields.insert(value_key("Title"), json!(""));
+        crate::dialogs::confirm(&mut app).unwrap();
+        let sets = app.run("dataset.list", json!({})).unwrap();
+        assert_eq!(sets["datasets"][1]["values"]["Title"], json!(null));
+        // Typing into the empty text value's field and emptying it again unsets it too, and so
+        // does the same edit through `ui.dialog.set`.
+        for typed in [true, false] {
+            app.run("dataset.set", json!({"name": "Row 1", "values": {"Title": "", "Badge": true}})).unwrap();
+            app.run("dataset.select", json!({"name": "Row 1"})).unwrap();
+            open(&mut app, "dataset.set", None);
+            {
+                let d = app.ui.dialog.as_mut().unwrap();
+                for s in ["a", ""] {
+                    if typed {
+                        set_text(d, &value_key("Title"), s.into());
+                    } else {
+                        crate::dialogs::set_field(d, value_key("Title"), json!(s));
+                    }
+                }
+            }
+            crate::dialogs::confirm(&mut app).unwrap();
+            let sets = app.run("dataset.list", json!({})).unwrap();
+            assert_eq!(sets["datasets"][0]["values"]["Title"], json!(null), "typed: {typed}");
+        }
+    }
+
     #[test]
     fn editing_a_dataset_opens_with_the_rows_own_values() {
         let mut app = app_with_variables();