From dd92908bc82eecc32896dd6b9db80068ae39ab16 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sat, 7 Nov 2026 15:06:49 +0100 Subject: [PATCH] Project panel: items dropped on New Folder go into a new folder, as After Effects project.newFolder takes optional `items` (ids or names) and moves them into the new folder in the same undo step, out of any folder they were in; a folder can't go into a new folder made inside itself. Dropping dragged Project items (the dragged item, or the selection it belongs to) on the panel's New Folder button runs it, with the drop outline New Composition shows; the new folder opens with its name ready to type, like one made with the button. Fixes #3196 --- crates/engine/src/cmd/file_more.rs | 36 +++++++++++- crates/engine/src/cmd/project_items.rs | 2 +- crates/engine/src/tests/project_items.rs | 46 +++++++++++++++ crates/ui-egui/src/menus/invoke.rs | 12 +++- crates/ui-egui/src/panels/project.rs | 20 +++++-- .../ui-egui/tests/it/ui_project_new_folder.rs | 58 +++++++++++++++++++ 6 files changed, 162 insertions(+), 12 deletions(-) diff --git a/crates/engine/src/cmd/file_more.rs b/crates/engine/src/cmd/file_more.rs index 168529c47..104907f74 100644 --- a/crates/engine/src/cmd/file_more.rs +++ b/crates/engine/src/cmd/file_more.rs @@ -56,15 +56,39 @@ fn items_p(s: &Session, p: &Value) -> Vec { } } +/// `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 { + 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 = 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 { @@ -1202,7 +1226,15 @@ fn reveal_in_finder(s: &mut Session, p: &Value) -> Result { pub fn specs() -> Vec { 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), diff --git a/crates/engine/src/cmd/project_items.rs b/crates/engine/src/cmd/project_items.rs index 4d76861dc..e513a0511 100644 --- a/crates/engine/src/cmd/project_items.rs +++ b/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 { +pub(super) fn item_ref(s: &Session, v: &Value, cmd: &str) -> Result { 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), diff --git a/crates/engine/src/tests/project_items.rs b/crates/engine/src/tests/project_items.rs index 105174c35..27e8e7818 100644 --- a/crates/engine/src/tests/project_items.rs +++ b/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(); diff --git a/crates/ui-egui/src/menus/invoke.rs b/crates/ui-egui/src/menus/invoke.rs index 0c7573c91..0f53a8372 100644 --- a/crates/ui-egui/src/menus/invoke.rs +++ b/crates/ui-egui/src/menus/invoke.rs @@ -195,10 +195,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(¶ms) { + // `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(¶ms) || 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); } diff --git a/crates/ui-egui/src/panels/project.rs b/crates/ui-egui/src/panels/project.rs index 8d67fd9ee..21163391c 100644 --- a/crates/ui-egui/src/panels/project.rs +++ b/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::(&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 = 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); } } diff --git a/crates/ui-egui/tests/it/ui_project_new_folder.rs b/crates/ui-egui/tests/it/ui_project_new_folder.rs index 63a1c6ae3..b86bf663d 100644 --- a/crates/ui-egui/tests/it/ui_project_new_folder.rs +++ b/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 { + 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() {