diff --git a/crates/engine/src/cmd/appearance.rs b/crates/engine/src/cmd/appearance.rs index f6aecc32d..de4885564 100644 --- a/crates/engine/src/cmd/appearance.rs +++ b/crates/engine/src/cmd/appearance.rs @@ -44,7 +44,7 @@ pub fn specs() -> Vec { "Add New Fill", ["Window", "Appearance"], Some("Cmd+/"), - "{ids?|id?, target?: \"object\"|\"contents\"} add a fill on top of each selected object's own stack, on an image right below its Contents row (a copy of its top fill; type without one: its characters' fill; else the default fill)", + "{ids?|id?, target?: \"object\"|\"contents\"} add a fill on top of each selected object's own stack, on an image right below its Contents row (a copy of its top fill; type without one: its characters' fill; else the default fill). With a fill row active (appearance.setActiveItem), no `ids` and no `target: \"contents\"`, the new fill is a copy of that row, right above it, and becomes the active row", has_doc, |s, p| add_item(s, p, true) ), @@ -470,12 +470,45 @@ fn set_active_item(s: &mut Session, p: &Value) -> Result { fn add_item(s: &mut Session, p: &Value, fill: bool) -> Result { let ids = require_targets(s, p, if fill { "appearance.addFill" } else { "appearance.addStroke" })?; let defaults = s.paint.clone(); + // The panel's active Fill row, when the new fill goes on the selection: the new fill is a copy + // of that row, right above it in each target that has it (others get a copy of their top fill). // Where the new item went in on the active item's object, when it is an image. let active = s.active_appearance_item.as_ref().and_then(|a| a.objects.first().copied()); + // The active row is a row of the active object's stack, which need not come first in `ids` + // (they are in paint order): match the others against it. + let anchor = active.and_then(|a| ids.iter().position(|id| *id == a)); + let selected = match (s.appearance_item(), anchor) { + (Some(index), Some(_)) if fill && p.get("ids").is_none() && p.get("id").is_none() && !targets_contents(p)? => { + ItemTarget::Item { index, explicit: false, aligned: true }.of_kind(s, true) + } + _ => ItemTarget::Top, + }; + let per: Vec> = match (selected, anchor) { + (ItemTarget::Item { index, .. }, Some(k)) => { + let mut order = ids.clone(); + order.swap(0, k); + let mut per: Vec> = + items_per_target(&s.doc()?.doc, &order, &[index], true).into_iter().map(|v| v.first().copied()).collect(); + per.swap(0, k); + per + } + _ => vec![None; ids.len()], + }; let mut inserted = None; + // Where the new fill went in on the active item's object, when it went above the active row. + let mut above = None; s.edit(if fill { "Add New Fill" } else { "Add New Stroke" }, |d, _| { - for id in &ids { + for (id, row) in ids.iter().zip(&per) { if let Some(n) = d.node_mut(*id) { + if let Some(i) = n.appearance.item_of_kind(*row, true) + && let Some(paint) = n.appearance.paint_at(Some(i), true).cloned() + { + n.appearance.insert_item(i + 1, AppearanceItem::Fill(FillLayer::new(paint))); + if active == Some(*id) { + above = Some(i + 1); + } + continue; + } // A copy of the top fill (stroke); without one, type's characters' and otherwise the // default paint, so a new fill on a group or type shows. let paint = match (n.appearance.paint_at(None, fill), &n.kind) { @@ -509,8 +542,9 @@ fn add_item(s: &mut Session, p: &Value, fill: bool) -> Result { } Ok(()) })?; - // The rows above the new item move up by one. - s.remap_appearance_item(&ids, |a| Some(a + usize::from(inserted.is_some_and(|k| k <= a)))); + // A fill added above the active row becomes the active row; otherwise the rows above the new + // item move up by one. + s.remap_appearance_item(&ids, |a| above.or(Some(a + usize::from(inserted.is_some_and(|k| k <= a))))); ok() } diff --git a/crates/engine/src/tests/appearance.rs b/crates/engine/src/tests/appearance.rs index d6336b86d..7d8cb2e0f 100644 --- a/crates/engine/src/tests/appearance.rs +++ b/crates/engine/src/tests/appearance.rs @@ -288,6 +288,74 @@ fn add_fill_and_stroke_copy_the_top_ones() { assert_eq!(node(&s, id).appearance.items.len(), 3); } +/// With a fill row active, Add New Fill puts a copy of that row right above it and makes the copy +/// the active row; the rows above it stay where they are. +#[test] +fn add_fill_copies_the_active_fill_row_right_above_it() { + let mut s = session(); + let id = rect(&mut s, 100.0); + run(&mut s, "paint.setFill", json!({"color": "#ff0000"})); + run(&mut s, "appearance.addFill", json!({})); + run(&mut s, "appearance.setItem", json!({"index": 2, "color": "#0000ff"})); // [red, Stroke, blue] + run(&mut s, "appearance.setActiveItem", json!({"index": 0})); + let steps = s.doc().unwrap().history.undo.len(); + run(&mut s, "appearance.addFill", json!({})); + let n = node(&s, id); + assert_eq!(n.appearance.items.iter().map(AppearanceItem::kind_name).collect::>(), ["fill", "fill", "stroke", "fill"]); + let hex = |i: usize| fill_paint(&n, i).color().unwrap().to_hex(); + assert_eq!((hex(0), hex(1), hex(3)), ("#ff0000".to_string(), "#ff0000".to_string(), "#0000ff".to_string())); + assert_eq!(s.appearance_item(), Some(1), "the new fill is the active row"); + // The next edit goes to the new fill. + run(&mut s, "paint.setFill", json!({"color": "#00ff00"})); + let n = node(&s, id); + assert_eq!((fill_paint(&n, 0).color().unwrap().to_hex(), fill_paint(&n, 1).color().unwrap().to_hex()), ("#ff0000".into(), "#00ff00".into())); + assert_eq!(s.doc().unwrap().history.undo.len(), steps + 2); + run(&mut s, "edit.undo", json!({})); + run(&mut s, "edit.undo", json!({})); + assert_eq!(node(&s, id).appearance.items.len(), 3); + // An active stroke row, or ids, leave the new fill on top. + run(&mut s, "appearance.setActiveItem", json!({"index": 1})); + run(&mut s, "appearance.addFill", json!({})); + assert_eq!(node(&s, id).appearance.items.iter().map(AppearanceItem::kind_name).collect::>(), ["fill", "stroke", "fill", "fill"]); + run(&mut s, "appearance.setActiveItem", json!({"index": 0})); + run(&mut s, "appearance.addFill", json!({"ids": [id.0]})); + assert_eq!(fill_paint(&node(&s, id), 4).color().unwrap().to_hex(), "#0000ff"); + assert_eq!(s.appearance_item(), Some(0)); +} + +/// In a mixed selection the active fill row goes above itself in the first object; an object that +/// doesn't share that row gets a copy of its top fill on top, as without an active row. +#[test] +fn add_fill_above_an_unshared_active_row_tops_the_other_objects() { + let mut s = session(); + let (a, b) = mixed_circles(&mut s, true); // a [yellow, Stroke], b [red, green, Stroke] + run(&mut s, "appearance.setActiveItem", json!({"index": 0})); + run(&mut s, "appearance.addFill", json!({})); + let (na, nb) = (node(&s, a), node(&s, b)); + assert_eq!(na.appearance.items.iter().map(AppearanceItem::kind_name).collect::>(), ["fill", "fill", "stroke"]); + assert_eq!(fill_paint(&na, 1).color().unwrap().to_hex(), "#ffff00"); + assert_eq!(nb.appearance.items.iter().map(AppearanceItem::kind_name).collect::>(), ["fill", "fill", "stroke", "fill"]); + assert_eq!(fill_paint(&nb, 3).color().unwrap().to_hex(), "#00ff00"); + assert_eq!(s.appearance_item(), Some(1)); +} + +/// The active row is a row of the first selected object, also when that object paints above the +/// others. +#[test] +fn add_fill_goes_above_the_active_row_of_the_first_selected_object() { + let mut s = session(); + let (a, b) = mixed_circles(&mut s, true); // a [yellow, Stroke], b [red, green, Stroke] + run(&mut s, "select.set", json!({"ids": [b.0, a.0]})); + run(&mut s, "appearance.setActiveItem", json!({"index": 1})); // b's green fill + run(&mut s, "appearance.addFill", json!({})); + let (na, nb) = (node(&s, a), node(&s, b)); + assert_eq!(nb.appearance.items.iter().map(AppearanceItem::kind_name).collect::>(), ["fill", "fill", "fill", "stroke"]); + assert_eq!(fill_paint(&nb, 2).color().unwrap().to_hex(), "#00ff00"); + assert_eq!(na.appearance.items.iter().map(AppearanceItem::kind_name).collect::>(), ["fill", "stroke", "fill"]); + assert_eq!(fill_paint(&na, 2).color().unwrap().to_hex(), "#ffff00"); + assert_eq!(s.appearance_item(), Some(2)); +} + #[test] fn set_item_and_remove_item() { let mut s = session(); diff --git a/crates/ui-egui/src/panels/appearance.rs b/crates/ui-egui/src/panels/appearance.rs index 19a578137..09df1544a 100644 --- a/crates/ui-egui/src/panels/appearance.rs +++ b/crates/ui-egui/src/panels/appearance.rs @@ -88,15 +88,16 @@ pub(crate) fn select_row(app: &mut VectorcraftApp, ctx: &egui::Context, sel: Sel } let owner = app.session.active().and_then(|d| d.selection.subjects().first().copied()); set_pstate(ctx, "ap-sel", (owner, sel)); - set_pstate(ctx, "ap-multi", (owner, sel.item().into_iter().collect::>())); + set_pstate(ctx, "ap-multi", (owner, sel.item().into_iter().collect::>(), sel.item())); } /// The selected fill/stroke rows of `n` (paint order, ascending): the active row plus those -/// Cmd/Shift-clicked with it. +/// Cmd/Shift-clicked with it. When the active row has changed since (Add New Fill makes the new +/// fill active), only the active row. fn selected_items(ctx: &egui::Context, n: Option<&Node>, sel: Sel) -> Vec { let (Some(n), Sel::Item(active)) = (n, sel) else { return vec![] }; - let (owner, mut set): (Option, Vec) = pstate(ctx, "ap-multi"); - if owner != Some(n.id) || !set.contains(&active) { + let (owner, mut set, at): (Option, Vec, Option) = pstate(ctx, "ap-multi"); + if owner != Some(n.id) || at != Some(active) || !set.contains(&active) { return vec![active]; } set.retain(|i| *i < n.appearance.items.len()); @@ -130,7 +131,7 @@ fn click_item(app: &mut VectorcraftApp, ctx: &egui::Context, n: &Node, sel: Sel, } }; select_row(app, ctx, Sel::Item(active)); - set_pstate(ctx, "ap-multi", (Some(n.id), set)); + set_pstate(ctx, "ap-multi", (Some(n.id), set, Some(active))); } /// An effect as the fx menu lists it: (id, label without the ellipsis, Effect-menu path). diff --git a/crates/ui-egui/src/panels/appearance/tests.rs b/crates/ui-egui/src/panels/appearance/tests.rs index 16bc55039..dfd7ccb63 100644 --- a/crates/ui-egui/src/panels/appearance/tests.rs +++ b/crates/ui-egui/src/panels/appearance/tests.rs @@ -438,6 +438,28 @@ fn groups_show_a_contents_row_and_type_a_characters_row_in_their_slot() { assert_eq!(rows(frame(&mut app)), ["Fill:", "Characters"]); } +/// Add New Fill (button, menu or shortcut) with rows selected: the copy of the active row lands +/// right above it and is then the only selected row. +#[test] +fn add_new_fill_selects_the_new_fill_alone() { + let mut app = VectorcraftApp::new(vectorcraft_engine::Session::new(), Default::default()); + app.run("file.new", json!({"width": 100, "height": 100})).unwrap(); + let id = NodeId(app.run("shape.rectangle", json!({"x": 0, "y": 0, "width": 20, "height": 20})).unwrap()["id"].as_u64().unwrap()); + app.run("appearance.addFill", json!({})).unwrap(); // [Fill, Stroke, Fill] + let ctx = egui::Context::default(); + let n = app.session.doc().unwrap().doc.node(id).unwrap().clone(); + // Rows 0 and 1 (the stroke) Cmd-clicked together, row 0 active. + click_item(&mut app, &ctx, &n, Sel::None, 0, egui::Modifiers::NONE); + click_item(&mut app, &ctx, &n, Sel::Item(0), 1, egui::Modifiers::COMMAND); + assert_eq!(selected_items(&ctx, Some(&n), Sel::Item(0)), [0, 1]); + app.run("appearance.addFill", json!({})).unwrap(); + let n = app.session.doc().unwrap().doc.node(id).unwrap().clone(); + assert_eq!(n.appearance.items.iter().map(AppearanceItem::kind_name).collect::>(), ["fill", "fill", "stroke", "fill"]); + let sel = current_sel(&app, &ctx, Some(&n)); + assert_eq!(sel, Sel::Item(1)); + assert_eq!(selected_items(&ctx, Some(&n), sel), [1]); +} + #[test] fn images_show_a_contents_row_with_new_fills_and_strokes_below_it() { let mut app = VectorcraftApp::new(vectorcraft_engine::Session::new(), Default::default());