diff --git a/crates/engine/src/commands/prop_groups.rs b/crates/engine/src/commands/prop_groups.rs index d070713a..e4d80b16 100644 --- a/crates/engine/src/commands/prop_groups.rs +++ b/crates/engine/src/commands/prop_groups.rs @@ -2,7 +2,7 @@ //! effect, mask, shape item or text animator (what scripting's `PropertyGroup.name`, `enabled`, //! `remove()`, `duplicate()` and `moveTo()` do, and what agents need without a UI selection). -use effectcraft_project::{GroupKind, ItemId, LayerId, LayerSource, Node, PropGroup, Uid}; +use effectcraft_project::{ItemId, LayerId, LayerSource, Node, PropGroup, Uid}; use serde_json::{Value, json}; use super::{CommandSpec, b_p, bad, has_layers, layer_mut, layer_p, layers_p, str_p}; @@ -10,7 +10,7 @@ use crate::{EngineError, Result, Session, cmd}; /// Groups that can be renamed, removed, duplicated and reordered: instances in indexed lists. fn is_instance(g: &PropGroup) -> bool { - matches!(g.kind, GroupKind::Indexed | GroupKind::Effect { .. } | GroupKind::Mask { .. } | GroupKind::Tracker { .. }) + g.is_instance() } /// `{layer, prop: uid}` → (comp, layer, group uid), checking that the group is an instance. diff --git a/crates/engine/src/tests_project_items.rs b/crates/engine/src/tests_project_items.rs index b9363f65..8da72d31 100644 --- a/crates/engine/src/tests_project_items.rs +++ b/crates/engine/src/tests_project_items.rs @@ -167,3 +167,26 @@ fn delete_removes_selected_groups_not_their_layer() { delete(&mut s); assert!(s.active_comp().unwrap().layer(LayerId(t)).is_none(), "a fixed group selected: the layer goes"); } + +/// #1061: text animators, masks and shape groups rename like effects, with undo; fixed groups and +/// empty names are refused. +#[test] +fn text_animators_masks_and_shape_groups_rename() { + let mut s = Session::default(); + s.execute("comp.new", json!({"name": "C", "width": 64, "height": 64, "duration": 1})).unwrap(); + let t = s.execute("layer.newText", json!({"text": "ABCD"})).unwrap()["layer"].as_u64().unwrap(); + s.execute("layer.addTextAnimator", json!({"layer": t, "properties": ["tracking"]})).unwrap(); + let sh = s.execute("layer.newShape", json!({"kind": "star"})).unwrap()["layer"].as_u64().unwrap(); + s.execute("layer.addMask", json!({"layer": sh, "shape": "ellipse"})).unwrap(); + let uid = |s: &Session, l: u64, path: &str| s.active_comp().unwrap().layer(LayerId(l)).unwrap().props.group(path).unwrap().groups().next().unwrap().uid; + let name = |s: &Session, l: u64, u: u64| s.active_comp().unwrap().layer(LayerId(l)).unwrap().props.find_group(u).unwrap().name.clone(); + for (l, path, new) in [(t, "text/animators", "Wiggle In"), (sh, "masks", "Reveal"), (sh, "contents", "Star Body")] { + let u = uid(&s, l, path); + let old = name(&s, l, u); + s.execute("prop.renameGroup", json!({"layer": l, "prop": u, "name": new})).unwrap(); + assert_eq!(name(&s, l, u), new, "{path}"); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(name(&s, l, u), old, "{path} undone"); + assert!(s.execute("prop.renameGroup", json!({"layer": l, "prop": u, "name": " "})).is_err()); + } +} diff --git a/crates/project/src/props.rs b/crates/project/src/props.rs index f1d5149d..18b44edb 100644 --- a/crates/project/src/props.rs +++ b/crates/project/src/props.rs @@ -337,6 +337,12 @@ pub fn parse_path(s: &str) -> Vec { } impl PropGroup { + /// An instance in an indexed list (effect, mask, shape item, text animator, tracker): these + /// can be renamed, removed, duplicated and reordered; fixed groups (Transform…) can't. + pub fn is_instance(&self) -> bool { + matches!(self.kind, GroupKind::Indexed | GroupKind::Effect { .. } | GroupKind::Mask { .. } | GroupKind::Tracker { .. }) + } + pub fn new(uid: Uid, match_id: &str, name: &str) -> PropGroup { PropGroup { uid, match_id: match_id.into(), name: name.into(), kind: GroupKind::Plain, enabled: true, children: vec![] } } diff --git a/crates/ui-egui/src/panels/effect_controls.rs b/crates/ui-egui/src/panels/effect_controls.rs index 9b70265d..d7826a86 100644 --- a/crates/ui-egui/src/panels/effect_controls.rs +++ b/crates/ui-egui/src/panels/effect_controls.rs @@ -1070,6 +1070,14 @@ fn histogram_only(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painter // --------------------------------------------------------------------------------------------- +/// Open the rename field on effect `uid` (Enter with the effect selected), name selected. +pub fn start_rename(ctx: &egui::Context, uid: u64, name: &str) { + let id = egui::Id::new(("ec-rename", uid)); + ctx.data_mut(|d| d.insert_temp(id, name.to_string())); + ctx.data_mut(|d| d.insert_temp(id.with("select"), true)); + ctx.memory_mut(|m| m.request_focus(id.with("field"))); +} + pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { let t = app.tokens; let p = ui.painter().with_clip_rect(rect); @@ -1147,6 +1155,9 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { response.request_focus(); } app.auto.add(&format!("effectControls.effect.{}.nameEdit", g.uid), name_rect, "Effect name"); + if ctx.data_mut(|d| d.remove_temp::(rename_id.with("select"))).unwrap_or(false) { + widgets::select_all(&ctx, response.id, &value); + } // Escape cancels. egui drops the field's focus on Escape in the same frame, so the // key is checked on the focus being lost as well as held. let escape = (response.has_focus() || response.lost_focus()) && ui.input(|i| i.key_pressed(egui::Key::Escape)); diff --git a/crates/ui-egui/src/panels/timeline.rs b/crates/ui-egui/src/panels/timeline.rs index 5042f368..55edb4ba 100644 --- a/crates/ui-egui/src/panels/timeline.rs +++ b/crates/ui-egui/src/panels/timeline.rs @@ -596,6 +596,41 @@ pub fn locate(app: &EffectcraftApp, layer: u64, time: Option) -> Option<(f3 /// Start renaming the first selected layer (Enter in the Timeline). pub fn begin_rename(app: &mut EffectcraftApp, ctx: &egui::Context) { + // A selected effect, mask, shape group or text animator renames before its layer (as in + // After Effects, #1061). + let group = app.session.state.selected_props.iter().rev().find_map(|(lid, uid)| { + let g = app.session.active_comp()?.layer(*lid)?.props.find_group(*uid).filter(|g| g.is_instance())?; + Some((lid.0, g.uid, g.name.clone(), matches!(g.kind, GroupKind::Effect { .. }))) + }); + if let Some((layer, uid, name, is_fx)) = group { + // An effect in a visible Effect Controls panel renames there, the rest in the Timeline. + if is_fx && app.auto.previous.iter().chain(app.auto.elements.iter()).any(|e| e.id == format!("effectControls.effect.{uid}.name")) { + super::effect_controls::start_rename(ctx, uid, &name); + } else { + // Show the group's row first: the field lives in it. + fn ancestors(g: &PropGroup, uid: u64, out: &mut Vec) -> bool { + for sg in g.groups() { + if sg.uid == uid || ancestors(sg, uid, out) { + out.push(sg.uid); + return true; + } + } + false + } + let mut open = vec![]; + if let Some(l) = app.session.active_comp().and_then(|c| c.layer(LayerId(layer))) { + ancestors(&l.props, uid, &mut open); + } + open.retain(|u| *u != uid); + app.ui.timeline.open_layers.insert(layer); + app.ui.timeline.open_groups.extend(open); + ctx.data_mut(|d| { + d.insert_temp(group_rename_id(), (layer, uid, name)); + d.insert_temp(rename_focus_id(), true); + }); + } + return; + } if let Some(l) = app.session.state.selected_layers.first().and_then(|id| app.session.active_comp().and_then(|c| c.layer(*id))) { start_rename(ctx, l.id.0, &l.name); } @@ -621,6 +656,11 @@ fn start_rename(ctx: &egui::Context, id: u64, name: &str) { }); } +/// The Timeline's open group rename field: (layer, group uid, text). +fn group_rename_id() -> egui::Id { + egui::Id::new("tl-group-rename") +} + fn rename_focus_id() -> egui::Id { egui::Id::new("tl-rename-focus") } @@ -2304,7 +2344,27 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { } app.auto.add(&format!("timeline.group.{uid}.eye"), er, name); } - lp.text(pos2(indent + 10.0, cy), Align2::LEFT_CENTER, name, Tokens::ui(12.0), t.text); + let group_edit: Option<(u64, u64, String)> = ctx.data(|d| d.get_temp(group_rename_id())); + if let Some((_, _, mut buf)) = group_edit.filter(|(rl, ru, _)| *rl == layer.id.0 && ru == uid) { + let field = Rect::from_min_max(pos2(indent + 8.0, r.min.y + 2.0), pos2((cw.switches - 4.0).max(indent + 60.0), r.max.y - 2.0)); + let mut child = ui.new_child(egui::UiBuilder::new().max_rect(field)); + let er = child.add(egui::TextEdit::singleline(&mut buf).font(Tokens::ui(12.0)).desired_width(field.width())); + app.auto.add(&format!("timeline.group.{uid}.nameEdit"), er.rect, &buf); + if ctx.data_mut(|d| d.remove_temp::(rename_focus_id())).unwrap_or(false) { + er.request_focus(); + widgets::select_all(&ctx, er.id, &buf); + } + if er.lost_focus() { + if !ui.input(|i| i.key_pressed(egui::Key::Escape)) && !buf.trim().is_empty() && buf != *name { + actions.push(("prop.renameGroup".into(), json!({"layer": layer.id.0, "prop": uid, "name": buf}))); + } + ctx.data_mut(|d| d.remove::<(u64, u64, String)>(group_rename_id())); + } else { + ctx.data_mut(|d| d.insert_temp(group_rename_id(), (layer.id.0, *uid, buf))); + } + } else { + lp.text(pos2(indent + 10.0, cy), Align2::LEFT_CENTER, name, Tokens::ui(12.0), t.text); + } text_anim_popups(app, ui, &lp, layer, *uid, cw.switches, cy, &mut actions); shape_add_popup(app, ui, &lp, layer, *uid, cw.switches, cy, &mut actions); if *uid == GEOM_BIT | layer.id.0 { diff --git a/crates/ui-egui/tests/ui_group_rename.rs b/crates/ui-egui/tests/ui_group_rename.rs new file mode 100644 index 00000000..cf56ac6f --- /dev/null +++ b/crates/ui-egui/tests/ui_group_rename.rs @@ -0,0 +1,106 @@ +//! Enter renames the selected effect, text animator, mask or shape group, not only the layer +//! (#1061). Enter commits, Escape cancels. + +use effectcraft_engine::Session; +use effectcraft_ui_egui::EffectcraftApp; +use egui::{Event, Key, Modifiers}; +use egui_kittest::Harness; +use serde_json::json; + +struct Fx { + text: u64, + shape: u64, + effect: u64, + anim: u64, + mask: u64, + group: u64, +} + +fn harness() -> (Harness<'static, EffectcraftApp>, Fx) { + let mut s = Session::default(); + s.execute("comp.new", json!({"name": "Main", "width": 320, "height": 180, "duration": 4, "fps": 24})).unwrap(); + let text = s.execute("layer.newText", json!({"text": "ABCD"})).unwrap()["layer"].as_u64().unwrap(); + s.execute("layer.addTextAnimator", json!({"layer": text, "properties": ["tracking"]})).unwrap(); + let effect = s.execute("effect.apply", json!({"layer": text, "effect": "Gaussian Blur"})).unwrap()["effects"][0].as_u64().unwrap(); + let shape = s.execute("layer.newShape", json!({"kind": "star"})).unwrap()["layer"].as_u64().unwrap(); + s.execute("layer.addMask", json!({"layer": shape, "shape": "ellipse"})).unwrap(); + let first = |s: &Session, l: u64, path: &str| { + s.active_comp().unwrap().layer(effectcraft_engine::project::LayerId(l)).unwrap().props.group(path).unwrap().groups().next().unwrap().uid + }; + let (anim, mask, group) = (first(&s, text, "text/animators"), first(&s, shape, "masks"), first(&s, shape, "contents")); + let mut h = Harness::builder().with_size(egui::vec2(1700.0, 1100.0)).with_step_dt(1.0 / 60.0).build_eframe(|_| EffectcraftApp::new(s)); + h.run_steps(6); + (h, Fx { text, shape, effect, anim, mask, group }) +} + +fn key(h: &mut Harness<'_, EffectcraftApp>, key: Key) { + h.input_mut().events.push(Event::Key { key, physical_key: None, pressed: true, repeat: false, modifiers: Modifiers::NONE }); + h.input_mut().events.push(Event::Key { key, physical_key: None, pressed: false, repeat: false, modifiers: Modifiers::NONE }); + h.run_steps(3); +} + +fn name(h: &Harness<'_, EffectcraftApp>, layer: u64, uid: u64) -> String { + h.state().session.active_comp().unwrap().layer(effectcraft_engine::project::LayerId(layer)).unwrap().props.find_group(uid).unwrap().name.clone() +} + +fn layer_name(h: &Harness<'_, EffectcraftApp>, layer: u64) -> String { + h.state().session.active_comp().unwrap().layer(effectcraft_engine::project::LayerId(layer)).unwrap().name.clone() +} + +fn select(h: &mut Harness<'_, EffectcraftApp>, layer: u64, uid: u64) { + let s = &mut h.state_mut().session; + s.execute("layer.select", json!({"layers": [layer]})).unwrap(); + s.execute("prop.select", json!({"layer": layer, "prop": uid, "selectKeys": false})).unwrap(); + h.run_steps(3); +} + +fn editing(h: &Harness<'_, EffectcraftApp>, uid: u64) -> bool { + let ids = [format!("timeline.group.{uid}.nameEdit"), format!("effectControls.effect.{uid}.nameEdit")]; + h.state().auto.previous.iter().chain(h.state().auto.elements.iter()).any(|e| ids.contains(&e.id)) +} + +fn rename_with_enter(h: &mut Harness<'_, EffectcraftApp>, layer: u64, uid: u64, new: &str) { + select(h, layer, uid); + key(h, Key::Enter); + assert!(editing(h, uid), "Enter opens a rename field on group {uid}"); + h.key_press_modifiers(Modifiers::COMMAND, Key::A); + h.event(Event::Text(new.into())); + h.step(); + key(h, Key::Enter); + assert!(!editing(h, uid), "Enter closes the field"); +} + +#[test] +fn enter_renames_the_selected_group_not_the_layer() { + let (mut h, x) = harness(); + let before = (layer_name(&h, x.text), layer_name(&h, x.shape)); + for (layer, uid, new) in [(x.text, x.effect, "Delay"), (x.text, x.anim, "Wiggle"), (x.shape, x.mask, "Reveal"), (x.shape, x.group, "Body")] { + rename_with_enter(&mut h, layer, uid, new); + assert_eq!(name(&h, layer, uid), new); + } + assert_eq!((layer_name(&h, x.text), layer_name(&h, x.shape)), before, "the layers keep their names"); +} + +#[test] +fn escape_cancels_a_group_rename_and_a_layer_still_renames_without_a_group() { + let (mut h, x) = harness(); + let old = name(&h, x.text, x.anim); + select(&mut h, x.text, x.anim); + key(&mut h, Key::Enter); + assert!(editing(&h, x.anim)); + h.key_press_modifiers(Modifiers::COMMAND, Key::A); + h.event(Event::Text("Nope".into())); + h.step(); + key(&mut h, Key::Escape); + assert!(!editing(&h, x.anim)); + assert_eq!(name(&h, x.text, x.anim), old); + // No group selected: Enter renames the layer as before. + h.state_mut().session.execute("layer.select", json!({"layers": [x.shape]})).unwrap(); + h.run_steps(3); + key(&mut h, Key::Enter); + h.key_press_modifiers(Modifiers::COMMAND, Key::A); + h.event(Event::Text("Plate".into())); + h.step(); + key(&mut h, Key::Enter); + assert_eq!(layer_name(&h, x.shape), "Plate"); +}