From 60bf43b9e842ca1e8477c5e0b9b5129a281f13c1 Mon Sep 17 00:00:00 2001 From: moppeeer Date: Mon, 12 Oct 2026 00:06:18 +0200 Subject: [PATCH] fix(ui): A tint keeps each object's swatch, and no stroke stays none Changing the Tint in the Fill menu sent the first object's swatch with it, so with several objects selected every fill became that swatch; and a Stroke tint on a frame without a stroke gave it the default 1 pt black stroke. object.fill now takes a tint alone and keeps each object's swatch, and object.stroke with only a tint leaves a [None] stroke alone; the menu sends just the tint. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/object.rs | 63 +++++++++++++++++++++++-- crates/ui-egui/src/panels/properties.rs | 31 ++++++++++-- 2 files changed, 86 insertions(+), 8 deletions(-) diff --git a/crates/engine/src/cmd/object.rs b/crates/engine/src/cmd/object.rs index 17c44c27a..b9847432b 100644 --- a/crates/engine/src/cmd/object.rs +++ b/crates/engine/src/cmd/object.rs @@ -160,11 +160,31 @@ pub fn specs() -> Vec { ) } ), - cmd!("object.fill", "Fill", [], None, "{swatch, tint?: 0..1, ids?}", has_selection, |s, p| { - let sw = str_param(p, "swatch").ok_or_else(|| bad("object.fill", "missing swatch"))?.to_string(); - let tint = f64_or(p, "tint", 1.0) as f32; - set_flag(s, p, move |i| i.fill = Fill { swatch: sw.clone(), tint, ..i.fill.clone() }, false) - }), + cmd!( + "object.fill", + "Fill", + [], + None, + "{swatch?, tint?: 0..1, ids?} — a swatch (tint 100% unless given), or a tint alone that keeps each object's swatch", + has_selection, + |s, p| { + let sw = str_param(p, "swatch").map(str::to_string); + let tint = p.get("tint").and_then(Value::as_f64).filter(|t| t.is_finite()).map(|t| t.clamp(0.0, 1.0) as f32); + if sw.is_none() && tint.is_none() { + return Err(bad("object.fill", "missing swatch")); + } + set_flag( + s, + p, + move |i| { + let swatch = sw.clone().unwrap_or_else(|| i.fill.swatch.clone()); + let tint = tint.unwrap_or(if sw.is_some() { 1.0 } else { i.fill.tint }); + i.fill = Fill { swatch, tint, ..i.fill.clone() }; + }, + false, + ) + } + ), cmd!( "object.stroke", "Stroke", @@ -174,10 +194,15 @@ pub fn specs() -> Vec { has_selection, |s, p| { let p2 = p.clone(); + // A tint alone changes no stroke into one (it would become the default 1 pt black). + let tint_only = p.as_object().is_some_and(|o| o.keys().all(|k| k == "tint" || k == "ids")); set_flag( s, p, move |i| { + if tint_only && i.stroke.is_none() { + return; + } let mut st = if i.stroke.is_none() && p2.get("swatch").is_none() { Stroke::default() } else { i.stroke.clone() }; if let Some(v) = p2.get("swatch").and_then(Value::as_str) { st.swatch = v.into(); @@ -2714,3 +2739,31 @@ mod blend_mode_tests { assert_eq!(it.opacity, 1.0); } } + +#[cfg(test)] +mod tint_alone_tests { + use serde_json::json; + + #[test] + fn a_tint_alone_keeps_each_swatch_and_leaves_no_stroke_alone() { + let mut s = crate::Session::new(); + s.execute("file.new", &json!({})).unwrap(); + let a = s.execute("frame.create", &json!({"rect": [0, 0, 50, 50]})).unwrap()["id"].as_u64().unwrap(); + let b = s.execute("frame.create", &json!({"rect": [60, 0, 110, 50]})).unwrap()["id"].as_u64().unwrap(); + s.execute("object.fill", &json!({"swatch": "[Black]", "ids": [a]})).unwrap(); + s.execute("object.fill", &json!({"swatch": "C=100 M=0 Y=0 K=0", "ids": [b]})).unwrap(); + s.execute("object.stroke", &json!({"swatch": "[None]", "ids": [a, b]})).unwrap(); + // Fill tint on both: each keeps its swatch. + s.execute("object.fill", &json!({"tint": 0.25, "ids": [a, b]})).unwrap(); + let item = |s: &crate::Session, id: u64| s.doc().unwrap().doc.item(designcraft_doc::ItemId(id)).unwrap().clone(); + assert_eq!((item(&s, a).fill.swatch.as_str(), item(&s, a).fill.tint), ("[Black]", 0.25)); + assert_eq!((item(&s, b).fill.swatch.as_str(), item(&s, b).fill.tint), ("C=100 M=0 Y=0 K=0", 0.25)); + // A swatch alone is 100%. + s.execute("object.fill", &json!({"swatch": "[Paper]", "ids": [a]})).unwrap(); + assert_eq!(item(&s, a).fill.tint, 1.0); + // A stroke tint on no stroke: still no stroke. + s.execute("object.stroke", &json!({"tint": 0.5, "ids": [a]})).unwrap(); + assert!(item(&s, a).stroke.is_none(), "{:?}", item(&s, a).stroke); + assert!(s.execute("object.fill", &json!({"ids": [a]})).is_err(), "neither swatch nor tint"); + } +} diff --git a/crates/ui-egui/src/panels/properties.rs b/crates/ui-egui/src/panels/properties.rs index 55263ecb5..81d37599b 100644 --- a/crates/ui-egui/src/panels/properties.rs +++ b/crates/ui-egui/src/panels/properties.rs @@ -736,9 +736,6 @@ fn swatch_menu( if let Some(v) = p.remove(tint_key) { p.insert("tint".into(), v); } - if !stroke && !p.contains_key("swatch") { - p.insert("swatch".into(), json!(cur)); - } let _ = app.run(if stroke { "object.stroke" } else { "object.fill" }, Value::Object(p)); } }; @@ -2316,6 +2313,34 @@ mod tests { assert_eq!((fill.swatch.as_str(), fill.tint), (CYAN, 0.25), "the tint, on the same swatch"); } + #[test] + fn a_tint_on_several_objects_keeps_each_ones_swatch() { + let mut app = DesignApp::new(designcraft_engine::Session::new(), crate::Services::default()); + app.run("file.new", json!({})).unwrap(); + let a = app.run("frame.create", json!({"rect": [72, 72, 200, 200]})).unwrap()["id"].as_u64().unwrap(); + let b = app.run("frame.create", json!({"rect": [220, 72, 300, 200]})).unwrap()["id"].as_u64().unwrap(); + app.run("object.fill", json!({"swatch": CYAN, "ids": [a]})).unwrap(); + app.run("object.fill", json!({"swatch": "[Black]", "ids": [b]})).unwrap(); + app.run("selection.set", json!({"ids": [a, b]})).unwrap(); + let mut h = panel(app); + h.get_by_label("Fill swatch").click(); + h.run_steps(3); + let row = h.get_by_label("Tint:").rect(); + h.query_all_by(|n| n.role() == egui::accesskit::Role::TextInput && n.value().as_deref() == Some("100%")) + .find(|n| (n.rect().center().y - row.center().y).abs() < 8.0) + .expect("the Tint field") + .click(); + h.run_steps(2); + h.event(egui::Event::Key { key: egui::Key::A, physical_key: None, pressed: true, repeat: false, modifiers: egui::Modifiers::COMMAND }); + h.event(egui::Event::Text("25".into())); + h.key_press(egui::Key::Enter); + h.run_steps(3); + let doc = &h.state().session.active().unwrap().doc; + let fill = |id: u64| doc.item(designcraft_doc::ids::ItemId(id)).unwrap().fill.clone(); + assert_eq!((fill(a).swatch.as_str(), fill(a).tint), (CYAN, 0.25)); + assert_eq!((fill(b).swatch.as_str(), fill(b).tint), ("[Black]", 0.25), "the second keeps its own swatch"); + } + #[test] fn apply_to_text_colours_a_text_frames_text_not_the_frame() { let mut app = DesignApp::new(designcraft_engine::Session::new(), crate::Services::default());