From 99b7fb0ab8471a0c70f99be059a820d3627ecd15 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Tue, 10 Nov 2026 23:58:23 -0800 Subject: [PATCH] Appearance: aligned Remove Item and Duplicate Item keep the active row After an aligned Remove Item or Duplicate Item, the active row was re-pointed by the indices of the first object in paint order. When the first selected object paints above the others, its own rows that changed are different ones, so the active row could be dropped or land on the wrong row, and a later paint edit changed that row instead. The active row now follows the rows removed or copied in the first selected object's own stack. Follows #5591. --- crates/engine/src/cmd/appearance.rs | 9 +++++++-- crates/engine/src/tests/appearance.rs | 22 ++++++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) 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]