From 37a845bc688aec491852076d685806be1c09c168 Mon Sep 17 00:00:00 2001 From: NDelic Date: Mon, 12 Oct 2026 11:17:58 +0200 Subject: [PATCH 1/2] #1241: Select All in Effect Controls selects its keyframes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With Effect Controls focused, Edit ▸ Select All (⌘A / Ctrl+A) selected every clip in the Timeline. It now selects every keyframe the panel shows (effect parameters and mask parameters of the shown clips) and leaves the Timeline selection alone. With no keyframes shown, or another panel focused, Select All behaves as before. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/menus.rs | 2 +- crates/ui-egui/src/panels/effect_controls.rs | 27 ++++++++++++++++++++ crates/ui-egui/tests/keyframes_ui.rs | 26 +++++++++++++++++++ 3 files changed, 54 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 04d79ac3..10587abf 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -176,7 +176,7 @@ pub fn targets_source(app: &FilmcraftApp, params: &Value) -> bool { /// Execute a UI or engine command by id. pub fn invoke(app: &mut FilmcraftApp, ctx: &egui::Context, id: &str, mut params: Value) -> Result { - if id == "edit.selectAll" && crate::panels::graphics::select_all_edited_text(app) { + if id == "edit.selectAll" && (crate::panels::graphics::select_all_edited_text(app) || crate::panels::effect_controls::select_all_keyframes(app)) { return Ok(Value::Null); } if filmcraft_engine::source_monitor::source_command(id) && targets_source(app, ¶ms) && params.get("target").is_none() { diff --git a/crates/ui-egui/src/panels/effect_controls.rs b/crates/ui-egui/src/panels/effect_controls.rs index 1750022a..962e6e36 100644 --- a/crates/ui-egui/src/panels/effect_controls.rs +++ b/crates/ui-egui/src/panels/effect_controls.rs @@ -63,6 +63,33 @@ fn keyframe_exists(it: &TrackItem, k: &KeyframeRef) -> bool { param.is_some_and(|p| p.keyframes.iter().any(|kf| kf.time == k.time)) } +/// Edit ▸ Select All (⌘A) with Effect Controls focused selects every keyframe the panel shows +/// instead of the Timeline's clips (#1241). False when the panel is not focused or shows none. +pub(crate) fn select_all_keyframes(app: &mut FilmcraftApp) -> bool { + if app.ui.focused != crate::dock::PanelKind::EffectControls { + return false; + } + let mut keys = Vec::new(); + for (clip, it, _) in selected_clips(app) { + for (effect, e) in it.effects.iter().enumerate() { + let key = |param: &str, mask, time| KeyframeRef { clip: clip.0, effect, param: param.to_string(), mask, time }; + for (param, p) in &e.params { + keys.extend(p.keyframes.iter().map(|kf| key(param, None, kf.time))); + } + for (m, mask) in e.masks.iter().enumerate() { + for param in filmcraft_project::mask::MASK_PARAMS { + keys.extend(mask.param(param).into_iter().flat_map(|p| &p.keyframes).map(|kf| key(param, Some(m), kf.time))); + } + } + } + } + if keys.is_empty() { + return false; + } + app.ui.keyframe_selection = keys; + true +} + fn selected_clip(app: &FilmcraftApp) -> Option<(ClipId, TrackItem, TrackKind)> { selected_clips(app).into_iter().next() } diff --git a/crates/ui-egui/tests/keyframes_ui.rs b/crates/ui-egui/tests/keyframes_ui.rs index 3edd7ef5..d0549a01 100644 --- a/crates/ui-egui/tests/keyframes_ui.rs +++ b/crates/ui-egui/tests/keyframes_ui.rs @@ -311,6 +311,32 @@ fn clicking_a_lane_keyframe_selects_only_that_keyframe() { assert_eq!(selection(&mut d), json!([]), "a deleted keyframe is not selected"); } +#[test] +fn select_all_in_effect_controls_selects_its_keyframes_not_the_clips() { + // #1241 + let (mut d, clip) = Driver::demo(); + d.seek(1.0); + for param in ["scale", "position"] { + d.exec("effects.addKeyframe", json!({"clip": clip.id, "effect": "motion", "param": param})); + } + d.seek(2.0); + d.exec("effects.addKeyframe", json!({"clip": clip.id, "effect": "motion", "param": "scale"})); + let clips = d.ok("ui.inspect", json!({}))["selection"].clone(); + d.ok("ui.set", json!({"focused": "EffectControls"})); + d.ok("ui.key", json!({"key": "Cmd+A"})); + d.frames(2); + let inspect = d.ok("ui.inspect", json!({})); + let keys = inspect["ui"]["keyframe_selection"].as_array().cloned().unwrap_or_default(); + assert_eq!(keys.len(), 3, "{keys:?}"); + assert!(keys.iter().all(|k| k["clip"] == clip.id)); + assert_eq!(inspect["selection"], clips, "the Timeline selection is unchanged"); + // with the Timeline focused, Select All still selects the clips + d.ok("ui.set", json!({"focused": "Timeline"})); + d.ok("ui.key", json!({"key": "Cmd+A"})); + d.frames(2); + assert!(d.ok("ui.inspect", json!({}))["selection"].as_array().map_or(0, Vec::len) > clips.as_array().map_or(0, Vec::len)); +} + /// Effect Controls' divider (#643): dragging it widens the effect list or the keyframe area, each /// keeping a minimum width, and the width stays. The mouse wheel scrolls the rows over the /// keyframe area too, not only over the effect list. From d74ed39c24914bd850c45cbe400e95a1edb87d48 Mon Sep 17 00:00:00 2001 From: NDelic Date: Mon, 12 Oct 2026 11:17:58 +0200 Subject: [PATCH 2/2] Contributors: add n-delic Co-Authored-By: Claude Opus 5.5 --- contributors/people.toml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/contributors/people.toml b/contributors/people.toml index 04ed7243..5a64a93d 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -77,3 +77,7 @@ sync_github_name = true real_name = "Valentin Klein" display_name = "Valentin Klein" sync_github_name = false + +[people.n-delic] +display_name = "n-delic" +sync_github_name = false