From 4b8cd307570cf81bdcb81da0a73ad503230b87f7 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Fri, 13 Nov 2026 09:10:23 -0800 Subject: [PATCH] Artboards: a batch's Fit to Artwork Bounds step records the first artboard it defaults to A Fit to Artwork Bounds step of `command.batch` that gets no artboards from the Artboard tool or the batch fits the first artboard, as the command does by default, and its journal entry now records that `index`. Before, the entry kept the step without it, so a replay with the Artboard tool choosing two or more artboards fitted those instead. --- crates/engine/src/cmd/fileio/batch.rs | 27 +++++++++++-------- crates/engine/src/tests/journal.rs | 37 +++++++++++++++------------ 2 files changed, 37 insertions(+), 27 deletions(-) diff --git a/crates/engine/src/cmd/fileio/batch.rs b/crates/engine/src/cmd/fileio/batch.rs index 7b41894772..5863dd4ff7 100644 --- a/crates/engine/src/cmd/fileio/batch.rs +++ b/crates/engine/src/cmd/fileio/batch.rs @@ -83,7 +83,8 @@ pub(super) fn batch(s: &mut Session, p: &Value) -> Result { /// `index` nor `indices`, while the Artboard tool is chosen with two or more artboards selected when /// the step runs, gets those as its `indices`. Otherwise a step that resizes the active artboard /// ([`crate::FIT_ACTIVE_ARTBOARD`]) and names no `index` (nor `indices`, for Fit to Artwork Bounds) -/// gets that artboard as its `index`. A Fit to Artwork Bounds step's journal entry records the +/// gets that artboard as its `index`. A Fit to Artwork Bounds step that still names neither gets +/// `index` 0, the command's default. A Fit to Artwork Bounds step's journal entry records the /// `indices` or `index` it got. /// `grouped` gets each step's undo groups, as [`Session::batch_steps`] keeps them. fn run_steps( @@ -118,17 +119,13 @@ fn run_steps( && (params.get("artboards").is_some() || (params.get("artboard").is_none() && !crate::cmd::edit::artboard_tool_picks(s).is_empty())); // A Fit to Artwork Bounds step that names neither `index` nor `indices`, while the Artboard // tool is chosen with two or more artboards selected, fits those. - let fit_picks = if id == "artboard.fitToArt" && params.get("index").is_none() && params.get("indices").is_none() { - crate::cmd::edit::artboard_tool_picks(s) - } else { - Vec::new() - }; + let fit_unnamed = id == "artboard.fitToArt" && params.get("index").is_none() && params.get("indices").is_none(); + let fit_picks = if fit_unnamed { crate::cmd::edit::artboard_tool_picks(s) } else { Vec::new() }; if !fit_picks.is_empty() && let Some(o) = params.as_object_mut() { o.insert("indices".into(), json!(fit_picks)); } - let mut fit_index = false; if let Some(a) = artboard && let Some(key) = key && params.get(key).is_none() @@ -138,7 +135,15 @@ fn run_steps( && let Some(o) = params.as_object_mut() { o.insert(key.into(), json!(usize::try_from(a).map_or(last, |a| a.min(last)))); - fit_index = id == "artboard.fitToArt"; + } + // With neither, the step fits the command's default, the first artboard, named here so the + // journal records it. + if fit_unnamed + && params.get("index").is_none() + && params.get("indices").is_none() + && let Some(o) = params.as_object_mut() + { + o.insert("index".into(), json!(0)); } // The documents with an undo group open, held so that the step's first edit in one copies it. let start = s.active().map(|d| d.uid); @@ -151,9 +156,9 @@ fn run_steps( grouped.push(step_groups(s, start, &held)); // The step's journal entry records what it took from the preferences or the clock (a // save's date, a new document's), as a command of its own would, and the artboards a Fit to - // Artwork Bounds step took from the Artboard tool or the batch, so a replay does not fit the - // artboards the Artboard tool has selected then. - if (noted != params || !fit_picks.is_empty() || fit_index) + // Artwork Bounds step took from the Artboard tool, the batch or the command's default, so a + // replay does not fit the artboards the Artboard tool has selected then. + if (noted != params || fit_unnamed) && let Some(c) = c.as_object_mut() { c.insert("params".into(), noted); diff --git a/crates/engine/src/tests/journal.rs b/crates/engine/src/tests/journal.rs index 5b7c3b2322..a3f0e0a017 100644 --- a/crates/engine/src/tests/journal.rs +++ b/crates/engine/src/tests/journal.rs @@ -274,29 +274,34 @@ fn a_batch_fit_selection_replays_but_an_action_uses_the_current_selection() { } /// The journal records the `index` a batch's Fit to Artwork Bounds step took from the batch's -/// `artboard`, so a replay fits that artboard even when the Artboard tool then has several selected. +/// `artboard`, or the command's default (the first artboard) when the batch names none, so a replay +/// fits that artboard even when the Artboard tool then has several selected. #[test] fn a_batch_fit_step_records_the_batchs_artboard() { let fitted = three_fitted_artboards(); let boards = |s: &Session| s.doc().unwrap().doc.artboards.iter().map(|a| a.rect).collect::>(); - let mut s = Session::new(); - let was = three_artboards_with_art(&mut s); - let start = s.journal.len(); let steps = json!([{"command": "artboard.fitToArt", "params": {}}]); - s.execute("command.batch", &json!({"artboard": 1, "commands": steps})).unwrap(); - assert_eq!(boards(&s), [was[0], fitted[1], was[2]]); - let (id, p) = s.journal.last().unwrap().clone(); - assert_eq!((id.as_str(), &p["commands"][0]["params"]), ("command.batch", &json!({"index": 1}))); - let mut r = Session::new(); - for (n, (id, p)) in s.journal.iter().enumerate() { - if n == start { - r.select_tool("artboard", crate::tooling::ViewInfo::default()).unwrap(); - r.set_tool_option("selected", &json!([0, 2])); + for (batch, index, picks) in [(json!({"artboard": 1, "commands": steps}), 1, json!([0, 2])), (json!({"commands": steps}), 0, json!([1, 2]))] { + let mut s = Session::new(); + let was = three_artboards_with_art(&mut s); + let start = s.journal.len(); + s.execute("command.batch", &batch).unwrap(); + let mut want = was.clone(); + want[index] = fitted[index]; + assert_eq!(boards(&s), want, "{batch}"); + let (id, p) = s.journal.last().unwrap().clone(); + assert_eq!((id.as_str(), &p["commands"][0]["params"]), ("command.batch", &json!({ "index": index }))); + let mut r = Session::new(); + for (n, (id, p)) in s.journal.iter().enumerate() { + if n == start { + r.select_tool("artboard", crate::tooling::ViewInfo::default()).unwrap(); + r.set_tool_option("selected", &picks); + } + r.execute(id, p).unwrap(); } - r.execute(id, p).unwrap(); + assert_eq!(boards(&r), want, "{batch}: the replay fits artboard {index}, not the selected ones"); + assert_eq!(s.journal_for_action(start), [("command.batch".to_string(), json!({"commands": steps}))]); } - assert_eq!(boards(&r), boards(&s), "the replay fits artboard 2, not the selected ones"); - assert_eq!(s.journal_for_action(start), [("command.batch".to_string(), json!({"commands": steps}))]); } /// A batch step with `"params": null` gets the Artboard tool's selected artboards, or else the