From 048d6c4a25ce6596c97906042a0b59db71e3aa83 Mon Sep 17 00:00:00 2001 From: Christopher Linnett Date: Sat, 24 Oct 2026 14:35:45 +0800 Subject: [PATCH 1/8] Text: duplicate ends the open text edit and selects the copy (#2226) A deselected layer never keeps an open text edit: duplicate now commits any open edit through the same route as text.endEdit before the selection moves, from every route (menu, shortcut, palette, MCP, script). The editor-held keyboard matches the chord against each command's current binding, like Deselect All already did, so rebound shortcuts keep working and keys a focused editor must never surrender stay typing. --- crates/engine/src/commands/edit.rs | 5 ++ crates/engine/src/tests_text_edit.rs | 66 +++++++++++++++++++ crates/ui-egui/src/panels/viewer_text.rs | 21 +++--- crates/ui-egui/tests/it/ui_text_edit.rs | 82 ++++++++++++++++++++++++ 4 files changed, 166 insertions(+), 8 deletions(-) diff --git a/crates/engine/src/commands/edit.rs b/crates/engine/src/commands/edit.rs index 3630efeee..ad4c10228 100644 --- a/crates/engine/src/commands/edit.rs +++ b/crates/engine/src/commands/edit.rs @@ -126,6 +126,11 @@ fn duplicate(s: &mut Session, p: &Value) -> Result { return super::prop_groups::duplicate_contents(s, &items); } } + // A deselected layer never keeps an open text edit: the copies take the selection, so commit + // an open edit first, exactly as `text.endEdit` (typed text kept, an empty Type-tool layer + // discarded) and as Deselect All does — one call on the command path so every route (menu, + // shortcut, palette, MCP, script) ends the edit and leaves the same final state. + super::text_edit::end_edit(s, &Value::Null)?; let (cid, ids) = layers_p(s, p)?; let new = s.edit("Duplicate", None, |proj, st| { let mut next = proj.next_id; diff --git a/crates/engine/src/tests_text_edit.rs b/crates/engine/src/tests_text_edit.rs index b4d2ff8c0..a44e44856 100644 --- a/crates/engine/src/tests_text_edit.rs +++ b/crates/engine/src/tests_text_edit.rs @@ -427,3 +427,69 @@ fn deselect_all_discards_an_empty_type_tool_layer() { s.execute("edit.undo", json!({})).unwrap(); assert_eq!(count(&s), before, "that removal is one undo step of its own, as on text.endEdit"); } + +#[test] +fn duplicate_ends_an_edit_on_already_committed_text() { + // The engine route (MCP, control channel, script) with text that was committed before the + // edit opened and nothing typed inside it: the edit still ends and the copy is selected. + let (mut s, t) = session("Hello"); + s.execute("text.edit", json!({"layer": t, "caret": 5})).unwrap(); + let copy = s.execute("edit.duplicate", json!({})).unwrap()[0].as_u64().unwrap(); + assert!(s.state.text_edit.is_none(), "the open edit ends on a direct edit.duplicate"); + assert_eq!(s.state.selected_layers, vec![effectcraft_project::LayerId(copy)], "and the copy is selected"); + assert_eq!(doc(&s, t).text, "Hello", "the committed text is kept on the original"); + assert_eq!(doc(&s, copy).text, "Hello", "and carried by the copy"); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(s.active_comp().map_or(0, |c| c.layers.len()), 1, "undo removes the copy"); + assert!(s.state.text_edit.is_none(), "undo does not reopen the edit"); +} + +#[test] +fn duplicate_ends_the_open_edit_and_selects_the_copy() { + let (mut s, t) = session("Hello"); + s.execute("text.edit", json!({"layer": t, "caret": 5})).unwrap(); + s.execute("text.insert", json!({"text": "!"})).unwrap(); + assert_eq!(doc(&s, t).text, "Hello!"); + let copy = s.execute("edit.duplicate", json!({})).unwrap()[0].as_u64().unwrap(); + // A deselected layer never keeps an open text edit: Duplicate commits the edit through the + // same route as text.endEdit before the selection moves, so every route (menu, shortcut, + // palette, MCP, script) leaves one final state. + assert!(s.state.text_edit.is_none(), "Duplicate ends the open text edit, as Deselect All does"); + assert_eq!(s.state.selected_layers, vec![effectcraft_project::LayerId(copy)], "and selects the copy"); + assert_eq!(doc(&s, t).text, "Hello!", "the original keeps the typed text"); + assert_eq!(doc(&s, copy).text, "Hello!", "the copy carries it too"); + // The edit ending is no undoable step of its own (as with text.endEdit): undo takes the + // duplicate, it does not reopen the edit or undo the typing. + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(s.active_comp().map_or(0, |c| c.layers.len()), 1, "undo removed the copy"); + assert_eq!(doc(&s, t).text, "Hello!", "the typed text stays committed"); + assert!(s.state.text_edit.is_none(), "undo does not reopen the edit"); + // With no open edit Duplicate is untouched by all of this: same behavior, no extra steps. + s.execute("edit.duplicate", json!({"layers": [t]})).unwrap(); + assert_eq!(s.active_comp().map_or(0, |c| c.layers.len()), 2, "a plain duplicate still duplicates"); + assert!(s.state.text_edit.is_none()); +} + +#[test] +fn duplicate_of_an_empty_type_tool_layer_discards_it_as_endedit_does() { + let (mut s, _keep) = session("Hello"); + let new = s.execute("layer.newText", json!({"text": "", "position": [80.0, 80.0], "edit": true})).unwrap()["layer"].as_u64().unwrap(); + assert!( + s.state.text_edit.as_ref().is_some_and(|e| e.created && e.layer == effectcraft_project::LayerId(new)), + "the Type tool opens the edit on its new, empty layer" + ); + let count = |s: &Session| s.active_comp().map_or(0, |c| c.layers.len()); + let before = count(&s); + s.execute("edit.duplicate", json!({})).unwrap(); + assert!(s.state.text_edit.is_none(), "the edit ends the way text.endEdit ends it"); + assert_eq!(count(&s), before - 1, "a Type-tool layer left empty is discarded, not duplicated"); + assert!(s.state.selected_layers.is_empty(), "nothing is selected: the layer that would be copied is gone"); + // The edit is committed before Duplicate resolves its layers, so naming the very layer + // the empty edit was on addresses nothing: the commit still happened, the duplicate errors. + let new2 = s.execute("layer.newText", json!({"text": "", "position": [90.0, 90.0], "edit": true})).unwrap()["layer"].as_u64().unwrap(); + assert!(s.execute("edit.duplicate", json!({"layers": [new2]})).is_err(), "the named layer is gone once the edit commits"); + assert!(s.state.text_edit.is_none(), "and the edit ended all the same"); + assert_eq!(count(&s), before - 1, "with the empty layer discarded, nothing left to duplicate"); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(count(&s), before, "that removal is one undo step of its own, as on text.endEdit"); +} diff --git a/crates/ui-egui/src/panels/viewer_text.rs b/crates/ui-egui/src/panels/viewer_text.rs index c7169f416..8c77e069a 100644 --- a/crates/ui-egui/src/panels/viewer_text.rs +++ b/crates/ui-egui/src/panels/viewer_text.rs @@ -504,14 +504,19 @@ fn keyboard(app: &mut EffectcraftApp, ui: &mut egui::Ui, viewer: Rect) { exec(app, "layer.setText", json!({"layer": sel.layer.0, "range": [r.start, r.end], "size": ns})); } // The app-wide shortcut dispatcher is skipped while this editor holds the - // keyboard, so the Deselect All chord (Cmd/Ctrl+Shift+A or F2 by default) - // must be matched here and run through the command dispatcher like undo - // above. Matching the command's current binding, not a fixed key, keeps - // rebound shortcuts working and gives every route one final state; the - // forwarding test also refuses chords a focused editor must never surrender - // (see `forwardable_during_text_edit`). - _ if crate::menus::chord_matches(&app.session, "edit.deselectAll", key, m) && crate::menus::forwardable_during_text_edit(m, key) => { - let _ = crate::menus::invoke(app, &ctx, "edit.deselectAll", json!({})); + // keyboard, so commands that end the edit by moving the selection — Deselect + // All (Cmd/Ctrl+Shift+A or F2 by default) and Duplicate (Cmd/Ctrl+D) — must be + // matched here and run through the command dispatcher like undo above. + // Matching each command's current binding, not a fixed key, keeps rebound + // shortcuts working and gives every route one final state; a chord bound to + // both commands resolves in list order. The forwarding test also refuses + // chords a focused editor must never surrender (see + // `forwardable_during_text_edit`). + _ if crate::menus::forwardable_during_text_edit(m, key) + && let Some(cmd_id) = + ["edit.deselectAll", "edit.duplicate"].into_iter().find(|id| crate::menus::chord_matches(&app.session, id, key, m)) => + { + let _ = crate::menus::invoke(app, &ctx, cmd_id, json!({})); } _ => {} } diff --git a/crates/ui-egui/tests/it/ui_text_edit.rs b/crates/ui-egui/tests/it/ui_text_edit.rs index ed5baca2d..65eccb589 100644 --- a/crates/ui-egui/tests/it/ui_text_edit.rs +++ b/crates/ui-egui/tests/it/ui_text_edit.rs @@ -724,3 +724,85 @@ fn deselect_all_menu_item_follows_the_edit_while_the_project_panel_is_focused() // With no edit, the item deselects Project items again: nothing selected there, nothing to do. assert!(!enabled(&h), "Deselect All follows the empty Project selection once the edit is over"); } + +/// Layer ids of the active comp, bottom to top. +fn layer_ids(h: &Harness<'_, EffectcraftApp>) -> Vec { + h.state().session.active_comp().map_or(vec![], |c| c.layers.iter().map(|l| l.id.0).collect()) +} + +#[test] +fn duplicate_chord_ends_a_type_tool_edit_and_selects_the_copy() { + let (mut h, lid) = type_tool_edit(app(), "Hello"); + key(&mut h, Key::D, Modifiers::COMMAND); + let s = &h.state().session; + assert!(s.state.text_edit.is_none(), "Duplicate ends the open edit, as Deselect All does"); + let copy = *s.state.selected_layers.first().expect("the copy is selected"); + assert_ne!(copy.0, lid, "the copy (not the edited original) holds the selection"); + assert_eq!(layer_doc(s, LayerId(lid)).unwrap().text, "Hello", "the original keeps its text"); + assert_eq!(layer_doc(s, copy).unwrap().text, "Hello", "the copy carries it too"); +} + +#[test] +fn duplicate_chord_ends_an_edit_started_while_the_project_panel_is_focused() { + let (mut h, lid) = project_focused_text_edit("Hi"); + key(&mut h, Key::D, Modifiers::COMMAND); + let s = &h.state().session; + assert!(s.state.text_edit.is_none(), "Cmd+D ends the edit even with Project focused"); + let copy = s.state.selected_layers.first().expect("the copy is selected"); + assert_eq!(layer_doc(s, LayerId(lid)).unwrap().text, "Hi", "the typed text is kept"); + assert_eq!(layer_doc(s, *copy).unwrap().text, "Hi", "on the copy as well"); +} + +#[test] +fn rebound_duplicate_chord_forwards_from_inside_the_edit() { + let mut a = app(); + a.session.execute("shortcuts.set", json!({"command": "edit.duplicate", "keys": ["Cmd+Alt+D"]})).unwrap(); + let (mut h, lid) = type_tool_edit(a, "Hello"); + // The default key no longer belongs to Duplicate: while the editor holds the keyboard a + // chord that matches no command does nothing, and the current binding is what forwards. + let n = layer_ids(&h).len(); + key(&mut h, Key::D, Modifiers::COMMAND); + assert!(h.state().session.state.text_edit.is_some(), "Cmd+D was rebound away: the edit stays open"); + assert_eq!(layer_ids(&h).len(), n, "and nothing was duplicated"); + key(&mut h, Key::D, Modifiers { alt: true, ..Modifiers::COMMAND }); + let s = &h.state().session; + assert!(s.state.text_edit.is_none(), "the rebound chord ends the edit"); + assert_eq!(layer_doc(s, LayerId(lid)).unwrap().text, "Hello", "the text is kept"); + assert_eq!(layer_ids(&h).len(), n + 1, "and the copy exists"); +} + +#[test] +fn duplicate_rebound_onto_a_bare_letter_stays_typing() { + let mut a = app(); + a.session.execute("shortcuts.set", json!({"command": "edit.duplicate", "keys": ["D"]})).unwrap(); + let (mut h, lid) = type_tool_edit(a, "Hello"); + let n = layer_ids(&h).len(); + // A binding without a command modifier is text as far as the editor is concerned: the editor + // must not surrender a key that types (same invariant as the app-wide dispatcher). + key(&mut h, Key::D, Modifiers::NONE); + assert!(h.state().session.state.text_edit.is_some(), "a bare-letter Duplicate binding does not end the edit"); + assert_eq!(layer_ids(&h).len(), n, "and does not duplicate"); + type_text(&mut h, "D"); + let (lid2, text, _, _) = edited(&h); + assert_eq!(lid2, lid, "the same edit on the same layer continues"); + assert_eq!(text, "HelloD", "and the letter typed into the text"); +} + +#[test] +fn a_chord_bound_to_both_forwarded_commands_resolves_in_list_order() { + let mut a = app(); + // Shortcuts may hold one chord for several commands (the conflict is a toast, not a + // refusal), so the editor's forwarding resolves a shared chord in list order — Deselect + // All before Duplicate. This pins that documented contract: the shared chord must end the + // edit like Deselect All, without running Duplicate behind it. + a.session.execute("shortcuts.set", json!({"command": "edit.deselectAll", "keys": ["Cmd+Alt+G"]})).unwrap(); + a.session.execute("shortcuts.set", json!({"command": "edit.duplicate", "keys": ["Cmd+Alt+G"]})).unwrap(); + let (mut h, lid) = type_tool_edit(a, "Hello"); + let n = layer_ids(&h).len(); + key(&mut h, Key::G, Modifiers { alt: true, ..Modifiers::COMMAND }); + let s = &h.state().session; + assert!(s.state.text_edit.is_none(), "the shared chord still ends the edit"); + assert!(s.state.selected_layers.is_empty(), "Deselect All wins the shared chord: nothing is selected"); + assert_eq!(layer_ids(&h).len(), n, "so the chord did not duplicate"); + assert_eq!(layer_doc(s, LayerId(lid)).unwrap().text, "Hello", "and the text is kept"); +} From 9928f6181bfdbc08a8018a537b545f97a3bddd7a Mon Sep 17 00:00:00 2001 From: Christopher Linnett Date: Wed, 28 Oct 2026 00:22:35 +0800 Subject: [PATCH 2/8] 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(); From eeedd6b540e229fdfba987a6b842e4e6cb48b2d1 Mon Sep 17 00:00:00 2001 From: Christopher Linnett Date: Wed, 28 Oct 2026 00:26:52 +0800 Subject: [PATCH 3/8] Text: duplicate during a text edit leaves no effect selected on the original (#2226) Duplicate passes over the property selection an open edit kept and copies the edited layer, but left that selection in place on the original. With the copy selected, the next Duplicate copied the original's effect, shape item or text animator, and Clear would have removed it. Ending the edit through Duplicate now clears that property selection along with it. --- crates/engine/src/cmd/edit.rs | 8 +++++++- crates/engine/src/tests/text_edit.rs | 7 ++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 8d4bde60c..585e4459e 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -105,10 +105,11 @@ pub(crate) fn reid(layer: &mut Layer, next: &mut u64) { } fn duplicate(s: &mut Session, p: &Value) -> Result { + let editing = s.state.text_edit.is_some(); // 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() { + if p.get("layers").is_none() && !editing { let fx = super::effect::selected_effects(s); if !fx.is_empty() { let mut out = vec![]; @@ -151,6 +152,11 @@ fn duplicate(s: &mut Session, p: &Value) -> Result { relink_copies(comp, &map); proj.next_id = next; st.selected_layers = created.clone(); + // The property selection the edit kept was passed over above and belongs to the + // originals: left in place, the next Duplicate or Clear would act on it, not the copies. + if editing { + st.selected_props.clear(); + } Ok(created) })?; Ok(json!(new.iter().map(|l| l.0).collect::>())) diff --git a/crates/engine/src/tests/text_edit.rs b/crates/engine/src/tests/text_edit.rs index 64ee2f0e3..1713cd459 100644 --- a/crates/engine/src/tests/text_edit.rs +++ b/crates/engine/src/tests/text_edit.rs @@ -1265,11 +1265,16 @@ fn duplicate_during_an_edit_copies_the_layer_not_a_selected_property_group() { 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"); + // The selection the edit passed over goes with it: the next Duplicate copies the selected + // copy rather than reaching back to the original's {what}. + s.execute("edit.duplicate", json!({})).unwrap(); + assert_eq!(items(&s).len(), n, "{what}: a second Duplicate leaves the original's {what} alone"); + assert_eq!(layers(&s), before + 2, "{what}: and copies the copy"); // 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"); + assert_eq!(layers(&s), before + 2, "{what}: and no layer"); } } From 0e3b894cd906befdd0c01f2df1201b1975eaaae6 Mon Sep 17 00:00:00 2001 From: Christopher Linnett Date: Wed, 28 Oct 2026 00:30:00 +0800 Subject: [PATCH 4/8] Text: the text-edit tests count layers with the shared helper; forwarding docs name Duplicate (#2226) The engine's text-edit tests repeated one layer-count closure in five places; they now use the layer_count helper the 3D tests already had. The doc of forwardable_during_text_edit and of the Type-tool test helper still spoke of Deselect All alone, though the editor now forwards Duplicate too. --- crates/engine/src/tests/text_edit.rs | 34 ++++++++++++------------- crates/engine/src/tests/three_d.rs | 2 +- crates/ui-egui/src/menus/shortcuts.rs | 4 +-- crates/ui-egui/tests/it/ui_text_edit.rs | 2 +- 4 files changed, 20 insertions(+), 22 deletions(-) diff --git a/crates/engine/src/tests/text_edit.rs b/crates/engine/src/tests/text_edit.rs index 1713cd459..f129479f6 100644 --- a/crates/engine/src/tests/text_edit.rs +++ b/crates/engine/src/tests/text_edit.rs @@ -7,6 +7,7 @@ use serde_json::json; use crate::Session; use crate::commands::text_edit::{MAX_IME_TEXT_BYTES, ime_stamp_revision, ime_surrounding, layer_doc}; +use crate::tests::three_d::layer_count; fn session(text: &str) -> (Session, u64) { let mut s = Session::default(); @@ -278,7 +279,7 @@ fn type_tool_layers_point_box_and_empty_cleanup() { let pos = l.props.prop("transform/position").unwrap().value.as_vec3(); assert_eq!([pos[0], pos[1]], [190.0, 110.0]); s.execute("text.endEdit", json!({})).unwrap(); - assert_eq!(s.active_comp().unwrap().layers.len(), 1); + assert_eq!(layer_count(&s), 1); // Resizing the box (layer space) is an attribute too. s.execute("layer.setText", json!({"layer": b, "box": [-100, -60, 200, 120]})).unwrap(); assert_eq!(doc(&s, b).box_size, Some([200.0, 120.0])); @@ -1209,14 +1210,13 @@ fn deselect_all_discards_an_empty_type_tool_layer() { s.state.text_edit.as_ref().is_some_and(|e| e.created && e.layer == effectcraft_project::LayerId(new)), "the Type tool opens the edit on its new, empty layer" ); - let count = |s: &Session| s.active_comp().map_or(0, |c| c.layers.len()); - let before = count(&s); + let before = layer_count(&s); s.execute("edit.deselectAll", json!({})).unwrap(); assert!(s.state.text_edit.is_none()); assert!(s.state.selected_layers.is_empty()); - assert_eq!(count(&s), before - 1, "a Type-tool layer left empty is removed, as text.endEdit does"); + assert_eq!(layer_count(&s), before - 1, "a Type-tool layer left empty is removed, as text.endEdit does"); s.execute("edit.undo", json!({})).unwrap(); - assert_eq!(count(&s), before, "that removal is one undo step of its own, as on text.endEdit"); + assert_eq!(layer_count(&s), before, "that removal is one undo step of its own, as on text.endEdit"); } #[test] @@ -1230,7 +1230,7 @@ fn duplicate_ends_an_edit_on_already_committed_text() { assert_eq!(doc(&s, t).text, "Hello", "the committed text is kept on the original"); assert_eq!(doc(&s, copy).text, "Hello", "and carried by the copy"); s.execute("edit.undo", json!({})).unwrap(); - assert_eq!(s.active_comp().map_or(0, |c| c.layers.len()), 1, "undo removes the copy"); + assert_eq!(layer_count(&s), 1, "undo removes the copy"); assert!(s.state.text_edit.is_none(), "undo does not reopen the edit"); } @@ -1256,25 +1256,24 @@ fn duplicate_during_an_edit_copies_the_layer_not_a_selected_property_group() { |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); + let before = layer_count(&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!(layer_count(&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"); // The selection the edit passed over goes with it: the next Duplicate copies the selected // copy rather than reaching back to the original's {what}. s.execute("edit.duplicate", json!({})).unwrap(); assert_eq!(items(&s).len(), n, "{what}: a second Duplicate leaves the original's {what} alone"); - assert_eq!(layers(&s), before + 2, "{what}: and copies the copy"); + assert_eq!(layer_count(&s), before + 2, "{what}: and copies the copy"); // 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 + 2, "{what}: and no layer"); + assert_eq!(layer_count(&s), before + 2, "{what}: and no layer"); } } @@ -1294,12 +1293,12 @@ fn duplicate_ends_the_open_edit_and_selects_the_copy() { // The edit ending is no undoable step of its own (as with text.endEdit): undo takes the // duplicate, it does not reopen the edit or undo the typing. s.execute("edit.undo", json!({})).unwrap(); - assert_eq!(s.active_comp().map_or(0, |c| c.layers.len()), 1, "undo removed the copy"); + assert_eq!(layer_count(&s), 1, "undo removed the copy"); assert_eq!(doc(&s, t).text, "Hello!", "the typed text stays committed"); assert!(s.state.text_edit.is_none(), "undo does not reopen the edit"); // With no open edit Duplicate is untouched by all of this: same behavior, no extra steps. s.execute("edit.duplicate", json!({"layers": [t]})).unwrap(); - assert_eq!(s.active_comp().map_or(0, |c| c.layers.len()), 2, "a plain duplicate still duplicates"); + assert_eq!(layer_count(&s), 2, "a plain duplicate still duplicates"); assert!(s.state.text_edit.is_none()); } @@ -1311,18 +1310,17 @@ fn duplicate_of_an_empty_type_tool_layer_discards_it_as_endedit_does() { s.state.text_edit.as_ref().is_some_and(|e| e.created && e.layer == effectcraft_project::LayerId(new)), "the Type tool opens the edit on its new, empty layer" ); - let count = |s: &Session| s.active_comp().map_or(0, |c| c.layers.len()); - let before = count(&s); + let before = layer_count(&s); s.execute("edit.duplicate", json!({})).unwrap(); assert!(s.state.text_edit.is_none(), "the edit ends the way text.endEdit ends it"); - assert_eq!(count(&s), before - 1, "a Type-tool layer left empty is discarded, not duplicated"); + assert_eq!(layer_count(&s), before - 1, "a Type-tool layer left empty is discarded, not duplicated"); assert!(s.state.selected_layers.is_empty(), "nothing is selected: the layer that would be copied is gone"); // The edit is committed before Duplicate resolves its layers, so naming the very layer // the empty edit was on addresses nothing: the commit still happened, the duplicate errors. let new2 = s.execute("layer.newText", json!({"text": "", "position": [90.0, 90.0], "edit": true})).unwrap()["layer"].as_u64().unwrap(); assert!(s.execute("edit.duplicate", json!({"layers": [new2]})).is_err(), "the named layer is gone once the edit commits"); assert!(s.state.text_edit.is_none(), "and the edit ended all the same"); - assert_eq!(count(&s), before - 1, "with the empty layer discarded, nothing left to duplicate"); + assert_eq!(layer_count(&s), before - 1, "with the empty layer discarded, nothing left to duplicate"); s.execute("edit.undo", json!({})).unwrap(); - assert_eq!(count(&s), before, "that removal is one undo step of its own, as on text.endEdit"); + assert_eq!(layer_count(&s), before, "that removal is one undo step of its own, as on text.endEdit"); } diff --git a/crates/engine/src/tests/three_d.rs b/crates/engine/src/tests/three_d.rs index 07a48af1c..dc28ae42a 100644 --- a/crates/engine/src/tests/three_d.rs +++ b/crates/engine/src/tests/three_d.rs @@ -26,7 +26,7 @@ fn auto_orient_command() { assert!(!effectcraft_render::three_d::camera::is_two_node(&s.active_comp().unwrap().layers[0])); } -fn layer_count(s: &Session) -> usize { +pub(crate) fn layer_count(s: &Session) -> usize { s.active_comp().unwrap().layers.len() } diff --git a/crates/ui-egui/src/menus/shortcuts.rs b/crates/ui-egui/src/menus/shortcuts.rs index 7e3dfcec2..f931667ea 100644 --- a/crates/ui-egui/src/menus/shortcuts.rs +++ b/crates/ui-egui/src/menus/shortcuts.rs @@ -136,8 +136,8 @@ pub fn chord_matches(session: &effectcraft_engine::Session, command: &str, key: bindings(session).into_iter().any(|(m, k, id, _)| id == command && k == key && mods_match(m, mods)) } -/// Whether a Deselect All chord matched by [`chord_matches`] may run while a text editor holds -/// the keyboard. Chords carrying Cmd/Ctrl/Alt are command chords (the editor treats them as +/// Whether a Deselect All or Duplicate chord matched by [`chord_matches`] may run while a text +/// editor holds the keyboard. Chords carrying Cmd/Ctrl/Alt are command chords (the editor treats them as /// such for undo and the rest), and function keys never type, so those forward from the editor; /// a chord that could instead produce a character — a bare or Shift-only letter, digit or /// punctuation key, as from a pathological bare-letter rebinding — stays with the editor so diff --git a/crates/ui-egui/tests/it/ui_text_edit.rs b/crates/ui-egui/tests/it/ui_text_edit.rs index a63631e8a..4fe647e54 100644 --- a/crates/ui-egui/tests/it/ui_text_edit.rs +++ b/crates/ui-egui/tests/it/ui_text_edit.rs @@ -580,7 +580,7 @@ fn type_tool_selected_text_preference_skips_ineligible_layers_and_falls_back_to_ } /// Opens a fresh Type-tool edit holding `text` and returns the harness with the new layer id, -/// so the Deselect All tests start from the same state. +/// so the Deselect All and Duplicate tests start from the same state. fn type_tool_edit(a: EffectcraftApp, text: &str) -> (Harness<'static, EffectcraftApp>, u64) { let mut h = harness(a); h.run_steps(3); From c8b07b4b06d9293690bd4ca9ae35882ecc845632 Mon Sep 17 00:00:00 2001 From: Christopher Linnett Date: Wed, 28 Oct 2026 00:50:43 +0800 Subject: [PATCH 5/8] Text: duplicate during a text edit leaves no keyframe or mask vertex selected on the original (#2226) Opening the editor keeps the keyframe and mask-vertex selection as well as the property one, and Duplicate cleared only the properties. With the copy selected, Clear then deleted a key or vertex of the original instead of the copy. During an edit the copies now take the selection the way a click on them does: layer.select's rule, moved into one helper both use. With no edit open, Duplicate's selection is unchanged. --- crates/engine/src/cmd/edit.rs | 10 +++++--- crates/engine/src/cmd/layer.rs | 1 + crates/engine/src/cmd/layer/select.rs | 16 ++++++++---- crates/engine/src/tests/text_edit.rs | 35 +++++++++++++++++++++++++++ 4 files changed, 53 insertions(+), 9 deletions(-) diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 585e4459e..aaef8052a 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -151,11 +151,13 @@ fn duplicate(s: &mut Session, p: &Value) -> Result { } relink_copies(comp, &map); proj.next_id = next; - st.selected_layers = created.clone(); - // The property selection the edit kept was passed over above and belongs to the - // originals: left in place, the next Duplicate or Clear would act on it, not the copies. + // The property, keyframe and mask-vertex selection the edit kept belongs to the originals; + // left in place, the next Duplicate or Clear would act on it, not on the copies. So during + // an edit the copies take the selection the way a click on them would. if editing { - st.selected_props.clear(); + super::layer::select_only(st, created.clone()); + } else { + st.selected_layers = created.clone(); } Ok(created) })?; diff --git a/crates/engine/src/cmd/layer.rs b/crates/engine/src/cmd/layer.rs index f6d2feb04..7ad73b8ba 100644 --- a/crates/engine/src/cmd/layer.rs +++ b/crates/engine/src/cmd/layer.rs @@ -28,6 +28,7 @@ pub(crate) use create::{color_p, index_p, insert_layer, insert_layer_at, place, pub(crate) use parent::parent_fixes; use parent::set_parent; use replace::{has_replace_source, replace_source}; +pub(crate) use select::select_only; use select::{select, select_next, select_prev, select_step}; use switches::{blend, rename, set_comment, set_switch, track_matte}; use timing::{arrange, move_frames, precompose, slip, timing}; diff --git a/crates/engine/src/cmd/layer/select.rs b/crates/engine/src/cmd/layer/select.rs index b7fded70d..8008bcb7e 100644 --- a/crates/engine/src/cmd/layer/select.rs +++ b/crates/engine/src/cmd/layer/select.rs @@ -25,15 +25,21 @@ pub(super) fn select(s: &mut Session, p: &Value) -> Result { } } } else { - s.state.selected_layers = ids; - s.state.selected_props.clear(); - s.state.selected_keys.clear(); - let keep = s.state.selected_layers.clone(); - s.state.selected_vertices.retain(|v| keep.contains(&v.layer)); + select_only(&mut s.state, ids); } Ok(json!(s.state.selected_layers.iter().map(|l| l.0).collect::>())) } +/// Make `ids` the whole layer selection, as a plain click on them does: no property or keyframe +/// stays selected, and mask vertices stay selected only on layers that are still selected. +pub(crate) fn select_only(st: &mut crate::EditorState, ids: Vec) { + st.selected_layers = ids; + st.selected_props.clear(); + st.selected_keys.clear(); + let keep = st.selected_layers.clone(); + st.selected_vertices.retain(|v| keep.contains(&v.layer)); +} + pub(super) fn select_step(s: &mut Session, p: &Value, dir: i64) -> Result { let comp = s.active_comp().ok_or(EngineError::NoComp)?; if comp.layers.is_empty() { diff --git a/crates/engine/src/tests/text_edit.rs b/crates/engine/src/tests/text_edit.rs index f129479f6..176fe2ae6 100644 --- a/crates/engine/src/tests/text_edit.rs +++ b/crates/engine/src/tests/text_edit.rs @@ -1277,6 +1277,41 @@ fn duplicate_during_an_edit_copies_the_layer_not_a_selected_property_group() { } } +#[test] +fn duplicate_during_an_edit_leaves_no_key_or_vertex_selected_on_the_original() { + use effectcraft_project::LayerId; + // Opening the editor keeps the keyframe and mask-vertex selection too. Once the copy takes the + // selection, Clear deletes the copy, not a key or a vertex of the original. + for what in ["keyframe", "mask vertex"] { + let (mut s, t) = session("Hello"); + let mask = match what { + "keyframe" => { + for time in [0.0, 1.0] { + s.execute("prop.addKey", json!({"layer": t, "path": "transform/opacity", "time": time, "value": 50})).unwrap(); + } + s.execute("keys.select", json!({"keys": [{"layer": t, "path": "transform/opacity", "time": 0.0}]})).unwrap(); + None + } + // The Pen leaves the new mask's last vertex selected. + _ => Some(s.execute("mask.new", json!({"layer": t, "vertices": [[0, 0], [40, 0], [40, 40]], "closed": true})).unwrap()["mask"].as_u64().unwrap()), + }; + let original = |s: &Session| { + let l = s.active_comp().unwrap().layer(LayerId(t)).unwrap(); + match mask { + None => l.props.prop("transform/opacity").unwrap().keys.len(), + Some(m) => l.props.find_group(m).unwrap().get("path").unwrap().value.as_path().unwrap().vertices.len(), + } + }; + let (n, before) = (original(&s), layer_count(&s)); + begin_edit_at(&mut s, t, 5); + let copy = s.execute("edit.duplicate", json!({})).unwrap()[0].as_u64().unwrap(); + assert_eq!(s.state.selected_layers, vec![LayerId(copy)], "{what}: the copy is selected"); + s.execute("edit.clear", json!({})).unwrap(); + assert_eq!(layer_count(&s), before, "{what}: Clear deletes the selected copy"); + assert_eq!(original(&s), n, "{what}: and leaves the original's {what} alone"); + } +} + #[test] fn duplicate_ends_the_open_edit_and_selects_the_copy() { let (mut s, t) = editing("Hello", 5); From c074612acc61bd487ceb734c56afea8359c905e7 Mon Sep 17 00:00:00 2001 From: Christopher Linnett Date: Thu, 29 Oct 2026 18:53:45 +0800 Subject: [PATCH 6/8] Text: duplicate during a text edit copies the edited layer even after another is selected (#2226) --- crates/engine/src/cmd.rs | 4 ++-- crates/engine/src/cmd/edit.rs | 16 ++++++++++++--- crates/engine/src/cmd/params.rs | 7 ++++++- crates/engine/src/tests/text_edit.rs | 29 ++++++++++++++++++++++++++++ crates/engine/src/tests/three_d.rs | 1 + 5 files changed, 51 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/cmd.rs b/crates/engine/src/cmd.rs index cba6df6ef..eb308011d 100644 --- a/crates/engine/src/cmd.rs +++ b/crates/engine/src/cmd.rs @@ -163,8 +163,8 @@ pub use enabled::always; pub(crate) use enabled::{has_comp, has_keys, has_layers, has_project_selection, not_yet, not_yet_run}; pub use params::match_path_of; pub(crate) use params::{ - b_p, bad, comp_id, f_p, frontend, layer_mut, layer_p, layers_p, merge_p, resolve_effect_path, resolve_layer, selectable, selected_leaf_props, str_p, - time_p, unlocked, visible_p, + b_p, bad, comp_id, f_p, frontend, layer_mut, layer_p, layers_or, layers_p, merge_p, resolve_effect_path, resolve_layer, selectable, selected_leaf_props, + str_p, time_p, unlocked, visible_p, }; pub use schema::{accepted_params, check_params, params_schema}; diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index f89974c80..758a2a4a0 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -6,7 +6,7 @@ use effectcraft_project::{Comp, Expression, ItemId, ItemKind, Layer, LayerId, La use effectcraft_time::Tick; use serde_json::{Value, json}; -use super::{CommandSpec, has_comp, has_layers, layers_p, match_path_of, selected_leaf_props, str_p}; +use super::{CommandSpec, has_comp, has_layers, layers_or, layers_p, match_path_of, selected_leaf_props, str_p}; use crate::{EngineError, KeyRef, LinkClip, Result, Session, cmd}; fn can_undo(s: &Session) -> std::result::Result<(), String> { @@ -104,6 +104,10 @@ pub(crate) fn reid(layer: &mut Layer, next: &mut u64) { *next += 1; } +/// `edit.duplicate`: copies the targets in place. Without `layers` the targets are, in order: +/// during a text edit the edited layer (the edit is committed first, as `text.endEdit` does), +/// else the selected effects, shape items or text animators, else the selected layers. Layer +/// copies are selected and their ids returned; the other paths return their delegated result. fn duplicate(s: &mut Session, p: &Value) -> Result { let editing = s.state.text_edit.is_some(); // Effects selected → duplicate them (Edit ▸ Duplicate in Effect Controls). An open text edit @@ -132,9 +136,15 @@ fn duplicate(s: &mut Session, p: &Value) -> Result { // A deselected layer never keeps an open text edit: the copies take the selection, so commit // an open edit first, exactly as `text.endEdit` (typed text kept, an empty Type-tool layer // discarded) and as Deselect All does — one call on the command path so every route (menu, - // shortcut, palette, MCP, script) ends the edit and leaves the same final state. + // shortcut, palette, MCP, script) ends the edit and leaves the same final state. Unnamed, the + // target is the edited layer even when an engine route selected another one meanwhile; it is + // read before the commit, which may discard an empty Type-tool layer and leave nothing to copy. + let edited = s.state.text_edit.as_ref().map(|e| e.layer); super::text_edit::end_edit(s, &Value::Null)?; - let (cid, ids) = layers_p(s, p)?; + let (cid, ids) = match edited { + Some(layer) => layers_or(s, p, &[layer])?, + None => layers_p(s, p)?, + }; let new = s.edit("Duplicate", None, |proj, st| { let mut next = proj.next_id; let comp = proj.comp_mut(cid).ok_or(crate::EngineError::NoComp)?; diff --git a/crates/engine/src/cmd/params.rs b/crates/engine/src/cmd/params.rs index 7c4e1c292..794d3484a 100644 --- a/crates/engine/src/cmd/params.rs +++ b/crates/engine/src/cmd/params.rs @@ -142,6 +142,11 @@ pub(crate) fn layer_p(s: &Session, p: &Value, cmd: &str) -> Result<(ItemId, Laye /// `layers` param (array) or the selection. pub(crate) fn layers_p(s: &Session, p: &Value) -> Result<(ItemId, Vec)> { + layers_or(s, p, &s.state.selected_layers) +} + +/// `layers` param (array), else `layer`, else those of `fallback` still in the comp. +pub(crate) fn layers_or(s: &Session, p: &Value, fallback: &[LayerId]) -> Result<(ItemId, Vec)> { let cid = comp_id(s, p)?; let comp = s.project.comp(cid).ok_or(EngineError::NoComp)?; // Explicit references that match no layer are an error (agents otherwise get a silent no-op). @@ -153,7 +158,7 @@ pub(crate) fn layers_p(s: &Session, p: &Value) -> Result<(ItemId, Vec)> if let Some(v) = p.get("layer").filter(|v| !v.is_null()) { return Ok((cid, vec![resolve(v)?])); } - Ok((cid, s.state.selected_layers.iter().copied().filter(|l| comp.layer(*l).is_some()).collect())) + Ok((cid, fallback.iter().copied().filter(|l| comp.layer(*l).is_some()).collect())) } /// Whether a layer can be picked by the user (Select All, Ctrl+Up/Down, a Timeline click): not diff --git a/crates/engine/src/tests/text_edit.rs b/crates/engine/src/tests/text_edit.rs index 176fe2ae6..dfd2596cd 100644 --- a/crates/engine/src/tests/text_edit.rs +++ b/crates/engine/src/tests/text_edit.rs @@ -1312,6 +1312,35 @@ fn duplicate_during_an_edit_leaves_no_key_or_vertex_selected_on_the_original() { } } +#[test] +fn duplicate_during_an_edit_copies_the_edited_layer_after_another_is_selected() { + use effectcraft_project::LayerId; + // An engine route (MCP, control channel, script) can select another layer while the edit + // stays open. The implicit Duplicate still means the edited layer, as Clear does; a named + // target keeps its meaning. + let (mut s, t) = session("Hello"); + let other = s.execute("layer.newText", json!({"text": "Other"})).unwrap()["layer"].as_u64().unwrap(); + begin_edit_at(&mut s, t, 5); + s.execute("layer.select", json!({"layer": other})).unwrap(); + assert!(s.state.text_edit.as_ref().is_some_and(|e| e.layer == LayerId(t)), "selecting another layer leaves the edit open"); + let before = layer_count(&s); + let copy = s.execute("edit.duplicate", json!({})).unwrap()[0].as_u64().unwrap(); + assert!(s.state.text_edit.is_none(), "Duplicate ends the open edit"); + assert_eq!(layer_count(&s), before + 1, "one layer is copied"); + assert_eq!(doc(&s, copy).text, "Hello", "the edited layer is copied, not the newly selected one"); + assert_eq!(s.state.selected_layers, vec![LayerId(copy)], "and its copy is selected"); + // Null parameters name no layer, as everywhere else layers are read. + for unnamed in [json!({"layers": null}), json!({"layer": null})] { + begin_edit_at(&mut s, t, 5); + s.execute("layer.select", json!({"layer": other})).unwrap(); + let copy = s.execute("edit.duplicate", unnamed.clone()).unwrap()[0].as_u64().unwrap(); + assert_eq!(doc(&s, copy).text, "Hello", "{unnamed}: the edited layer is copied"); + } + begin_edit_at(&mut s, t, 5); + let named = s.execute("edit.duplicate", json!({"layer": other})).unwrap()[0].as_u64().unwrap(); + assert_eq!(doc(&s, named).text, "Other", "a named layer is copied as named"); +} + #[test] fn duplicate_ends_the_open_edit_and_selects_the_copy() { let (mut s, t) = editing("Hello", 5); diff --git a/crates/engine/src/tests/three_d.rs b/crates/engine/src/tests/three_d.rs index dc28ae42a..f10c74043 100644 --- a/crates/engine/src/tests/three_d.rs +++ b/crates/engine/src/tests/three_d.rs @@ -26,6 +26,7 @@ fn auto_orient_command() { assert!(!effectcraft_render::three_d::camera::is_two_node(&s.active_comp().unwrap().layers[0])); } +/// Layers in the active comp. pub(crate) fn layer_count(s: &Session) -> usize { s.active_comp().unwrap().layers.len() } From 54a141d0a07a6250a6bfa9573659058a0adf5a58 Mon Sep 17 00:00:00 2001 From: Christopher Linnett Date: Thu, 29 Oct 2026 19:20:46 +0800 Subject: [PATCH 7/8] Text: Duplicate stays enabled during a text edit with nothing selected (#2226) --- crates/engine/src/cmd/edit.rs | 7 ++++++- crates/engine/src/tests/text_edit.rs | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 758a2a4a0..eca532039 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -989,7 +989,7 @@ pub fn specs() -> Vec { ["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, + layers_or_text_edit, duplicate ), cmd!("edit.splitLayer", "Split Layer", ["Edit"], Some("Cmd+Shift+D"), "{layers?}", has_layers, split), @@ -1061,6 +1061,11 @@ fn layers_or_keys(s: &Session) -> std::result::Result<(), String> { if !s.state.selected_keys.is_empty() || !s.state.selected_vertices.is_empty() { super::has_comp(s) } else { has_layers(s) } } +/// Duplicate's unnamed target: the edited layer during a text edit, else the selection. +fn layers_or_text_edit(s: &Session) -> std::result::Result<(), String> { + if s.state.text_edit.is_some() { super::has_comp(s) } else { has_layers(s) } +} + fn always_ok(_: &Session) -> std::result::Result<(), String> { Ok(()) } diff --git a/crates/engine/src/tests/text_edit.rs b/crates/engine/src/tests/text_edit.rs index dfd2596cd..7a8598a72 100644 --- a/crates/engine/src/tests/text_edit.rs +++ b/crates/engine/src/tests/text_edit.rs @@ -1341,6 +1341,25 @@ fn duplicate_during_an_edit_copies_the_edited_layer_after_another_is_selected() assert_eq!(doc(&s, named).text, "Other", "a named layer is copied as named"); } +#[test] +fn duplicate_during_an_edit_with_nothing_selected_copies_the_edited_layer() { + use effectcraft_project::LayerId; + // An engine route can also clear the selection while the edit stays open. Duplicate stays + // available, since its unnamed target is the edited layer, not the selection. + let (mut s, _) = editing("Hello", 5); + s.execute("layer.select", json!({"layers": []})).unwrap(); + assert!(s.state.selected_layers.is_empty() && s.state.text_edit.is_some(), "the edit is open with nothing selected"); + let copy = s.execute("edit.duplicate", json!({})).unwrap()[0].as_u64().unwrap(); + assert!(s.state.text_edit.is_none(), "Duplicate ends the open edit"); + assert_eq!(doc(&s, copy).text, "Hello", "and copies the edited layer"); + assert_eq!(s.state.selected_layers, vec![LayerId(copy)], "which is selected"); + // With no edit open, nothing selected still means nothing to duplicate. + s.execute("layer.select", json!({"layers": []})).unwrap(); + let before = layer_count(&s); + assert!(s.execute("edit.duplicate", json!({})).is_err(), "outside an edit Duplicate needs a selection"); + assert_eq!(layer_count(&s), before, "and copies nothing"); +} + #[test] fn duplicate_ends_the_open_edit_and_selects_the_copy() { let (mut s, t) = editing("Hello", 5); From cdaba8cce5e1201cd20b6230fd76a37884d33d36 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Fri, 30 Oct 2026 10:46:18 +0700 Subject: [PATCH 8/8] rustfmt --- crates/engine/src/tests/text_edit.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/crates/engine/src/tests/text_edit.rs b/crates/engine/src/tests/text_edit.rs index 5053ba396..eb1de6f89 100644 --- a/crates/engine/src/tests/text_edit.rs +++ b/crates/engine/src/tests/text_edit.rs @@ -1410,10 +1410,7 @@ fn duplicate_of_an_empty_type_tool_layer_ends_the_edit_and_copies_it() { use effectcraft_project::LayerId; let (mut s, _keep) = session("Hello"); let new = s.execute("layer.newText", json!({"text": "", "position": [80.0, 80.0], "edit": true})).unwrap()["layer"].as_u64().unwrap(); - assert!( - s.state.text_edit.as_ref().is_some_and(|e| e.created && e.layer == LayerId(new)), - "the Type tool opens the edit on its new, empty layer" - ); + assert!(s.state.text_edit.as_ref().is_some_and(|e| e.created && e.layer == LayerId(new)), "the Type tool opens the edit on its new, empty layer"); let before = layer_count(&s); let copy = s.execute("edit.duplicate", json!({})).unwrap()[0].as_u64().unwrap(); assert!(s.state.text_edit.is_none(), "the edit ends the way text.endEdit ends it");