From bc2af8df351ca027e9b430c71042c2fb96ea0e75 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 4 Nov 2026 04:00:32 -0800 Subject: [PATCH] Appearance: the active row is matched in each selected object on its own Reduce to Basic Appearance and Add New Fill matched the Appearance panel's active row only when every selected object had an equal row. With three objects whose third had a different lower fill, the second object lost the match too: Reduce kept its top fill instead of the active one, and Add New Fill put the copy on top instead of above the active row. items_per_target now matches each object against the first one separately, so an object keeps the match whenever it has an equal row. --- crates/engine/src/cmd/appearance.rs | 20 +++++++++++--- crates/engine/src/tests/appearance.rs | 39 +++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cmd/appearance.rs b/crates/engine/src/cmd/appearance.rs index 990a4d97d..5ba393f58 100644 --- a/crates/engine/src/cmd/appearance.rs +++ b/crates/engine/src/cmd/appearance.rs @@ -357,11 +357,23 @@ pub(crate) fn appearances_differ(d: &vectorcraft_doc::Document, ids: &[NodeId]) /// Indices `idx` of the first target's items (aligned with `aligned`) in each of `ids`: the first /// target's own, and in each other one the items equal to them there (see [`SharedStack`]), each -/// index on its own: one the targets don't share is left out of the others (never taken as the -/// same index there). Not aligned, or one target: the same indices in each. +/// index and each target on its own: an index a target doesn't share with the first is left out +/// of that target (never taken as the same index there), whatever the other targets have. Not +/// aligned, or one target: the same indices in each. pub(crate) fn items_per_target(d: &vectorcraft_doc::Document, ids: &[NodeId], idx: &[usize], aligned: bool) -> Vec> { - match shared_stack(d, ids).filter(|_| aligned) { - Some(sh) => (0..ids.len()).map(|k| if k == 0 { idx.to_vec() } else { idx.iter().filter_map(|i| sh.item_in(*i, k)).collect() }).collect(), + let aps: Option> = ids.iter().map(|id| d.node(*id).map(|n| &*n.appearance)).collect(); + match aps.filter(|aps| aligned && aps.len() >= 2) { + Some(aps) => aps + .iter() + .enumerate() + .map(|(k, ap)| match aps.first() { + Some(first) if k > 0 => { + let sh = SharedStack::of(&[*first, *ap]); + idx.iter().filter_map(|i| sh.item_in(*i, 1)).collect() + } + _ => idx.to_vec(), + }) + .collect(), None => vec![idx.to_vec(); ids.len()], } } diff --git a/crates/engine/src/tests/appearance.rs b/crates/engine/src/tests/appearance.rs index a49bae19a..284a809de 100644 --- a/crates/engine/src/tests/appearance.rs +++ b/crates/engine/src/tests/appearance.rs @@ -463,6 +463,45 @@ fn reduce_to_basic_keeps_the_active_row_of_the_first_selected_object() { assert_eq!(fill_paint(&node(&s, a), 0).color().unwrap().to_hex(), "#0000ff"); } +/// Three objects with equal stacks, the third's lower fill changed; the lower, green fill row of +/// the first is active, selected a, b, c. +fn three_with_one_changed(s: &mut Session) -> [NodeId; 3] { + let ids = [rect(s, 50.0), rect(s, 200.0), rect(s, 300.0)]; + run(s, "select.set", json!({"ids": ids.map(|id| id.0)})); + run(s, "appearance.addFill", json!({})); + run(s, "appearance.setItem", json!({"index": 0, "color": "#00ff00"})); + run(s, "appearance.setItem", json!({"index": 2, "color": "#ff0000"})); // [Fill green, Stroke, Fill red] + run(s, "appearance.setItem", json!({"index": 0, "color": "#0000ff", "ids": [ids[2].0]})); + run(s, "appearance.setActiveItem", json!({"index": 0})); + ids +} + +/// The active row is matched in each other object on its own: one object without an equal row +/// does not stop another that has one from keeping it. +#[test] +fn reduce_to_basic_keeps_the_active_row_in_each_object_that_has_it() { + let mut s = session(); + let [a, b, c] = three_with_one_changed(&mut s); + run(&mut s, "appearance.reduceToBasic", json!({})); + assert_eq!(fill_paint(&node(&s, a), 0).color().unwrap().to_hex(), "#00ff00"); + assert_eq!(fill_paint(&node(&s, b), 0).color().unwrap().to_hex(), "#00ff00"); + assert_eq!(fill_paint(&node(&s, c), 0).color().unwrap().to_hex(), "#ff0000"); +} + +/// Add New Fill puts its copy of the active row right above that row in each object that has an +/// equal row, even when another selected object has none. +#[test] +fn add_fill_copies_the_active_row_in_each_object_that_has_it() { + let mut s = session(); + let [a, b, _] = three_with_one_changed(&mut s); + run(&mut s, "appearance.addFill", json!({})); + for id in [a, b] { + let n = node(&s, id); + assert_eq!(fill_paint(&n, 1).color().unwrap().to_hex(), "#00ff00", "{:?}", n.appearance); + assert_eq!(fill_paint(&n, 3).color().unwrap().to_hex(), "#ff0000", "{:?}", n.appearance); + } +} + /// b `[Fill red, Fill white, Stroke]` above a `[Fill white, Stroke]`, selected b first: b's rows 1 /// and 2 equal a's rows 0 and 1. fn reversed_pair(s: &mut Session) -> (NodeId, NodeId) {