From b45c41a02c124d53b89ce3ae2f69f4f0024766ae Mon Sep 17 00:00:00 2001 From: MortenHusted Date: Wed, 28 Oct 2026 08:35:33 +0100 Subject: [PATCH 1/3] M2: auto layout settings in one call don't depend on how their keys are spelled `apply_order` gave `layout` the rank of every other key, so the props map's alphabetical order decided: `gap` applied before an object `layout`, which then replaced the whole auto layout (the gap went back to its default), and `padding` applied after `layout: null` and turned auto layout back on. `layout`/`layoutMode` now apply right after the geometry keys and before the keys that tune them, and auto layout keys beside a `layout` of null or "none" are refused, naming them. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/al_behaviour_tests.rs | 42 ++++++++++++++++++++++++- crates/engine/src/cmd/props.rs | 34 +++++++++++++++----- docs/mcp.md | 4 ++- 3 files changed, 70 insertions(+), 10 deletions(-) diff --git a/crates/engine/src/al_behaviour_tests.rs b/crates/engine/src/al_behaviour_tests.rs index a7a4d1d7..e9f2ea86 100644 --- a/crates/engine/src/al_behaviour_tests.rs +++ b/crates/engine/src/al_behaviour_tests.rs @@ -6,7 +6,7 @@ use serde_json::{Value, json}; use crate::Session; use crate::params::id_of; -use figcraft_doc::{CellAlign, MainAlign, NodeId, Sizing}; +use figcraft_doc::{CellAlign, CrossAlign, LayoutMode, MainAlign, NodeId, Sizing}; fn id(v: Value) -> NodeId { id_of(&v["id"]).unwrap() @@ -233,3 +233,43 @@ fn a_size_given_with_a_new_path_fixes_that_axis() { assert_eq!(sizing(&s, v).0, Sizing::Fixed); assert!((size(&s, v).0 - 30.0).abs() < 1e-6, "{:?}", size(&s, v)); } + +/// What one call's auto layout keys give, whichever way they are spelled: the mode (a string, or +/// an object that replaces the whole auto layout) applies first and the other keys tune it. +#[test] +fn auto_layout_keys_in_one_call_tune_the_mode_given_beside_them() { + for mode in [json!("vertical"), json!({"mode": "vertical"})] { + let props = json!({"layout": mode, "gap": 8, "padding": 4, "crossAlign": "center"}); + let mut s = Session::new(); + let created = id(s.execute("node.create", &json!({"type": "frame", "props": props})).unwrap()); + let set_on = id(s.execute("frame.create", &json!({"x": 0, "y": 0, "width": 100, "height": 100})).unwrap()); + set(&mut s, set_on, props.clone()); + for f in [created, set_on] { + let l = s.doc().unwrap().node(f).unwrap().layout.clone().unwrap(); + assert_eq!((l.mode, l.gap, l.padding, l.cross_align), (LayoutMode::Vertical, 8.0, [4.0; 4], CrossAlign::Center), "{props}"); + } + } +} + +/// Auto layout keys beside a `layout` that turns auto layout off would turn it back on: the call +/// is refused and the layer stays as it was. +#[test] +fn auto_layout_keys_beside_no_auto_layout_are_refused() { + let mut s = Session::new(); + let f = id(s.execute("frame.create", &json!({"x": 0, "y": 0, "width": 100, "height": 100})).unwrap()); + for props in + [json!({"layout": null, "padding": 4}), json!({"layout": "none", "gap": 8, "padding": 4}), json!({"layoutMode": "NONE", "wrap": true})] + { + let e = s.execute("node.set", &json!({"id": f, "props": props})).unwrap_err().to_string(); + assert!(e.contains("turns auto layout off"), "{props}: {e}"); + assert!(s.doc().unwrap().node(f).unwrap().layout.is_none(), "{props}"); + let e = s.execute("node.create", &json!({"type": "frame", "props": props})).unwrap_err().to_string(); + assert!(e.contains("turns auto layout off"), "{props}: {e}"); + } + let e = s.execute("node.set", &json!({"id": f, "props": {"layout": null, "gap": 8, "padding": 4}})).unwrap_err().to_string(); + assert!(e.contains("`gap`, `padding` are auto layout properties"), "{e}"); + // Without other auto layout keys, `layout: null` still removes it. + set(&mut s, f, json!({"layout": "vertical", "gap": 8})); + set(&mut s, f, json!({"layout": null, "name": "Plain"})); + assert!(s.doc().unwrap().node(f).unwrap().layout.is_none()); +} diff --git a/crates/engine/src/cmd/props.rs b/crates/engine/src/cmd/props.rs index 30a10393..e0cb996e 100644 --- a/crates/engine/src/cmd/props.rs +++ b/crates/engine/src/cmd/props.rs @@ -383,20 +383,37 @@ fn paint_none(v: &Value) -> bool { v.is_null() || v.as_str().is_some_and(|s| s.eq_ignore_ascii_case("none")) } -/// The order [`apply`] takes keys in: geometry first so rotation applies to the final size, style -/// links after the values so a link wins over values its style sets, variable bindings last so -/// paints set in the same call don't detach them. +/// The order [`apply`] takes keys in: geometry first so rotation applies to the final size, the +/// auto layout mode before the keys that tune it (an object `layout` replaces the whole auto +/// layout), style links after the values so a link wins over values its style sets, variable +/// bindings last so paints set in the same call don't detach them. fn apply_order(k: &str) -> u8 { match k { "path" | "width" | "height" | "w" | "h" => 0, - "rotation" => 2, - "x" | "y" => 3, - "fillStyle" | "strokeStyle" | "effectStyle" | "textStyle" | "gridStyle" => 4, - "boundVariables" => 5, - _ => 1, + "layout" | "layoutMode" => 1, + "rotation" => 3, + "x" | "y" => 4, + "fillStyle" | "strokeStyle" | "effectStyle" | "textStyle" | "gridStyle" => 5, + "boundVariables" => 6, + _ => 2, } } +/// Auto layout keys given beside a `layout` (or `layoutMode`) that turns auto layout off: they +/// would turn it back on (`layout_mut`), so the call is refused rather than one of them losing. +fn layout_keys_beside_none(cmd: &str, props: &Map) -> Result<()> { + let off = ["layout", "layoutMode"] + .iter() + .filter_map(|k| props.get(*k)) + .any(|v| v.is_null() || v.as_str().is_some_and(|s| s.eq_ignore_ascii_case("none"))); + let tuning: Vec = props.keys().filter(|k| LAYOUT_KEYS.contains(&k.as_str())).map(|k| format!("`{k}`")).collect(); + if !off || tuning.is_empty() { + return Ok(()); + } + let (what, are) = if tuning.len() == 1 { ("an auto layout property", "is") } else { ("auto layout properties", "are") }; + Err(bad(cmd, format!("{} {are} {what}, and `layout` in the same call turns auto layout off", tuning.join(", ")))) +} + /// An explicit width (`is_w`) or height makes hugging or filling on that axis fixed, like typing /// into W or H. fn fix_axis(doc: &mut Document, id: NodeId, is_w: bool) -> Result<()> { @@ -411,6 +428,7 @@ fn fix_axis(doc: &mut Document, id: NodeId, is_w: bool) -> Result<()> { /// Apply a property map to one node. `x`/`y` are relative to the parent. pub fn apply(cmd: &str, doc: &mut Document, id: NodeId, props: &Map) -> Result<()> { + layout_keys_beside_none(cmd, props)?; let mut unknown = Vec::new(); let mut keys: Vec<(&String, &Value)> = props.iter().collect(); keys.sort_by_key(|(k, _)| apply_order(k)); diff --git a/docs/mcp.md b/docs/mcp.md index 05dfd1e6..a71a0def 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -157,7 +157,9 @@ Or in a project's `.mcp.json`: ignored); a layer a mask hides can't be clicked there, and a mask group's box and exports are the mask's (plus any layers under the mask). - Children of an auto-layout frame are positioned by the layout: set `layout`, `gap`, `padding`, - `mainAlign`, `crossAlign`, `sizingH`/`sizingV` instead of x/y. More auto layout props: + `mainAlign`, `crossAlign`, `sizingH`/`sizingV` instead of x/y. In one call, `layout` applies + before the keys that tune it, so `{"layout": {"mode": "vertical"}, "gap": 8}` keeps the gap; + auto layout keys beside a `layout` of `null` or `"none"` are refused. More auto layout props: - `sizingH` / `sizingV`: `fixed`, `hug` (auto layout frames and text) or `fill` (children in an auto layout flow). A child can't fill an axis its parent hugs: setting `fill` turns the parent `fixed` on that axis, and a parent set to `hug` turns the children that filled it `fixed`. A From f98da27882f21841e109fdda018643343f75f562 Mon Sep 17 00:00:00 2001 From: MortenHusted Date: Wed, 28 Oct 2026 08:49:34 +0100 Subject: [PATCH 2/3] #1458: name the key that turns auto layout off `{"layoutMode": "NONE", "wrap": true}` was refused with a message naming `layout`, a key the call didn't have. The error now names the key the call used. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/al_behaviour_tests.rs | 4 +++- crates/engine/src/cmd/props.rs | 11 ++++------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/crates/engine/src/al_behaviour_tests.rs b/crates/engine/src/al_behaviour_tests.rs index e9f2ea86..030b30cf 100644 --- a/crates/engine/src/al_behaviour_tests.rs +++ b/crates/engine/src/al_behaviour_tests.rs @@ -267,7 +267,9 @@ fn auto_layout_keys_beside_no_auto_layout_are_refused() { assert!(e.contains("turns auto layout off"), "{props}: {e}"); } let e = s.execute("node.set", &json!({"id": f, "props": {"layout": null, "gap": 8, "padding": 4}})).unwrap_err().to_string(); - assert!(e.contains("`gap`, `padding` are auto layout properties"), "{e}"); + assert!(e.contains("`gap`, `padding` are auto layout properties, and `layout` in the same call"), "{e}"); + let e = s.execute("node.set", &json!({"id": f, "props": {"layoutMode": "NONE", "wrap": true}})).unwrap_err().to_string(); + assert!(e.contains("`wrap` is an auto layout property, and `layoutMode` in the same call"), "{e}"); // Without other auto layout keys, `layout: null` still removes it. set(&mut s, f, json!({"layout": "vertical", "gap": 8})); set(&mut s, f, json!({"layout": null, "name": "Plain"})); diff --git a/crates/engine/src/cmd/props.rs b/crates/engine/src/cmd/props.rs index e0cb996e..d570e822 100644 --- a/crates/engine/src/cmd/props.rs +++ b/crates/engine/src/cmd/props.rs @@ -403,15 +403,12 @@ fn apply_order(k: &str) -> u8 { /// would turn it back on (`layout_mut`), so the call is refused rather than one of them losing. fn layout_keys_beside_none(cmd: &str, props: &Map) -> Result<()> { let off = ["layout", "layoutMode"] - .iter() - .filter_map(|k| props.get(*k)) - .any(|v| v.is_null() || v.as_str().is_some_and(|s| s.eq_ignore_ascii_case("none"))); + .into_iter() + .find(|k| props.get(*k).is_some_and(|v| v.is_null() || v.as_str().is_some_and(|s| s.eq_ignore_ascii_case("none")))); let tuning: Vec = props.keys().filter(|k| LAYOUT_KEYS.contains(&k.as_str())).map(|k| format!("`{k}`")).collect(); - if !off || tuning.is_empty() { - return Ok(()); - } + let Some(off) = off.filter(|_| !tuning.is_empty()) else { return Ok(()) }; let (what, are) = if tuning.len() == 1 { ("an auto layout property", "is") } else { ("auto layout properties", "are") }; - Err(bad(cmd, format!("{} {are} {what}, and `layout` in the same call turns auto layout off", tuning.join(", ")))) + Err(bad(cmd, format!("{} {are} {what}, and `{off}` in the same call turns auto layout off", tuning.join(", ")))) } /// An explicit width (`is_w`) or height makes hugging or filling on that axis fixed, like typing From 81769cbe742346f98c807728ce2dc1bbd52781b6 Mon Sep 17 00:00:00 2001 From: MortenHusted Date: Wed, 28 Oct 2026 08:51:02 +0100 Subject: [PATCH 3/3] #1458: test layoutMode beside the keys that tune it too Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/al_behaviour_tests.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/al_behaviour_tests.rs b/crates/engine/src/al_behaviour_tests.rs index 030b30cf..d36b35b8 100644 --- a/crates/engine/src/al_behaviour_tests.rs +++ b/crates/engine/src/al_behaviour_tests.rs @@ -238,8 +238,9 @@ fn a_size_given_with_a_new_path_fixes_that_axis() { /// an object that replaces the whole auto layout) applies first and the other keys tune it. #[test] fn auto_layout_keys_in_one_call_tune_the_mode_given_beside_them() { - for mode in [json!("vertical"), json!({"mode": "vertical"})] { - let props = json!({"layout": mode, "gap": 8, "padding": 4, "crossAlign": "center"}); + for (key, mode) in ["layout", "layoutMode"].into_iter().flat_map(|k| [(k, json!("vertical")), (k, json!({"mode": "vertical"}))]) { + let mut props = json!({"gap": 8, "padding": 4, "crossAlign": "center"}); + props[key] = mode; let mut s = Session::new(); let created = id(s.execute("node.create", &json!({"type": "frame", "props": props})).unwrap()); let set_on = id(s.execute("frame.create", &json!({"x": 0, "y": 0, "width": 100, "height": 100})).unwrap());