Merge pull request #5522 from dotCooCoo/fix/same-font-isolated-group-on-layer

Select: the font Same commands look through a selected group that holds the isolated group

crates/engine/src/cmd/docmenu.rs

@@ -328,9 +328,9 @@ fn same_text(s: &mut Session, cmd: &str, eq: fn(&CharStyle, &CharStyle) -> bool)
 
 /// Call `f` with each type object among the selected objects and inside them, in selection order,
 /// keeping only those [`walk_selectable`] visits: none that is hidden, locked, on a hidden, locked
-/// or template layer, or outside the isolated group. A selected layer or sublayer gives the type
-/// on it that passes the same test. A type object inside two selected objects is given twice. The
-/// walk stops when `f` returns false.
+/// or template layer, or outside the isolated group. A selected layer or sublayer, or a selected
+/// group that holds the isolated group, gives the type in it that passes the same test. A type
+/// object inside two selected objects is given twice. The walk stops when `f` returns false.
 fn each_selected_text(s: &Session, mut f: impl FnMut(NodeId) -> bool) {
     fn inside(n: &Node, f: &mut impl FnMut(NodeId) -> bool) -> bool {
         if !n.visible || n.locked {
@@ -341,18 +341,23 @@ fn each_selected_text(s: &Session, mut f: impl FnMut(NodeId) -> bool) {
         }
         n.children().into_iter().flatten().all(|c| inside(c, f))
     }
-    fn from(n: &Node, selectable: &impl Fn(NodeId) -> bool, f: &mut impl FnMut(NodeId) -> bool) -> bool {
-        if n.is_layer() {
-            return n.children().into_iter().flatten().all(|c| from(c, selectable, f));
+    fn from(n: &Node, selectable: &impl Fn(NodeId) -> bool, holds_isolated: &[NodeId], f: &mut impl FnMut(NodeId) -> bool) -> bool {
+        if selectable(n.id) {
+            return inside(n, f);
         }
-        !selectable(n.id) || inside(n, f)
+        if n.is_layer() || holds_isolated.contains(&n.id) {
+            return n.children().into_iter().flatten().all(|c| from(c, selectable, holds_isolated, f));
+        }
+        true
     }
     let Some(st) = s.active() else { return };
     let locks = st.doc.locked_artboards();
     let selectable = |id| is_selectable(&st.doc, st.isolation, &locks, id);
+    // The isolated group and the groups and layers that hold it.
+    let holds_isolated = st.isolation.and_then(|i| st.doc.ancestry(i)).unwrap_or_default();
     for id in &st.selection.objects {
         if let Some(n) = st.doc.node(*id)
-            && !from(n, &selectable, &mut f)
+            && !from(n, &selectable, &holds_isolated, &mut f)
         {
             return;
         }

crates/engine/src/tests/menucmds.rs

@@ -2025,6 +2025,30 @@ fn select_same_font_takes_its_reference_from_the_type_on_a_selected_layer() {
     }
 }
 
+/// A selected layer that holds the isolated group gives the type inside that group as the
+/// reference of the font Same commands, and not the type on the layer outside it.
+#[test]
+fn select_same_font_takes_its_reference_from_the_isolated_group_on_a_selected_layer() {
+    let mut s = session();
+    let typed =
+        |s: &mut Session, y: f64, font: &str| id_of(&s.execute("text.create", &json!({"x": 0, "y": y, "text": "type", "font": font})).unwrap());
+    let l2 = id_of(&s.execute("layer.new", &json!({"top": true})).unwrap());
+    let outside = typed(&mut s, 20.0, "Arial");
+    let courier = typed(&mut s, 60.0, "Courier");
+    let other_courier = typed(&mut s, 100.0, "Courier");
+    sel(&mut s, &[courier, other_courier]);
+    let g = id_of(&s.execute("object.group", &json!({})).unwrap());
+    s.execute("object.isolate", &json!({"id": g.0})).unwrap();
+    sel(&mut s, &[l2]);
+    assert_eq!(s.execute("select.same.fontFamily", &json!({})).unwrap()["count"], 2);
+    let mut got = selected(&s);
+    got.sort();
+    let mut want = vec![courier, other_courier];
+    want.sort();
+    assert_eq!(got, want);
+    assert!(!got.contains(&outside));
+}
+
 /// Select › Same › Text Fill & Stroke Color selects the type with characters whose fill and stroke
 /// both match the first character of the selected type: the same fill with another stroke, or the
 /// same stroke with another fill, is left out, and in type with several runs one run has to match