From 9f280cb2e0f06f6176a4e294957e6d5bc1fb156d Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 24 Oct 2026 05:23:20 -0700 Subject: [PATCH] M3: style variable pickers offer only variables scoped to the field The variable picker opened from a style's field listed every variable of the field's type, and the Fill and Stroke style picker's Color variables listed every color variable, whatever their scopes. Both now offer only variables scoped to the property, as the Design panel's pickers do. A paint style's colors take variables scoped to a fill or to strokes, an effect style's fields take effect variables, and a text style's fields take the variables of their font property. Layout grid fields still take any number variable. style.bind still binds a variable outside its scopes, as variable.bind does. --- crates/engine/src/style_vars.rs | 23 +++++ crates/engine/src/style_vars_tests.rs | 40 +++++++++ crates/ui-egui/src/style_picker.rs | 6 +- crates/ui-egui/src/variable_ui.rs | 116 ++++++++++++++++++++++++++ 4 files changed, 184 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/style_vars.rs b/crates/engine/src/style_vars.rs index 97226aed..b2e5c182 100644 --- a/crates/engine/src/style_vars.rs +++ b/crates/engine/src/style_vars.rs @@ -42,6 +42,29 @@ pub fn field_type(kind: &StyleKind, key: &str) -> Option { } } +/// The variable scopes a picker offers for `key` of a style of `kind`: a variable scoped to any one +/// of them fits. A paint style's colors take fill and stroke variables, an effect style's fields +/// take effect variables, and a text style's fields take the variables of their font property. +/// None when every variable of the field's type fits (a grid's fields, as on layers) or the style +/// has no such field. +pub fn field_scopes(kind: &StyleKind, key: &str) -> Option<&'static [&'static str]> { + field_type(kind, key)?; + Some(match (kind, list_binding(key)) { + (StyleKind::Paint { .. }, _) => &["FRAME_FILL", "SHAPE_FILL", "TEXT_FILL", "STROKE_COLOR"], + (StyleKind::Effect { .. }, Some((_, _, "color"))) => &["EFFECT_COLOR"], + (StyleKind::Effect { .. }, _) => &["EFFECT_FLOAT"], + (StyleKind::Text { .. }, _) => match key { + "fontSize" => &["FONT_SIZE"], + "lineHeight" => &["LINE_HEIGHT"], + "letterSpacing" => &["LETTER_SPACING"], + "fontFamily" => &["FONT_FAMILY"], + "fontStyle" => &["FONT_STYLE"], + _ => return None, + }, + _ => return None, + }) +} + /// The key a style stores a binding of `key` under: the first paint's colour has one spelling. pub fn canonical(key: &str) -> String { match list_binding(key) { diff --git a/crates/engine/src/style_vars_tests.rs b/crates/engine/src/style_vars_tests.rs index dae9ef56..66768daa 100644 --- a/crates/engine/src/style_vars_tests.rs +++ b/crates/engine/src/style_vars_tests.rs @@ -182,6 +182,46 @@ fn what_a_style_cannot_bind_is_refused() { } } +#[test] +fn style_fields_offer_their_properties_scopes() { + let mut s = Session::new(); + let (_, _, a, _, sid) = two_modes(&mut s); + run(&mut s, "effect.add", json!({"ids": [a], "type": "dropShadow"})); + let fx = run(&mut s, "style.create", json!({"type": "effect", "name": "Drop", "from": a}))["id"].as_str().unwrap().to_string(); + let light = node(&s, a).parent.unwrap(); + run(&mut s, "grid.add", json!({"ids": [light]})); + let grid = run(&mut s, "style.create", json!({"type": "grid", "name": "Base", "from": light}))["id"].as_str().unwrap().to_string(); + let body = run(&mut s, "style.create", json!({"type": "text", "name": "Body", "value": {"fontFamily": "Inter", "fontSize": 14}}))["id"] + .as_str() + .unwrap() + .to_string(); + let doc = s.doc().unwrap(); + let scopes = |st: &str, key: &str| crate::style_vars::field_scopes(&doc.style(st).unwrap().kind, key); + let fills: &[&str] = &["FRAME_FILL", "SHAPE_FILL", "TEXT_FILL", "STROKE_COLOR"]; + for (st, key, want) in [ + (&sid, "fill", Some(fills)), + (&sid, "fills/0/color", Some(fills)), + (&fx, "effects/0/color", Some(&["EFFECT_COLOR"][..])), + (&fx, "effects/0/radius", Some(&["EFFECT_FLOAT"][..])), + (&fx, "effects/0/offsetX", Some(&["EFFECT_FLOAT"][..])), + (&body, "fontSize", Some(&["FONT_SIZE"][..])), + (&body, "lineHeight", Some(&["LINE_HEIGHT"][..])), + (&body, "letterSpacing", Some(&["LETTER_SPACING"][..])), + (&body, "fontFamily", Some(&["FONT_FAMILY"][..])), + (&body, "fontStyle", Some(&["FONT_STYLE"][..])), + // Grid fields take any number variable, as on layers; fields a style lacks take none. + (&grid, "layoutGrids/0/size", None), + (&sid, "fills/3/color", None), + (&body, "opacity", None), + ] { + assert_eq!(scopes(st, key), want, "{st} {key}"); + } + // As on layers, scopes limit pickers only: an out-of-scope variable still binds. + run(&mut s, "variable.update", json!({"variable": "size", "scopes": ["CORNER_RADIUS"]})); + run(&mut s, "style.bind", json!({"style": body, "field": "fontSize", "variable": "size"})); + assert!(s.doc().unwrap().style(&body).unwrap().bound.contains_key("fontSize")); +} + #[test] fn rest_and_fig_carry_style_bindings() { let mut s = Session::new(); diff --git a/crates/ui-egui/src/style_picker.rs b/crates/ui-egui/src/style_picker.rs index 22713000..53046d8a 100644 --- a/crates/ui-egui/src/style_picker.rs +++ b/crates/ui-egui/src/style_picker.rs @@ -555,7 +555,11 @@ fn listing(app: &FigApp, ui: &mut Ui, doc: &Document, p: &mut Picker, out: &mut } } let colors = if slot.paints() { - crate::variable_ui::groups(doc, &libs, |_, v| v.kind == VariableType::Color && (q.is_empty() || v.name.to_lowercase().contains(&q))) + // Variables scoped to the selection's fill (or stroke), as the Design panel's pickers offer. + let scope = p.ids.first().and_then(|id| figcraft_engine::cmd::variables::field_scope(doc, *id, slot.key())); + crate::variable_ui::groups(doc, &libs, |_, v| { + v.kind == VariableType::Color && (q.is_empty() || v.name.to_lowercase().contains(&q)) && scope.is_none_or(|s| v.in_scope(s)) + }) } else { Vec::new() }; diff --git a/crates/ui-egui/src/variable_ui.rs b/crates/ui-egui/src/variable_ui.rs index 39c5396e..3dfeeec7 100644 --- a/crates/ui-egui/src/variable_ui.rs +++ b/crates/ui-egui/src/variable_ui.rs @@ -399,6 +399,11 @@ pub fn picker(app: &mut FigApp, ctx: &egui::Context) { Target::Alias { variable, .. } => (None, doc.variables.get(variable)), Target::Proto(_) | Target::Style { .. } => (None, None), }; + // A style's fields offer variables scoped to one of the properties the field sets. + let style_scopes = match &p.target { + Target::Style { style, field } => doc.style(style).and_then(|st| figcraft_engine::style_vars::field_scopes(&st.kind, field)), + _ => None, + }; // Prototypes and styles use this file's variables. let local_only = matches!(p.target, Target::Proto(_) | Target::Style { .. }); let field = match &p.target { @@ -410,6 +415,7 @@ pub fn picker(app: &mut FigApp, ctx: &egui::Context) { && !(local_only && lib.is_some()) && (q.is_empty() || v.name.to_lowercase().contains(&q)) && scope.is_none_or(|s| v.in_scope(s)) + && style_scopes.is_none_or(|list| list.iter().any(|s| v.in_scope(s))) && (lib.is_some() || own.is_none_or(|o| figcraft_engine::cmd::variables::alias_target(&doc.variables, o, &v.id).is_ok())) }; let groups = groups(&doc, &libs, keep); @@ -518,3 +524,113 @@ mod kind_tests { assert!(kind_fits(None, None, Boolean)); } } + +#[cfg(test)] +mod scope_tests { + use figcraft_engine::Session; + + use super::*; + use crate::layers_drag_tests::Ui as TestUi; + use crate::style_picker::StyleSlot; + + fn run(s: &mut Session, cmd: &str, p: serde_json::Value) -> serde_json::Value { + s.execute(cmd, &p).unwrap_or_else(|e| panic!("{cmd} {p}: {e}")) + } + + /// A rectangle (selected, with a drop shadow), a frame with a layout grid, the styles "Brand" + /// (paint), "Shadow" (effect, from the rectangle), "Base" (grid, from the frame) and "Body" + /// (text), and variables each scoped to one property (`any` to none). + fn session() -> Session { + let mut s = Session::new(); + let r = run(&mut s, "shape.rectangle", json!({"x": 0, "y": 0, "width": 40, "height": 40}))["id"].clone(); + run(&mut s, "effect.add", json!({"ids": [r], "type": "dropShadow"})); + let f = run(&mut s, "frame.create", json!({"x": 100, "y": 0, "width": 100, "height": 100}))["id"].clone(); + run(&mut s, "grid.add", json!({"ids": [f]})); + run(&mut s, "style.create", json!({"type": "paint", "name": "Brand", "value": [{"type": "solid", "color": "#0D99FF"}]})); + run(&mut s, "style.create", json!({"type": "effect", "name": "Shadow", "from": r})); + run(&mut s, "style.create", json!({"type": "grid", "name": "Base", "from": f})); + run(&mut s, "style.create", json!({"type": "text", "name": "Body", "value": {"fontFamily": "Inter", "fontSize": 14}})); + run(&mut s, "variable.createCollection", json!({"name": "Tokens", "modes": ["Base"]})); + for (name, ty, value, scope) in [ + ("frame fill", "color", json!("#FF0000"), Some("FRAME_FILL")), + ("shape fill", "color", json!("#00FF00"), Some("SHAPE_FILL")), + ("stroke color", "color", json!("#0000FF"), Some("STROKE_COLOR")), + ("shadow color", "color", json!("#000000"), Some("EFFECT_COLOR")), + ("radius", "number", json!(8), Some("CORNER_RADIUS")), + ("font size", "number", json!(18), Some("FONT_SIZE")), + ("blur", "number", json!(4), Some("EFFECT_FLOAT")), + ("any", "number", json!(2), None), + ("family", "string", json!("Inter"), Some("FONT_FAMILY")), + ("weight", "string", json!("Bold"), Some("FONT_STYLE")), + ] { + run(&mut s, "variable.create", json!({"collection": "Tokens", "name": name, "type": ty, "values": {"Base": value}})); + if let Some(sc) = scope { + run(&mut s, "variable.update", json!({"variable": name, "scopes": [sc]})); + } + } + run(&mut s, "select.set", json!({"ids": [r]})); + s + } + + /// The texts drawn in one frame. + fn drawn(u: &mut TestUi) -> Vec { + fn walk(shape: &egui::Shape, out: &mut Vec) { + match shape { + egui::Shape::Text(t) => out.push(t.galley.text().to_string()), + egui::Shape::Vec(v) => v.iter().for_each(|s| walk(s, out)), + _ => {} + } + } + let mut out = Vec::new(); + for c in u.frame_output(vec![]).shapes { + walk(&c.shape, &mut out); + } + out + } + + fn check(texts: &[String], shown: &[&str], hidden: &[&str], what: &str) { + for name in shown { + assert!(texts.iter().any(|t| t == name), "{what}: {name:?} offered, in {texts:?}"); + } + for name in hidden { + assert!(!texts.iter().any(|t| t == name), "{what}: {name:?} not offered, in {texts:?}"); + } + } + + #[test] + fn a_styles_variable_picker_offers_the_fields_scopes_only() { + use VariableType::{Color, Number, String}; + let mut u = TestUi::new(session()); + // (style, field, type, offered, not offered) + for (style, field, kind, shown, hidden) in [ + ("Brand", "fill", Color, &["frame fill", "shape fill", "stroke color"][..], &["shadow color"][..]), + ("Shadow", "effects/0/color", Color, &["shadow color"], &["frame fill", "shape fill", "stroke color"]), + ("Shadow", "effects/0/radius", Number, &["blur", "any"], &["radius", "font size"]), + ("Base", "layoutGrids/0/size", Number, &["radius", "font size", "blur", "any"], &[]), + ("Body", "fontSize", Number, &["font size", "any"], &["radius", "blur"]), + ("Body", "fontFamily", String, &["family"], &["weight"]), + ("Body", "fontStyle", String, &["weight"], &["family"]), + ] { + let id = u.app.session.doc().unwrap().style(style).unwrap().id.clone(); + let target = Target::Style { style: id, field: field.to_string() }; + u.ctx.data_mut(|d| d.insert_temp(picker_id(), Picker { target, kind: Some(kind), at: pos2(400.0, 100.0) })); + u.frame(vec![]); + check(&drawn(&mut u), shown, hidden, &format!("{style} {field}")); + } + } + + #[test] + fn the_fill_and_stroke_style_pickers_offer_scoped_color_variables() { + let mut u = TestUi::new(session()); + for (slot, shown, hidden) in [ + (StyleSlot::Fill, &["shape fill"][..], &["frame fill", "stroke color", "shadow color"][..]), + (StyleSlot::Stroke, &["stroke color"], &["frame fill", "shape fill", "shadow color"]), + ] { + crate::style_picker::open_for_selection(&mut u.app, slot, None); + u.frame(vec![]); + let texts = drawn(&mut u); + assert!(texts.iter().any(|t| t == "Color variables"), "{slot:?}: {texts:?}"); + check(&texts, shown, hidden, &format!("{slot:?}")); + } + } +}