Merge pull request #3232 from vangel76/project-drag-new-folder-3196

Project panel: items dropped on New Folder go into a new folder, as After Effects

crates/engine/src/cmd/file_more.rs

@@ -56,15 +56,39 @@ fn items_p(s: &Session, p: &Value) -> Vec<ItemId> {
     }
 }
 
+/// `project.newFolder {name?, parent?, items?}`. With `items` (ids or names), they move into the
+/// new folder in the same undo step: Project items dropped on the panel's New Folder button, as
+/// in After Effects (#3196). A folder can't end up inside itself.
 fn new_folder(s: &mut Session, p: &Value) -> Result<Value> {
+    let c = "project.newFolder";
     let name = str_p(p, "name").unwrap_or("Untitled Folder").to_string();
     let parent = p.get("parent").and_then(Value::as_u64).map(ItemId).filter(|i| s.project.item(*i).is_some_and(|x| x.is_folder()));
+    let mut items: Vec<ItemId> = vec![];
+    if let Some(v) = p.get("items") {
+        let Value::Array(a) = v else { return Err(bad(c, "`items` must be an array of item ids or names")) };
+        for v in a {
+            let id = super::project_items::item_ref(s, v, c)?;
+            if !items.contains(&id) {
+                items.push(id);
+            }
+        }
+        if let Some(f) = parent
+            && let Some(inside) = items.iter().find(|i| super::project_items::is_within(&s.project, f, **i))
+        {
+            return Err(bad(c, format!("can't move folder {} into a folder inside itself", inside.0)));
+        }
+    }
     let id = s.edit("New Folder", None, |proj, st| {
         let id = proj.add_item(&name, Label::Yellow, parent, ItemKind::Folder);
+        for i in &items {
+            if let Some(it) = proj.item_mut(*i) {
+                it.parent = Some(id);
+            }
+        }
         st.project_selection = vec![id];
         Ok(id)
     })?;
-    Ok(json!({"item": id.0}))
+    if p.get("items").is_some() { Ok(json!({"item": id.0, "moved": items.len()})) } else { Ok(json!({"item": id.0})) }
 }
 
 fn close(s: &mut Session, p: &Value) -> Result<Value> {
@@ -1211,7 +1235,15 @@ fn reveal_in_finder(s: &mut Session, p: &Value) -> Result<Value> {
 
 pub fn specs() -> Vec<CommandSpec> {
     vec![
-        cmd!("project.newFolder", "New Folder", ["File", "New"], Some("Cmd+Alt+Shift+N"), "{name?, parent?}", always, new_folder),
+        cmd!(
+            "project.newFolder",
+            "New Folder",
+            ["File", "New"],
+            Some("Cmd+Alt+Shift+N"),
+            "{name?, parent?, items?: [id|name] (moved into it)}",
+            always,
+            new_folder
+        ),
         cmd!("file.close", "Close", ["File"], Some("Cmd+W"), "{comp?}", has_comp, close),
         cmd!("file.closeProject", "Close Project", ["File"], None, "{}", always, close_project),
         cmd!("file.saveCopy", "Save a Copy...", ["File", "Save As"], None, "{path}", always, save_copy),

crates/engine/src/cmd/project_items.rs

@@ -8,7 +8,7 @@ use super::{CommandSpec, always, b_p, bad, str_p};
 use crate::{EngineError, Result, Session, cmd, query};
 
 /// `item` (id or name) → id.
-fn item_ref(s: &Session, v: &Value, cmd: &str) -> Result<ItemId> {
+pub(super) fn item_ref(s: &Session, v: &Value, cmd: &str) -> Result<ItemId> {
     match v {
         Value::Number(n) => n.as_u64().map(ItemId).filter(|i| s.project.item(*i).is_some()),
         Value::String(name) => s.project.find_by_name(name).map(|i| i.id),

crates/engine/src/tests/project_items.rs

@@ -40,6 +40,52 @@ fn move_into_folder_rename_label_comment_with_undo() {
     assert_eq!(s.project.item(comp).unwrap().comment, "");
 }
 
+/// #3196: `project.newFolder` with `items` (items dropped on the Project panel's New Folder
+/// button) makes the folder and moves them into it, out of the folder they were in, in one undo
+/// step. A folder can't go into a new folder made inside itself.
+#[test]
+fn new_folder_with_items_moves_them_into_it_in_one_undo_step() {
+    let mut s = Session::default();
+    let a = ItemId(s.execute("comp.new", json!({"name": "A", "width": 64, "height": 64, "duration": 1})).unwrap()["comp"].as_u64().unwrap());
+    let b = ItemId(s.execute("comp.new", json!({"name": "B", "width": 64, "height": 64, "duration": 1})).unwrap()["comp"].as_u64().unwrap());
+    let bin = ItemId(s.execute("project.newFolder", json!({"name": "Bin"})).unwrap()["item"].as_u64().unwrap());
+    s.execute("project.move", json!({"items": [b.0], "folder": bin.0})).unwrap();
+    let before = s.project.clone();
+    let undo = s.history.undo.len();
+
+    let r = s.execute("project.newFolder", json!({"items": [a.0, "B", a.0]})).unwrap();
+    let f = ItemId(r["item"].as_u64().unwrap());
+    assert_eq!(r["moved"], 2, "duplicates count once");
+    let folder = s.project.item(f).unwrap();
+    assert!(folder.is_folder());
+    assert_eq!((folder.name.as_str(), folder.parent), ("Untitled Folder", None), "the New Folder button's name and place");
+    assert_eq!(s.project.item(a).unwrap().parent, Some(f));
+    assert_eq!(s.project.item(b).unwrap().parent, Some(f), "out of the folder it was in");
+    assert_eq!(s.state.project_selection, [f], "the new folder is selected");
+    assert_eq!(s.history.undo.len(), undo + 1, "one undo step");
+    s.execute("edit.undo", json!({})).unwrap();
+    assert_eq!(s.project, before, "one undo takes back the folder and the move");
+
+    // A folder dropped on New Folder goes into the new folder; a new folder made inside the
+    // folder that is moving into it is refused, and nothing changes.
+    let r = s.execute("project.newFolder", json!({"items": [bin.0]})).unwrap();
+    let f = ItemId(r["item"].as_u64().unwrap());
+    assert_eq!(s.project.item(bin).unwrap().parent, Some(f));
+    s.execute("edit.undo", json!({})).unwrap();
+    // Inside another folder (`parent`), and an empty list moves nothing.
+    let r = s.execute("project.newFolder", json!({"parent": bin.0, "items": [a.0]})).unwrap();
+    let f = ItemId(r["item"].as_u64().unwrap());
+    assert_eq!((s.project.item(f).unwrap().parent, s.project.item(a).unwrap().parent), (Some(bin), Some(f)));
+    s.execute("edit.undo", json!({})).unwrap();
+    assert_eq!(s.execute("project.newFolder", json!({"items": []})).unwrap()["moved"], 0);
+    s.execute("edit.undo", json!({})).unwrap();
+    let before = s.project.clone();
+    assert!(s.execute("project.newFolder", json!({"parent": bin.0, "items": [bin.0]})).is_err());
+    assert!(s.execute("project.newFolder", json!({"items": [999_999]})).is_err(), "an unknown item");
+    assert!(s.execute("project.newFolder", json!({"items": a.0})).is_err(), "not an array");
+    assert_eq!(s.project, before);
+}
+
 #[test]
 fn delete_and_duplicate_items() {
     let mut s = Session::default();

crates/ui-egui/src/menus/invoke.rs

@@ -203,10 +203,16 @@ pub fn invoke(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, params: V
     }
     // A new folder (File ▸ New, its shortcut, the Project panel's button and context menu) opens
     // with its name ready to type, as in After Effects (#1658). The command selects it.
-    // `engine.execute` keeps the headless default.
-    if id == "project.newFolder" && no_params(&params) {
+    // `engine.execute` keeps the headless default. Items dropped on the New Folder button (only
+    // `items`, #3196) open the name too.
+    if id == "project.newFolder" && (no_params(&params) || params.as_object().is_some_and(|m| m.len() == 1 && m.contains_key("items"))) {
+        let dropped = params.get("items").is_some();
         let r = run_engine(app, ctx, id, params);
-        if r.is_ok() {
+        if let Ok(v) = &r {
+            // Open a folder that items were dropped into, so they stay in view.
+            if dropped && let Some(f) = v.get("item").and_then(Value::as_u64) {
+                app.ui.project_open_folders.insert(f);
+            }
             app.show_panel(PanelKind::Project);
             crate::panels::project::begin_rename(app, ctx);
         }

crates/ui-egui/src/panels/project.rs

@@ -3,7 +3,8 @@
 //! Media Duration, Frame Rate, File Path, Comment — shown or hidden from the header's context
 //! menu), folders (drag items into and out of them), renaming (Enter, or double-click the name),
 //! the label colour picker, and the bottom bar (interpret, new folder, new comp: click, or drop
-//! items on it for a comp from them, bit depth, delete), and the keyboard (the arrow keys select,
+//! items on New Folder for a folder holding them, or on New Comp for a comp from them; bit depth,
+//! delete), and the keyboard (the arrow keys select,
 //! extend the selection and open / close folders; rows and buttons carry AccessKit names and
 //! states for screen readers). Edits are engine commands, so they are undoable.
 
@@ -905,17 +906,24 @@ fn footer(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painter, foot:
             }
         }
         // Items dropped on Create a new Composition: New Comp from Selection with them (After
-        // Effects; several items ask how in its dialog).
-        if id == "newComp"
+        // Effects; several items ask how in its dialog). Dropped on Create a new Folder: a new
+        // folder holding them (#3196). The dragged item, or the selection it belongs to.
+        if matches!(id, "newComp" | "newFolder")
             && let Some(DragPayload::Item(iid)) = egui::DragAndDrop::payload::<DragPayload>(&ctx).as_deref()
             && ui.rect_contains_pointer(r)
         {
             p.rect_stroke(r, 4.0, Stroke::new(2.0, t.accent), egui::StrokeKind::Inside);
             if ctx.input(|i| i.pointer.any_released()) {
-                if !app.session.state.project_selection.contains(&ItemId(*iid)) {
-                    app.session.state.project_selection = vec![ItemId(*iid)];
+                let selected = app.session.state.project_selection.contains(&ItemId(*iid));
+                if id == "newComp" {
+                    if !selected {
+                        app.session.state.project_selection = vec![ItemId(*iid)];
+                    }
+                    actions.push(("file.newCompFromSelection".into(), json!({})));
+                } else {
+                    let items: Vec<u64> = if selected { app.session.state.project_selection.iter().map(|i| i.0).collect() } else { vec![*iid] };
+                    actions.push(("project.newFolder".into(), json!({"items": items})));
                 }
-                actions.push(("file.newCompFromSelection".into(), json!({})));
                 egui::DragAndDrop::clear_payload(&ctx);
             }
         }

crates/ui-egui/tests/it/ui_project_new_folder.rs

@@ -143,6 +143,64 @@ fn the_file_menu_opens_the_name_in_view() {
     assert_eq!(name(&h, folder).as_deref(), Some("Renders"));
 }
 
+/// Press on `from`, move to `to` in steps, release there.
+fn drag(h: &mut Harness<'_, EffectcraftApp>, from: Pos2, to: Pos2) {
+    h.input_mut().events.push(Event::PointerMoved(from));
+    h.step();
+    h.input_mut().events.push(Event::PointerButton { pos: from, button: PointerButton::Primary, pressed: true, modifiers: Modifiers::NONE });
+    h.step();
+    for k in 1..=10 {
+        h.input_mut().events.push(Event::PointerMoved(from + (to - from) * (k as f32 / 10.0)));
+        h.step();
+    }
+    h.input_mut().events.push(Event::PointerButton { pos: to, button: PointerButton::Primary, pressed: false, modifiers: Modifiers::NONE });
+    h.run_steps(3);
+}
+
+fn item_named(h: &Harness<'_, EffectcraftApp>, name: &str) -> ItemId {
+    h.state().session.project.items.values().find(|i| i.name == name).map(|i| i.id).unwrap_or_else(|| panic!("no item {name}"))
+}
+
+fn parent(h: &Harness<'_, EffectcraftApp>, id: ItemId) -> Option<ItemId> {
+    h.state().session.project.item(id).and_then(|i| i.parent)
+}
+
+/// #3196: two selected items (one at the root, one in a folder) dragged onto the New Folder
+/// button go into a new folder, out of the folder they were in, with its name open as the
+/// button's; one undo takes it all back.
+#[test]
+fn items_dropped_on_the_new_folder_button_go_into_a_new_folder() {
+    let (mut h, bin) = harness(2);
+    let (a, b) = (item_named(&h, "Comp 000"), item_named(&h, "Comp 001"));
+    h.state_mut().session.execute("project.move", json!({"items": [b.0], "folder": bin.0})).unwrap();
+    h.state_mut().session.state.project_selection = vec![a, b];
+    h.run_steps(3);
+    let before = folders(&h);
+    let project = h.state().session.project.clone();
+    let from = rect(&h, &format!("project.item.{}.name", a.0)).center();
+    let to = rect(&h, "project.newFolder").center();
+    drag(&mut h, from, to);
+    let [folder] = new_folders(&h, &before)[..] else { panic!("one new folder") };
+    assert_eq!((parent(&h, a), parent(&h, b)), (Some(folder), Some(folder)), "both dragged items are in it");
+    assert_eq!(parent(&h, folder), None, "where the button makes folders");
+    assert_eq!(h.state().session.state.project_selection, [folder]);
+    assert!(renaming(&h, folder), "its name is open, as from the button");
+    assert!(h.state().ui.project_open_folders.contains(&folder.0), "open, so the moved items stay in view");
+    key(&mut h, Key::Escape);
+    assert_eq!(name(&h, folder).as_deref(), Some("Untitled Folder"));
+    h.state_mut().session.execute("edit.undo", json!({})).unwrap();
+    assert_eq!(h.state().session.project, project, "one undo takes back the folder and the move");
+
+    // An item that is not selected goes alone.
+    h.state_mut().session.state.project_selection = vec![b];
+    h.run_steps(3);
+    let before = folders(&h);
+    let from = rect(&h, &format!("project.item.{}.name", a.0)).center();
+    drag(&mut h, from, to);
+    let [folder] = new_folders(&h, &before)[..] else { panic!("one new folder") };
+    assert_eq!((parent(&h, a), parent(&h, b)), (Some(folder), Some(bin)), "only the dragged item");
+}
+
 /// Agents and scripts: `engine.execute` only creates the folder.
 #[test]
 fn engine_execute_only_creates_the_folder() {