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() {