diff --git a/crates/engine/src/cmd/edit/props.rs b/crates/engine/src/cmd/edit/props.rs index f793b50c..97e88409 100644 --- a/crates/engine/src/cmd/edit/props.rs +++ b/crates/engine/src/cmd/edit/props.rs @@ -104,6 +104,9 @@ pub(super) fn set(s: &mut Session, p: &Value) -> Result { if props.is_empty() { return Err(bad("node.set", "no properties given")); } + for (k, v) in &props { + crate::cmd::props::variable_prop("node.set", k, v)?; + } // A new rotation for the selection turns it about its moved rotation origin: each layer's // point under the origin stays put. let ds = s.ds()?; diff --git a/crates/engine/src/cmd/props.rs b/crates/engine/src/cmd/props.rs index 30a10393..ae2d842b 100644 --- a/crates/engine/src/cmd/props.rs +++ b/crates/engine/src/cmd/props.rs @@ -13,7 +13,7 @@ pub const STROKE_SIDES: [&str; 4] = ["strokeTopWeight", "strokeRightWeight", "st /// Keys `node.set` understands (documented for agents). pub const KEYS: &str = "name, visible, locked, opacity (0-1), blend, x, y (relative to the parent frame; groups don't count), width, height, rotation (degrees), \ -fills (paints or colours; gradients as {type: linearGradient|radialGradient|angularGradient|diamondGradient, stops: [\"#hex\", ...], from?, to?} too), fill (colour, or \"none\"/null: no fill), strokes, stroke (colour, or \"none\"/null: no stroke), strokeWeight, strokeAlign (inside|center|outside), strokeSides [t,r,b,l], strokeTopWeight / strokeRightWeight / strokeBottomWeight / strokeLeftWeight (one side; turns on individual strokes), \ +fills (paints or colours; gradients as {type: linearGradient|radialGradient|angularGradient|diamondGradient, stops: [\"#hex\", ...], from?, to?} too), fill (colour, {$var: variable id or name}, or \"none\"/null: no fill), strokes, stroke (colour, {$var: variable id or name}, or \"none\"/null: no stroke), strokeWeight, strokeAlign (inside|center|outside), strokeSides [t,r,b,l], strokeTopWeight / strokeRightWeight / strokeBottomWeight / strokeLeftWeight (one side; turns on individual strokes), \ strokeCap / startCap / endCap, strokeJoin, miterAngle (degrees: sharper joins bevel; or miterLimit as a ratio), dashes [..], widthProfile (preset uniform|wedge|taper|eye|rounded|pinch, [[t, scale], ...] or null), brush (preset id, {preset, spacing, sizeJitter, rotationJitter, scatter, seed} or null), cornerRadius (n or [tl,tr,br,bl]), cornerSmoothing (0-1), effects, \ clip, isMask, layout (null|horizontal|vertical|grid|{...}), gap, crossGap, padding (n or [t,r,b,l]), paddingTop/Right/Bottom/Left, \ mainAlign (min|center|max, or the auto gaps spaceBetween|spaceAround|spaceEvenly), crossAlign (min|center|max|baseline: first text baselines, horizontal layouts), contentAlign (wrapped lines: auto = packed where crossAlign says | spaceBetween), wrap (rows of a horizontal layout, columns of a vertical one), strokesIncluded (the frame's inside stroke takes up space), reverseZ (canvas stacking: true = first on top), \ @@ -383,10 +383,31 @@ fn paint_none(v: &Value) -> bool { v.is_null() || v.as_str().is_some_and(|s| s.eq_ignore_ascii_case("none")) } +/// A top-level variable wrapper. Validate before `node.set` skips type-specific properties too, +/// so an unsupported wrapper is always an error, even on a mixed selection. +pub(crate) fn variable_prop<'a>(cmd: &str, key: &str, value: &'a Value) -> Result> { + let Some(o) = value.as_object().filter(|o| o.contains_key("$var")) else { return Ok(None) }; + if !matches!(key, "fill" | "stroke") { + return Err(bad(cmd, format!("`{key}`: only `fill` and `stroke` take variables in props; use `variable.bind` for other fields"))); + } + if o.len() != 1 { + return Err(bad(cmd, format!("`{key}`: a `$var` wrapper takes only `$var`, with no extra keys"))); + } + let name = o + .get("$var") + .and_then(Value::as_str) + .filter(|s| !s.is_empty()) + .ok_or_else(|| bad(cmd, format!("`{key}`: `$var` must be a variable id or name (a non-empty string)")))?; + Ok(Some(name)) +} + /// The order [`apply`] takes keys in: geometry first so rotation applies to the final size, style /// links after the values so a link wins over values its style sets, variable bindings last so -/// paints set in the same call don't detach them. -fn apply_order(k: &str) -> u8 { +/// paints set in the same call don't detach them. `$var` wrappers also win over style links. +fn apply_order(k: &str, v: &Value) -> u8 { + if v.get("$var").is_some() { + return 6; + } match k { "path" | "width" | "height" | "w" | "h" => 0, "rotation" => 2, @@ -413,11 +434,26 @@ fn fix_axis(doc: &mut Document, id: NodeId, is_w: bool) -> Result<()> { pub fn apply(cmd: &str, doc: &mut Document, id: NodeId, props: &Map) -> Result<()> { let mut unknown = Vec::new(); let mut keys: Vec<(&String, &Value)> = props.iter().collect(); - keys.sort_by_key(|(k, _)| apply_order(k)); + keys.sort_by_key(|(k, v)| apply_order(k, v)); // Where a new `path` had its own box, for `x`/`y` in the same call (as `node.create` places it). let mut path_offset = (0.0, 0.0); for (k, v) in keys { let k = k.as_str(); + if let Some(name) = variable_prop(cmd, k, v)? { + let vid = super::variables::resolve_strict(cmd, doc, name)?; + // All aliases of the first paint name one slot; other paint slots stay bound. + super::styles::unbind_slot(doc.get_mut(id)?, k == "fill", 0); + super::styles::bind_variable(cmd, doc, &[id], k, Some(&vid))?; + // A paint bound through props follows the variable, not its former style. Keep this + // local to `$var`, including when the style and variable happen to give the same colour. + let n = doc.get_mut(id)?; + if k == "fill" { + n.styles.fill = None; + } else { + n.styles.stroke = None; + } + continue; + } match k { "path" => path_offset = set_path(cmd, doc, id, v, props)?, // A new path takes the size asked for as its scale (`set_path`); the size still fixes diff --git a/crates/engine/src/cmd/styles.rs b/crates/engine/src/cmd/styles.rs index 9c2922d2..c59f67f1 100644 --- a/crates/engine/src/cmd/styles.rs +++ b/crates/engine/src/cmd/styles.rs @@ -772,13 +772,20 @@ pub fn bind(s: &mut Session, p: &Value) -> Result { Some(v) => Some(doc.variables.get(v).map(|x| x.id.clone()).ok_or_else(|| bad(cmd, "no such variable"))?), None => None, }; + bind_variable(cmd, doc, &ids, &field, vid.as_deref())?; + ok() +} + +/// Bind an already resolved variable id, or unbind, inside the caller's transaction. Props and +/// `variable.bind` share type checks and paint alpha handling; normalisation applies the value. +pub(crate) fn bind_variable(cmd: &str, doc: &mut Document, ids: &[NodeId], field: &str, vid: Option<&str>) -> Result<()> { // An instance's component property takes a variable of its type; other fields the types their // scopes belong to. - if let Some(v) = vid.as_ref().and_then(|v| doc.variables.get(v)) { - for i in &ids { + if let Some(v) = vid.and_then(|v| doc.variables.get(v)) { + for i in ids { match field.strip_prefix(crate::components::PROP_BINDING) { Some(name) => super::component::check_prop_binding(doc, *i, name, v.kind).map_err(|e| bad(cmd, e))?, - None if !super::variables::field_takes(doc, *i, &field, v.kind) => { + None if !super::variables::field_takes(doc, *i, field, v.kind) => { return Err(bad(cmd, format!("`{field}` doesn't take a {} variable", v.kind.id()))); } None => {} @@ -786,22 +793,32 @@ pub fn bind(s: &mut Session, p: &Value) -> Result { } } for i in ids { - let n = doc.get_mut(i)?; - match &vid { + let n = doc.get_mut(*i)?; + match vid { Some(v) => { - if n.bound.get(&field) != Some(v) { - rebind_paint_alpha(&mut n.fills, &mut n.strokes, &field, true); + if n.bound.get(field).map(String::as_str) != Some(v) { + rebind_paint_alpha(&mut n.fills, &mut n.strokes, field, true); } - n.bound.insert(field.clone(), v.clone()); + n.bound.insert(field.to_string(), v.to_string()); } None => { - if n.bound.remove(&field).is_some() { - rebind_paint_alpha(&mut n.fills, &mut n.strokes, &field, false); + if n.bound.remove(field).is_some() { + rebind_paint_alpha(&mut n.fills, &mut n.strokes, field, false); } } } } - ok() + Ok(()) +} + +/// Remove every binding key of the slot (`fill` and `fills/0/color` name the same one); true when +/// one was there. +pub(crate) fn unbind_slot(n: &mut figcraft_doc::Node, fills: bool, index: usize) -> bool { + use super::lists::{BoundList, list_binding}; + let list = if fills { BoundList::Fills } else { BoundList::Strokes }; + let before = n.bound.len(); + n.bound.retain(|key, _| list_binding(key).is_none_or(|(l, i, field)| !(l == list && i == index && field == "color"))); + n.bound.len() != before } /// A colour variable bound to a solid paint owns its whole colour, alpha included. Binding takes the diff --git a/crates/engine/src/cmd/variables.rs b/crates/engine/src/cmd/variables.rs index 53b62ee4..5a3b7b1c 100644 --- a/crates/engine/src/cmd/variables.rs +++ b/crates/engine/src/cmd/variables.rs @@ -43,6 +43,34 @@ pub fn specs() -> Vec { ] } +/// Props resolve an exact id first, then an unambiguous exact name or `Collection/name`. Compare +/// whole names: both collection and variable names can contain `/`. Other commands keep their +/// existing first-match lookup. +pub(crate) fn resolve_strict(cmd: &str, doc: &Document, name: &str) -> Result { + let vars = &doc.variables; + if let Some(v) = vars.variables.iter().find(|v| v.id == name) { + return Ok(v.id.clone()); + } + let collections: std::collections::BTreeMap<_, _> = vars.collections.iter().map(|c| (c.id.as_str(), c.name.as_str())).collect(); + let mut candidates = std::collections::BTreeMap::new(); + for v in &vars.variables { + let collection = collections.get(v.collection.as_str()).copied().unwrap_or(v.collection.as_str()); + let qualified = format!("{collection}/{}", v.name); + if v.name == name || qualified == name { + candidates.insert(v.id.as_str(), qualified); + } + } + match candidates.keys().next().copied() { + None => Err(bad(cmd, format!("no such variable `{name}`"))), + Some(id) if candidates.len() == 1 => Ok(id.to_string()), + Some(_) => { + let mut names: Vec<_> = candidates.into_values().collect(); + names.sort(); + Err(bad(cmd, format!("ambiguous variable `{name}`: {} (use a variable id to choose)", names.join(", ")))) + } + } +} + /// The first of `base`, `base 2`, `base 3`… that no collection is named. pub(crate) fn free_collection_name(doc: &Document, base: &str) -> String { (1..1000) diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 60f240cb..06c86e9f 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -482,6 +482,8 @@ mod tiff_tests; #[cfg(test)] mod transform_behaviour_tests; #[cfg(test)] +mod variable_props_tests; +#[cfg(test)] mod variable_tests; #[cfg(test)] mod variables_behaviour_tests; diff --git a/crates/engine/src/selection/selection_colors.rs b/crates/engine/src/selection/selection_colors.rs index 56bc4b35..3c5eb96c 100644 --- a/crates/engine/src/selection/selection_colors.rs +++ b/crates/engine/src/selection/selection_colors.rs @@ -10,7 +10,7 @@ use serde::{Deserialize, Serialize}; use serde_json::{Value, json}; use crate::cmd::lists::{BoundList, list_binding, rebind_edited_paint}; -use crate::cmd::styles::rebind_paint_alpha; +use crate::cmd::styles::{rebind_paint_alpha, unbind_slot}; use crate::params::*; use crate::{CommandSpec, Result, Session, bad, spec}; @@ -238,15 +238,6 @@ fn list_field(doc: &Document, target: &PaintTarget) -> Option { Some(format!("{}/{}/color", list.prefix(), target.index)) } -/// Remove every binding key of the slot (`fill` and `fills/0/color` name the same one); true when -/// one was there. -fn unbind_slot(n: &mut figcraft_doc::Node, fills: bool, index: usize) -> bool { - let list = if fills { BoundList::Fills } else { BoundList::Strokes }; - let before = n.bound.len(); - n.bound.retain(|key, _| list_binding(key).is_none_or(|(l, i, field)| !(l == list && i == index && field == "color"))); - n.bound.len() != before -} - /// Why `variable` can't be applied to `targets` (`selection.bindColorVariable`), if it can't: a /// variable that isn't a color, or a gradient. Scopes don't refuse, as with `variable.bind`; they /// only filter what the menu offers ([`in_scope`]). diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 811724f2..d5d6b59a 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -404,6 +404,11 @@ fn every_command_survives_junk_params() { json!({"variations": {"wght": 1e308, "": "x", "slnt": null}, "props": {"fontVariations": [1], "variationSettings": {"wdth": "-1e9"}}, "text": 7, "cursor": -3}), json!({"style": 3, "props": {"listStyle": 5, "indentation": 1e308, "listSpacing": "NaN", "hangingList": "yes", "start": -5, "end": 1e12, "textDecorationThickness": {"unit": "pixels", "value": "x"}, "textDecorationOffset": -1e308, "textDecorationColor": [1]}}), + // Paint variable props: malformed wrappers, unsupported fields and mixed references. + json!({"id": 2, "type": "rectangle", "props": {"fill": {"$var": null}, "stroke": {"$var": 7}}}), + json!({"id": 2, "type": "rectangle", "props": {"fill": {"$var": ""}, "stroke": {"$var": []}}}), + json!({"id": 2, "type": "rectangle", "props": {"fill": {"$var": "x", "extra": true}, "width": {"$var": "x"}}}), + json!({"id": 2, "type": "rectangle", "props": {"fill": {"$var": {"$ref": "x"}}, "stroke": {"$var": "x", "$ref": "x"}}}), // Templates and the Getting started tips as junk. json!({"template": {"id": 1}, "name": [1e308], "show": "yes"}), // Links (dev resources) and indexes as junk. diff --git a/crates/engine/src/variable_props_tests.rs b/crates/engine/src/variable_props_tests.rs new file mode 100644 index 00000000..0d4790d5 --- /dev/null +++ b/crates/engine/src/variable_props_tests.rs @@ -0,0 +1,389 @@ +//! Top-level paint variable props share `variable.bind` behaviour in one transaction (#1000). + +use figcraft_color::Color; +use figcraft_doc::NodeId; +use serde_json::{Value, json}; + +use crate::{Session, params::id_of}; + +fn run(s: &mut Session, command: &str, params: Value) -> Value { + s.execute(command, ¶ms).unwrap_or_else(|e| panic!("{command}: {e}")) +} + +fn setup() -> (Session, String) { + let mut s = Session::new(); + run(&mut s, "variable.createCollection", json!({"name": "Theme", "modes": ["Light", "Dark"]})); + let v = run(&mut s, "variable.create", json!({"collection": "Theme", "name": "palette/surface", "type": "color", "value": "#0D99FF80"})); + (s, v["id"].as_str().unwrap().to_string()) +} + +fn create(s: &mut Session, props: Value) -> NodeId { + id_of(&run(s, "node.create", json!({"type": "rectangle", "width": 10, "height": 10, "props": props}))["id"]).unwrap() +} + +fn color(s: &Session, id: NodeId, field: &str) -> Color { + let n = s.doc().unwrap().node(id).unwrap(); + let paints = if field == "fill" { &n.fills } else { &n.strokes }; + paints[0].solid_color().unwrap() +} + +fn assert_bound(s: &Session, id: NodeId, field: &str, variable: &str) { + assert_eq!(s.doc().unwrap().node(id).unwrap().bound.get(field).map(String::as_str), Some(variable)); + assert_eq!(color(s, id, field).to_hex(), "0D99FF"); + assert!((color(s, id, field).a - 128.0 / 255.0).abs() < 1e-6); +} + +#[test] +fn create_and_set_take_variable_ids_names_and_collection_names_for_both_paints() { + let (mut s, vid) = setup(); + for field in ["fill", "stroke"] { + for name in [&vid, "palette/surface", "Theme/palette/surface"] { + let id = create(&mut s, json!({field: {"$var": name}})); + assert_bound(&s, id, field, &vid); + let id = create(&mut s, json!({"fill": "none", "stroke": "none"})); + run(&mut s, "node.set", json!({"id": id, "props": {field: {"$var": name}}})); + assert_bound(&s, id, field, &vid); + } + } +} + +#[test] +fn ambiguous_and_unknown_variable_names_are_errors_but_ids_still_work() { + let (mut s, vid) = setup(); + run(&mut s, "variable.createCollection", json!({"name": "Other"})); + run(&mut s, "variable.create", json!({"collection": "Other", "name": "palette/surface", "type": "color", "value": "#FF0000"})); + let e = s.execute("node.create", &json!({"type": "rectangle", "props": {"fill": {"$var": "palette/surface"}}})).unwrap_err().to_string(); + assert!(e.contains("ambiguous variable `palette/surface`"), "{e}"); + assert!(e.contains("Other/palette/surface") && e.contains("Theme/palette/surface"), "{e}"); + let id = create(&mut s, json!({"fill": {"$var": vid}})); + assert_bound(&s, id, "fill", &vid); + let id = create(&mut s, json!({"fill": {"$var": "Theme/palette/surface"}})); + assert_bound(&s, id, "fill", &vid); + let e = s.execute("node.set", &json!({"id": id, "props": {"fill": {"$var": "missing/surface"}}})).unwrap_err().to_string(); + assert!(e.contains("no such variable `missing/surface`"), "{e}"); + // The existing command still uses the first match, not the new strict resolver. + run(&mut s, "variable.bind", json!({"id": id, "field": "fill", "variable": "palette/surface"})); + assert_bound(&s, id, "fill", &vid); +} + +#[test] +fn variable_ids_choose_between_variables_with_the_same_name() { + let (mut s, first) = setup(); + run(&mut s, "variable.createCollection", json!({"name": "Other"})); + let second = run(&mut s, "variable.create", json!({"collection": "Other", "name": "palette/surface", "type": "color", "value": "#FF0000"}))["id"] + .as_str() + .unwrap() + .to_string(); + let id = create(&mut s, json!({"fill": {"$var": second}})); + assert_eq!(s.doc().unwrap().node(id).unwrap().bound.get("fill"), Some(&second)); + assert_eq!(color(&s, id, "fill").to_hex(), "FF0000"); + // `variable.bind` deliberately keeps its first id-or-name match. + run(&mut s, "variable.bind", json!({"id": id, "field": "fill", "variable": "palette/surface"})); + assert_bound(&s, id, "fill", &first); + run(&mut s, "node.set", json!({"id": id, "props": {"fill": {"$var": second}}})); + assert_eq!(s.doc().unwrap().node(id).unwrap().bound.get("fill"), Some(&second)); + assert_eq!(color(&s, id, "fill").to_hex(), "FF0000"); +} + +#[test] +fn whole_name_matches_and_qualified_matches_share_one_candidate_set() { + let (mut s, _) = setup(); + run(&mut s, "variable.createCollection", json!({"name": "Other"})); + run(&mut s, "variable.create", json!({"collection": "Other", "name": "Theme/palette/surface", "type": "color", "value": "#FF0000"})); + let e = s.execute("node.create", &json!({"type": "rectangle", "props": {"fill": {"$var": "Theme/palette/surface"}}})).unwrap_err().to_string(); + assert!(e.contains("ambiguous variable"), "{e}"); + assert!(e.contains("Other/Theme/palette/surface") && e.contains("Theme/palette/surface"), "{e}"); + // Collection names can contain slashes too; no splitting at the first slash. + run(&mut s, "variable.createCollection", json!({"name": "Brand/Theme"})); + let v = run(&mut s, "variable.create", json!({"collection": "Brand/Theme", "name": "palette/surface", "type": "color", "value": "#0D99FF80"})); + let id = create(&mut s, json!({"fill": {"$var": "Brand/Theme/palette/surface"}})); + assert_bound(&s, id, "fill", v["id"].as_str().unwrap()); +} + +#[test] +fn wrong_variable_type_rolls_back_create_and_set() { + let (mut s, _) = setup(); + run(&mut s, "variable.create", json!({"collection": "Theme", "name": "size", "type": "number", "value": 20})); + let id = create(&mut s, json!({"fill": "#FF0000"})); + for field in ["fill", "stroke"] { + for command in ["node.create", "node.set"] { + let before = s.doc().unwrap().clone(); + let selection = s.selection().to_vec(); + let depth = s.undo_depth(); + let e = s.execute(command, &json!({"type": "rectangle", "id": id, "props": {"name": "must roll back", "fill": {"$var": "palette/surface"}, field: {"$var": "size"}}})).unwrap_err().to_string(); + assert!(e.contains(&format!("`{field}` doesn't take a number variable")), "{e}"); + assert_eq!(s.doc().unwrap(), &before); + assert_eq!(s.selection(), selection); + assert_eq!(s.undo_depth(), depth); + } + } +} + +#[test] +fn variable_props_follow_modes_and_later_literal_paints_detach() { + let (mut s, vid) = setup(); + // Start with both paints present: mode alpha on a newly created paint is an existing, + // separate `variable.bind` bug, not part of variable props. + let id = create(&mut s, json!({"strokes": ["#FFFFFF"], "fill": {"$var": vid}, "stroke": {"$var": vid}})); + run(&mut s, "variable.set", json!({"variable": vid, "mode": "Dark", "value": "#20212440"})); + for mode in ["Dark", "Light", "Dark"] { + run(&mut s, "variable.setMode", json!({"collection": "Theme", "mode": mode})); + for field in ["fill", "stroke"] { + let c = color(&s, id, field); + assert_eq!(c.to_hex(), if mode == "Dark" { "202124" } else { "0D99FF" }); + let alpha = if mode == "Dark" { 64.0 } else { 128.0 } / 255.0; + assert!((c.a - alpha).abs() < 1e-6, "{mode}: {c:?}"); + } + } + run(&mut s, "node.set", json!({"id": id, "props": {"fill": "#FF0000", "stroke": "#00FF00"}})); + assert!(s.doc().unwrap().node(id).unwrap().bound.is_empty()); + run(&mut s, "variable.set", json!({"variable": vid, "mode": "Dark", "value": "#000000"})); + assert_eq!(color(&s, id, "fill").to_hex(), "FF0000"); + assert_eq!(color(&s, id, "stroke").to_hex(), "00FF00"); +} + +#[test] +fn create_with_variables_is_one_undo_entry_and_redo_restores_it() { + let (mut s, vid) = setup(); + let before = s.doc().unwrap().clone(); + let depth = s.undo_depth(); + let id = create(&mut s, json!({"fill": {"$var": vid}, "stroke": {"$var": vid}})); + assert_eq!(s.undo_depth(), (depth.0 + 1, 0)); + let after = s.doc().unwrap().clone(); + run(&mut s, "edit.undo", json!({})); + assert_eq!(s.doc().unwrap(), &before); + assert!(!s.doc().unwrap().contains(id)); + run(&mut s, "edit.redo", json!({})); + assert_eq!(s.doc().unwrap(), &after); + assert_bound(&s, id, "fill", &vid); + assert_bound(&s, id, "stroke", &vid); +} + +#[test] +fn paint_variable_props_match_explicit_binding_including_typed_alpha_and_opacity() { + let (mut s, vid) = setup(); + for field in ["fill", "stroke"] { + let list = if field == "fill" { "fills" } else { "strokes" }; + for opacity in [1.0, 0.25] { + let paints = json!({list: [{"type": "solid", "color": "#FF000080", "opacity": opacity}]}); + let explicit = create(&mut s, paints.clone()); + run(&mut s, "variable.bind", json!({"id": explicit, "field": field, "variable": vid})); + let mut props = paints.clone(); + props[field] = json!({"$var": vid}); + let inline = create(&mut s, props); + let set = create(&mut s, paints); + run(&mut s, "node.set", json!({"id": set, "props": {field: {"$var": vid}}})); + for id in [inline, set] { + let doc = s.doc().unwrap(); + let (a, b) = (doc.node(id).unwrap(), doc.node(explicit).unwrap()); + assert_eq!(a.fills, b.fills); + assert_eq!(a.strokes, b.strokes); + assert_eq!(a.bound, b.bound); + assert!((color(&s, id, field).a - 128.0 / 255.0 * opacity as f32).abs() < 1e-6); + } + // Repeating the same binding does not accumulate alpha. + run(&mut s, "node.set", json!({"id": inline, "props": {field: {"$var": vid}}})); + assert_eq!(color(&s, inline, field), color(&s, explicit, field)); + } + } +} + +#[test] +fn variable_props_win_over_literal_paint_lists_and_style_links() { + let (mut s, vid) = setup(); + let st = run(&mut s, "style.create", json!({"type": "paint", "name": "Red", "value": ["#FF000080"]}))["id"].clone(); + for field in ["fill", "stroke"] { + let style_key = if field == "fill" { "fillStyle" } else { "strokeStyle" }; + let list = if field == "fill" { "fills" } else { "strokes" }; + let props = json!({list: ["#00FF00"], style_key: st, field: {"$var": vid}}); + let created = create(&mut s, props.clone()); + let set = create(&mut s, json!({})); + run(&mut s, "node.set", json!({"id": set, "props": props})); + for id in [created, set] { + assert_bound(&s, id, field, &vid); + let n = s.doc().unwrap().node(id).unwrap(); + assert!(if field == "fill" { n.styles.fill.is_none() } else { n.styles.stroke.is_none() }); + } + run(&mut s, "style.update", json!({"style": st, "value": ["#FF00FF"]})); + for id in [created, set] { + assert_bound(&s, id, field, &vid); + } + } +} + +#[test] +fn variable_props_unlink_a_preexisting_paint_style_even_when_the_colour_is_unchanged() { + let (mut s, vid) = setup(); + run(&mut s, "variable.set", json!({"variable": vid, "value": "#FF0000"})); + let style = run(&mut s, "style.create", json!({"type": "paint", "name": "Red", "value": ["#FF0000"]}))["id"].clone(); + for field in ["fill", "stroke"] { + let style_key = if field == "fill" { "fillStyle" } else { "strokeStyle" }; + let id = create(&mut s, json!({style_key: style})); + let n = s.doc().unwrap().node(id).unwrap(); + assert!(if field == "fill" { n.styles.fill.is_some() } else { n.styles.stroke.is_some() }); + run(&mut s, "node.set", json!({"id": id, "props": {field: {"$var": vid}}})); + let n = s.doc().unwrap().node(id).unwrap(); + assert_eq!(n.bound.get(field), Some(&vid)); + assert!(if field == "fill" { n.styles.fill.is_none() } else { n.styles.stroke.is_none() }); + assert_eq!(color(&s, id, field).to_hex(), "FF0000"); + } +} + +#[test] +fn malformed_and_unsupported_variable_props_are_errors_and_leave_no_edit() { + let (mut s, _) = setup(); + let id = create(&mut s, json!({})); + for props in [ + json!({"fill": {"$var": null}}), + json!({"fill": {"$var": 5}}), + json!({"stroke": {"$var": []}}), + json!({"fill": {"$var": {}}}), + json!({"fill": {"$var": ""}}), + json!({"fill": {}}), + json!({"fill": {"$var": "palette/surface", "extra": true}}), + json!({"fill": {"$var": "palette/surface", "$ref": "x"}}), + json!({"fill": {"$var": "palette/surface", "$step": 0}}), + ] { + for command in ["node.create", "node.set"] { + let before = s.doc().unwrap().clone(); + let depth = s.undo_depth(); + let e = s.execute(command, &json!({"type": "rectangle", "id": id, "props": props})).unwrap_err().to_string(); + assert!(e.contains("$var") || e.contains("must be a colour"), "{e}"); + assert_eq!(s.doc().unwrap(), &before, "{props}"); + assert_eq!(s.undo_depth(), depth); + } + } + for key in ["width", "fills", "strokes", "opacity", "gap", "path", "pluginData", "fillStyle", "unknown"] { + for command in ["node.create", "node.set"] { + let e = s.execute(command, &json!({"type": "rectangle", "id": id, "props": {key: {"$var": "palette/surface"}}})).unwrap_err().to_string(); + assert!(e.contains("only `fill` and `stroke` take variables in props") && e.contains("variable.bind"), "{e}"); + } + } +} + +#[test] +fn batch_references_resolve_inside_variable_wrappers_and_mixed_keys_roll_back() { + for reference in [json!({"$ref": "surface"}), json!({"$step": 1})] { + let mut s = Session::new(); + let commands = json!([ + {"command": "variable.createCollection", "params": {"name": "Theme"}}, + {"command": "variable.create", "as": "surface", "params": {"collection": "Theme", "name": "surface", "type": "color", "value": "#0D99FF80"}}, + {"command": "node.create", "params": {"type": "rectangle", "props": {"fill": {"$var": reference}}}} + ]); + let result = run(&mut s, "command.batch", json!({"commands": commands})); + let id = id_of(&result[2]["id"]).unwrap(); + let vid = result[1]["id"].as_str().unwrap(); + assert_bound(&s, id, "fill", vid); + assert_eq!(s.undo_depth(), (1, 0)); + run(&mut s, "edit.undo", json!({})); + assert!(s.doc().unwrap().variables.variables.is_empty()); + assert!(!s.doc().unwrap().contains(id)); + + let before = s.doc().unwrap().clone(); + let mut bad_commands = commands.clone(); + bad_commands[2]["params"]["props"]["fill"] = json!({"$var": "surface", "$ref": "surface"}); + let e = s.execute("command.batch", &json!({"commands": bad_commands})).unwrap_err().to_string(); + assert!(e.contains("not `$var`"), "{e}"); + assert_eq!(s.doc().unwrap(), &before); + } +} + +#[test] +fn variable_props_replace_first_paint_aliases_but_keep_other_slots_bound() { + let (mut s, vid) = setup(); + let old = run(&mut s, "variable.create", json!({"collection": "Theme", "name": "old", "type": "color", "value": "#FF0000"}))["id"] + .as_str() + .unwrap() + .to_string(); + for (field, list, aliases) in [ + ("fill", "fills", vec!["fill", "fills", "fills/0", "fills/0/color", "fills/00/color"]), + ("stroke", "strokes", vec!["stroke", "strokes/0/color", "strokes/00/color"]), + ] { + for alias in aliases { + let first = format!("{list}/0/color"); + let second = format!("{list}/1/color"); + let props = json!({list: ["#FFFFFF", "#FFFFFF"], "boundVariables": {alias: old, second.clone(): old}}); + let set = create(&mut s, props.clone()); + run(&mut s, "node.set", json!({"id": set, "props": {field: {"$var": vid}}})); + let mut props = props; + props[field] = json!({"$var": vid}); + let created = create(&mut s, props); + for id in [set, created] { + assert_bound(&s, id, field, &vid); + let n = s.doc().unwrap().node(id).unwrap(); + assert_eq!(n.bound.len(), 2, "{alias}"); + assert_eq!(n.bound.get(&second), Some(&old)); + if alias != field { + assert!(!n.bound.contains_key(alias), "{alias}"); + } + assert!(!n.bound.contains_key(&first)); + let paints = if field == "fill" { &n.fills } else { &n.strokes }; + assert_eq!(paints[1].solid_color().unwrap().to_hex(), "FF0000"); + } + } + } +} + +#[test] +fn variable_props_bind_a_mixed_selection_and_unsupported_wrappers_leave_no_edit() { + let (mut s, vid) = setup(); + let ids: Vec<_> = ["frame", "rectangle", "text"] + .into_iter() + .map(|kind| id_of(&run(&mut s, "node.create", json!({"type": kind, "width": 10, "height": 10}))["id"]).unwrap()) + .collect(); + run(&mut s, "select.set", json!({"ids": ids})); + run(&mut s, "node.set", json!({"props": {"fill": {"$var": vid}, "stroke": {"$var": vid}}})); + for &id in &ids { + for field in ["fill", "stroke"] { + assert_bound(&s, id, field, &vid); + } + } + let before = s.doc().unwrap().clone(); + let selection = s.selection().to_vec(); + let depth = s.undo_depth(); + let e = s.execute("node.set", &json!({"props": {"name": "must not change", "cornerRadius": {"$var": vid}}})).unwrap_err().to_string(); + assert!(e.contains("only `fill` and `stroke` take variables in props"), "{e}"); + assert_eq!(s.doc().unwrap(), &before); + assert_eq!(s.selection(), selection); + assert_eq!(s.undo_depth(), depth); +} + +#[test] +fn variable_keys_inside_typed_paint_lists_stay_literal_but_bare_wrappers_error() { + let (mut s, vid) = setup(); + let id = create(&mut s, json!({})); + for command in ["node.create", "node.set"] { + // Typed paints ignore extra keys today: `$var` is only a wrapper in top-level props. + let result = run( + &mut s, + command, + json!({"type": "rectangle", "id": id, "props": { + "fills": [{"type": "solid", "color": "#FF0000", "$var": vid}] + }}), + ); + let painted = if command == "node.create" { id_of(&result["id"]).unwrap() } else { id }; + assert_eq!(color(&s, painted, "fill").to_hex(), "FF0000"); + assert!(s.doc().unwrap().node(painted).unwrap().bound.is_empty()); + let before = s.doc().unwrap().clone(); + let depth = s.undo_depth(); + assert!(s.execute(command, &json!({"type": "rectangle", "id": id, "props": {"fills": [{"$var": vid}]}})).is_err()); + assert_eq!(s.doc().unwrap(), &before); + assert_eq!(s.undo_depth(), depth); + } +} + +#[test] +fn variable_bind_keeps_its_legacy_type_lookup_when_a_name_collides_with_an_id() { + let mut s = Session::new(); + run(&mut s, "variable.createCollection", json!({"name": "Theme"})); + let number = run(&mut s, "variable.create", json!({"collection": "Theme", "name": "size", "type": "number", "value": 20}))["id"].clone(); + let paint = run(&mut s, "variable.create", json!({"collection": "Theme", "name": "surface", "type": "color", "value": "#FF0000"}))["id"].clone(); + run(&mut s, "variable.update", json!({"variable": number, "name": paint})); + let id = create(&mut s, json!({})); + let before = s.doc().unwrap().clone(); + let depth = s.undo_depth(); + let e = s.execute("variable.bind", &json!({"id": id, "field": "fill", "variable": "surface"})).unwrap_err().to_string(); + assert!(e.contains("`fill` doesn't take a number variable"), "{e}"); + assert_eq!(s.doc().unwrap(), &before); + assert_eq!(s.undo_depth(), depth); +} diff --git a/docs/mcp.md b/docs/mcp.md index 05dfd1e6..dc8e5719 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -281,7 +281,13 @@ Or in a project's `.mcp.json`: `null`), so a later `style.update` doesn't overwrite the edit; `style.apply` replaces variables bound to what the style sets; a text style goes on text layers only. In one call, a style link (`fillStyle`, `textStyle`…) wins over values it sets; set a deviation in a later call, which - unlinks it. Names are trimmed per `/` folder part. `style.move {styles, before?, folder?}` + unlinks it. Names are trimmed per `/` folder part. A `fill` or `stroke` can also bind a variable: + `node.create {type: "rectangle", props: {fill: {"$var": "Theme/surface"}}}` creates and binds in + one undo step, and `node.set` takes the same wrapper. `$var` takes an exact variable id first, + else an unambiguous exact name or `Collection/name` (names can contain `/`); unknown or ambiguous + names are errors. The wrapper is applied after literal values and style links, and clears that + paint's style link even when the colour is unchanged. Only `fill` and `stroke` take variables in + props; use `variable.bind` for other fields. `style.move {styles, before?, folder?}` reorders (and with `folder`, renames into it), `style.duplicate {style}` adds " copy" right below, `style.renameFolder {folder, name}`. - **Variables**: `variable.bind` refuses a variable of a type the field doesn't take (a colour on diff --git a/docs/roadmap.md b/docs/roadmap.md index 14d734ba..8d95700b 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -1,6 +1,6 @@ # FigCraft roadmap: milestones and what's next -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-26 · **Change:** minor (Android Vector Drawable export; a selected unfilled shape drags over a filled layer below it (#1370); text truncation fills the last line, survives resizing, and auto height takes Max lines (#1375, #1376, #1377); M11 widget fields and property menus; M12 Boards: timer and voting; Installed weights of an embedded font family (#1246); `.sketch` import, best effort; Adobe Fonts found on macOS (#298); M12 Boards; community fixes in #507, #510, #529 and #530: auto-save, a text box by dragging, batch step references, removing someone from a live session, Russian and Turkish catalogs reviewed, Czech and Indonesian drafts, the Japanese draft completed, zoom previews, the web clipboard, Alt-drag snapping and Cmd+D repeat, recovery copies, crash and rollback fixes, colour emoji, `.fig` import fixes; earlier changes are in the revision history below) · **Target:** Figma Design (desktop app 126.10.7) +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-27 · **Change:** minor (variables in paint props (M3, #1000); Android Vector Drawable export; a selected unfilled shape drags over a filled layer below it (#1370); text truncation fills the last line, survives resizing, and auto height takes Max lines (#1375, #1376, #1377); M11 widget fields and property menus; M12 Boards: timer and voting; Installed weights of an embedded font family (#1246); `.sketch` import, best effort; Adobe Fonts found on macOS (#298); M12 Boards; community fixes in #507, #510, #529 and #530: auto-save, a text box by dragging, batch step references, removing someone from a live session, Russian and Turkish catalogs reviewed, Czech and Indonesian drafts, the Japanese draft completed, zoom previews, the web clipboard, Alt-drag snapping and Cmd+D repeat, recovery copies, crash and rollback fixes, colour emoji, `.fig` import fixes; earlier changes are in the revision history below) · **Target:** Figma Design (desktop app 126.10.7) Forward-looking plan. Where FigCraft stands today: [`ROADMAP.md`](../ROADMAP.md) and [`target-app-parity.md`](target-app-parity.md); the full ranked list: [`gaps.md`](gaps.md). Hours @@ -75,6 +75,7 @@ blockers. | Date | Change | Summary | |---|---|---| +| 2026-10-27 | minor | Variables in props (M3, #1000): `node.create` and `node.set` take `{$var: variable id or name}` for `fill` and `stroke`, including unambiguous `Collection/name` lookups; bindings apply after literal values and style links and clear the paint's style link, with the same type checks, alpha and modes as `variable.bind`, in one undo step. Regression tests cover lookups, ordering, batch references, rollback and undo/redo. Numbers unchanged | | 2026-10-26 | minor | Dragging a selected unfilled shape (M1, #1370): a press inside a selected stroke-only shape over a filled layer stacked below it moved that layer; the drag-from-inside fallback of #910 now treats any layer painted below the selection like the frame or canvas behind it, so the selection drags, while a layer painted above it still takes the press. Regression test in the selection behaviour suite. Numbers unchanged | | 2026-10-26 | minor | Text truncation (M4, #1375, #1376, #1377): the last line shown takes as much of the next word as fits before the ellipsis ("Lorem consec…", not "Lorem…"); an explicit height, `transform.resize`, a resize handle or fixed sizing on a truncated text keeps it truncating instead of turning it fixed size; Max lines on auto-height text hugs up to that many lines and ends in an ellipsis (`text.maxLines`, `node.set {maxLines}`, Type settings, `.fig` export and import), and `node.get` / `get_node` report `maxLines`. Regression tests in the text and engine behaviour suites. Numbers unchanged | | 2026-10-24 | minor | Installed weights of an embedded font family (M7, #1246): release builds embed some families with only a few styles (Poppins Regular, Italic and Bold), and the first font scan dropped every installed face of such a family, so an installed Poppins SemiBold or Medium was never offered (a rescan did offer them). The first scan now keeps the embedded faces, which still win for the styles they cover, and offers the installed styles they lack beside them. Regression test in the font database tests. Numbers unchanged |