Merge pull request #5602 from mockydevs/fix/area-options-each-frame

Area Type Options: applying to several frames keeps each frame's own options

crates/engine/src/cmd/typecmd.rs

@@ -87,7 +87,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Area Type Options…",
             ["Type"],
             None,
-            "{ids?|id?, width?: pt, height?: pt, rows?, columns?, gutter?: pt (between rows and columns), rowGutter?: pt|null (between rows when it differs from gutter, which is then between columns; gutter given alone sets both; the reply gives the rows' gutter either way), inset?: pt, firstBaseline?: ascent|capHeight|xHeight|leading|fixed, firstBaselineMin?: pt, verticalAlign?: top|center|bottom|justify, flow?: columns|rows (Text Flow: down each column's rows, then across, the default; or across each row's columns, then down), fit?: none|autoHeight|shrinkText, fitMinPercent?: 10..100, …the keys of its own reply (it can be passed back; the values it only reports are ignored)} set the selected area type's options; width and height size the type area from its top-left corner, along the type's own axes, and the text reflows at its size (with fit autoHeight, Auto Size, the frame's height follows the text after every edit and height is ignored; setting the height by hand turns it off); fit shrinkText scales overflowing text down (size, leading, baseline shift) by the largest factor down to fitMinPercent % (default 50) that makes it fit (none given: query) → the first object's options, plus overflow (the text doesn't fit its frame) and fitScale (Shrink Text's factor, 1 unshrunk)",
+            "{ids?|id?, width?: pt, height?: pt, rows?, columns?, gutter?: pt (between rows and columns), rowGutter?: pt|null (between rows when it differs from gutter, which is then between columns; gutter given alone sets both; the reply gives the rows' gutter either way), inset?: pt, firstBaseline?: ascent|capHeight|xHeight|leading|fixed, firstBaselineMin?: pt, verticalAlign?: top|center|bottom|justify, flow?: columns|rows (Text Flow: down each column's rows, then across, the default; or across each row's columns, then down), fit?: none|autoHeight|shrinkText, fitMinPercent?: 10..100, …the keys of its own reply (it can be passed back; the values it only reports are ignored)} set the selected area type's options (each frame takes the options given over its own and keeps the rest; fit resolves against each frame's own; options that change nothing add no undo step); width and height size the type area from its top-left corner, along the type's own axes, and the text reflows at its size (with fit autoHeight, Auto Size, the frame's height follows the text after every edit and height is ignored; setting the height by hand turns it off); fit shrinkText scales overflowing text down (size, leading, baseline shift) by the largest factor down to fitMinPercent % (default 50) that makes it fit (none given: query) → the first object's options, plus overflow (the text doesn't fit its frame) and fitScale (Shrink Text's factor, 1 unshrunk)",
             has_selection,
             area_options
         ),
@@ -1080,46 +1080,74 @@ fn area_options(s: &mut Session, p: &Value) -> Result<Value> {
         let t = area_text(s.doc()?.doc.node(first)).ok_or_else(|| bad(C, "select area type (text in a frame)"))?;
         area_options_json(t)
     };
+    // Checked once up front (its errors don't depend on the frame); each frame resolves `fit`
+    // against its own setting below.
     let cur = area_text(s.doc()?.doc.node(first)).map(|t| t.area.clone()).unwrap_or_default();
-    let fit = fit_param(p, cur.fit, C)?;
-    let mut v = serde_json::to_value(&cur).map_err(|e| EngineError::Other(e.to_string()))?;
+    fit_param(p, cur.fit, C)?;
+    let fit_given = p.get("fit").is_some_and(|v| !v.is_null()) || p.get("fitMinPercent").is_some_and(|v| !v.is_null());
     let size = |k: &str| p.get(k).and_then(Value::as_f64).filter(|x| x.is_finite()).map(|x| x.clamp(1.0, 100_000.0));
