diff --git a/crates/engine/src/cmd.rs b/crates/engine/src/cmd.rs index 8442ad74e..1168c431b 100644 --- a/crates/engine/src/cmd.rs +++ b/crates/engine/src/cmd.rs @@ -164,7 +164,7 @@ 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, popup_options, resolve_effect_path, resolve_layer, selectable, + b_p, bad, comp_id, f_p, frontend, layer_mut, layer_p, layers_or, layers_p, merge_p, popup_options, 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 dd1685722..b33f4cc5c 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,9 +104,16 @@ 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 { - // Effects selected → duplicate them (Edit ▸ Duplicate in Effect Controls). - if p.get("layers").is_none() { + 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() && !editing { let fx = super::effect::selected_effects(s); if !fx.is_empty() { let mut out = vec![]; @@ -126,7 +133,17 @@ fn duplicate(s: &mut Session, p: &Value) -> Result { return super::prop_groups::duplicate_contents(s, &items); } } - let (cid, ids) = layers_p(s, p)?; + // A deselected layer never keeps an open text edit (#2226): the copies take the selection, so + // commit an open edit first, exactly as `text.endEdit` and Deselect All do — one call on the + // command path so every route (menu, 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. + let edited = s.state.text_edit.as_ref().map(|e| e.layer); + super::text_edit::end_edit(s, &Value::Null)?; + 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)?; @@ -143,14 +160,16 @@ fn duplicate(s: &mut Session, p: &Value) -> Result { } relink_copies(comp, &map); proj.next_id = next; - st.selected_layers = created.clone(); + // 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 { + super::layer::select_only(st, created.clone()); + } else { + st.selected_layers = created.clone(); + } Ok(created) })?; - // Duplicating selects the copies, so an open text edit on a now-deselected layer ends - // (#2226): typing must not keep editing an unselected layer. - if s.state.text_edit.as_ref().is_some_and(|e| !s.state.selected_layers.contains(&e.layer)) { - super::text_edit::end_edit(s, &Value::Null)?; - } Ok(json!(new.iter().map(|l| l.0).collect::>())) } @@ -968,7 +987,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)", + layers_or_text_edit, + 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), @@ -1038,6 +1065,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/cmd/layer.rs b/crates/engine/src/cmd/layer.rs index eb9641547..6b6a0ecd9 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 0edf7a72c..efe038a0a 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/cmd/params.rs b/crates/engine/src/cmd/params.rs index ff4d443ea..2929d3347 100644 --- a/crates/engine/src/cmd/params.rs +++ b/crates/engine/src/cmd/params.rs @@ -148,6 +148,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). @@ -159,7 +164,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 7a04ea155..eb1de6f89 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(); @@ -1231,10 +1232,188 @@ fn deselect_all_keeps_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, "an empty Type-tool layer is kept, as in After Effects"); + assert_eq!(layer_count(&s), before, "an empty Type-tool layer is kept, as in After Effects"); +} + +#[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) = editing("Hello", 5); + 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!(layer_count(&s), 1, "undo removes the copy"); + 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 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!(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!(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!(layer_count(&s), before + 2, "{what}: and no layer"); + } +} + +#[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_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_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); + 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!(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!(layer_count(&s), 2, "a plain duplicate still duplicates"); + assert!(s.state.text_edit.is_none()); +} + +#[test] +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"); + 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"); + assert_eq!(layer_count(&s), before + 1, "the empty layer is kept, as text.endEdit keeps it, and copied"); + assert_eq!(s.state.selected_layers, vec![LayerId(copy)], "and the copy is selected"); } diff --git a/crates/engine/src/tests/three_d.rs b/crates/engine/src/tests/three_d.rs index 748f8a32f..5b5af594d 100644 --- a/crates/engine/src/tests/three_d.rs +++ b/crates/engine/src/tests/three_d.rs @@ -26,7 +26,8 @@ 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 { +/// Layers in the active comp. +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 f50fbdb3c..b343d2e40 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/src/panels/viewer_text.rs b/crates/ui-egui/src/panels/viewer_text.rs index ce387f6ca..d4a5c8ccf 100644 --- a/crates/ui-egui/src/panels/viewer_text.rs +++ b/crates/ui-egui/src/panels/viewer_text.rs @@ -1362,14 +1362,18 @@ fn key_event( 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 ea4b56d26..1573dd135 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); @@ -728,6 +728,110 @@ fn deselect_all_menu_item_follows_the_edit_while_the_project_panel_is_focused() 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 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(); + // 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"); +} + /// The on-canvas editor's outgoing IME producer: the descriptor carries the edited layer's /// committed text, the real owner, a stable generation and an explicit change cause. A normal /// typed character is application work; an accepted input-method commit is not.