diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 01aa82e2..c83bb03f 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -1307,9 +1307,20 @@ pub fn handle_shortcuts(app: &mut EffectcraftApp, ctx: &egui::Context) { if key == egui::Key::Enter && !mods.any() && app.ui.focused == PanelKind::Project { continue; } - // Delete/Backspace clears selection. + // The Project panel has its own selection; a previously selected Timeline layer + // must not be deleted while the Project panel has focus. if matches!(key, egui::Key::Delete | egui::Key::Backspace) && !mods.any() { - let _ = invoke(app, ctx, "edit.clear", json!({})); + let command = if app.ui.focused == PanelKind::Project { + if app.session.state.project_selection.is_empty() { + continue; + } + "project.delete" + } else { + "edit.clear" + }; + if let Err(e) = invoke(app, ctx, command, json!({})) { + app.ui.status = e; + } continue; } if let Some((_, _, id, params)) = binds.iter().find(|(m, k, ..)| *k == key && mods_match(*m, mods)) { diff --git a/crates/ui-egui/tests/ui_project_delete.rs b/crates/ui-egui/tests/ui_project_delete.rs new file mode 100644 index 00000000..a343740d --- /dev/null +++ b/crates/ui-egui/tests/ui_project_delete.rs @@ -0,0 +1,141 @@ +//! Project-panel deletion uses its selection, even when a Timeline layer is still selected. + +use effectcraft_engine::Session; +use effectcraft_engine::project::{Footage, FootageKind, ItemId, ItemKind, LayerId}; +use effectcraft_ui_egui::EffectcraftApp; +use effectcraft_ui_egui::dock::PanelKind; +use egui::{Event, Key, Modifiers, pos2}; +use egui_kittest::Harness; +use serde_json::json; + +fn harness() -> (Harness<'static, EffectcraftApp>, Vec, LayerId) { + let mut s = Session::default(); + s.execute("comp.new", json!({"name": "Delete me", "width": 320, "height": 180, "duration": 4})).unwrap(); + let comp = s.active_comp_id().unwrap(); + s.execute("comp.new", json!({"name": "Working composition", "width": 320, "height": 180, "duration": 4})).unwrap(); + s.execute("layer.newSolid", json!({"name": "Solid", "color": "#406080"})).unwrap(); + let solid = s.project.items.values().find(|i| matches!(i.kind, ItemKind::Solid(_))).unwrap().id; + let footage = std::sync::Arc::make_mut(&mut s.project).add_item( + "Data asset", + Default::default(), + None, + ItemKind::Footage(Footage { kind: FootageKind::Data, path: "original.json".into(), data: Some("{}".into()), ..Default::default() }), + ); + let folder = ItemId(s.execute("project.newFolder", json!({"name": "Folder"})).unwrap()["item"].as_u64().unwrap()); + let keep = LayerId(s.execute("layer.newNull", json!({"name": "Keep this layer"})).unwrap()["layer"].as_u64().unwrap()); + let mut h = Harness::builder().with_size(egui::vec2(1600.0, 1000.0)).build_eframe(|_| EffectcraftApp::new(s)); + let folders: Vec = h.state().session.project.items.values().filter(|i| i.is_folder()).map(|i| i.id.0).collect(); + h.state_mut().ui.project_open_folders.extend(folders); + h.run_steps(3); + (h, vec![comp, solid, footage, folder], keep) +} + +fn click(h: &mut Harness<'_, EffectcraftApp>, id: &str, modifiers: Modifiers) { + let e = h.state().auto.find(id).unwrap_or_else(|| panic!("no {id}")).clone(); + let p = pos2(e.rect[0] + e.rect[2] / 2.0, e.rect[1] + e.rect[3] / 2.0); + h.input_mut().events.push(Event::ModifiersChanged(modifiers)); + h.input_mut().events.push(Event::PointerMoved(p)); + h.input_mut().events.push(Event::PointerButton { pos: p, button: egui::PointerButton::Primary, pressed: true, modifiers }); + h.step(); + h.input_mut().events.push(Event::PointerButton { pos: p, button: egui::PointerButton::Primary, pressed: false, modifiers }); + h.run_steps(2); + h.input_mut().events.push(Event::ModifiersChanged(Modifiers::NONE)); +} + +fn key(h: &mut Harness<'_, EffectcraftApp>, key: Key, modifiers: Modifiers) { + h.input_mut().events.push(Event::Key { key, physical_key: None, pressed: true, repeat: false, modifiers }); + h.input_mut().events.push(Event::Key { key, physical_key: None, pressed: false, repeat: false, modifiers }); + h.run_steps(2); +} + +#[test] +fn delete_and_backspace_remove_project_items_and_undo() { + for delete_key in [Key::Delete, Key::Backspace] { + for target in 0..4 { + let (mut h, items, keep) = harness(); + let item = items[target]; + click(&mut h, &format!("project.item.{}.name", item.0), Modifiers::NONE); + assert_eq!(h.state().ui.focused, PanelKind::Project); + assert_eq!(h.state().session.state.project_selection, vec![item]); + assert_eq!(h.state().session.state.selected_layers, vec![keep]); + let project = h.state().session.project.clone(); + let undo = h.state().session.history.undo.len(); + key(&mut h, delete_key, Modifiers::NONE); + assert!(h.state().session.project.item(item).is_none(), "{delete_key:?} should delete project item {target}"); + assert!(h.state().session.active_comp().unwrap().layer(keep).is_some(), "the unrelated Timeline layer stays"); + assert!(h.state().session.state.project_selection.is_empty()); + assert_eq!(h.state().session.project.items.len(), project.items.len() - 1); + assert_eq!(h.state().session.history.undo.len(), undo + 1); + h.state_mut().session.execute("edit.undo", json!({})).unwrap(); + assert_eq!(h.state().session.project, project, "one undo restores the item and its references"); + } + } +} + +#[test] +fn project_multi_selection_deletes_together_and_undoes_once() { + let (mut h, items, keep) = harness(); + for (i, item) in items.iter().enumerate() { + click(&mut h, &format!("project.item.{}.name", item.0), if i == 0 { Modifiers::NONE } else { Modifiers::SHIFT }); + } + assert_eq!(h.state().session.state.project_selection, items); + let project = h.state().session.project.clone(); + let undo = h.state().session.history.undo.len(); + key(&mut h, Key::Backspace, Modifiers::NONE); + assert!(items.iter().all(|i| h.state().session.project.item(*i).is_none())); + assert!(h.state().session.active_comp().unwrap().layer(keep).is_some()); + assert_eq!(h.state().session.history.undo.len(), undo + 1); + h.state_mut().session.execute("edit.undo", json!({})).unwrap(); + assert_eq!(h.state().session.project, project); +} + +#[test] +fn empty_project_selection_does_not_delete_a_timeline_layer() { + let (mut h, items, keep) = harness(); + click(&mut h, &format!("project.item.{}.name", items[0].0), Modifiers::NONE); + h.state_mut().session.execute("project.select", json!({"items": []})).unwrap(); + let undo = h.state().session.history.undo.len(); + for delete_key in [Key::Delete, Key::Backspace] { + key(&mut h, delete_key, Modifiers::NONE); + assert!(h.state().session.active_comp().unwrap().layer(keep).is_some()); + assert_eq!(h.state().session.history.undo.len(), undo); + } +} + +#[test] +fn viewer_focus_keeps_layer_deletion_with_a_project_selection() { + for delete_key in [Key::Delete, Key::Backspace] { + let (mut h, items, keep) = harness(); + click(&mut h, &format!("project.item.{}.name", items[0].0), Modifiers::NONE); + click(&mut h, "viewer.comp", Modifiers::NONE); + h.state_mut().session.execute("layer.select", json!({"layers": [keep.0]})).unwrap(); + assert_eq!(h.state().ui.focused, PanelKind::Composition); + key(&mut h, delete_key, Modifiers::NONE); + assert!(h.state().session.active_comp().unwrap().layer(keep).is_none()); + assert!(h.state().session.project.item(items[0]).is_some()); + } +} + +#[test] +fn typing_dialogs_and_modified_keys_do_not_delete_project_items() { + let (mut h, items, _) = harness(); + let item = items[0]; + click(&mut h, &format!("project.item.{}.name", item.0), Modifiers::NONE); + let project = h.state().session.project.clone(); + for delete_key in [Key::Delete, Key::Backspace] { + key(&mut h, delete_key, Modifiers::SHIFT); + assert_eq!(h.state().session.project, project); + } + h.state_mut().dialog = Some(effectcraft_ui_egui::Dialog::About); + key(&mut h, Key::Delete, Modifiers::NONE); + assert_eq!(h.state().session.project, project); + h.state_mut().dialog = None; + h.run_steps(2); + key(&mut h, Key::Enter, Modifiers::NONE); + h.run_steps(2); + assert!(h.ctx.egui_wants_keyboard_input(), "the inline rename field has focus"); + for delete_key in [Key::Backspace, Key::Delete] { + key(&mut h, delete_key, Modifiers::NONE); + assert_eq!(h.state().session.project, project, "typing must not delete the item"); + } +} diff --git a/docs/gaps.md b/docs/gaps.md index 062d2b95..1f4124c9 100644 --- a/docs/gaps.md +++ b/docs/gaps.md @@ -67,6 +67,11 @@ The most important missing piece: it turns every other estimate here into a meas - Run the headless snapshot and control-channel checks on Linux (X11 and Wayland) and Windows, not only macOS. Interactions that only fail with real input (docking drags, viewer pan, drag-and-drop import, inline rename) need scripted input tests. +- Regression evidence for [#67](https://github.com/storytold/effectcraft/issues/67): scripted + input tests in `ui_project_delete`, checked headlessly on Linux, cover Delete/Backspace on + Project items, multiple selection and undo. They check that an unrelated Timeline layer stays, + an empty Project selection does nothing, viewer focus still deletes layers, and typing or + dialogs do not delete items. - Done when: no open bug blocks a basic workflow (import, arrange, animate, preview, render) on any of the three desktop platforms, and every bug users report gets triaged within a day.