From 9928f6181bfdbc08a8018a537b545f97a3bddd7a Mon Sep 17 00:00:00 2001 From: Christopher Linnett Date: Wed, 28 Oct 2026 00:22:35 +0800 Subject: [PATCH] Text: duplicate during a text edit copies the layer, not a selected effect (#2226) Opening the editor keeps the property selection, so Cmd+D while typing duplicated an effect, shape item or text animator selected before and left the edit open. An open edit now comes first, as it does for Clear: Duplicate ends it and copies the edited layer. With no edit open, selected effects and items keep their precedence, and the command's description says so. --- crates/engine/src/cmd/edit.rs | 16 ++++++++-- crates/engine/src/tests/text_edit.rs | 39 +++++++++++++++++++++++++ crates/ui-egui/tests/it/ui_text_edit.rs | 22 ++++++++++++++ 3 files changed, 74 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index e9ec7eca1..8d4bde60c 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -105,8 +105,10 @@ pub(crate) fn reid(layer: &mut Layer, next: &mut u64) { } fn duplicate(s: &mut Session, p: &Value) -> Result { - // Effects selected → duplicate them (Edit ▸ Duplicate in Effect Controls). - if p.get("layers").is_none() { + // Effects selected → duplicate them (Edit ▸ Duplicate in Effect Controls). An open text edit + // comes first, as it does for Clear: opening the editor keeps the property selection, but + // while typing, Duplicate means the edited layer, not an effect or item selected before. + if p.get("layers").is_none() && s.state.text_edit.is_none() { let fx = super::effect::selected_effects(s); if !fx.is_empty() { let mut out = vec![]; @@ -961,7 +963,15 @@ pub fn specs() -> Vec { paste_reversed ), cmd!("edit.clear", "Clear", ["Edit"], Some("Delete"), "{layers?}", layers_or_keys, delete), - cmd!("edit.duplicate", "Duplicate", ["Edit"], Some("Cmd+D"), "{layers?} (effects or shape items when they are selected)", has_layers, duplicate), + cmd!( + "edit.duplicate", + "Duplicate", + ["Edit"], + Some("Cmd+D"), + "{layers?} (without layers: during a text edit the edited layer, else the selected effects, shape items or text animators, else the selected layers)", + has_layers, + duplicate + ), cmd!("edit.splitLayer", "Split Layer", ["Edit"], Some("Cmd+Shift+D"), "{layers?}", has_layers, split), cmd!("edit.liftWorkArea", "Lift Work Area", ["Edit"], None, "{layers?}", has_comp, lift), cmd!("edit.extractWorkArea", "Extract Work Area", ["Edit"], None, "{layers?}", has_comp, extract), diff --git a/crates/engine/src/tests/text_edit.rs b/crates/engine/src/tests/text_edit.rs index 14fef7576..64ee2f0e3 100644 --- a/crates/engine/src/tests/text_edit.rs +++ b/crates/engine/src/tests/text_edit.rs @@ -1234,6 +1234,45 @@ fn duplicate_ends_an_edit_on_already_committed_text() { assert!(s.state.text_edit.is_none(), "undo does not reopen the edit"); } +#[test] +fn duplicate_during_an_edit_copies_the_layer_not_a_selected_property_group() { + use effectcraft_project::LayerId; + // Opening the editor by command, as a Type-tool click does, keeps the property selection, so + // an effect, shape item or text animator can still be selected while the user types. + for what in ["effect", "shape item", "text animator"] { + let (mut s, t) = session("Hello"); + let (owner, path) = match what { + "effect" => { + s.execute("effect.apply", json!({"layers": [t], "effect": "Gaussian Blur"})).unwrap(); + (t, "effects") + } + "shape item" => (s.execute("layer.newShape", json!({"kind": "rect"})).unwrap()["layer"].as_u64().unwrap(), "contents"), + _ => { + s.execute("layer.addTextAnimator", json!({"layer": t, "properties": ["opacity"]})).unwrap(); + (t, "text/animators") + } + }; + let items = + |s: &Session| s.active_comp().unwrap().layer(LayerId(owner)).unwrap().props.group(path).unwrap().groups().map(|g| g.uid).collect::>(); + let item = items(&s)[0]; + let n = items(&s).len(); + let layers = |s: &Session| s.active_comp().map_or(0, |c| c.layers.len()); + let before = layers(&s); + s.execute("prop.select", json!({"layer": owner, "prop": item})).unwrap(); + begin_edit_at(&mut s, t, 5); + let r = s.execute("edit.duplicate", json!({})).unwrap(); + assert!(s.state.text_edit.is_none(), "{what}: Duplicate ends the open edit"); + assert_eq!(items(&s).len(), n, "{what}: the selected {what} is not duplicated"); + assert_eq!(layers(&s), before + 1, "{what}: the edited layer is"); + assert_eq!(s.state.selected_layers, vec![LayerId(r[0].as_u64().unwrap())], "{what}: and its copy is selected"); + // With no edit open, the selected item keeps its precedence over the layers. + s.execute("prop.select", json!({"layer": owner, "prop": item})).unwrap(); + s.execute("edit.duplicate", json!({})).unwrap(); + assert_eq!(items(&s).len(), n + 1, "{what}: outside an edit Duplicate copies the selected {what}"); + assert_eq!(layers(&s), before + 1, "{what}: and no layer"); + } +} + #[test] fn duplicate_ends_the_open_edit_and_selects_the_copy() { let (mut s, t) = editing("Hello", 5); diff --git a/crates/ui-egui/tests/it/ui_text_edit.rs b/crates/ui-egui/tests/it/ui_text_edit.rs index 0612e2923..a63631e8a 100644 --- a/crates/ui-egui/tests/it/ui_text_edit.rs +++ b/crates/ui-egui/tests/it/ui_text_edit.rs @@ -791,6 +791,28 @@ fn duplicate_rebound_onto_a_bare_letter_stays_typing() { assert_eq!(text, "HelloD", "and the letter typed into the text"); } +#[test] +fn duplicate_chord_copies_the_edited_layer_not_an_effect_still_selected() { + let mut a = app(); + let lid = a.session.execute("layer.newText", json!({"text": "alpha beta", "size": 40, "box": [40, 60, 250, 100]})).unwrap()["layer"].as_u64().unwrap(); + // Applying the blur selects it, as clicking it in Effect Controls does. + a.session.execute("effect.apply", json!({"layers": [lid], "effect": "Gaussian Blur"})).unwrap(); + a.ui.tool = effectcraft_ui_egui::state::Tool::Type; + let mut h = harness(a); + h.run_steps(3); + let comp = rect(&h, "viewer.comp"); + click(&mut h, comp.min + egui::vec2(80.0, 85.0) * (comp.width() / 640.0), Modifiers::NONE); + assert_eq!(edited(&h).0, lid, "the Type tool's click opens the text layer for editing"); + assert!(!h.state().session.state.selected_props.is_empty(), "and the blur stays selected"); + let n = layer_ids(&h).len(); + key(&mut h, Key::D, Modifiers::COMMAND); + let s = &h.state().session; + assert!(s.state.text_edit.is_none(), "Duplicate ends the edit"); + assert_eq!(layer_ids(&h).len(), n + 1, "and copies the edited layer"); + let blurs = s.active_comp().unwrap().layer(LayerId(lid)).unwrap().effects().map_or(0, |fx| fx.groups().count()); + assert_eq!(blurs, 1, "not the blur"); +} + #[test] fn a_chord_bound_to_both_forwarded_commands_resolves_in_list_order() { let mut a = app();