mirror of
https://github.com/storytold/figcraft.git
synced 2026-10-26 19:36:46 -06:00
node.set: refuse auto layout keys off frames, and font styles the family lacks
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Edivan Teixeira
co-authored by
Claude Opus 5.5
Claude Opus 5.5
parent
9dcfe4ecbc
commit
c8ba7b4a4b
@@ -260,6 +260,23 @@ fn ensure_layout(n: &mut Node) -> &mut AutoLayout {
|
||||
n.layout.get_or_insert_with(|| AutoLayout { padding: [0.0; 4], gap: 0.0, ..Default::default() })
|
||||
}
|
||||
|
||||
/// The auto layout of a frame-like node, created when it has none yet. Other nodes can't hold an
|
||||
/// auto layout: an auto layout key on them is refused, not stored where nothing reads it.
|
||||
fn layout_mut<'a>(cmd: &str, k: &str, n: &'a mut Node) -> Result<&'a mut AutoLayout> {
|
||||
if !n.kind.is_frame_like() {
|
||||
return Err(bad(
|
||||
cmd,
|
||||
format!(
|
||||
"`{k}` is an auto layout property, and layer {} “{}” is a {} (only frames, components and instances have auto layout)",
|
||||
n.id,
|
||||
n.name,
|
||||
n.kind.type_name()
|
||||
),
|
||||
));
|
||||
}
|
||||
Ok(ensure_layout(n))
|
||||
}
|
||||
|
||||
/// Apply a property map to one node. `x`/`y` are relative to the parent.
|
||||
pub fn apply(cmd: &str, doc: &mut Document, id: NodeId, props: &Map<String, Value>) -> Result<()> {
|
||||
let mut unknown = Vec::new();
|
||||
@@ -447,56 +464,56 @@ pub fn apply(cmd: &str, doc: &mut Document, id: NodeId, props: &Map<String, Valu
|
||||
}
|
||||
}
|
||||
"gap" | "itemSpacing" | "columnGap" => {
|
||||
ensure_layout(doc.get_mut(id)?).gap = f64_of(v).ok_or_else(|| bad(cmd, format!("`{k}` must be a number")))?
|
||||
layout_mut(cmd, k, doc.get_mut(id)?)?.gap = f64_of(v).ok_or_else(|| bad(cmd, format!("`{k}` must be a number")))?
|
||||
}
|
||||
"crossGap" | "counterAxisSpacing" | "rowGap" => {
|
||||
ensure_layout(doc.get_mut(id)?).cross_gap = f64_of(v).ok_or_else(|| bad(cmd, format!("`{k}` must be a number")))?
|
||||
layout_mut(cmd, k, doc.get_mut(id)?)?.cross_gap = f64_of(v).ok_or_else(|| bad(cmd, format!("`{k}` must be a number")))?
|
||||
}
|
||||
"padding" => {
|
||||
let p = four(v).ok_or_else(|| bad(cmd, "padding: n or [t,r,b,l]"))?;
|
||||
ensure_layout(doc.get_mut(id)?).padding = p.map(|x| x.max(0.0));
|
||||
layout_mut(cmd, k, doc.get_mut(id)?)?.padding = p.map(|x| x.max(0.0));
|
||||
}
|
||||
"paddingTop" | "paddingRight" | "paddingBottom" | "paddingLeft" => {
|
||||
let i = ["paddingTop", "paddingRight", "paddingBottom", "paddingLeft"].iter().position(|x| *x == k).unwrap_or(0);
|
||||
let val = f64_of(v).ok_or_else(|| bad(cmd, format!("`{k}` must be a number")))?.max(0.0);
|
||||
if let Some(slot) = ensure_layout(doc.get_mut(id)?).padding.get_mut(i) {
|
||||
if let Some(slot) = layout_mut(cmd, k, doc.get_mut(id)?)?.padding.get_mut(i) {
|
||||
*slot = val;
|
||||
}
|
||||
}
|
||||
"paddingHorizontal" => {
|
||||
let val = f64_of(v).unwrap_or(0.0).max(0.0);
|
||||
let l = ensure_layout(doc.get_mut(id)?);
|
||||
let l = layout_mut(cmd, k, doc.get_mut(id)?)?;
|
||||
l.padding[1] = val;
|
||||
l.padding[3] = val;
|
||||
}
|
||||
"paddingVertical" => {
|
||||
let val = f64_of(v).unwrap_or(0.0).max(0.0);
|
||||
let l = ensure_layout(doc.get_mut(id)?);
|
||||
let l = layout_mut(cmd, k, doc.get_mut(id)?)?;
|
||||
l.padding[0] = val;
|
||||
l.padding[2] = val;
|
||||
}
|
||||
"mainAlign" | "primaryAxisAlignItems" => {
|
||||
ensure_layout(doc.get_mut(id)?).main_align =
|
||||
layout_mut(cmd, k, doc.get_mut(id)?)?.main_align =
|
||||
enum_of(v).ok_or_else(|| bad(cmd, "mainAlign: min|center|max|spaceBetween|spaceAround|spaceEvenly"))?
|
||||
}
|
||||
"crossAlign" | "counterAxisAlignItems" => {
|
||||
ensure_layout(doc.get_mut(id)?).cross_align = enum_of(v).ok_or_else(|| bad(cmd, "crossAlign: min|center|max|baseline"))?
|
||||
layout_mut(cmd, k, doc.get_mut(id)?)?.cross_align = enum_of(v).ok_or_else(|| bad(cmd, "crossAlign: min|center|max|baseline"))?
|
||||
}
|
||||
"contentAlign" | "counterAxisAlignContent" => {
|
||||
ensure_layout(doc.get_mut(id)?).content_align = enum_of(v).ok_or_else(|| bad(cmd, "contentAlign: auto|spaceBetween"))?
|
||||
layout_mut(cmd, k, doc.get_mut(id)?)?.content_align = enum_of(v).ok_or_else(|| bad(cmd, "contentAlign: auto|spaceBetween"))?
|
||||
}
|
||||
"wrap" => ensure_layout(doc.get_mut(id)?).wrap = v.as_bool().unwrap_or(false),
|
||||
"wrap" => layout_mut(cmd, k, doc.get_mut(id)?)?.wrap = v.as_bool().unwrap_or(false),
|
||||
"gridColumns" | "gridRows" => {
|
||||
let count = f64_of(v).ok_or_else(|| bad(cmd, format!("`{k}` must be a number")))?.clamp(1.0, 1000.0) as u32;
|
||||
let l = ensure_layout(doc.get_mut(id)?);
|
||||
let l = layout_mut(cmd, k, doc.get_mut(id)?)?;
|
||||
// Tracks past the new count go away (new tracks are 1fr).
|
||||
let (n, tracks) =
|
||||
if k == "gridColumns" { (&mut l.grid_columns, &mut l.column_tracks) } else { (&mut l.grid_rows, &mut l.row_tracks) };
|
||||
*n = count;
|
||||
tracks.truncate(count as usize);
|
||||
}
|
||||
"strokesIncluded" => ensure_layout(doc.get_mut(id)?).strokes_included = v.as_bool().unwrap_or(false),
|
||||
"reverseZ" => ensure_layout(doc.get_mut(id)?).reverse_z = v.as_bool().unwrap_or(false),
|
||||
"strokesIncluded" => layout_mut(cmd, k, doc.get_mut(id)?)?.strokes_included = v.as_bool().unwrap_or(false),
|
||||
"reverseZ" => layout_mut(cmd, k, doc.get_mut(id)?)?.reverse_z = v.as_bool().unwrap_or(false),
|
||||
"sizingH" | "layoutSizingHorizontal" => {
|
||||
let s = sizing(v).ok_or_else(|| bad(cmd, "sizing: fixed|hug|fill"))?;
|
||||
set_sizing(cmd, doc, id, s, true)?;
|
||||
@@ -528,7 +545,7 @@ pub fn apply(cmd: &str, doc: &mut Document, id: NodeId, props: &Map<String, Valu
|
||||
"columnTracks" | "gridColumnTracks" | "rowTracks" | "gridRowTracks" => {
|
||||
let a = v.as_array().ok_or_else(|| bad(cmd, format!("`{k}` must be an array of tracks")))?;
|
||||
let tracks = a.iter().map(track_of).collect::<Option<Vec<_>>>().ok_or_else(|| bad(cmd, TRACK_HELP))?;
|
||||
let l = ensure_layout(doc.get_mut(id)?);
|
||||
let l = layout_mut(cmd, k, doc.get_mut(id)?)?;
|
||||
if k.contains("olumn") {
|
||||
l.grid_columns = (tracks.len() as u32).clamp(1, 1000);
|
||||
l.column_tracks = tracks;
|
||||
@@ -852,6 +869,15 @@ pub fn apply_text_style(cmd: &str, t: &mut TextData, start: usize, end: usize, k
|
||||
}
|
||||
"fontStyle" => {
|
||||
let st = string_val(v).ok_or_else(|| bad(cmd, "`fontStyle` must be a string"))?;
|
||||
// A style the family doesn't have would draw in another face without a word; refuse it
|
||||
// and list the family's styles. A family that isn't installed keeps any name (files
|
||||
// from elsewhere name their fonts' styles).
|
||||
for (_, _, span) in t.spans().into_iter().filter(|(a, b, _)| *b > start && *a < end.max(start + 1)) {
|
||||
let styles = figcraft_text::family_styles(&span.font_family);
|
||||
if !styles.is_empty() && !styles.iter().any(|x| x.eq_ignore_ascii_case(&st)) {
|
||||
return Err(bad(cmd, format!("{} has no `{st}` style (it has: {})", span.font_family, styles.join(", "))));
|
||||
}
|
||||
}
|
||||
t.restyle(start, end, |s| s.set_font_style(st.clone()));
|
||||
}
|
||||
"fontWeight" => {
|
||||
|
||||
@@ -1580,6 +1580,8 @@ mod menu_gaps_tests;
|
||||
#[cfg(test)]
|
||||
mod nested_override_tests;
|
||||
#[cfg(test)]
|
||||
mod node_set_refusals_tests;
|
||||
#[cfg(test)]
|
||||
mod online_tests;
|
||||
#[cfg(test)]
|
||||
mod open_issues_tests;
|
||||
|
||||
@@ -0,0 +1,55 @@
|
||||
//! `node.set` refuses what can't apply instead of storing it where nothing reads it: auto layout
|
||||
//! keys on layers that can't have auto layout, and a `fontStyle` the font family doesn't have.
|
||||
|
||||
use serde_json::{Value, json};
|
||||
|
||||
use crate::{EngineError, Session};
|
||||
|
||||
fn ex(s: &mut Session, cmd: &str, p: Value) -> Value {
|
||||
s.execute(cmd, &p).unwrap_or_else(|e| panic!("{cmd}: {e}"))
|
||||
}
|
||||
|
||||
fn id(v: &Value) -> crate::NodeId {
|
||||
crate::params::id_of(v).unwrap()
|
||||
}
|
||||
|
||||
fn bad_msg(r: crate::Result<Value>) -> String {
|
||||
match r {
|
||||
Err(EngineError::BadParams { msg, .. }) => msg,
|
||||
other => panic!("expected BadParams, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn auto_layout_keys_on_layers_without_auto_layout_are_refused() {
|
||||
let mut s = Session::new();
|
||||
let t = id(&ex(&mut s, "node.create", json!({"type": "text", "text": "Hi"}))["id"]);
|
||||
let r = id(&ex(&mut s, "node.create", json!({"type": "rectangle"}))["id"]);
|
||||
for (target, key, value) in [(t, "gap", json!(8)), (r, "padding", json!(4)), (t, "mainAlign", json!("center")), (r, "wrap", json!(true))] {
|
||||
let msg = bad_msg(s.execute("node.set", &json!({"ids": [target], "props": {key: value}})));
|
||||
assert!(msg.contains(key) && msg.contains("auto layout"), "{msg}");
|
||||
assert!(s.doc().unwrap().node(target).unwrap().layout.is_none(), "no hidden auto layout on {target}");
|
||||
}
|
||||
// Frames still take them, with or without a layout mode first.
|
||||
let f = id(&ex(&mut s, "node.create", json!({"type": "frame"}))["id"]);
|
||||
ex(&mut s, "node.set", json!({"ids": [f], "props": {"gap": 8, "padding": 4}}));
|
||||
assert_eq!(s.doc().unwrap().node(f).unwrap().layout.as_ref().map(|l| l.gap), Some(8.0));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_font_style_the_family_lacks_is_refused_with_the_styles_it_has() {
|
||||
figcraft_text::scan_system_fonts();
|
||||
// Any installed family with a known list of styles will do (CI machines differ).
|
||||
let Some((family, styles)) =
|
||||
figcraft_text::family_names().into_iter().map(|f| (f.clone(), figcraft_text::family_styles(&f))).find(|(_, st)| !st.is_empty())
|
||||
else {
|
||||
return;
|
||||
};
|
||||
let mut s = Session::new();
|
||||
let t = id(&ex(&mut s, "node.create", json!({"type": "text", "text": "Hi", "props": {"fontFamily": family}}))["id"]);
|
||||
let msg = bad_msg(s.execute("node.set", &json!({"ids": [t], "props": {"fontStyle": "Definitely Not A Style"}})));
|
||||
assert!(msg.contains(&family) && msg.contains(&styles[0]), "{msg}");
|
||||
ex(&mut s, "node.set", json!({"ids": [t], "props": {"fontStyle": styles[0]}}));
|
||||
// A family that isn't installed keeps any style name (files from elsewhere name theirs).
|
||||
ex(&mut s, "node.set", json!({"ids": [t], "props": {"fontFamily": "No Such Family 7f3a", "fontStyle": "Hairline"}}));
|
||||
}
|
||||
@@ -163,7 +163,7 @@ pub fn tool_definitions() -> Vec<Value> {
|
||||
tool(
|
||||
"set_properties",
|
||||
"Set properties",
|
||||
"Set properties on one or more layers as one undo step (engine `node.set`). Keys: name, visible, locked, opacity (0-1), blend, x, y (relative to the parent frame; groups don't count), width, height, rotation (degrees), fill (colour) / fills (array of paints: \"#hex\" or {\"type\":\"solid\",\"color\":\"#hex\"} or {\"type\":\"linearGradient\",\"stops\":[\"#hex\",\"#hex\"]} or stops [{\"position\":0..1,\"color\":\"#hex\"}], optional from/to handles in the unit box; radialGradient too), stroke / strokes, strokeWeight, strokeAlign (inside|center|outside), cornerRadius (n or [tl,tr,br,bl]), effects (array, e.g. {\"type\":\"dropShadow\",\"color\":\"#00000040\",\"offset\":[0,4],\"radius\":8}), clip, layout (null|horizontal|vertical|grid), gap, padding (n or [t,r,b,l]), mainAlign (min|center|max|spaceBetween|spaceAround|spaceEvenly), crossAlign, wrap, contentAlign (auto|spaceBetween: wrapped lines), sizingH / sizingV (fixed|hug|fill), constraints {horizontal, vertical}, characters, fontFamily, fontStyle, fontSize, lineHeight, letterSpacing, textAlign, textAutoResize. Run list_commands with filter \"node.set\" for the full list. Unknown keys are an error. Returns the updated layers.",
|
||||
"Set properties on one or more layers as one undo step (engine `node.set`). Keys: name, visible, locked, opacity (0-1), blend, x, y (relative to the parent frame; groups don't count), width, height, rotation (degrees), fill (colour) / fills (array of paints: \"#hex\" or {\"type\":\"solid\",\"color\":\"#hex\"} or {\"type\":\"linearGradient\",\"stops\":[\"#hex\",\"#hex\"]} or stops [{\"position\":0..1,\"color\":\"#hex\"}], optional from/to handles in the unit box; radialGradient too), stroke / strokes, strokeWeight, strokeAlign (inside|center|outside), cornerRadius (n or [tl,tr,br,bl]), effects (array, e.g. {\"type\":\"dropShadow\",\"color\":\"#00000040\",\"offset\":[0,4],\"radius\":8}), clip, layout (null|horizontal|vertical|grid), gap, padding (n or [t,r,b,l]), mainAlign (min|center|max|spaceBetween|spaceAround|spaceEvenly), crossAlign, wrap, contentAlign (auto|spaceBetween: wrapped lines), sizingH / sizingV (fixed|hug|fill), constraints {horizontal, vertical}, characters, fontFamily, fontStyle, fontSize, lineHeight, letterSpacing, textAlign, textAutoResize. Auto layout keys (gap, padding, mainAlign, crossAlign, wrap, …) apply to frames, components and instances only; on other layers they are an error, as is a fontStyle the font family doesn't have (the error lists its styles). Run list_commands with filter \"node.set\" for the full list. Unknown keys are an error. Returns the updated layers.",
|
||||
obj(
|
||||
json!({"ids": ids_schema("Layer ids (default: the selection)"), "props": {"type": "object", "description": "Property values to set"}}),
|
||||
&["props"],
|
||||
|
||||
Reference in New Issue
Block a user