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]