Merge #814

crates/engine/src/cmd/props.rs

@@ -297,6 +297,12 @@ pub const LAYOUT_KEYS: &[&str] = &[
     "gridRowTracks",
 ];
 
+/// Drop the bindings of auto layout fields (gap, padding…): they go with the auto layout, or
+/// re-applying them would bring it back.
+pub fn drop_layout_bindings(n: &mut Node) {
+    n.bound.retain(|k, _| !LAYOUT_KEYS.contains(&k.as_str()));
+}
+
 /// The auto layout of a frame-like node, created when it has none yet. Other nodes can't hold an
 /// auto layout: an auto layout key on them is refused, not stored where nothing reads it
 /// (`node.set` on several layers skips them instead, see `edit::set`).
@@ -500,8 +506,14 @@ pub fn apply(cmd: &str, doc: &mut Document, id: NodeId, props: &Map<String, Valu
                     return Err(bad(cmd, "auto layout needs a frame"));
                 }
                 match v {
-                    Value::Null => n.layout = None,
-                    Value::String(s) if s.eq_ignore_ascii_case("none") => n.layout = None,
+                    Value::Null => {
+                        n.layout = None;
+                        drop_layout_bindings(n);
+                    }
+                    Value::String(s) if s.eq_ignore_ascii_case("none") => {
+                        n.layout = None;
+                        drop_layout_bindings(n);
+                    }
                     Value::String(s) => {
                         let mode = match s.to_ascii_lowercase().as_str() {
                             "horizontal" | "row" => LayoutMode::Horizontal,

crates/engine/src/cmd/structure.rs

@@ -10,6 +10,8 @@ use super::{CommandSpec, ok};
 use crate::params::*;
 use crate::{EngineError, Result, Session, bad, spec};
 
+/// The layer structure commands: group, frame, mask, boolean operations, flatten, auto layout
+/// add/remove, move to page and vector path edits.
 pub fn specs() -> Vec<CommandSpec> {
     vec![
         spec!("object.group", "Group selection", "{ids?, name?}", true, |s, p| wrap(s, p, "object.group", Wrap::Group)),
@@ -71,7 +73,9 @@ pub fn specs() -> Vec<CommandSpec> {
                 // Hugging needs auto layout and the children that filled have nothing to fill: both
                 // become fixed (the frame keeps filling its own auto layout parent).
                 let doc = s.doc_mut()?;
-                doc.get_mut(i)?.layout = None;
+                let n = doc.get_mut(i)?;
+                n.layout = None;
+                super::props::drop_layout_bindings(n);
                 figcraft_layout::coherent_sizing(doc, i);
             }
             ok()
@@ -286,7 +290,7 @@ fn convert_to_section(s: &mut Session, p: &Value) -> Result<Value> {
         n.grids.clear();
         n.guides.clear();
         n.styles.grid = None;
-        n.bound.retain(|k, _| !k.starts_with("padding") && !matches!(k.as_str(), "gap" | "counterAxisSpacing"));
+        super::props::drop_layout_bindings(n);
         let kids = n.children.clone();
         for k in kids {
             // Out of the flow there is nothing to fill or to be absolute against.

crates/engine/src/cmd/styles.rs

@@ -1105,6 +1105,8 @@ pub(crate) fn apply_paint_bindings_on_load(doc: &mut Document) {
     doc.mark_dirty(pending);
 }
 
+/// Re-apply the variable bindings whose field passes `include`, on the `only` nodes (every bound
+/// node when `None`). Auto layout fields wait while their node has no auto layout.
 fn apply_bindings_matching(doc: &mut Document, only: Option<&std::collections::BTreeSet<NodeId>>, include: impl Fn(&str) -> bool) {
     use super::lists::{BoundList, list_binding};
     if doc.variables.variables.is_empty() {
@@ -1119,6 +1121,7 @@ fn apply_bindings_matching(doc: &mut Document, only: Option<&std::collections::B
         let modes = doc.modes_for(id);
         let Some(n) = doc.node(id) else { continue };
         let binds = n.bound.clone();
+        let has_layout = n.layout.is_some();
         let mut sets = serde_json::Map::new();
         let mut effect_sets: Vec<(usize, String, VariableValue)> = Vec::new();
         let mut paint_sets: Vec<(bool, usize, Option<usize>, Color)> = Vec::new();
@@ -1128,6 +1131,10 @@ fn apply_bindings_matching(doc: &mut Document, only: Option<&std::collections::B
             if !include(&field) {
                 continue;
             }
+            // A binding never creates an auto layout: without one it waits for the next.
+            if !has_layout && super::props::LAYOUT_KEYS.contains(&field.as_str()) {
+                continue;
+            }
             let Some(val) = doc.variables.resolve(&var, &modes) else { continue };
             // One list item's value: a paint's colour, an effect's or a layout grid's field.
             if let Some((list, i, prop)) = list_binding(&field) {

crates/engine/src/variables_behaviour_tests.rs

@@ -279,6 +279,34 @@ fn bindings_by_field_and_type() {
     }
 }
 
+#[test]
+fn auto_layout_bindings_go_with_the_auto_layout() {
+    // Docs silent; pinned (#810): removing a frame's auto layout drops its gap and padding bindings,
+    // and a binding never brings an auto layout back. One made without auto layout waits for it.
+    let mut s = Session::new();
+    run(&mut s, "variable.createCollection", json!({"name": "C"}));
+    run(&mut s, "variable.create", json!({"collection": "C", "name": "g", "type": "number", "value": 12}));
+    let removals =
+        [("layout.remove", json!({})), ("node.set", json!({"props": {"layout": null}})), ("node.set", json!({"props": {"layout": "none"}}))];
+    for (cmd, mut p) in removals {
+        let f = frame(&mut s, None);
+        run(&mut s, "layout.add", json!({"ids": [f]}));
+        for field in ["gap", "crossGap", "paddingTop"] {
+            run(&mut s, "variable.bind", json!({"ids": [f], "field": field, "variable": "g"}));
+        }
+        p["ids"] = json!([f]);
+        run(&mut s, cmd, p.clone());
+        let n = node(&s, f);
+        assert!(n.layout.is_none(), "{cmd} {p}: auto layout stays removed");
+        assert!(n.bound.is_empty(), "{cmd} {p}: layout bindings dropped");
+    }
+    let f = frame(&mut s, None);
+    run(&mut s, "variable.bind", json!({"ids": [f], "field": "gap", "variable": "g"}));
+    assert!(node(&s, f).layout.is_none(), "binding gap adds no auto layout");
+    run(&mut s, "layout.add", json!({"ids": [f]}));
+    assert_eq!(node(&s, f).layout.map(|l| l.gap), Some(12.0), "the waiting binding applies to the new auto layout");
+}
+
 #[test]
 fn deleting_a_bound_variable_keeps_the_last_value() {
     // Documented: deleting a variable unbinds what used it, which keeps showing its last value.