diff --git a/ROADMAP.md b/ROADMAP.md index 73e866f..355caea 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -235,7 +235,7 @@ Scores by behaviour, 0–100%. Hours are what remains (one agent). | Auto layout & constraints | 9 | 72% | visual QA of nested edge cases, arrow-key moves across nested auto layout frames | 2–4 | | Text | 8 | 67% | right-to-left layer names in the panels, text style picker depth, axis hover preview | 5–8 | | Components & variants | 9 | 76% | QA with larger real-world libraries | 1 | -| Styles & variables | 5 | 84% | bindings that follow effects when they are reordered, grid and component property bindings | 1–2 | +| Styles & variables | 5 | 86% | grid and component property bindings, reordering paints and effects in the panel | 1–2 | | Prototyping | 6 | 84% | building conditionals in the interaction editor, hotspot hints, comments and inspect in the presenter | 3–6 | | Import/export | 6 | 66% | .fig import (best effort), variable-mode lookups in prototype expressions, PDF print options (bleed, CMYK) | 10–20 | | Dev mode / inspect | 2 | 78% | component code with properties as parameters, visual QA | 2–3 | diff --git a/crates/engine/src/binding_lists_tests.rs b/crates/engine/src/binding_lists_tests.rs new file mode 100644 index 0000000..8e83c86 --- /dev/null +++ b/crates/engine/src/binding_lists_tests.rs @@ -0,0 +1,99 @@ +//! Variable bindings on paint and effect lists: a bound fill colour changes only the first fill, +//! effect bindings follow their effects when the list is reordered or shrinks, and removing the +//! first fill takes its binding with it. + +use figcraft_color::{Color, PaintKind}; +use figcraft_doc::{EffectKind, NodeId}; +use serde_json::json; + +use crate::Session; + +fn id(v: serde_json::Value) -> NodeId { + NodeId(v["id"].as_u64().unwrap()) +} + +fn ex(s: &mut Session, cmd: &str, p: serde_json::Value) { + s.execute(cmd, &p).unwrap_or_else(|e| panic!("{cmd}: {e}")); +} + +fn setup() -> (Session, NodeId) { + let mut s = Session::new(); + let r = id(s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 10, "height": 10})).unwrap()); + ex(&mut s, "variable.createCollection", json!({"name": "T"})); + for (name, kind, value) in [("brand", "color", json!("#FF0000")), ("small", "number", json!(2)), ("large", "number", json!(30))] { + ex(&mut s, "variable.create", json!({"collection": "T", "name": name, "type": kind, "value": value})); + } + (s, r) +} + +fn blur(s: &Session, r: NodeId, i: usize) -> f64 { + match s.doc().unwrap().node(r).unwrap().effects.get(i).map(|e| &e.kind) { + Some(EffectKind::LayerBlur { radius } | EffectKind::BackgroundBlur { radius }) => *radius, + k => panic!("{k:?}"), + } +} + +fn bound(s: &Session, r: NodeId) -> Vec<(String, String)> { + s.doc().unwrap().node(r).unwrap().bound.iter().map(|(k, v)| (k.clone(), v.to_string())).collect() +} + +#[test] +fn a_bound_fill_colour_keeps_the_other_fills() { + let (mut s, r) = setup(); + ex(&mut s, "fill.add", json!({"ids": [r], "color": "#00FF00"})); + ex(&mut s, "fill.set", json!({"ids": [r], "index": 0, "color": "#0000FF", "opacity": 0.5})); + ex(&mut s, "variable.bind", json!({"ids": [r], "field": "fill", "variable": "brand"})); + let fills = s.doc().unwrap().node(r).unwrap().fills.clone(); + assert_eq!(fills.len(), 2, "the second fill stays"); + assert_eq!(fills[0].kind, PaintKind::Solid { color: Color::from_rgb8_const(0xFF, 0, 0) }); + assert_eq!(fills[0].opacity, 0.5, "the bound paint keeps its opacity"); + assert_eq!(fills[1].kind, PaintKind::Solid { color: Color::from_rgb8_const(0, 0xFF, 0) }); + // Removing the bound (first) fill takes the binding with it. + ex(&mut s, "fill.remove", json!({"ids": [r], "index": 0})); + assert!(bound(&s, r).is_empty()); + ex(&mut s, "variable.set", json!({"variable": "brand", "value": "#123456"})); + let fills = s.doc().unwrap().node(r).unwrap().fills.clone(); + assert_eq!(fills.len(), 1); + assert_eq!(fills[0].kind, PaintKind::Solid { color: Color::from_rgb8_const(0, 0xFF, 0) }, "the remaining fill isn't recoloured"); +} + +#[test] +fn effect_bindings_follow_their_effects() { + let (mut s, r) = setup(); + for _ in 0..3 { + ex(&mut s, "effect.add", json!({"ids": [r], "type": "layerBlur"})); + } + ex(&mut s, "variable.bind", json!({"ids": [r], "field": "effects/0/radius", "variable": "small"})); + ex(&mut s, "variable.bind", json!({"ids": [r], "field": "effects/2/radius", "variable": "large"})); + assert_eq!((blur(&s, r, 0), blur(&s, r, 2)), (2.0, 30.0)); + // Move the "small" effect to the top: its binding goes with it. + ex(&mut s, "effect.move", json!({"ids": [r], "from": 0, "to": 2})); + assert_eq!((blur(&s, r, 1), blur(&s, r, 2)), (30.0, 2.0)); + ex(&mut s, "variable.set", json!({"variable": "small", "value": 5})); + assert_eq!((blur(&s, r, 1), blur(&s, r, 2)), (30.0, 5.0)); + // Removing the effect under the "large" one shifts its binding down. + ex(&mut s, "effect.remove", json!({"ids": [r], "index": 0})); + ex(&mut s, "variable.set", json!({"variable": "large", "value": 40})); + assert_eq!((blur(&s, r, 0), blur(&s, r, 1)), (40.0, 5.0)); + // Removing a bound effect drops its binding. + ex(&mut s, "effect.remove", json!({"ids": [r], "index": 1})); + let keys: Vec = bound(&s, r).into_iter().map(|(k, _)| k).collect(); + assert_eq!(keys, vec!["effects/0/radius".to_string()]); + // Undo restores the list and its bindings together. + s.undo().unwrap(); + assert_eq!(bound(&s, r).len(), 2); +} + +#[test] +fn effect_moves_and_removals_reject_junk() { + let (mut s, r) = setup(); + ex(&mut s, "effect.add", json!({"ids": [r], "type": "layerBlur"})); + for bad in [json!({"ids": [r], "from": -1, "to": 0}), json!({"ids": [r], "from": 0}), json!({"ids": [r], "from": "x", "to": 0})] { + assert!(s.execute("effect.move", &bad).is_err(), "{bad}"); + } + assert!(s.execute("effect.remove", &json!({"ids": [r], "index": -1})).is_err()); + // Out-of-range indices change nothing. + ex(&mut s, "effect.move", json!({"ids": [r], "from": 0, "to": 5})); + ex(&mut s, "effect.remove", json!({"ids": [r], "index": 7})); + assert_eq!(s.doc().unwrap().node(r).unwrap().effects.len(), 1); +} diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index bedf76d..b581db5 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -206,15 +206,42 @@ pub fn specs() -> Vec { effect_add ), spec!("effect.remove", "Remove effect", "{ids?, index}", true, |s, p| { - let i = req_num("effect.remove", p, "index")? as usize; + let i = req_index("effect.remove", p, "index")?; for id in req_targets("effect.remove", s, p)? { let n = s.doc_mut()?.get_mut(id)?; if i < n.effects.len() { n.effects.remove(i); + // Its bindings go with it; later effects' move down one. + super::styles::remap_effect_bindings(n, |k| match k.cmp(&i) { + std::cmp::Ordering::Less => Some(k), + std::cmp::Ordering::Equal => None, + std::cmp::Ordering::Greater => Some(k - 1), + }); } } ok() }), + spec!("effect.move", "Move effect", "{ids?, from, to} (indices in the effect list, bottom first)", true, |s, p| { + let from = req_index("effect.move", p, "from")?; + let to = req_index("effect.move", p, "to")?; + for id in req_targets("effect.move", s, p)? { + let n = s.doc_mut()?.get_mut(id)?; + if from >= n.effects.len() || to >= n.effects.len() || from == to { + continue; + } + let e = n.effects.remove(from); + n.effects.insert(to, e); + // Bindings follow their effects. + super::styles::remap_effect_bindings(n, |k| { + if k == from { + return Some(to); + } + let k = if k > from { k - 1 } else { k }; + Some(if k >= to { k + 1 } else { k }) + }); + } + ok() + }), spec!("effect.set", "Set effect", "{ids?, index, effect {..}}", true, |s, p| { let i = req_num("effect.set", p, "index")? as usize; let e: Effect = parse("effect.set", p, "effect")?.ok_or_else(|| bad("effect.set", "`effect` required"))?; @@ -984,6 +1011,13 @@ fn paint_list(s: &mut Session, p: &Value, fills: bool, add: bool) -> Result = Vec::new(); + let mut paint_sets: Vec<(bool, Color)> = Vec::new(); for (field, var) in binds { let Some(val) = doc.variables.resolve(&var, &modes) else { continue }; + // The first fill's or stroke's colour (the rest of the list stays). + let paint = match field.as_str() { + "fill" | "fills" | "fills/0" | "fills/0/color" => Some(true), + "stroke" | "strokes/0/color" => Some(false), + _ => None, + }; + if let (Some(fills), VariableValue::Color(c)) = (paint, &val) { + paint_sets.push((fills, *c)); + continue; + } // `effects//`: one effect's value. if let Some((i, prop)) = field.strip_prefix("effects/").and_then(|r| r.split_once('/')) && let Ok(i) = i.parse::() @@ -681,12 +692,59 @@ pub fn apply_bindings(doc: &mut Document) { sets.insert(key.to_string(), jv); } let _ = super::props::apply("variables", doc, id, &sets); + if !paint_sets.is_empty() { + apply_paint_bindings(doc, id, &paint_sets); + } if !effect_sets.is_empty() { apply_effect_bindings(doc, id, &effect_sets); } } } +/// Write bound colours into a node's first fill (`true`) or stroke, keeping the rest of the list and +/// the paint's opacity, visibility and blend mode (only what changed, so unchanged layers stay +/// shared). +fn apply_paint_bindings(doc: &mut Document, id: NodeId, sets: &[(bool, Color)]) { + let Some(n) = doc.node(id) else { return }; + let (mut fills, mut strokes) = (n.fills.clone(), n.strokes.clone()); + for (is_fill, c) in sets { + let list = if *is_fill { &mut fills } else { &mut strokes }; + match list.first_mut() { + Some(p) => p.kind = PaintKind::Solid { color: *c }, + None => list.push(Paint::solid(*c)), + } + } + if doc.node(id).is_some_and(|n| n.fills != fills || n.strokes != strokes) + && let Ok(n) = doc.get_mut(id) + { + n.fills = fills; + n.strokes = strokes; + } +} + +/// Keep a node's bindings on its effects' fields (`effects//`) with their effects +/// when the list changes: `map` gives an effect's new index, or None when it was removed. +pub(crate) fn remap_effect_bindings(n: &mut Node, map: impl Fn(usize) -> Option) { + let moved: Vec<(String, Option)> = n + .bound + .keys() + .filter_map(|k| { + let (i, field) = k.strip_prefix("effects/")?.split_once('/')?; + let i = i.parse::().ok()?; + Some((k.clone(), map(i).map(|j| format!("effects/{j}/{field}")))) + }) + .collect(); + let mut kept = Vec::new(); + for (old, new) in moved { + if let Some(var) = n.bound.remove(&old) + && let Some(new) = new + { + kept.push((new, var)); + } + } + n.bound.extend(kept); +} + /// Write bound values into a node's effects (only what changed, so unchanged layers stay shared). fn apply_effect_bindings(doc: &mut Document, id: NodeId, sets: &[(usize, String, VariableValue)]) { let Some(mut effects) = doc.node(id).map(|n| n.effects.clone()) else { return }; diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 7bdc9dc..8ee5acb 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -904,6 +904,8 @@ mod al_nesting_tests; #[cfg(test)] mod autosave_tests; #[cfg(test)] +mod binding_lists_tests; +#[cfg(test)] mod devinspect_tests; #[cfg(test)] mod effects_tests;