From 7eaba0ddd5fdf286e058eb9a75882f6d75b206f6 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sat, 10 Oct 2026 16:43:42 +0200 Subject: [PATCH] #456: Select All in the Project panel takes only the shown bin Problem: with the Project panel focused, Cmd+A ran project.selectAll, which selected every item in the project: items in closed bins and in bins of other tabs too. Fix: project.selectAll takes an optional `bin` (its direct items) and `expanded` (sub-bins twirled open in List view, whose items are shown and count). Unknown or malformed ids are an error and leave the selection alone; without `bin` it still selects the whole project. The UI passes the shown tab's bin and, in List view, the open bins; while a search filters the panel it selects exactly the rows shown. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/project_panel.rs | 46 +++++++++++++-- crates/engine/src/project_panel_tests.rs | 74 +++++++++++++++++++++++- crates/ui-egui/src/lib.rs | 12 +++- crates/ui-egui/src/panels/project.rs | 15 +++++ crates/ui-egui/tests/project_panel_ui.rs | 38 ++++++++++++ 5 files changed, 176 insertions(+), 9 deletions(-) diff --git a/crates/engine/src/project_panel.rs b/crates/engine/src/project_panel.rs index 84659296..735f5b28 100644 --- a/crates/engine/src/project_panel.rs +++ b/crates/engine/src/project_panel.rs @@ -17,9 +17,9 @@ //! | `project.freeform.saveArrangement` / `.restoreArrangement` / `.arrangements` / `.deleteArrangement` | saved arrangements | //! | `project.freeform.options` | Freeform View Options… (grid, snap, names, durations) | //! | `project.renameBin` | rename a bin (inline rename, undoable) | -//! | `project.selectAll` / `project.deselectAll` | select every item in the project / clear the Project panel selection (Cmd+A / Cmd+Shift+A with the panel focused) | +//! | `project.selectAll` / `project.deselectAll` | select the items a bin shows (`bin`, plus the sub-bins open in List view in `expanded`; without `bin` every item in the project) / clear the Project panel selection (Cmd+A / Cmd+Shift+A with the panel focused) | -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; use filmcraft_project::{Bin, BinEntry, BinId, ItemId, ItemKind, MediaRef, Project, ProjectItem, TrackKind}; use filmcraft_time::{Tick, TimeDisplay, format_time}; @@ -1171,6 +1171,43 @@ fn rename_bin(s: &mut Session, p: &Value) -> Result { Ok(json!({"bin": bin.0, "name": name})) } +/// Select All in the Project panel. With `bin`, only what a panel showing that bin lists: its +/// items, plus the items of the sub-bins named in `expanded` (bins twirled open in List view), +/// never those of a closed bin. Without `bin`, every item in the project. +fn select_all(s: &mut Session, p: &Value) -> Result { + const CMD: &str = "project.selectAll"; + // strict: a negative or fractional `bin` is an error, not bin 0 + let id = match p.get("bin") { + None | Some(Value::Null) => { + s.state.project_selection = s.project.items.keys().copied().collect(); + return Ok(json!({"selected": s.state.project_selection.len()})); + } + Some(b) => BinId(b.as_u64().ok_or_else(|| bad(CMD, "`bin` must be a bin id"))?), + }; + let bin = s.project.root.find_bin(id).ok_or_else(|| bad(CMD, format!("no bin {}", id.0)))?; + let expanded: BTreeSet = match p.get("expanded") { + None | Some(Value::Null) => Default::default(), + Some(Value::Array(a)) => { + a.iter().map(|v| v.as_u64().map(BinId).ok_or_else(|| bad(CMD, "`expanded` must be a list of bin ids"))).collect::>()? + } + Some(_) => return Err(bad(CMD, "`expanded` must be a list of bin ids")), + }; + // iterative walk: a hostile project file cannot overflow the stack with deeply nested bins + let mut sel = Vec::new(); + let mut stack = vec![bin]; + while let Some(b) = stack.pop() { + for e in &b.children { + match e { + BinEntry::Item(i) if s.project.item(*i).is_some_and(|it| listed(&s.project, it)) => sel.push(*i), + BinEntry::Bin(sub) if expanded.contains(&sub.id) => stack.push(sub), + _ => {} + } + } + } + s.state.project_selection = sel; + Ok(json!({"bin": id.0, "selected": s.state.project_selection.len()})) +} + // ------------------------------------------------------------------------------------ registry pub(crate) fn commands() -> Vec { @@ -1223,10 +1260,7 @@ pub(crate) fn commands() -> Vec { freeform_options, ), c("project.renameBin", "Rename Bin", r#"{"bin":binId,"name":str}"#, always, rename_bin), - c("project.selectAll", "Select All Project Items", "{}", always, |s, _| { - s.state.project_selection = s.project.items.keys().copied().collect(); - Ok(json!({"selected": s.state.project_selection.len()})) - }), + c("project.selectAll", "Select All Project Items", r#"{"bin":binId?,"expanded":[binId]?}"#, always, select_all), c("project.deselectAll", "Deselect All Project Items", "{}", always, |s, _| { s.state.project_selection.clear(); Ok(Value::Null) diff --git a/crates/engine/src/project_panel_tests.rs b/crates/engine/src/project_panel_tests.rs index 42d3c79e..b094a54f 100644 --- a/crates/engine/src/project_panel_tests.rs +++ b/crates/engine/src/project_panel_tests.rs @@ -2,7 +2,7 @@ //! sorting, view presets, Freeform positions / stacks / arrangements (undoable, saved with the //! project) and bin renames. -use filmcraft_project::{ItemId, ItemKind}; +use filmcraft_project::{BinEntry, ItemId, ItemKind}; use serde_json::{Value, json}; use crate::Session; @@ -305,3 +305,75 @@ fn select_all_selects_every_project_item() { assert!(!s.state.selection.is_empty()); assert_eq!(s.state.project_selection, before); } + +#[test] +fn select_all_in_a_bin_skips_closed_bins_and_other_bins() { + // #456: Cmd+A in the Project panel selected every item in the project, closed bins included + let mut s = demo(); + let outer = s.execute("file.newBin", json!({"name": "Footage A"})).unwrap()["bin"].as_u64().unwrap(); + let inner = s.execute("file.newBin", json!({"name": "Day 1", "parent": outer})).unwrap()["bin"].as_u64().unwrap(); + let ids = media(&s); + assert!(ids.len() >= 2); + s.execute("project.moveToBin", json!({"items": [ids[0].0], "bin": outer})).unwrap(); + s.execute("project.moveToBin", json!({"items": [ids[1].0], "bin": inner})).unwrap(); + s.state.project_selection.clear(); + + // only the bin's own items: the closed sub-bin's item stays unselected + let r = s.execute("project.selectAll", json!({"bin": outer})).unwrap(); + assert_eq!(r["selected"].as_u64(), Some(1)); + assert_eq!(s.state.project_selection, vec![ids[0]]); + + // a sub-bin twirled open in List view counts + s.execute("project.selectAll", json!({"bin": outer, "expanded": [inner]})).unwrap(); + let mut sel = s.state.project_selection.clone(); + sel.sort(); + assert_eq!(sel, vec![ids[0], ids[1]]); + + // the root: its direct items only, nothing from the (closed) bins + let root = s.project.root.id; + let mut want: Vec = s + .project + .root + .children + .iter() + .filter_map(|e| if let BinEntry::Item(i) = e { Some(*i) } else { None }) + .filter(|i| s.project.item(*i).is_some_and(|it| project_panel::listed(&s.project, it))) + .collect(); + want.sort(); + s.execute("project.selectAll", json!({"bin": root.0})).unwrap(); + let mut sel = s.state.project_selection.clone(); + sel.sort(); + assert_eq!(sel, want); + assert!(!sel.contains(&ids[0]) && !sel.contains(&ids[1])); + assert!(sel.len() < s.project.items.len()); + + // an open bin inside a closed one is not shown, so it does not count either + s.execute("project.selectAll", json!({"bin": root.0, "expanded": [inner]})).unwrap(); + assert!(!s.state.project_selection.contains(&ids[1])); + // ... but open all the way down, it does + s.execute("project.selectAll", json!({"bin": root.0, "expanded": [outer, inner]})).unwrap(); + assert!(s.state.project_selection.contains(&ids[0]) && s.state.project_selection.contains(&ids[1])); +} + +#[test] +fn select_all_rejects_bad_bins_without_touching_the_selection() { + let mut s = demo(); + let ids = media(&s); + let root = s.project.root.id.0; + s.execute("project.select", json!({"items": [ids[0].0]})).unwrap(); + for p in [ + json!({"bin": 987_654_321u64}), + json!({"bin": "footage"}), + json!({"bin": -1}), + json!({"bin": root, "expanded": "all"}), + json!({"bin": root, "expanded": ["x"]}), + ] { + assert!(s.execute("project.selectAll", p.clone()).is_err(), "{p}"); + assert_eq!(s.state.project_selection, vec![ids[0]], "{p}"); + } + // unknown ids in `expanded` are simply not open bins + assert!(s.execute("project.selectAll", json!({"bin": root, "expanded": [987_654_321u64]})).is_ok()); + // `bin: null` is the same as no bin: the whole project + let r = s.execute("project.selectAll", json!({"bin": null})).unwrap(); + assert_eq!(r["selected"].as_u64(), Some(s.project.items.len() as u64)); +} diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 857f42a5..727e80f7 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -1301,8 +1301,16 @@ impl FilmcraftApp { }); for mut id in fire { // Select All / Deselect All act on the Project panel's items when it has focus (#168). - if self.ui.focused == PanelKind::Project && matches!(id.as_str(), "edit.selectAll" | "edit.deselectAll") { - id = id.replacen("edit.", "project.", 1); + // Select All takes only what the shown bin lists, not the whole project (#456). + if self.ui.focused == PanelKind::Project && id == "edit.selectAll" { + let (cmd, params) = panels::project::select_all_command(self); + if let Err(e) = menus::invoke(self, ctx, &cmd, params) { + self.ui.status = e; + } + continue; + } + if self.ui.focused == PanelKind::Project && id == "edit.deselectAll" { + id = "project.deselectAll".into(); } // Mark In/Out in the Source monitor when it has focus. let params = if self.ui.focused == PanelKind::Source diff --git a/crates/ui-egui/src/panels/project.rs b/crates/ui-egui/src/panels/project.rs index 09b04069..3493ee93 100644 --- a/crates/ui-egui/src/panels/project.rs +++ b/crates/ui-egui/src/panels/project.rs @@ -254,6 +254,21 @@ pub fn visible_items(app: &FilmcraftApp, inst: Inst) -> Vec { crate::panels::project_views::items_in_view(app, &v, &app.ui.project_search.to_ascii_lowercase()) } +/// The command Select All (Cmd+A) runs with the Project panel focused: the items of the shown +/// bin, plus those of sub-bins twirled open in List view (`project.selectAll {bin, expanded}`). +/// Items in closed bins and in bins of other tabs stay unselected, as in Premiere (#456). While a +/// search filters the panel, exactly the rows it shows (`project.select {items}`). +pub fn select_all_command(app: &FilmcraftApp) -> (String, Value) { + let inst = shown_inst(app); + if !app.ui.project_search.is_empty() { + let items: Vec = visible_items(app, inst).into_iter().map(|i| i.0).collect(); + return ("project.select".into(), json!({"items": items})); + } + let v = view_of(app, inst); + let expanded: &[u64] = if v.mode == ViewMode::List { &app.ui.expanded_bins } else { &[] }; + ("project.selectAll".into(), json!({"bin": v.bin.0, "expanded": expanded})) +} + // ------------------------------------------------------------------------------------ panel pub fn show(app: &mut FilmcraftApp, ui: &mut egui::Ui, rect: Rect) { diff --git a/crates/ui-egui/tests/project_panel_ui.rs b/crates/ui-egui/tests/project_panel_ui.rs index bb504f32..c148b58c 100644 --- a/crates/ui-egui/tests/project_panel_ui.rs +++ b/crates/ui-egui/tests/project_panel_ui.rs @@ -447,6 +447,44 @@ fn bins_open_in_place_in_a_tab_or_in_a_window() { assert_eq!(name, "Camera A"); } +#[test] +fn select_all_takes_only_the_shown_bin() { + // #456: Cmd+A selected every item in the project, closed bins and other tabs included + let mut d = project_driver(); + d.exec("project.view.set", json!({"view": "list"})); + let (bin, items) = d.footage(); + assert!(!items.is_empty()); + let sorted = |v: &[ItemId]| { + let mut v: Vec = v.iter().map(|i| i.0).collect(); + v.sort(); + v + }; + let mut footage = items.clone(); + footage.sort(); + // the root with the Footage bin closed: none of its items + d.app().ui.expanded_bins.clear(); + d.frames(2); + d.key("Cmd+A"); + let sel = d.app().session.state.project_selection.clone(); + assert!(sel.iter().all(|i| !items.contains(&i.0)), "{sel:?}"); + assert!(sel.iter().all(|i| d.app().session.project.root.children.contains(&BinEntry::Item(*i))), "{sel:?}"); + // twirled open in List view, its items are shown and count + d.app().ui.expanded_bins.push(bin); + d.frames(2); + d.key("Cmd+A"); + let sel = d.app().session.state.project_selection.clone(); + assert!(items.iter().all(|i| sel.contains(&ItemId(*i))), "{sel:?}"); + d.key("Cmd+Shift+A"); + assert!(d.app().session.state.project_selection.is_empty()); + // the bin opened in its own tab: exactly its items + d.app().ui.expanded_bins.clear(); + d.app().session.prefs.general.bins_double_click = "openNewTab".into(); + double_click_bin(&mut d, bin, false); + assert_eq!(d.app().ui.project_panel.active_tab, Some(0)); + d.key("Cmd+A"); + assert_eq!(sorted(&d.app().session.state.project_selection), footage); +} + #[test] fn footer_buttons_are_wired() { let mut d = project_driver();