Merge pull request #499 from tylerjenningsw/enhance/project-shift-delete

This commit is contained in:
Claude committed 2026-10-10 12:51:16 +00:00
commit a5feb441e9
5 files changed
+264 -25

No files matched your search

+10
View File
@@ -106,5 +106,15 @@ pub fn specs() -> Vec<CommandSpec> {
fe!("layer.openLayer", "Open Layer", ["Layer"], None, "{layer?}", has_layers),
fe!("effect.manage", "Manage Effects...", ["Effect"], None, "{}", always),
fe!("anim.browsePresets", "Browse Presets...", ["Animation"], None, "{}", always),
// Project panel: delete the selected items without the prompt that Delete shows when
// compositions use them. The UI runs `project.delete` (agents can run it directly).
fe!(
"project.deleteWithoutConfirmation",
"Delete Project Items Without Confirmation",
[],
Some("Shift+Delete"),
"{} — the Project panel's selection, with the layers that use it; one undo step",
always
),
]
}
+59 -14
View File
@@ -300,6 +300,14 @@ pub fn invoke(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, params: V
None => Err("select a Render Queue item first".into()),
};
}
// Shift+Delete deletes the Project panel's selection without asking; it does nothing in the
// other panels.
if id == "project.deleteWithoutConfirmation" {
if app.ui.focused != PanelKind::Project {
return Ok(Value::Null);
}
return crate::panels::delete_items::delete_confirmed(app, ctx, json!({}));
}
let now = ctx.input(|i| i.time);
// Closing a modified project asks to save it first; the command runs once answered.
if crate::panels::unsaved::guard(app, id, &params) {
@@ -668,7 +676,7 @@ fn clipboard_note(s: &effectcraft_engine::Session) -> String {
}
}
fn run_engine(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, params: Value) -> Result<Value, String> {
pub(crate) fn run_engine(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, params: Value) -> Result<Value, String> {
let r = app.session.execute(id, params).map_err(|e| e.to_string());
if let Err(e) = &r {
app.ui.status = e.clone();
@@ -713,6 +721,8 @@ pub fn frontend(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, p: Valu
}
// View ▸ New Viewer.
"view.newViewer" => json!({"viewer": crate::panels::viewers::new_viewer(app)}),
// Run by id (control channel, MCP): the Project panel's selection, whatever has focus.
"project.deleteWithoutConfirmation" => crate::panels::delete_items::delete_confirmed(app, ctx, json!({}))?,
"window.scriptPanel" => {
let id = p.get("window").and_then(Value::as_u64).ok_or("no ScriptUI panel window")? as u32;
app.show_panel(PanelKind::ScriptPanel(id));
@@ -1461,8 +1471,27 @@ fn space_tap_id() -> egui::Id {
egui::Id::new("spacebar-tap")
}
/// The modifiers held when this frame's input began: the state the last pass ended with, if it
/// ran this (egui keeps only the state after the frame's events). Records this pass's state.
fn modifiers_at_start(ctx: &egui::Context) -> Option<egui::Modifiers> {
let id = egui::Id::new("shortcut-modifiers");
let pass = ctx.cumulative_pass_nr();
let now = ctx.input(|i| i.modifiers);
ctx.data_mut(|d| {
let last = d.get_temp::<(u64, egui::Modifiers)>(id);
d.insert_temp(id, (pass, now));
last.and_then(|(p, m)| (p.checked_add(1) == Some(pass)).then_some(m))
})
}
/// Only Shift held (Shift+Delete, not Ctrl/Alt+Shift+Delete).
fn only_shift(m: egui::Modifiers) -> bool {
m.shift && !m.alt && !m.ctrl && !m.command && !m.mac_cmd
}
/// Dispatch keyboard shortcuts (skipped while typing in a text field).
pub fn handle_shortcuts(app: &mut EffectcraftApp, ctx: &egui::Context) {
let start = modifiers_at_start(ctx);
// Dialog cancellation owns Escape even when a text field has keyboard focus.
if app.dialog.is_some() && ctx.input(|i| i.key_pressed(egui::Key::Escape)) {
if crate::panels::shortcut_editor::recording(app) || egui::Popup::is_any_open(ctx) {
@@ -1499,14 +1528,23 @@ pub fn handle_shortcuts(app: &mut EffectcraftApp, ctx: &egui::Context) {
}
// Text editing in the viewer takes the clipboard events itself.
let clipboard = app.session.state.text_edit.is_none();
let project = app.ui.focused == PanelKind::Project;
let events: Vec<(egui::Key, egui::Modifiers, bool)> = ctx.input(|i| {
// The windowing layer turns Ctrl+C / Ctrl+X / Ctrl+V into clipboard events instead of
// key presses: map them back to the keys (with the modifiers held) so Edit ▸ Copy, Cut,
// Paste and their variants (Ctrl+Alt+C…) run.
let held = if i.modifiers.command { i.modifiers } else { egui::Modifiers::COMMAND };
i.events
.iter()
.filter_map(|e| match e {
// Paste and their variants (Ctrl+Alt+C…) run. Clipboard events carry no modifiers and
// `i.modifiers` is the state after all of the frame's events, so each is read with the
// modifiers where it is in the stream: walk the events from the frame's starting state.
// `None` when that isn't known.
let changed = i.events.iter().any(|e| matches!(e, egui::Event::ModifiersChanged(_) | egui::Event::WindowFocused(false)));
let mut at = if changed { start } else { Some(i.modifiers) };
let held = |at: Option<egui::Modifiers>| at.filter(|m| m.command).unwrap_or(egui::Modifiers::COMMAND);
let mut out = Vec::new();
for e in &i.events {
match e {
egui::Event::ModifiersChanged(m) => at = Some(*m),
// egui forgets the modifiers held when the window loses focus.
egui::Event::WindowFocused(false) => at = Some(egui::Modifiers::NONE),
egui::Event::Key { key, pressed: true, modifiers, repeat, .. }
if !*repeat
|| matches!(
@@ -1514,15 +1552,22 @@ pub fn handle_shortcuts(app: &mut EffectcraftApp, ctx: &egui::Context) {
egui::Key::PageUp | egui::Key::PageDown | egui::Key::ArrowLeft | egui::Key::ArrowRight | egui::Key::ArrowUp | egui::Key::ArrowDown
) =>
{
Some((*key, *modifiers, true))
out.push((*key, *modifiers, true));
}
egui::Event::Key { key: egui::Key::Space, pressed: false, .. } => Some((egui::Key::Space, egui::Modifiers::NONE, false)),
egui::Event::Copy if clipboard => Some((egui::Key::C, held, true)),
egui::Event::Cut if clipboard => Some((egui::Key::X, held, true)),
egui::Event::Paste(_) if clipboard => Some((egui::Key::V, held, true)),
_ => None,
})
.collect()
egui::Event::Key { key: egui::Key::Space, pressed: false, .. } => out.push((egui::Key::Space, egui::Modifiers::NONE, false)),
egui::Event::Copy if clipboard => out.push((egui::Key::C, held(at), true)),
// On Windows, Shift+Delete is the old Cut key and arrives as Cut too. The Project
// panel has no clipboard: there a Cut with only Shift held is Shift+Delete (Delete
// Project Items Without Confirmation). Never when the modifiers aren't known.
egui::Event::Cut if clipboard && project && at.is_some_and(only_shift) => {
out.push((egui::Key::Delete, egui::Modifiers::SHIFT, true));
}
egui::Event::Cut if clipboard => out.push((egui::Key::X, held(at), true)),
egui::Event::Paste(_) if clipboard => out.push((egui::Key::V, held(at), true)),
_ => {}
}
}
out
});
// Numpad * (or `*` typed with Shift+8): Add Marker (#275). egui has no key for it, so it
// comes as text.
+12 -8
View File
@@ -1,7 +1,8 @@
//! Deleting Project items that compositions use asks first, as in After Effects: the Delete key
//! and the Project panel's trash button (`project.delete` through the UI) show how many layers in
//! how many compositions go with the items (`project.usage`). Items nothing uses are deleted at
//! once. Agents running `project.delete` with `engine.execute` are never asked.
//! once. Agents running `project.delete` with `engine.execute` are never asked, and neither is
//! Delete Project Items Without Confirmation (Shift+Delete in the Project panel).
//!
//! Automation ids: `dialog.deleteItems.delete`, `dialog.deleteItems.cancel`.
@@ -16,8 +17,6 @@ use crate::{Dialog, EffectcraftApp};
#[derive(Clone, Debug, Default)]
pub struct Pending {
pub command: Option<(Value, Value)>,
/// The next `project.delete` runs without asking (the user answered Delete).
pub answered: bool,
}
fn plural(n: u64, one: &str, many: &str) -> String {
@@ -27,7 +26,7 @@ fn plural(n: u64, one: &str, many: &str) -> String {
/// Whether `id` needs the prompt first. When it does, the prompt opens holding the command and
/// the caller must not run it (it runs once the user answers Delete).
pub fn guard(app: &mut EffectcraftApp, id: &str, params: &Value) -> bool {
if id != "project.delete" || std::mem::take(&mut app.dialog_state.delete_items.answered) {
if id != "project.delete" {
return false;
}
let Ok(usage) = app.session.execute("project.usage", params.clone()) else { return false };
@@ -79,9 +78,14 @@ pub fn show(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) {
app.dialog = None;
app.dialog_state.delete_items.command = None;
if delete {
app.dialog_state.delete_items.answered = true;
if let Err(e) = crate::menus::invoke(app, ctx, "project.delete", params) {
app.ui.status = e;
}
// An error is already in the status bar.
let _ = delete_confirmed(app, ctx, params);
}
}
/// Run `project.delete` with `params` without the prompt: this request is already confirmed (the
/// prompt's Delete, or Delete Project Items Without Confirmation). The confirmation goes with the
/// request, so it can't carry over to a later deletion.
pub fn delete_confirmed(app: &mut EffectcraftApp, ctx: &egui::Context, params: Value) -> Result<Value, String> {
crate::menus::run_engine(app, ctx, "project.delete", params)
}
+177 -3
View File
@@ -1,4 +1,5 @@
//! Project-panel deletion uses its selection, even when a Timeline layer is still selected.
//! Shift+Delete deletes it without asking (Delete Project Items Without Confirmation).
use effectcraft_engine::Session;
use effectcraft_engine::project::{Footage, FootageKind, ItemId, ItemKind, LayerId};
@@ -51,6 +52,31 @@ fn key(h: &mut Harness<'_, EffectcraftApp>, key: Key, modifiers: Modifiers) {
h.run_steps(2);
}
/// Shift+Delete as each platform delivers it: a key press (macOS, Linux), or on Windows a Cut
/// event with Shift held (the windowing layer turns Shift+Delete into Cut).
#[derive(Clone, Copy, Debug)]
enum ShiftDelete {
Key,
WindowsCut,
}
fn shift_delete(h: &mut Harness<'_, EffectcraftApp>, how: ShiftDelete) {
match how {
ShiftDelete::Key => key(h, Key::Delete, Modifiers::SHIFT),
// Shift down, Cut and Shift up all in one frame.
ShiftDelete::WindowsCut => frame(h, &[Event::ModifiersChanged(Modifiers::SHIFT), Event::Cut, Event::ModifiersChanged(Modifiers::NONE)]),
}
}
/// Queue `events` together and run them as one frame.
fn frame(h: &mut Harness<'_, EffectcraftApp>, events: &[Event]) {
h.input_mut().events.extend(events.iter().cloned());
h.step();
h.run_steps(2);
}
const SHIFT_DELETE: [ShiftDelete; 2] = [ShiftDelete::Key, ShiftDelete::WindowsCut];
/// Items that compositions use ask first, as in After Effects: answer Delete when asked.
/// Returns whether it asked.
fn confirm_if_asked(h: &mut Harness<'_, EffectcraftApp>) -> bool {
@@ -138,12 +164,22 @@ fn typing_dialogs_and_modified_keys_do_not_delete_project_items() {
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);
// Only plain Delete/Backspace and Shift+Delete delete Project items.
for (k, m) in [
(Key::Backspace, Modifiers::SHIFT),
(Key::Delete, Modifiers::ALT),
(Key::Delete, Modifiers::COMMAND),
(Key::Delete, Modifiers::SHIFT | Modifiers::COMMAND),
(Key::Delete, Modifiers::SHIFT | Modifiers::ALT),
] {
key(&mut h, k, m);
assert_eq!(h.state().session.project, project, "{m:?}+{k:?}");
}
h.state_mut().dialog = Some(effectcraft_ui_egui::Dialog::About);
key(&mut h, Key::Delete, Modifiers::NONE);
for how in SHIFT_DELETE {
shift_delete(&mut h, how);
}
assert_eq!(h.state().session.project, project);
h.state_mut().dialog = None;
h.run_steps(2);
@@ -154,6 +190,10 @@ fn typing_dialogs_and_modified_keys_do_not_delete_project_items() {
key(&mut h, delete_key, Modifiers::NONE);
assert_eq!(h.state().session.project, project, "typing must not delete the item");
}
for how in SHIFT_DELETE {
shift_delete(&mut h, how);
assert_eq!(h.state().session.project, project, "typing must not delete the item ({how:?})");
}
}
/// Deleting an item a composition uses asks first, as in After Effects, and says what goes with
@@ -190,3 +230,137 @@ fn deleting_items_in_use_asks_first() {
h.state_mut().session.execute("project.delete", json!({"items": [solid.0]})).unwrap();
assert!(h.state().session.project.item(solid).is_none());
}
/// Shift+Delete deletes the selected item without asking, even one a composition uses, with the
/// layers that use it, in one undo step (Delete Project Items Without Confirmation).
#[test]
fn shift_delete_deletes_without_asking_and_undoes_once() {
for how in SHIFT_DELETE {
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);
let project = h.state().session.project.clone();
let undo = h.state().session.history.undo.len();
shift_delete(&mut h, how);
assert_eq!(h.state().dialog, None, "{how:?} item {target}: no prompt");
assert!(h.state().session.project.item(item).is_none(), "{how:?} 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.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 shift_delete_deletes_a_multi_selection_and_folder_contents_together() {
for how in SHIFT_DELETE {
let (mut h, items, keep) = harness();
let (solid, folder) = (items[1], items[3]);
// The solid (used by a layer) inside the folder: deleting the folder takes both.
h.state_mut().session.execute("project.move", json!({"items": [solid.0], "folder": folder.0})).unwrap();
h.run_steps(2);
click(&mut h, &format!("project.item.{}.name", items[0].0), Modifiers::NONE);
click(&mut h, &format!("project.item.{}.name", folder.0), Modifiers::COMMAND);
assert_eq!(h.state().session.state.project_selection, vec![items[0], folder]);
let project = h.state().session.project.clone();
let undo = h.state().session.history.undo.len();
shift_delete(&mut h, how);
assert_eq!(h.state().dialog, None, "{how:?}: no prompt");
for gone in [items[0], solid, folder] {
assert!(h.state().session.project.item(gone).is_none(), "{how:?}: {gone:?} goes");
}
assert!(h.state().session.project.item(items[2]).is_some(), "the unselected item stays");
let uses = h.state().session.project.comps().flat_map(|(_, c)| c.layers.iter()).filter(|l| l.source.item() == Some(solid)).count();
assert_eq!(uses, 0, "the layer using the solid goes with it");
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);
}
}
/// Shift+Delete with nothing selected deletes nothing, and skipping the prompt once (Shift+Delete,
/// or answering Delete) doesn't skip it for the next Delete.
#[test]
fn shift_delete_and_answered_prompts_dont_skip_the_next_prompt() {
for how in SHIFT_DELETE {
let (mut h, items, keep) = harness();
let solid = items[1];
click(&mut h, &format!("project.item.{}.name", items[0].0), Modifiers::NONE);
h.state_mut().session.execute("project.select", json!({"items": []})).unwrap();
let project = h.state().session.project.clone();
shift_delete(&mut h, how);
assert_eq!(h.state().dialog, None);
assert_eq!(h.state().session.project, project, "{how:?} with nothing selected deletes nothing");
assert!(h.state().session.active_comp().unwrap().layer(keep).is_some());
// Without asking once, then Delete on an item in use still asks.
click(&mut h, &format!("project.item.{}.name", items[2].0), Modifiers::NONE);
shift_delete(&mut h, how);
assert!(h.state().session.project.item(items[2]).is_none());
click(&mut h, &format!("project.item.{}.name", solid.0), Modifiers::NONE);
key(&mut h, Key::Delete, Modifiers::NONE);
assert_eq!(h.state().dialog, Some(Dialog::DeleteItems), "{how:?}: Delete still asks after Shift+Delete");
click(&mut h, "dialog.deleteItems.cancel", Modifiers::NONE);
assert!(h.state().session.project.item(solid).is_some());
}
// Answering Delete runs that deletion only: the next Delete asks again.
let (mut h, items, _) = harness();
let solid = items[1];
h.state_mut().session.execute("layer.newSolid", json!({"name": "Second solid", "color": "#806040"})).unwrap();
let second = h.state().session.project.items.values().find(|i| i.name == "Second solid").unwrap().id;
h.run_steps(2);
click(&mut h, &format!("project.item.{}.name", solid.0), Modifiers::NONE);
key(&mut h, Key::Delete, Modifiers::NONE);
assert!(confirm_if_asked(&mut h));
assert!(h.state().session.project.item(solid).is_none());
click(&mut h, &format!("project.item.{}.name", second.0), Modifiers::NONE);
key(&mut h, Key::Delete, Modifiers::NONE);
assert_eq!(h.state().dialog, Some(Dialog::DeleteItems), "the answer was for the first deletion only");
}
/// Shift+Delete only deletes Project items with the Project panel focused.
#[test]
fn shift_delete_in_other_panels_keeps_project_items() {
for how in SHIFT_DELETE {
let (mut h, items, _) = harness();
click(&mut h, &format!("project.item.{}.name", items[0].0), Modifiers::NONE);
click(&mut h, "viewer.comp", Modifiers::NONE);
assert_eq!(h.state().ui.focused, PanelKind::Composition);
assert_eq!(h.state().session.state.project_selection, vec![items[0]]);
shift_delete(&mut h, how);
assert_eq!(h.state().dialog, None);
assert!(items.iter().all(|i| h.state().session.project.item(*i).is_some()), "{how:?} in the Composition panel");
}
}
/// A Cut is read with the modifiers held where it is among the frame's events, not those held
/// after the frame: Shift released in the same frame is still Shift+Delete, and a Ctrl+X followed
/// in the same frame by Ctrl up and Shift down is not.
#[test]
fn a_cut_is_read_with_the_modifiers_where_it_is_in_the_frame() {
use Event::{Cut, ModifiersChanged as M};
let (shift, ctrl, none) = (Modifiers::SHIFT, Modifiers::COMMAND, Modifiers::NONE);
// (held from the frame before, this frame's events, whether the solid in use goes)
let cases: [(Modifiers, Vec<Event>, bool); 5] = [
(shift, vec![Cut, M(none)], true),
(ctrl, vec![Cut, M(none), M(shift)], false),
(none, vec![M(shift), Cut, M(none)], true),
(none, vec![M(ctrl), Cut, M(none), M(shift)], false),
(shift, vec![M(none), Cut, M(shift)], false),
];
for (n, (before, events, deletes)) in cases.into_iter().enumerate() {
let (mut h, items, _) = harness();
let solid = items[1];
click(&mut h, &format!("project.item.{}.name", solid.0), Modifiers::NONE);
frame(&mut h, &[M(before)]);
frame(&mut h, &events);
frame(&mut h, &[M(none)]);
assert_eq!(h.state().dialog, None, "case {n}: no prompt");
assert_eq!(h.state().session.project.item(solid).is_none(), deletes, "case {n}: {events:?} after {before:?}");
}
}
+6
View File
@@ -80,6 +80,12 @@ The most important missing piece: it turns every other estimate here into a meas
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.
- Regression evidence for [#484](https://github.com/storytold/effectcraft/issues/484): the
`shift_delete_*` input tests in `ui_project_delete`, checked headlessly on Windows, press
Shift+Delete both as a key and as the Cut event Windows sends for it. Items in use go without
the prompt, with their layers, in one undo step, for single items, multiple selections and
folder contents. Nothing happens with an empty selection, in other panels, while typing or with
a dialog open, and the next Delete still asks.
- Regression evidence for the reports of 6 October, checked headlessly on Windows: real pointer
drags in `ui_drag_drop` (Project items into the Timeline between layers and at a time, and onto
the viewer; an effect onto a layer in the viewer: #85, #88, #89), `ui_render_queue` (a template