From d57d0b8e675532b54c70bf651977986c10b9ee84 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 31 Oct 2026 23:00:53 -0700 Subject: [PATCH] Appearance: effect.move reports the effect's index in the object whose row it names With the Appearance panel's active row, the row belongs to the first selected object, which need not paint first. effect.move reported where the effect landed in the first target in paint order; it now reports where it landed in the first selected object for the active row, and in the first target for an explicit fromItem, as effect.apply does. --- crates/engine/src/cmd/effectcmd.rs | 11 +++++++---- crates/engine/src/tests/appearance.rs | 22 ++++++++++++++++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cmd/effectcmd.rs b/crates/engine/src/cmd/effectcmd.rs index feee6913f..a62a694cd 100644 --- a/crates/engine/src/cmd/effectcmd.rs +++ b/crates/engine/src/cmd/effectcmd.rs @@ -304,8 +304,8 @@ fn move_effect(s: &mut Session, p: &Value) -> Result { if aligned(p) && super::appearance::appearances_differ(&s.doc()?.doc, &roots) { return Err(bad(C, "effects of objects whose appearances differ can't be moved together")); } - // Where the effect landed in the first object. - let mut landed = None; + // Where the effect landed in each object it moved in, in the order of `ids`. + let mut landed = vec![]; let ids = s.edit(if copy { "Duplicate Effect" } else { "Move Effect" }, |d, _| { let mut done = vec![]; for id in &roots { @@ -317,7 +317,7 @@ fn move_effect(s: &mut Session, p: &Value) -> Result { let at = to.min(fx.len()); fx.insert(at, e); check_revolve_count(n, C)?; - landed.get_or_insert((at, di)); + landed.push((at, di)); done.push(*id); } if done.is_empty() { @@ -325,7 +325,10 @@ fn move_effect(s: &mut Session, p: &Value) -> Result { } Ok(done) })?; - let (index, item) = landed.unzip(); + // The reply names the object whose row the active item is (the first selected object, which + // need not paint first), as `effect.apply` does; the first target otherwise. + let anchor = if matches!(src, ItemTarget::Item { explicit: false, .. }) { src } else { dst }; + let (index, item) = landed.get(anchor.anchor(s, &ids)).copied().unzip(); Ok(json!({ "ids": ids_json(&ids), "index": index, "item": item.flatten() })) } diff --git a/crates/engine/src/tests/appearance.rs b/crates/engine/src/tests/appearance.rs index e1728c1ad..372c653d7 100644 --- a/crates/engine/src/tests/appearance.rs +++ b/crates/engine/src/tests/appearance.rs @@ -534,6 +534,28 @@ fn apply_reports_the_index_in_the_object_whose_row_it_names() { assert_eq!((r["item"].clone(), r["index"].clone()), (json!(0), json!(1)), "a's row 0 holds two effects"); } +/// `effect.move`'s reply names where the effect landed in the object whose row the active item is +/// (the first selected object), also when that object paints above the others. +#[test] +fn move_reports_the_index_in_the_object_whose_row_it_names() { + let mut s = session(); + let (a, b) = reversed_pair(&mut s); + run(&mut s, "select.set", json!({"ids": [a.0]})); + run(&mut s, "effect.apply", json!({"effect": "distort.twist", "item": 0})); + run(&mut s, "select.set", json!({"ids": [b.0]})); + run(&mut s, "effect.apply", json!({"effect": "distort.twist", "item": 0})); + run(&mut s, "effect.apply", json!({"effect": "distort.twist", "item": 0})); + run(&mut s, "select.set", json!({"ids": [b.0, a.0]})); + run(&mut s, "appearance.setActiveItem", json!({"index": 0})); + // Row 0's first effect goes to the end of the row: index 1 in b, 0 in a, which paints first. + let r = run(&mut s, "effect.move", json!({"from": 0, "to": 9})); + assert_eq!((node(&s, a).appearance.items[0].effects().len(), node(&s, b).appearance.items[0].effects().len()), (1, 2)); + assert_eq!((r["item"].clone(), r["index"].clone()), (json!(0), json!(1)), "b's row 0 holds two effects"); + // An explicit item is a row of the first target, a. + let r = run(&mut s, "effect.move", json!({"from": 0, "to": 9, "fromItem": 0})); + assert_eq!((r["item"].clone(), r["index"].clone()), (json!(0), json!(0)), "a's row 0 holds one effect"); +} + /// How many steps there are to undo and to redo, and whether the document has unsaved changes. fn history(s: &Session) -> (usize, usize, bool) { let st = s.doc().unwrap();