Merge pull request #6547 from dotCooCoo/fix/appearance-active-row-aligned-remove

Appearance: aligned Remove Item and Duplicate Item keep the active row

crates/engine/src/cmd/appearance.rs

@@ -939,7 +939,10 @@ fn remove_item(s: &mut Session, p: &Value) -> Result<Value> {
         let painted = super::graph::capture_series_top_paints(d, &top_paint_changed(&before, d, &ids));
         Ok(count_changed(d, &before, &ids) + usize::from(kept || painted))
     })?;
-    s.remap_appearance_item(&ids, |a| (!idx.contains(&a)).then(|| a - idx.iter().filter(|i| **i < a).count()));
+    // The rows removed from the active row's object, which need not come first in `ids`.
+    let active = s.active_appearance_item.as_ref().and_then(|a| a.objects.first().copied());
+    let gone = active.and_then(|a| ids.iter().position(|id| *id == a)).and_then(|k| per.get(k)).unwrap_or(&idx);
+    s.remap_appearance_item(&ids, |a| (!gone.contains(&a)).then(|| a - gone.iter().filter(|i| **i < a).count()));
     ok()
 }
 
@@ -1023,10 +1026,12 @@ fn duplicate_item(s: &mut Session, p: &Value) -> Result<Value> {
     // Rows at or above each copy move up by one (by the block, for copies placed at `to`).
     let m = idx.len();
     let landed = active_at.or(first_at);
+    // The rows copied in the active row's object, which need not come first in `ids`.
+    let copied = active.and_then(|a| ids.iter().position(|id| *id == a)).and_then(|k| per.get(k)).unwrap_or(&idx);
     s.remap_appearance_item(&ids, |a| {
         Some(match to {
             Some(_) => a + if landed.is_some_and(|at| at <= a) { m } else { 0 },
-            None => a + idx.iter().filter(|i| **i < a).count(),
+            None => a + copied.iter().filter(|i| **i < a).count(),
         })
     });
     ok()

crates/engine/src/tests/appearance.rs

@@ -647,6 +647,28 @@ fn edits_change_the_active_row_of_the_first_selected_object() {
     assert!(!e.contains("internal error"), "{e}");
 }
 
+/// After an aligned Remove Item or Duplicate Item, the active row follows the rows that changed in
+/// the first selected object, also when that object paints above the others.
+#[test]
+fn aligned_remove_and_duplicate_keep_the_active_row_of_the_first_selected_object() {
+    let mut s = session();
+    let (_, b) = reversed_pair(&mut s);
+    run(&mut s, "appearance.setActiveItem", json!({"index": 0})); // b's red fill
+    // a's row 0 is b's white fill, row 1, so b's red fill stays at row 0.
+    run(&mut s, "appearance.removeItem", json!({"indices": [0], "aligned": true}));
+    assert_eq!(node(&s, b).appearance.items.len(), 2);
+    assert_eq!(s.appearance_item(), Some(0));
+    assert_eq!(fill_paint(&node(&s, b), 0).color().unwrap().to_hex(), "#ff0000");
+
+    let mut s = session();
+    let (_, b) = reversed_pair(&mut s);
+    run(&mut s, "appearance.setActiveItem", json!({"index": 1})); // b's white fill, a's row 0
+    // The copy goes above b's white fill, at 2; the active row stays on the original.
+    run(&mut s, "appearance.duplicateItem", json!({"indices": [0], "aligned": true}));
+    assert_eq!(node(&s, b).appearance.items.len(), 4);
+    assert_eq!(s.appearance_item(), Some(1));
+}
+
 /// `effect.apply`'s reply names the new effect by its index in the object whose row `item` is,
 /// also when the objects painted after it already have effects there.
 #[test]