diff --git a/crates/engine/src/cmd/appearance.rs b/crates/engine/src/cmd/appearance.rs index 45b23a0cb..024358adf 100644 --- a/crates/engine/src/cmd/appearance.rs +++ b/crates/engine/src/cmd/appearance.rs @@ -939,7 +939,10 @@ fn remove_item(s: &mut Session, p: &Value) -> Result { 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 { // 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() diff --git a/crates/engine/src/tests/appearance.rs b/crates/engine/src/tests/appearance.rs index 8d7023727..8d12c8371 100644 --- a/crates/engine/src/tests/appearance.rs +++ b/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]