mirror of
https://github.com/storytold/figcraft.git
synced 2026-10-24 02:16:56 -06:00
M3: explicit edits of bound scalar fields detach their variable (#809)
An explicit edit of a bound width, height, opacity, gap, padding, corner radius, stroke weight, arc or text field reported ok, but normalize wrote the variable's value straight back. Now the edit sticks and that field's binding is dropped, as paint edits already do. - `styles::bound_before` / `detach_edited`: compare each layer-level bound field's stored value before and after the edit (`bound_field`), so keys written unchanged keep their bindings and fields moved along with another (proportions, min/max) lose theirs. A binding the edit itself made (`boundVariables`) stays. List and component property bindings are left to their own rules. - Hooked into node.set, transform.resize, object.opacityKeys, text.style and `Session::preview` (canvas drags of the selected layers). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
a0d1162976
commit
6a163721dc
@@ -528,6 +528,7 @@ pub(crate) fn resize_cmd(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
let (col, row) = string(p, "anchor").and_then(|a| crate::tools::anchor_of(&a)).unwrap_or((0, 0));
|
||||
let (ax, ay) = (f64::from(col) / 2.0, f64::from(row) / 2.0);
|
||||
let doc = s.doc_mut()?;
|
||||
let before = super::styles::bound_before(doc, &ids);
|
||||
for i in ids {
|
||||
let n = doc.get(i)?;
|
||||
let (ow, oh) = (n.width, n.height);
|
||||
@@ -551,6 +552,7 @@ pub(crate) fn resize_cmd(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
}
|
||||
super::props::restore_worlds(doc, &kept)?;
|
||||
}
|
||||
super::styles::detach_edited(doc, &before);
|
||||
ok()
|
||||
}
|
||||
|
||||
|
||||
@@ -931,9 +931,13 @@ pub fn opacity_from_digits(digits: &str) -> Option<f32> {
|
||||
fn opacity_keys(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
let digits = req_string("object.opacityKeys", p, "digits")?;
|
||||
let o = opacity_from_digits(&digits).ok_or_else(|| bad("object.opacityKeys", "`digits` must be one or two digits"))?;
|
||||
for i in req_targets("object.opacityKeys", s, p)? {
|
||||
s.doc_mut()?.get_mut(i)?.opacity = o;
|
||||
let ids = req_targets("object.opacityKeys", s, p)?;
|
||||
let doc = s.doc_mut()?;
|
||||
let before = super::styles::bound_before(doc, &ids);
|
||||
for i in ids {
|
||||
doc.get_mut(i)?.opacity = o;
|
||||
}
|
||||
super::styles::detach_edited(doc, &before);
|
||||
Ok(json!({"opacity": o}))
|
||||
}
|
||||
|
||||
@@ -1181,10 +1185,12 @@ fn set(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
}
|
||||
}
|
||||
let doc = s.doc_mut()?;
|
||||
let before = super::styles::bound_before(doc, &ids);
|
||||
for i in ids {
|
||||
let props = if skipped.contains(&i) { &without_layout } else { &props };
|
||||
super::props::apply("node.set", doc, i, props)?;
|
||||
}
|
||||
super::styles::detach_edited(doc, &before);
|
||||
if let Some(o) = pivot {
|
||||
for (i, q) in pinned {
|
||||
let d = o - doc.world_transform(i) * q;
|
||||
|
||||
@@ -950,6 +950,104 @@ pub fn apply_bindings_to(doc: &mut Document, only: Option<&std::collections::BTr
|
||||
apply_bindings_matching(doc, only, |_| true);
|
||||
}
|
||||
|
||||
/// The layers `ids` before an explicit edit, for [`detach_edited`] (only those with bindings).
|
||||
pub(crate) fn bound_before(doc: &Document, ids: &[NodeId]) -> Vec<std::sync::Arc<Node>> {
|
||||
ids.iter().filter_map(|i| doc.node_arc(*i)).filter(|n| !n.bound.is_empty()).cloned().collect()
|
||||
}
|
||||
|
||||
/// An explicit edit sticks: each layer's own bound field (not a list item's) whose value the edit
|
||||
/// changed drops its binding, which would otherwise write the variable's value back (#809).
|
||||
/// Compared by value, so keys written unchanged keep their bindings and fields moved along with
|
||||
/// another (proportions, min/max) lose theirs.
|
||||
pub(crate) fn detach_edited(doc: &mut Document, before: &[std::sync::Arc<Node>]) {
|
||||
for old in before {
|
||||
let Some(n) = doc.node(old.id) else { continue };
|
||||
let edited: Vec<String> = n
|
||||
.bound
|
||||
.keys()
|
||||
// A binding the edit itself made (`boundVariables`) stays.
|
||||
.filter(|f| old.bound.get(*f) == n.bound.get(*f))
|
||||
.filter(|f| super::lists::list_binding(f).is_none() && !f.starts_with(crate::components::PROP_BINDING))
|
||||
.filter(|f| bound_field(old, f) != bound_field(n, f))
|
||||
.cloned()
|
||||
.collect();
|
||||
if edited.is_empty() {
|
||||
continue;
|
||||
}
|
||||
if let Ok(n) = doc.get_mut(old.id) {
|
||||
for f in edited {
|
||||
n.bound.remove(&f);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// The stored value a layer-level binding writes, comparable (`None`: the layer has no such field).
|
||||
fn bound_field(n: &Node, field: &str) -> Option<String> {
|
||||
let text = |f: &dyn Fn(&TextStyle) -> String| match &n.kind {
|
||||
NodeKind::Text(t) => Some(std::iter::once(&t.style).chain(t.runs.iter().map(|r| &r.style)).map(f).collect::<Vec<_>>().join(",")),
|
||||
_ => None,
|
||||
};
|
||||
let layout = n.layout.as_ref();
|
||||
let cl = &n.child_layout;
|
||||
let pad = |i: usize| layout.and_then(|l| l.padding.get(i).copied());
|
||||
Some(match field {
|
||||
"width" => format!("{}", n.width),
|
||||
"height" => format!("{}", n.height),
|
||||
"minWidth" => format!("{:?}", cl.min_w),
|
||||
"maxWidth" => format!("{:?}", cl.max_w),
|
||||
"minHeight" => format!("{:?}", cl.min_h),
|
||||
"maxHeight" => format!("{:?}", cl.max_h),
|
||||
"opacity" => format!("{}", n.opacity),
|
||||
"visible" => format!("{}", n.visible),
|
||||
"gap" | "columnGap" => format!("{:?}", layout.map(|l| l.gap)),
|
||||
"crossGap" | "rowGap" => format!("{:?}", layout.map(|l| l.cross_gap)),
|
||||
"padding" => format!("{:?}", layout.map(|l| l.padding)),
|
||||
"paddingTop" => format!("{:?}", pad(0)),
|
||||
"paddingRight" => format!("{:?}", pad(1)),
|
||||
"paddingBottom" => format!("{:?}", pad(2)),
|
||||
"paddingLeft" => format!("{:?}", pad(3)),
|
||||
"paddingHorizontal" => format!("{:?}", (pad(1), pad(3))),
|
||||
"paddingVertical" => format!("{:?}", (pad(0), pad(2))),
|
||||
"cornerRadius" => format!("{:?}", n.corners.radii),
|
||||
"topLeftRadius" => format!("{:?}", n.corners.radii.first()),
|
||||
"topRightRadius" => format!("{:?}", n.corners.radii.get(1)),
|
||||
"bottomRightRadius" => format!("{:?}", n.corners.radii.get(2)),
|
||||
"bottomLeftRadius" => format!("{:?}", n.corners.radii.get(3)),
|
||||
"strokeWeight" => format!("{:?}", (n.stroke.weight, n.stroke.sides)),
|
||||
"strokeTopWeight" | "strokeRightWeight" | "strokeBottomWeight" | "strokeLeftWeight" => format!("{:?}", n.stroke.sides),
|
||||
"arcStart" | "arcSweep" | "arcInner" => match &n.kind {
|
||||
NodeKind::Ellipse { start, sweep, inner } => format!(
|
||||
"{}",
|
||||
if field == "arcStart" {
|
||||
start
|
||||
} else if field == "arcSweep" {
|
||||
sweep
|
||||
} else {
|
||||
inner
|
||||
}
|
||||
),
|
||||
_ => return None,
|
||||
},
|
||||
"characters" => match &n.kind {
|
||||
NodeKind::Text(t) => t.characters.clone(),
|
||||
_ => return None,
|
||||
},
|
||||
"paragraphSpacing" | "paragraphIndent" => match &n.kind {
|
||||
NodeKind::Text(t) => format!("{}", if field == "paragraphSpacing" { t.paragraph_spacing } else { t.paragraph_indent }),
|
||||
_ => return None,
|
||||
},
|
||||
"fontFamily" => text(&|s| s.font_family.clone())?,
|
||||
"fontStyle" => text(&|s| s.font_style.clone())?,
|
||||
"fontWeight" => text(&|s| s.weight().to_string())?,
|
||||
"fontSize" => text(&|s| s.font_size.to_string())?,
|
||||
"lineHeight" => text(&|s| format!("{:?}", s.line_height))?,
|
||||
"letterSpacing" => text(&|s| format!("{:?}", s.letter_spacing))?,
|
||||
// shortcut: other layer-level fields keep their binding on edits; add a reader when one is bound in practice.
|
||||
_ => return None,
|
||||
})
|
||||
}
|
||||
|
||||
/// Imported files (`.fig`, REST) keep a solid paint's alpha in its opacity, a bound paint's too,
|
||||
/// while the bound colour brings its own alpha: take the variable's alpha back out of the opacity
|
||||
/// (styles' paints and layers') before the bindings are first resolved, so it applies once.
|
||||
|
||||
@@ -364,6 +364,7 @@ pub fn specs() -> Vec<CommandSpec> {
|
||||
let ids = req_targets("text.style", s, p)?;
|
||||
let m = p.as_object().cloned().unwrap_or_default();
|
||||
let doc = s.doc_mut()?;
|
||||
let before = super::styles::bound_before(doc, &ids);
|
||||
for i in ids {
|
||||
let n = doc.get_mut(i)?;
|
||||
let NodeKind::Text(t) = &mut n.kind else { return Err(bad("text.style", format!("{i} is not text"))) };
|
||||
@@ -377,6 +378,7 @@ pub fn specs() -> Vec<CommandSpec> {
|
||||
super::props::apply_text_style("text.style", t, a, b, k, v)?;
|
||||
}
|
||||
}
|
||||
super::styles::detach_edited(doc, &before);
|
||||
ok()
|
||||
}
|
||||
),
|
||||
|
||||
@@ -1098,8 +1098,11 @@ impl Session {
|
||||
let Some(it) = ds.interaction.clone() else { return Err(EngineError::Other("no gesture in progress".into())) };
|
||||
let mut doc = (*it.doc).clone();
|
||||
let mut sel = it.selection.clone();
|
||||
// What the gesture changes on the selected layers is an explicit edit.
|
||||
let before = cmd::styles::bound_before(&doc, &it.selection);
|
||||
let res = f(&mut doc, &mut sel);
|
||||
if res.is_ok() {
|
||||
cmd::styles::detach_edited(&mut doc, &before);
|
||||
normalize(&mut doc, &it.doc);
|
||||
// The same preview again (a handle held still) keeps the document: a new one would
|
||||
// re-render the canvas for nothing.
|
||||
|
||||
@@ -365,3 +365,57 @@ fn number_variables_are_pixels_and_edits_are_one_undo_step() {
|
||||
s.undo().unwrap();
|
||||
assert_eq!(lh(&s), LineHeight::Pixels(20.0), "the value and the bound layer go back together");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn explicit_edits_of_bound_fields_stick_and_detach_that_binding() {
|
||||
// #809: an edit of a bound field detaches its binding (as a paint edit does) instead of
|
||||
// the variable's value coming back; bindings whose value the edit kept stay.
|
||||
let mut s = Session::new();
|
||||
run(&mut s, "variable.createCollection", json!({"name": "C"}));
|
||||
let w = vid(run(&mut s, "variable.create", json!({"collection": "C", "name": "w", "type": "number", "value": 48})));
|
||||
let o = vid(run(&mut s, "variable.create", json!({"collection": "C", "name": "o", "type": "number", "value": 40})));
|
||||
let r = rect(&mut s, None);
|
||||
for (field, v) in [("width", &w), ("height", &w), ("opacity", &o)] {
|
||||
run(&mut s, "variable.bind", json!({"ids": [r], "field": field, "variable": v}));
|
||||
}
|
||||
assert_eq!((node(&s, r).width, node(&s, r).opacity), (48.0, 0.4));
|
||||
|
||||
run(&mut s, "node.set", json!({"ids": [r], "width": 100, "opacity": 0.4}));
|
||||
let n = node(&s, r);
|
||||
assert_eq!(n.width, 100.0);
|
||||
assert!(!n.bound.contains_key("width"));
|
||||
assert!(n.bound.contains_key("height") && n.bound.contains_key("opacity"), "unchanged values keep their bindings");
|
||||
|
||||
run(&mut s, "transform.resize", json!({"ids": [r], "height": 30}));
|
||||
assert_eq!(node(&s, r).height, 30.0);
|
||||
assert!(!node(&s, r).bound.contains_key("height"));
|
||||
|
||||
run(&mut s, "object.opacityKeys", json!({"ids": [r], "digits": "9"}));
|
||||
assert_eq!(node(&s, r).opacity, 0.9);
|
||||
assert!(node(&s, r).bound.is_empty());
|
||||
|
||||
// A value set with its binding in the same call keeps the new binding.
|
||||
run(&mut s, "node.set", json!({"ids": [r], "width": 5, "boundVariables": {"width": w}}));
|
||||
assert_eq!((node(&s, r).width, node(&s, r).bound.get("width")), (48.0, Some(&w)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_gesture_or_a_text_style_on_a_bound_field_detaches_it() {
|
||||
let mut s = Session::new();
|
||||
run(&mut s, "variable.createCollection", json!({"name": "C"}));
|
||||
let v = vid(run(&mut s, "variable.create", json!({"collection": "C", "name": "r", "type": "number", "value": 8})));
|
||||
let r = rect(&mut s, None);
|
||||
run(&mut s, "variable.bind", json!({"ids": [r], "field": "cornerRadius", "variable": v}));
|
||||
s.begin_interaction("Edit").unwrap();
|
||||
let props = serde_json::Map::from_iter([("cornerRadius".to_string(), json!(2))]);
|
||||
s.preview(|d, _| crate::cmd::props::apply("node.set", d, r, &props)).unwrap();
|
||||
s.commit_interaction().unwrap();
|
||||
assert_eq!(node(&s, r).corners.radii, [2.0; 4]);
|
||||
assert!(node(&s, r).bound.is_empty());
|
||||
|
||||
let t = id(run(&mut s, "text.create", json!({"x": 0, "y": 0, "text": "Hi"})));
|
||||
run(&mut s, "variable.bind", json!({"ids": [t], "field": "fontSize", "variable": v}));
|
||||
run(&mut s, "text.style", json!({"id": t, "fontSize": 20}));
|
||||
assert!(node(&s, t).bound.is_empty());
|
||||
assert_eq!(node(&s, t).kind.text().unwrap().style.font_size, 20.0);
|
||||
}
|
||||
|
||||
+4
-1
@@ -267,7 +267,10 @@ Or in a project's `.mcp.json`:
|
||||
`style.duplicate {style}` adds "<name> copy" right below, `style.renameFolder {folder, name}`.
|
||||
- **Variables**: `variable.bind` refuses a variable of a type the field doesn't take (a colour on
|
||||
`width`); scopes only filter pickers. `variable.delete` / `variable.deleteCollection` unbind the
|
||||
layers using them (they keep their last value) and give aliases to them their values.
|
||||
layers using them (they keep their last value) and give aliases to them their values. An
|
||||
explicit edit that changes a bound field's value (`node.set`, `transform.resize`,
|
||||
`object.opacityKeys`, `text.style`, a canvas drag) detaches that field's variable; fields the edit
|
||||
left as they were stay bound.
|
||||
`variable.addMode {from}` duplicates a mode, `variable.moveMode {collection, mode, to}` reorders
|
||||
(index 0 is the default mode, which layers on Auto follow). `variable.update {codeSyntax: {WEB?,
|
||||
iOS?, ANDROID?}}` sets code names (`""` clears); generated CSS uses the Web one.
|
||||
|
||||
@@ -638,6 +638,7 @@ Carried over from the ROADMAP's "Shipped so far" (2026-10-10). The test count th
|
||||
|
||||
| Date | Change | Summary |
|
||||
|---|---|---|
|
||||
| 2026-10-19 | minor | Bound scalar fields (M3, #809): an explicit edit that changes a bound width, height, min/max size, opacity, visibility, gap, padding, corner radius, stroke weight, arc or text field (`node.set`, `transform.resize`, `object.opacityKeys`, `text.style`, canvas drags) detaches that binding instead of the variable's value coming back; compared by value, so fields the edit left alone stay bound. Numbers unchanged |
|
||||
| 2026-10-17 | minor | Menu breadth re-run with `cargo xtask parity`: 212 / 212 (100%; the catalog gained View ▸ Full screen with #227 on 2026-10-14), 561 commands. Rows for the 2026-10-17 landings: import/export (area 14) and the file formats dimension name the `.fig` import's new fidelity (instance identity, overrides by key, text style precedence, grid tracks, slots, connector labels, per-point joins, text on a path, image adjustments, list start numbers, vertical trim), budgets from the file's size, the import report, a `.fig` copy from Save as and paste from another design app; the localization evidence takes the re-measured catalog counts (2,272 entries, drafts at 95–99%, Indonesian and Czech drafts, Swedish and Turkish catalogs); the Automation line adds control requests answered while the window is minimised (#720); batch #753's Selection colors, smart-selection spacing, Present tab and auto layout on empty frames are in ui-parity, prototyping-parity and auto-layout-parity. Numbers unchanged: every `.fig` and paste change is checked on synthetic files or clipboard HTML made from our own export, and real files are what gap 1's blocker and the file-exchange discounts wait for; the UI changes (menus, the Actions palette, editor fixes) are not compared side by side, so UI/UX stays at 58%; the localization grade and hours stay (Indonesian's draft is offset by the drafts' new gaps) |
|
||||
| 2026-10-17 | minor | Automation roots (M8, #715, #481): `figcraft-cli mcp --automation-read-root DIR --automation-write-root DIR` (each repeatable, existing folders) and the desktop app with the same flags confine every file a command reads to the read and write roots and every file it writes to the write roots; paths are resolved (links followed, `..`, Windows `\\?\` and UNC paths, case) and compared by whole folder names, anything else is an ordinary error. The engine reaches the file system through one module (enforced by `crates/engine/clippy.toml`), and a sweep runs every command with paths outside the roots. `--attach` refuses the flags (the app confines itself); `ui.screenshot` and `ui.render` return PNG data without a `path`, so the attached MCP server writes no temporary files. Numbers unchanged. |
|
||||
| 2026-10-17 | minor | Number variables on text content (M3, #629): the Typography section's variable picker also lists number variables for a text layer's content, as the engine already accepted (the text shows the number, e.g. 42.5, and follows the variable). Other text fields keep their own types. Numbers unchanged. |
|
||||
|
||||
Reference in New Issue
Block a user