From 988bfeb6ec7175648d7e87861da7017fa8167363 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Tue, 10 Nov 2026 22:50:48 -0800 Subject: [PATCH] Symbols: Break Link keeps new art on a locked artboard selected Break Link to Symbol on an instance just placed on a locked artboard selected nothing: the art that replaced the instance has a new id, so the locked-artboard check did not see it as new art. The art of an instance that was new art is now checked without the locked artboards, and the session keeps it selected as new art. The art of an older instance on a locked artboard, named by id, is still left out of the selection. --- crates/engine/src/cmd/brushsym/symbols.rs | 25 +++++++++++++++-------- crates/engine/src/cmd/select.rs | 14 ++++++++++++- crates/engine/src/tests/brushsym.rs | 25 +++++++++++++++++++++++ 3 files changed, 55 insertions(+), 9 deletions(-) diff --git a/crates/engine/src/cmd/brushsym/symbols.rs b/crates/engine/src/cmd/brushsym/symbols.rs index 395c07052..4c5cd89ed 100644 --- a/crates/engine/src/cmd/brushsym/symbols.rs +++ b/crates/engine/src/cmd/brushsym/symbols.rs @@ -391,9 +391,10 @@ fn expanded_bound(d: &Document, inst: &Node, own: bool) -> u64 { } /// Replace instances `ids` with their art ([`expanded_instance`], `own` as there). A hidden or -/// locked instance's art stays hidden or locked. Returns the art's ids. Fails before any edit when -/// the instances together would make more than [`MAX_EXPAND_NODES`] objects. -fn break_links(d: &mut Document, ids: &[NodeId], own: bool) -> Result> { +/// locked instance's art stays hidden or locked. Returns each replaced instance's id with its art's +/// id. Fails before any edit when the instances together would make more than +/// [`MAX_EXPAND_NODES`] objects. +fn break_links(d: &mut Document, ids: &[NodeId], own: bool) -> Result> { let total = ids.iter().filter_map(|id| d.node(*id)).fold(0u64, |sum, n| sum.saturating_add(expanded_bound(d, n, own))); if total > MAX_EXPAND_NODES as u64 { return Err(EngineError::Other("Too many pieces to make at once".into())); @@ -408,7 +409,7 @@ fn break_links(d: &mut Document, ids: &[NodeId], own: bool) -> Result Result { } let iso = s.doc()?.isolation; let out = s.edit("Break Link to Symbol", |d, sel| { - let out = break_links(d, &ids, true)?; + let fresh = d.new_art.get(); + let made = break_links(d, &ids, true)?; // Art that is hidden or locked, or in a hidden or locked group or layer, is left out of the - // selection. + // selection, as is art on a locked artboard unless the instance it replaces was new art. let locks = d.locked_artboards(); - sel.set(out.iter().copied().filter(|id| crate::cmd::select::is_selectable(d, iso, &locks, *id))); - Ok(out) + sel.set(made.iter().filter_map(|(inst, art)| { + let keep = if fresh.contains(inst) { + crate::cmd::select::is_selectable_on_any_artboard(d, iso, *art) + } else { + crate::cmd::select::is_selectable(d, iso, &locks, *art) + }; + keep.then_some(*art) + })); + Ok(made.into_iter().map(|(_, art)| art).collect::>()) })?; Ok(json!({ "ids": ids_json(&out) })) } diff --git a/crates/engine/src/cmd/select.rs b/crates/engine/src/cmd/select.rs index 0f16246df..4ef1e6bc6 100644 --- a/crates/engine/src/cmd/select.rs +++ b/crates/engine/src/cmd/select.rs @@ -252,6 +252,18 @@ pub(crate) fn selectable_ids(d: &Document, iso: Option) -> BTreeSet, locks: &LockedArtboards, id: NodeId) -> bool { + is_selectable_within(d, iso, Some(locks), id) +} + +/// [`is_selectable`] without the locked artboards, for an edit's new selection: after the edit, +/// the session takes the art on a locked artboard out of the selection, except the new art the +/// edit made. +pub(crate) fn is_selectable_on_any_artboard(d: &Document, iso: Option, id: NodeId) -> bool { + is_selectable_within(d, iso, None, id) +} + +/// [`is_selectable`], leaving out the locked artboards when `locks` is `None`. +fn is_selectable_within(d: &Document, iso: Option, locks: Option<&LockedArtboards>, id: NodeId) -> bool { let Some(a) = d.ancestry(id) else { return false }; if !a.iter().all(|i| d.node(*i).is_some_and(|n| n.visible && !n.locked && !n.is_template())) { return false; @@ -262,7 +274,7 @@ pub(crate) fn is_selectable(d: &Document, iso: Option, locks: &LockedArt None => { // The object Select All takes: the outermost one that is not a layer. let top = a.iter().filter_map(|i| d.node(*i)).find(|n| !n.is_layer()); - top.is_some_and(|t| locks.is_empty() || !locks.holds(t)) + top.is_some_and(|t| locks.is_none_or(|l| l.is_empty() || !l.holds(t))) } } } diff --git a/crates/engine/src/tests/brushsym.rs b/crates/engine/src/tests/brushsym.rs index ccf277585..b86cf89ee 100644 --- a/crates/engine/src/tests/brushsym.rs +++ b/crates/engine/src/tests/brushsym.rs @@ -1386,6 +1386,31 @@ fn break_link_leaves_art_in_hidden_or_locked_groups_unselected() { assert!(!a.contains(&hidden) && !a.contains(&locked), "{a:?}"); } +/// Break Link to Symbol on an instance just placed on a locked artboard selects the art that +/// replaces it, which stays new art there. The art of an older instance there, named by id, is not +/// selected. +#[test] +fn break_link_keeps_new_art_on_a_locked_artboard_selected() { + let mut s = session(); + let (old, name, _) = make_symbol(&mut s); + s.execute("artboard.new", &json!({"x": 1000, "y": 0, "width": 400, "height": 400})).unwrap(); + s.execute("artboard.setProps", &json!({"index": 1, "locked": true})).unwrap(); + select(&mut s, &[old]); + s.execute("object.move", &json!({"dx": 1000, "dy": 0})).unwrap(); + assert!(s.doc().unwrap().selection.objects.is_empty(), "moved onto the locked artboard"); + let inst = NodeId(s.execute("symbol.place", &json!({"name": name, "x": 1100, "y": 100})).unwrap()["id"].as_u64().unwrap()); + assert_eq!(s.doc().unwrap().selection.objects.to_vec(), [inst], "placed art is selected"); + let r = s.execute("symbol.breakLink", &json!({})).unwrap(); + let art: Vec = r["ids"].as_array().unwrap().iter().map(|v| NodeId(v.as_u64().unwrap())).collect(); + assert_eq!(art.len(), 1, "{r}"); + assert_eq!(s.doc().unwrap().selection.objects.to_vec(), art, "the art that replaces it"); + s.execute("object.move", &json!({"dx": 10, "dy": 0})).unwrap(); + assert_eq!(s.doc().unwrap().selection.objects.to_vec(), art, "the art stays editable"); + let r = s.execute("symbol.breakLink", &json!({"ids": [old.0]})).unwrap(); + assert_eq!(r["ids"].as_array().unwrap().len(), 1, "{r}"); + assert!(s.doc().unwrap().selection.objects.is_empty(), "the older instance's art"); +} + /// Delete Symbol, expanding the instances, keeps a hidden instance's art hidden and a locked /// instance's art locked. #[test]