M2: Suggest auto layout warns about uneven cross offsets and refuses instances (#1823)

Bands that start at different offsets across the flow (a centred title above a card) are lined up at the smallest offset, which moves some of them, so the result now carries a warning for it. Instances and layers inside them are refused before anything changes, with the same error Convert to group gives; a main component still works.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

crates/engine/src/cmd/structure.rs

@@ -114,7 +114,7 @@ pub fn specs() -> Vec<CommandSpec> {
         spec!(
             "layout.suggest",
             "Suggest auto layout",
-            "{ids?, direction? (horizontal|vertical|grid)}  splits each frame's layers into rows or columns by the gaps between them, wraps every band of several layers in a frame without fill that hugs it and does the same inside, so the layers keep their place where the gaps are even; the frame keeps its size, and the result carries `warnings` for gaps that differ or layers that overlap. `direction` sets the top level's direction (nested frames follow the gaps); grid lays the layers out in one grid. A group is converted to a frame and several layers are wrapped in one, as Add auto layout does",
+            "{ids?, direction? (horizontal|vertical|grid)}  splits each frame's layers into rows or columns by the gaps between them, wraps every band of several layers in a frame without fill that hugs it and does the same inside, so the layers keep their place where the gaps are even and the bands start at the same offset across the flow; the frame keeps its size, and the result carries `warnings` for gaps that differ, bands that start at different offsets (they line up at the smallest and some move) and layers that overlap. Instances and the layers inside them are refused. `direction` sets the top level's direction (nested frames follow the gaps); grid lays the layers out in one grid. A group is converted to a frame and several layers are wrapped in one, as Add auto layout does",
             true,
             suggest::layout_suggest
         ),

crates/engine/src/cmd/structure/suggest.rs

@@ -8,7 +8,7 @@ use serde_json::{Value, json};
 
 use super::{Wrap, convert, infer_layout, sort_layout_children, wrap};
 use crate::params::{id_of, req_targets, string};
-use crate::{EngineError, Result, Session};
+use crate::{EngineError, Result, Session, bad};
 
 /// How deep nested band frames go before a frame is laid out as a single list.
 const MAX_DEPTH: usize = 16;
@@ -102,6 +102,18 @@ fn layout_for(bands: &[Band], vertical: bool, frame_w: f64, frame_h: f64, name:
     let main_end = (main_size - bands.last().map_or(0.0, |b| b.hi)).max(0.0);
     let cross_start = bands.iter().map(|b| b.cross_lo).fold(f64::INFINITY, f64::min).max(0.0);
     let cross_end = bands.iter().map(|b| cross_size - b.cross_hi).fold(f64::INFINITY, f64::min).max(0.0);
+    // Every band is lined up at the start across the flow, so one that starts further in (a
+    // centred title above a card) moves to the smallest offset.
+    let widest_start = bands.iter().map(|b| b.cross_lo).fold(f64::NEG_INFINITY, f64::max);
+    let smallest_start = bands.iter().map(|b| b.cross_lo).fold(f64::INFINITY, f64::min);
+    if widest_start - smallest_start > GAP_TOLERANCE {
+        let what = if vertical { "rows" } else { "columns" };
+        warnings.push(format!(
+            "\"{name}\": the {what} start at different offsets ({} to {}), so they are lined up at the smallest and some move",
+            finite_or_zero(smallest_start),
+            finite_or_zero(widest_start)
+        ));
+    }
     let [ms, me, cs, ce] = [main_start, main_end, cross_start, cross_end].map(finite_or_zero);
     AutoLayout {
         mode: if vertical { LayoutMode::Vertical } else { LayoutMode::Horizontal },
@@ -215,6 +227,17 @@ pub(super) fn layout_suggest(s: &mut Session, p: &Value) -> Result<Value> {
     let ids = req_targets("layout.suggest", s, p)?;
     let direction = string(p, "direction");
     let doc = s.doc()?;
+    // An instance's layers come from its main component: nothing is added to them. Checked
+    // before anything changes, with the words Convert to group uses.
+    for i in &ids {
+        let n = doc.get(*i)?;
+        if matches!(n.kind, NodeKind::Instance { .. }) {
+            return Err(bad("layout.suggest", format!("\"{}\" is an instance: its layers come from its main component (detach it first)", n.name)));
+        }
+        if crate::components::owning_instance(doc, *i).is_some() {
+            return Err(bad("layout.suggest", format!("\"{}\" is inside an instance: edit its main component, or detach the instance", n.name)));
+        }
+    }
     let all_frames = ids.iter().all(|i| doc.node(*i).is_some_and(|n| n.kind.is_frame_like() && n.layout.is_none()));
     let single_group = ids.len() == 1 && ids.first().and_then(|i| doc.node(*i)).is_some_and(|n| matches!(n.kind, NodeKind::Group));
     // A frame with auto layout already is wrapped, like a layer, and so are several layers.

crates/engine/src/layout_suggest_tests.rs

@@ -278,3 +278,48 @@ fn add_auto_layout_still_makes_a_single_row() {
     got.sort();
     assert_eq!(got, sq.to_vec(), "no nested frames");
 }
+
+#[test]
+fn a_centred_title_above_a_card_is_lined_up_at_the_card_edge_with_a_warning() {
+    let mut s = Session::new();
+    let f = frame(&mut s, 0.0, 0.0, 200.0, 200.0);
+    let title = rect(&mut s, f, 60.0, 0.0, 80.0, 30.0);
+    let card = rect(&mut s, f, 0.0, 40.0, 200.0, 100.0);
+    let result = ex(&mut s, "layout.suggest", json!({"ids": [f]}));
+    assert_eq!(mode(&s, f), Some(LayoutMode::Vertical));
+    assert_eq!(kids(&s, f), vec![title, card]);
+    let w = warnings(&result);
+    assert_eq!(w.len(), 1, "{result}");
+    assert!(w[0].contains("different offsets"), "{}", w[0]);
+    assert_eq!(page_box(&s, title), [0.0, 0.0, 80.0, 30.0], "the title's left edge is the card's now");
+    assert_eq!(page_box(&s, card), [0.0, 40.0, 200.0, 140.0], "the rest stays");
+}
+
+#[test]
+fn instances_and_layers_inside_them_are_refused_and_a_main_component_works() {
+    let mut s = Session::new();
+    let f = frame(&mut s, 0.0, 0.0, 200.0, 200.0);
+    rect(&mut s, f, 0.0, 0.0, 50.0, 50.0);
+    rect(&mut s, f, 0.0, 60.0, 50.0, 30.0);
+    let nested = id(&ex(&mut s, "frame.create", json!({"parent": f, "x": 0, "y": 100, "width": 100, "height": 50})));
+    rect(&mut s, nested, 0.0, 0.0, 20.0, 20.0);
+    rect(&mut s, nested, 40.0, 0.0, 20.0, 20.0);
+    let comp = id(&ex(&mut s, "component.create", json!({"ids": [f]})));
+    let inst = id(&ex(&mut s, "instance.create", json!({"component": comp, "x": 400, "y": 0})));
+    let inside: Vec<NodeId> = kids(&s, inst).into_iter().filter(|k| matches!(s.doc().unwrap().node(*k).unwrap().kind, NodeKind::Frame)).collect();
+    assert!(!inside.is_empty(), "the instance has a frame of its own");
+    let page = s.page().unwrap();
+    let snapshot = |s: &Session| (kids(s, page), kids(s, inst), kids(s, inside[0]));
+    let before = snapshot(&s);
+    let steps = s.ds().unwrap().history.undo.len();
+    for target in [inst, inside[0]] {
+        let e = s.execute("layout.suggest", &json!({"ids": [target]})).unwrap_err().to_string();
+        assert!(e.contains("instance"), "{e}");
+    }
+    assert_eq!(snapshot(&s), before, "nothing changed");
+    assert_eq!(s.ds().unwrap().history.undo.len(), steps, "no history step");
+    assert!(s.doc().unwrap().node(inst).unwrap().layout.is_none());
+    // The main component is a frame and works.
+    let result = ex(&mut s, "layout.suggest", json!({"ids": [comp]}));
+    assert_eq!(result["ids"], json!([comp]));
+}