-    let (w, mut h) = (size("width"), size("height"));
-    let mut changed = w.is_some() || h.is_some() || fit.is_some();
-    if let (Some(o), Some(src)) = (v.as_object_mut(), p.as_object()) {
-        for (k, val) in src.iter().filter(|(k, _)| AREA_KEYS.contains(&k.as_str())) {
-            o.insert(k.clone(), val.clone());
-            changed = true;
-        }
-    }
-    if !changed {
+    let (w, h) = (size("width"), size("height"));
+    let given: Vec<(&String, &Value)> = p.as_object().into_iter().flatten().filter(|(k, _)| AREA_KEYS.contains(&k.as_str())).collect();
+    if w.is_none() && h.is_none() && !fit_given && given.is_empty() {
         return options(s);
     }
-    let mut opts: vectorcraft_doc::AreaOptions = serde_json::from_value(v).map_err(|e| bad(C, e.to_string()))?;
-    opts.rows = opts.rows.clamp(1, vectorcraft_doc::AreaOptions::MAX_ROWS_COLUMNS);
-    opts.columns = opts.columns.clamp(1, vectorcraft_doc::AreaOptions::MAX_ROWS_COLUMNS);
-    let finite = |x: f64| if x.is_finite() { x } else { 0.0 };
-    opts.gutter = finite(opts.gutter).clamp(0.0, 10_000.0);
-    // A gutter given alone is the rows' and the columns'.
-    if p.get("gutter").is_some() && p.get("rowGutter").is_none() {
-        opts.row_gutter = None;
-    }
-    opts.row_gutter = opts.row_gutter.map(|g| finite(g).clamp(0.0, 10_000.0)).filter(|g| *g != opts.gutter);
-    opts.inset = finite(opts.inset).clamp(0.0, 10_000.0);
-    opts.first_baseline_min = finite(opts.first_baseline_min).clamp(0.0, 10_000.0);
-    opts.fit = fit.unwrap_or(cur.fit);
-    if opts.fit == AreaFit::AutoHeight {
+    // Each area type takes the options given over its own (#3759): setting one option on several
+    // frames leaves their other options as they were.
+    let merged = |own: &vectorcraft_doc::AreaOptions| -> Result<vectorcraft_doc::AreaOptions> {
+        let mut v = serde_json::to_value(own).map_err(|e| EngineError::Other(e.to_string()))?;
+        if let Some(o) = v.as_object_mut() {
+            for (k, val) in &given {
+                o.insert((*k).clone(), (*val).clone());
+            }
+        }
+        let mut opts: vectorcraft_doc::AreaOptions = serde_json::from_value(v).map_err(|e| bad(C, e.to_string()))?;
+        opts.rows = opts.rows.clamp(1, vectorcraft_doc::AreaOptions::MAX_ROWS_COLUMNS);
+        opts.columns = opts.columns.clamp(1, vectorcraft_doc::AreaOptions::MAX_ROWS_COLUMNS);
+        let finite = |x: f64| if x.is_finite() { x } else { 0.0 };
+        opts.gutter = finite(opts.gutter).clamp(0.0, 10_000.0);
+        // A gutter given alone is the rows' and the columns'.
+        if p.get("gutter").is_some() && p.get("rowGutter").is_none() {
+            opts.row_gutter = None;
+        }
+        opts.row_gutter = opts.row_gutter.map(|g| finite(g).clamp(0.0, 10_000.0)).filter(|g| *g != opts.gutter);
+        opts.inset = finite(opts.inset).clamp(0.0, 10_000.0);
+        opts.first_baseline_min = finite(opts.first_baseline_min).clamp(0.0, 10_000.0);
+        // Each frame's fit from its own (`fitMinPercent` alone changes only Shrink Text frames).
+        opts.fit = fit_param(p, own.fit, C)?.unwrap_or(own.fit);
+        Ok(opts)
+    };
+    let d = &s.doc()?.doc;
+    let planned: Vec<(NodeId, vectorcraft_doc::AreaOptions)> = ids
+        .iter()
+        .filter_map(|id| area_text(d.node(*id)).map(|t| (*id, &t.area)))
+        .map(|(id, own)| merged(own).map(|o| (id, o)))
+        .collect::<Result<_>>()?;
+    let apply = |t: &mut TextObject, opts: &vectorcraft_doc::AreaOptions| {
+        t.area = opts.clone();
         // Auto Size sets the height (Illustrator's dialog turns the Height field off).
-        h = None;
+        let h = if opts.fit == AreaFit::AutoHeight { None } else { h };
+        // A resized frame has laid its text out again; otherwise the new options do here.
+        if !size_area(t, w, h) {
+            refresh_bounds(t);
+        }
+    };
+    // Options that change nothing (the vertical alignment already set, `fitMinPercent` on frames
+    // without Shrink Text) add no undo step and keep redo, frame sizes and Auto Size included.
+    let unchanged = planned.iter().all(|(id, opts)| match d.node(*id).map(|n| &n.kind) {
+        Some(NodeKind::Text(t)) => {
+            let mut after = t.clone();
+            apply(&mut after, opts);
+            after == *t
+        }
+        _ => true,
+    });
+    if unchanged {
+        return options(s);
     }
     s.edit("Area Type Options", |d, _| {
-        for id in &ids {
+        for (id, opts) in &planned {
             if let Some(NodeKind::Text(t)) = d.node_mut(*id).map(|n| &mut n.kind) {
-                t.area = opts.clone();
-                // A resized frame has laid its text out again; otherwise the new options do here.
-                if !size_area(t, w, h) {
-                    refresh_bounds(t);
-                }
+                apply(t, opts);
             }
         }
         Ok(())

crates/engine/src/cmd/typecmd/area_tests.rs

@@ -1762,3 +1762,46 @@ fn area_options_set_a_gutter_between_rows_of_its_own() {
     s.execute("text.areaOptions", &json!({"rowGutter": null})).unwrap();
     assert_eq!(area(&s).row_gutter, None);
 }
+
+/// #3759: an option given for several area types goes onto each one's own options: the others
+/// stay as each had them, not copied from the first.
+#[test]
+fn area_options_for_several_frames_change_only_what_is_given() {
+    let mut s = Session::new();
+    s.execute("file.new", &json!({"width": 400, "height": 300})).unwrap();
+    let make = |s: &mut Session, x: f64| {
+        s.execute("text.create", &json!({"x": x, "y": 10, "text": "A", "area": {"width": 100, "height": 100}})).unwrap()["id"].as_u64().unwrap()
+    };
+    let (a, b) = (make(&mut s, 10.0), make(&mut s, 200.0));
+    s.execute("text.areaOptions", &json!({"ids": [a], "columns": 3, "gutter": 6})).unwrap();
+    s.execute("text.areaOptions", &json!({"ids": [b], "inset": 4})).unwrap();
+    s.execute("text.areaOptions", &json!({"ids": [a, b], "verticalAlign": "bottom"})).unwrap();
+    let area = |s: &Session, id: u64| match &s.doc().unwrap().doc.node(NodeId(id)).unwrap().kind {
+        NodeKind::Text(t) => t.area.clone(),
+        k => panic!("{k:?}"),
+    };
+    let (oa, ob) = (area(&s, a), area(&s, b));
+    assert_eq!((oa.vertical_align, ob.vertical_align), (vectorcraft_doc::VerticalAlign::Bottom, vectorcraft_doc::VerticalAlign::Bottom));
+    assert_eq!((oa.columns, oa.gutter, oa.inset), (3, 6.0, 0.0));
+    assert_eq!((ob.columns, ob.inset), (1, 4.0), "b keeps its own options");
+}
+
+/// `fitMinPercent` alone changes only the frames that shrink text to fit, each from its own
+/// setting (#3759).
+#[test]
+fn fit_min_percent_for_several_frames_keeps_each_frames_fit() {
+    let mut s = Session::new();
+    s.execute("file.new", &json!({"width": 400, "height": 300})).unwrap();
+    let make = |s: &mut Session, x: f64| {
+        s.execute("text.create", &json!({"x": x, "y": 10, "text": "A", "area": {"width": 100, "height": 100}})).unwrap()["id"].as_u64().unwrap()
+    };
+    let (plain, shrink) = (make(&mut s, 10.0), make(&mut s, 200.0));
+    s.execute("text.areaOptions", &json!({"ids": [shrink], "fit": "shrinkText"})).unwrap();
+    s.execute("text.areaOptions", &json!({"ids": [plain, shrink], "fitMinPercent": 30})).unwrap();
+    let fit = |s: &Session, id: u64| match &s.doc().unwrap().doc.node(NodeId(id)).unwrap().kind {
+        NodeKind::Text(t) => t.area.fit,
+        k => panic!("{k:?}"),
+    };
+    assert_eq!(fit(&s, plain), vectorcraft_doc::AreaFit::None, "the plain frame doesn't start shrinking");
+    assert_eq!(fit(&s, shrink), vectorcraft_doc::AreaFit::ShrinkText { min_percent: 30.0 });
+}

crates/engine/src/tests/areafit.rs

@@ -77,7 +77,7 @@ fn area_options_set_and_report_the_fit() {
     let n = undo_len(&s);
     let again = s.execute("text.areaOptions", &q).unwrap();
     assert_eq!(again, q);
-    assert_eq!(undo_len(&s), n + 1);
+    assert_eq!(undo_len(&s), n, "options that change nothing add no undo step");
     // Undo back to no fit.
     while text(&s, id).area.fit != AreaFit::None {
         s.execute("edit.undo", &json!({})).unwrap();

crates/engine/src/tests/typearea.rs

@@ -508,3 +508,25 @@ fn area_type_options_set_the_text_flow() {
     assert_eq!((text(&s, id).area.flow, second(&s)), (TextFlow::Columns, (0.0, 160.0)));
     assert!(cmd::find_command("text.areaOptions").unwrap().params.contains("flow?: columns|rows"));
 }
+
+/// #3759: options that change nothing (the vertical alignment already set, or
+/// `fitMinPercent` alone on a frame without Shrink Text) add no undo step and keep redo; a size
+/// change with them still applies.
+#[test]
+fn unchanged_area_type_options_add_no_undo_step() {
+    let mut s = session();
+    let id = area(&mut s, 40.0, 40.0, 200.0, 300.0, "Short story");
+    sel(&mut s, &[id]);
+    s.execute("text.areaOptions", &json!({"verticalAlign": "bottom"})).unwrap();
+    s.execute("edit.undo", &json!({})).unwrap();
+    s.execute("text.areaOptions", &json!({"verticalAlign": "center"})).unwrap();
+    let n = undo_len(&s);
+    let before = text(&s, id);
+    s.execute("text.areaOptions", &json!({"verticalAlign": "center"})).unwrap();
+    s.execute("text.areaOptions", &json!({"fitMinPercent": 40})).unwrap();
+    assert_eq!(undo_len(&s), n, "no step for options already set");
+    assert_eq!(text(&s, id), before);
+    // A real change still counts, and a new width with an unchanged option too.
+    s.execute("text.areaOptions", &json!({"verticalAlign": "center", "width": 180})).unwrap();
+    assert_eq!(undo_len(&s), n + 1);
+}

docs/mcp.md

@@ -5021,6 +5021,11 @@ command also accepts that object form. The query (and every reply) of `text.area
 `document.inspect` (and `document.node {summary: true}`) reports `overflow` for area type and type on a path (text past
 the end of the path), and `fit` and `fitScale` for area type.
 
+Given for several area types (`ids`, or a selection of several), `text.areaOptions` changes only the options given on
+each one: each frame keeps its other options (it used to take the first frame's), and `fitMinPercent` alone changes only
+the frames that already shrink text to fit. Options that change nothing (the vertical alignment already set, say) add
+no undo step and keep redo.
+
 Text that doesn't fit is drawn up to the last line that does, so a one-line headline in a frame lower than its line
 draws nothing. The commands that make type or change what it holds say so in their own reply: `text.create`,
 `text.createInPath`, `text.setText` and `text.setStyle` add `overflow: [id…]` (the area type and type on a path among