diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 62a17bda3..f252d09ff 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -498,6 +498,7 @@ fn undo(s: &mut Session, _: &Value) -> Result { super::symboledit::follow_history(st, editing_symbol.as_ref()); leave_isolation_if_gone(st); super::globaledit::follow_history(st); + st.drop_stale_variables_highlight(); Ok(json!({ "undone": label })) } @@ -525,6 +526,7 @@ fn redo(s: &mut Session, _: &Value) -> Result { super::symboledit::follow_history(st, editing_symbol.as_ref()); leave_isolation_if_gone(st); super::globaledit::follow_history(st); + st.drop_stale_variables_highlight(); Ok(json!({ "redone": label })) } diff --git a/crates/engine/src/cmd/variablecmds.rs b/crates/engine/src/cmd/variablecmds.rs index 9e589e883..33799bc85 100644 --- a/crates/engine/src/cmd/variablecmds.rs +++ b/crates/engine/src/cmd/variablecmds.rs @@ -231,9 +231,7 @@ fn delete(s: &mut Session, p: &Value) -> Result { let n = s.edit("Delete Variables", |d, _| Ok(d.variables.prune(&names)))?; // A highlight of a variable that is gone highlights nothing. let st = s.doc_mut()?; - if st.variables_highlight.as_ref().is_some_and(|h| st.doc.variables.variable(h).is_none()) { - st.variables_highlight = None; - } + st.drop_stale_variables_highlight(); st.revision += 1; Ok(json!({"deleted": n})) } diff --git a/crates/engine/src/cmd/variablecmds/library.rs b/crates/engine/src/cmd/variablecmds/library.rs index 937d4155e..b410af042 100644 --- a/crates/engine/src/cmd/variablecmds/library.rs +++ b/crates/engine/src/cmd/variablecmds/library.rs @@ -277,9 +277,7 @@ pub(super) fn load_library(s: &mut Session, p: &Value) -> Result { })?; // A highlight of a variable that is gone highlights nothing. let st = s.doc_mut()?; - if st.variables_highlight.as_ref().is_some_and(|h| st.doc.variables.variable(h).is_none()) { - st.variables_highlight = None; - } + st.drop_stale_variables_highlight(); st.revision += 1; let skipped: Vec = lib.other.iter().map(|(name, t)| json!({"name": name, "trait": t})).collect(); let mut out = json!({ diff --git a/crates/engine/src/doc_state.rs b/crates/engine/src/doc_state.rs index 30cf1e7e8..dc1df88be 100644 --- a/crates/engine/src/doc_state.rs +++ b/crates/engine/src/doc_state.rs @@ -352,6 +352,14 @@ impl DocState { self.revision += 1; } } + /// Clear the Variables panel highlight when the document defines no variable of that name + /// (deleted, or taken out by Undo or Redo), so a variable defined later under that name is + /// not highlighted without a click. + pub(crate) fn drop_stale_variables_highlight(&mut self) { + if self.variables_highlight.as_ref().is_some_and(|h| self.doc.variables.variable(h).is_none()) { + self.variables_highlight = None; + } + } /// Count the document as modified, as if never saved (a document restored by Data Recovery). pub fn mark_unsaved(&mut self) { self.saved_doc = Arc::new(Document::new(1.0, 1.0)); diff --git a/crates/engine/src/session/execute.rs b/crates/engine/src/session/execute.rs index d3ffe4163..f4ebd87c4 100644 --- a/crates/engine/src/session/execute.rs +++ b/crates/engine/src/session/execute.rs @@ -219,6 +219,7 @@ impl Session { st.interaction = interaction; st.history = history; cmd::globaledit::follow_history(st); + st.drop_stale_variables_highlight(); st.revision += 1; } if let Some(i) = active.and_then(|uid| self.docs.iter().position(|d| d.uid == uid)) { diff --git a/crates/engine/src/session/undo.rs b/crates/engine/src/session/undo.rs index 2f750dc13..c5f3f1595 100644 --- a/crates/engine/src/session/undo.rs +++ b/crates/engine/src/session/undo.rs @@ -102,6 +102,7 @@ impl Session { deselect_hidden(&self.menu, &st.doc, &mut st.selection); st.sync_new_art(); cmd::globaledit::follow_history(st); + st.drop_stale_variables_highlight(); st.revision += 1; // The journal drops only the canceled groups' own entries and keeps the ones made in other documents meanwhile. journal.extend(g.journal); @@ -230,6 +231,7 @@ impl Session { st.doc = doc; st.selection = selection; cmd::globaledit::follow_history(st); + st.drop_stale_variables_highlight(); deselect_hidden(&menu, &st.doc, &mut st.selection); st.history.recost_last(&st.doc); st.revision += 1; diff --git a/crates/engine/src/tests/variables.rs b/crates/engine/src/tests/variables.rs index e1bfd3e2f..5e0176f70 100644 --- a/crates/engine/src/tests/variables.rs +++ b/crates/engine/src/tests/variables.rs @@ -893,6 +893,49 @@ fn highlight_follows_rename_and_clears_with_delete() { assert!(s.doc().unwrap().variables_highlight.is_none(), "a highlight of a deleted variable highlights nothing"); } +/// Undo of a rename takes the new name away, so the highlight that followed the rename goes +/// too: a variable defined later under that name is not highlighted, and Delete without names +/// does not delete it. +#[test] +fn undoing_a_rename_clears_the_highlight_of_the_new_name() { + let mut s = session(); + s.execute("variable.define", &json!({"name": "A", "kind": "text"})).unwrap(); + s.execute("variable.highlight", &json!({"variable": "A"})).unwrap(); + s.execute("variable.rename", &json!({"name": "A", "newName": "B"})).unwrap(); + s.execute("edit.undo", &json!({})).unwrap(); + assert!(s.doc().unwrap().variables_highlight.is_none()); + s.execute("variable.define", &json!({"name": "B", "kind": "visibility"})).unwrap(); + assert!(s.doc().unwrap().variables_highlight.is_none(), "B was never clicked"); + assert!(s.execute("variable.delete", &json!({})).is_err(), "no variable highlighted"); + let vars = s.execute("variable.list", &json!({})).unwrap(); + assert_eq!(vars["variables"].as_array().unwrap().len(), 2); +} + +/// Undo of New Variable, Redo of Delete Variables and a canceled undo group each leave the +/// highlighted name undefined, and the highlight goes with it. +#[test] +fn undo_redo_and_a_canceled_group_clear_a_highlight_of_a_variable_they_remove() { + let mut s = session(); + s.execute("variable.define", &json!({"name": "N", "kind": "text"})).unwrap(); + s.execute("variable.highlight", &json!({"variable": "N"})).unwrap(); + s.execute("edit.undo", &json!({})).unwrap(); + assert!(s.doc().unwrap().variables_highlight.is_none(), "undo of New Variable"); + + s.execute("edit.redo", &json!({})).unwrap(); + s.execute("variable.delete", &json!({"names": ["N"]})).unwrap(); + s.execute("edit.undo", &json!({})).unwrap(); + s.execute("variable.highlight", &json!({"variable": "N"})).unwrap(); + s.execute("edit.redo", &json!({})).unwrap(); + assert!(s.doc().unwrap().variables_highlight.is_none(), "redo of Delete Variables"); + + s.execute("edit.undoGroup.begin", &json!({})).unwrap(); + s.execute("variable.define", &json!({"name": "G", "kind": "text"})).unwrap(); + s.execute("variable.highlight", &json!({"variable": "G"})).unwrap(); + s.execute("edit.undoGroup.end", &json!({"cancel": true})).unwrap(); + assert!(s.doc().unwrap().doc.variables.variable("G").is_none(), "the cancel took G back"); + assert!(s.doc().unwrap().variables_highlight.is_none(), "a canceled undo group"); +} + /// A row that hides an object must be able to show it again, and hiding it on the artboard /// (as the Layers panel does) is what a capture records: a visibility variable reads and /// writes its object's own visibility, which therefore never makes the object off limits.