From 77f60a4efb7a5e241b5d8412bd4532fae8317508 Mon Sep 17 00:00:00 2001 From: Fini Date: Thu, 2 Jul 2026 21:21:20 +0800 Subject: [PATCH] =?UTF-8?q?fix(editor-core):=20ref=20slot-children=20fallb?= =?UTF-8?q?ack=20=E2=80=94=20instance=20inline=20children=20fill=20empty-m?= =?UTF-8?q?aster=20shells=20(Pencil=20slot-component=20pattern)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pencil's slot-component pattern uses an empty styled master (slot:[...] + zero children) with the instance supplying content via its own inline children. expand_ref only populated children from the master, dropping instance content → empty shell → blank white circle in converted templates. Now falls back to instance children (remapped) when the master has none. Fixes converted-template stat-card rendering + composed-ref content. +2 ref_resolve tests +1 variables_resolve nested-token regression guard. --- crates/op-editor-core/src/ref_resolve.rs | 103 +++++++++++++++++- .../op-editor-core/src/variables_resolve.rs | 28 +++++ 2 files changed, 130 insertions(+), 1 deletion(-) diff --git a/crates/op-editor-core/src/ref_resolve.rs b/crates/op-editor-core/src/ref_resolve.rs index a693fd25a..017a7d125 100644 --- a/crates/op-editor-core/src/ref_resolve.rs +++ b/crates/op-editor-core/src/ref_resolve.rs @@ -152,7 +152,20 @@ fn expand_ref( .and_then(Value::as_str) .unwrap_or_default(); let overrides = ref_map.get("descendants").and_then(Value::as_object); - if let Some(Value::Array(children)) = component_map.get("children") { + // Pencil's slot-component pattern: the master is an empty styled + // shell carrying a `slot:[…]` id list, and each instance supplies + // its real content as its OWN inline `children`. When the master + // has no children to render, fall back to the instance's inline + // slot content so the shell isn't rendered empty (an empty + // `fit_content` frame collapses to its padding box → a white + // circle for a high-`cornerRadius` card). The master-has-children + // branch is unchanged, so the 115 normal refs and the master-with- + // default-content (ambiguous) refs keep TS-parity behaviour. + let children_source = match component_map.get("children") { + Some(Value::Array(children)) if !children.is_empty() => Some(children), + _ => ref_map.get("children").and_then(Value::as_array), + }; + if let Some(children) = children_source { let remapped: Vec = children .iter() .map(|child| remap_ids(child, ref_id, overrides)) @@ -475,4 +488,92 @@ mod tests { "the cyclic inner ref is dropped" ); } + + // Pencil slot-component pattern: the master is an empty styled + // shell (carrying a `slot` id list, no children of its own) and + // the instance supplies its real content as its OWN inline + // `children`. Without the fallback the expanded instance renders + // empty (a high-`cornerRadius` `fit_content` card collapses to a + // white circle). + const DOC_WITH_SLOT_INSTANCE: &str = r##"{ + "version":"0.8.0", + "children":[ + {"type":"frame","id":"cardMaster","name":"Card/Default","reusable":true, + "width":"fit_content","height":"fit_content","cornerRadius":24,"padding":16, + "slot":["regionA","regionB"], + "fill":[{"type":"solid","color":"#f5f5f5"}]}, + {"type":"ref","id":"card1","ref":"cardMaster","x":40,"y":40, + "children":[ + {"type":"text","id":"label","name":"Label","content":"Revenue"}, + {"type":"text","id":"value","name":"Value","content":"$12.4k"} + ]} + ] + }"##; + + #[test] + fn empty_master_falls_back_to_instance_slot_children() { + let doc = doc_from(DOC_WITH_SLOT_INSTANCE); + let resolved = resolve_refs_for_canvas(&doc); + assert_eq!(resolved.children.len(), 2); + let PenNode::Frame(card) = &resolved.children[1] else { + panic!("slot instance expands to the master's Frame type"); + }; + assert_eq!(card.base.id, "card1"); + // The master shell's style is preserved (still the white card). + assert_eq!(card.base.x, Some(40.0)); + let children = card + .children + .as_ref() + .expect("instance slot content fills the empty master shell"); + assert_eq!( + children.len(), + 2, + "instance's own inline children render instead of an empty shell" + ); + let PenNode::Text(label) = &children[0] else { + panic!("first slot child survives"); + }; + // Slot children are remapped into the instance's virtual id + // space exactly like master children would be. + assert_eq!(label.base.id, "card1__label"); + assert_eq!( + label.content, + jian_ops_schema::node::text::TextContent::Plain("Revenue".into()) + ); + } + + #[test] + fn master_children_win_over_instance_children_no_regression() { + // A master WITH its own children keeps TS-parity behaviour: + // the instance's inline children never replace the master + // subtree (this guards the 115 normal + ambiguous refs against + // the slot-fallback path). + let doc = doc_from( + r##"{ + "version":"0.8.0", + "children":[ + {"type":"frame","id":"master","name":"Master","reusable":true, + "width":200,"height":100, + "children":[{"type":"text","id":"masterText","name":"M","content":"FromMaster"}]}, + {"type":"ref","id":"inst","ref":"master","x":10,"y":10, + "children":[{"type":"text","id":"instText","name":"I","content":"FromInstance"}]} + ] + }"##, + ); + let resolved = resolve_refs_for_canvas(&doc); + let PenNode::Frame(inst) = &resolved.children[1] else { + panic!("frame"); + }; + let children = inst.children.as_ref().unwrap(); + assert_eq!(children.len(), 1); + let PenNode::Text(text) = &children[0] else { + panic!("master text wins"); + }; + assert_eq!(text.base.id, "inst__masterText"); + assert_eq!( + text.content, + jian_ops_schema::node::text::TextContent::Plain("FromMaster".into()), + "instance inline children do not override a master that has children" + ); + } } diff --git a/crates/op-editor-core/src/variables_resolve.rs b/crates/op-editor-core/src/variables_resolve.rs index 51613b06a..56a56ad1c 100644 --- a/crates/op-editor-core/src/variables_resolve.rs +++ b/crates/op-editor-core/src/variables_resolve.rs @@ -848,6 +848,34 @@ mod tests { assert_eq!(resolve_color_ref("$not-a-token", None, &theme_light), None); } + #[test] + fn nested_namespaced_ref_resolves_against_literal_keyed_var() { + // Pencil emits namespaced tokens like `$surface/surface` and + // keys its variable table with the same literal nested string. + // `strip_prefix('$')` + a direct `vars.get("surface/surface")` + // hit already resolves them — no slash special-casing needed. + // This is the exact fill the converted Pencil cards carry, so + // it guards the resolver against a future nested-token regress. + let vars = vars_with( + "surface/surface", + color_var(VariableValue::Scalar(VariableScalar::Str("#f5f5f5".into()))), + ); + let theme = Theme::new(); + assert_eq!( + resolve_color_ref("$surface/surface", Some(&vars), &theme), + Some("#f5f5f5".to_string()) + ); + // A flat token sharing no slash still resolves unchanged. + let flat = vars_with( + "accent", + color_var(VariableValue::Scalar(VariableScalar::Str("#2563eb".into()))), + ); + assert_eq!( + resolve_color_ref("$accent", Some(&flat), &theme), + Some("#2563eb".to_string()) + ); + } + #[test] fn circular_ref_is_guarded() { let vars = vars_with(