From b5a315a7482002343fcb2a4411b4ea6c3b82d2f3 Mon Sep 17 00:00:00 2001 From: RiceDee <345114586+RiceDee@users.noreply.github.com> Date: Tue, 20 Oct 2026 16:32:16 +1100 Subject: [PATCH] #903: name a new component set from its variants' path, not their first property A component set took its name from the first variant's name up to the first '/' or '=', so variants named "State=Default, Size=Small" made a set called "State". The set now takes the path before the last '/' ("Button/State=Hover" -> "Button", "Icons/Arrow/Dir=Up" -> "Icons/Arrow"), keeps a plain name as is ("Button"), and otherwise gets the next numbered default ("Component 1"), as a new component does. --- crates/engine/src/cmd/component.rs | 15 +++++---- .../engine/src/components_behaviour_tests.rs | 32 +++++++++++++++++++ 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/cmd/component.rs b/crates/engine/src/cmd/component.rs index bf99c592..47ea804c 100644 --- a/crates/engine/src/cmd/component.rs +++ b/crates/engine/src/cmd/component.rs @@ -330,6 +330,14 @@ fn create(s: &mut Session, p: &Value) -> Result { Ok(json!({"id": target})) } +/// The name a new component set takes from its first variant: the path before the last '/' +/// ("Button/State=Hover" → "Button"), or a plain name as is ("Button"). A bare "prop=value" name +/// ("State=Default, Size=Small") names no set. +fn set_name_from(variant: &str) -> Option { + let base = variant.rsplit_once('/').map_or(variant, |(path, _)| path).trim(); + (!base.is_empty() && !base.contains('=')).then(|| base.to_string()) +} + /// Wrap components in a new component set (named after the first) placed around them. fn wrap_in_set(doc: &mut Document, ids: &[NodeId]) -> Result { let cmd = "component.combineVariants"; @@ -341,12 +349,7 @@ fn wrap_in_set(doc: &mut Document, ids: &[NodeId]) -> Result { let bounds = doc.bounds_of(ids).unwrap_or_default(); let pw = doc.world_transform(parent); let local = figcraft_geom::transform_rect_bbox(pw.inverse(), bounds.inflate(20.0, 20.0)); - let base = ids - .first() - .and_then(|i| doc.node(*i)) - .map(|n| n.name.split(['/', '=']).next().unwrap_or("Component").trim().to_string()) - .filter(|n| !n.is_empty() && !n.contains(',')) - .unwrap_or_else(|| "Component".into()); + let base = ids.first().and_then(|i| doc.node(*i)).and_then(|n| set_name_from(&n.name)).unwrap_or_else(|| doc.next_name("Component")); let set = doc.alloc_id(); let mut n = Node::new(set, NodeKind::ComponentSet { description: String::new() }, base, local.width(), local.height()); n.transform = figcraft_geom::Affine::translate((local.x0, local.y0)); diff --git a/crates/engine/src/components_behaviour_tests.rs b/crates/engine/src/components_behaviour_tests.rs index ff78f73b..7a5c1f90 100644 --- a/crates/engine/src/components_behaviour_tests.rs +++ b/crates/engine/src/components_behaviour_tests.rs @@ -879,3 +879,35 @@ fn auto_layout_instances_follow_their_main() { assert_eq!(node(&s, i).child_layout.min_w, Some(80.0)); healthy(&s); } + +#[test] +fn a_new_component_set_is_named_after_its_first_variant() { + // "Button/State=Hover" groups under "Button": the set takes the path before the last '/'. + // A bare property name ("State=Default, Size=Small") names no set, and neither does an empty + // name: the set gets the next numbered default, like a new component. A plain name is kept. + let rows = [ + ("Button/State=Hover", "Button", "slash-prefixed: the path before the last '/'"), + ("Icons/Arrow/Dir=Up", "Icons/Arrow", "nested path: everything before the last '/'"), + ("State=Default, Size=Small", "Component 1", "bare prop=value: not \"State\""), + ("", "Component 1", "empty name: the numbered default"), + ]; + for (name, expected, why) in rows { + let mut s = Session::new(); + let f = idv(&ex(&mut s, "frame.create", json!({"x": 0, "y": 0, "width": 40, "height": 20}))); + ex(&mut s, "node.set", json!({"id": f, "props": {"name": name}})); + let c = idv(&ex(&mut s, "component.create", json!({"ids": [f]}))); + assert_eq!(node(&s, c).name, name, "{why}: the variant keeps its name"); + let set = idv(&ex(&mut s, "component.combineVariants", json!({"ids": [c]}))); + assert_eq!(node(&s, set).name, expected, "{why}"); + healthy(&s); + } + + // Add variant on a lone component (a variant property on it) wraps it in a set named after it. + let mut s = Session::new(); + let f = idv(&ex(&mut s, "frame.create", json!({"x": 0, "y": 0, "width": 40, "height": 20, "name": "Button"}))); + let c = idv(&ex(&mut s, "component.create", json!({"ids": [f]}))); + let set = idv(&ex(&mut s, "component.addProperty", json!({"id": c, "name": "State", "type": "variant"}))); + assert_eq!(node(&s, set).name, "Button"); + assert_eq!(node(&s, c).name, "State=Default"); + healthy(&s); +}