fix(ui): Save Arrangement refuses an unknown bin (#3774 follow-up) (#3900)

* fix(ui): Save Arrangement refuses an unknown bin (#3774)

The engine lays out the root bin when it does not find the one asked for, so
a Save Arrangement for a bin that does not exist (a scripted call, or a bin
deleted while the dialog was open) could overwrite a root arrangement of the
same name. The dialog now refuses a bin that does not exist when it opens and
checks it again before saving.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(ui): Save Arrangement refuses a bin that is not a bin id (#3774)

A `bin` such as -1, a string or 1.5 is not an unsigned id, so it was read
as no bin and the dialog opened for the shown view. Any non-null `bin` that
is not a bin id is now refused; an omitted or null one still means the shown
view's. The test checks the arrangements list it meant to (`arrangements`),
and covers these values.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
John
2026-11-02 07:38:07 +02:00
committed by GitHub
parent 8c171a0ab8
commit d57a29b776
3 changed files with 45 additions and 6 deletions
+18 -5
View File
@@ -958,11 +958,24 @@ pub fn route(app: &mut FilmcraftApp, ctx: &egui::Context, op: &str, params: &Val
Ok(Value::Null)
}
"saveArrangement" => {
// the canvas the menu was opened on, which may be a floating bin window
let bin = params.get("bin").and_then(Value::as_u64).or_else(|| Some(view_of(app, inst).bin.0));
let width = params.get("width").and_then(Value::as_f64);
app.ui.project_panel.dialog = Some(ProjectDialog::SaveArrangement { name: String::new(), bin, width });
Ok(Value::Null)
// the canvas the menu was opened on, which may be a floating bin window; a `bin` that is
// not a bin id, or names no bin, is refused, as the engine would save to the root bin
// instead (an omitted or null `bin` is the shown view's)
let given = params.get("bin").filter(|v| !v.is_null());
let id = given.map(|v| v.as_u64().ok_or_else(|| format!("`bin` is a bin id, not {v}")));
if let Some(Err(e)) = id {
Err(e)
} else if let Some(Ok(b)) = id
&& app.session.project.root.find_bin(BinId(b)).is_none()
{
Err(format!("no bin {b}"))
} else {
let given = id.and_then(Result::ok);
let bin = given.or_else(|| Some(view_of(app, inst).bin.0));
let width = params.get("width").and_then(Value::as_f64);
app.ui.project_panel.dialog = Some(ProjectDialog::SaveArrangement { name: String::new(), bin, width });
Ok(Value::Null)
}
}
"revealProject" => {
let Some(p) = app.session.path.clone() else { return Some(Err("the project has not been saved".into())) };
+8 -1
View File
@@ -79,7 +79,14 @@ pub fn show(app: &mut FilmcraftApp, ctx: &egui::Context) {
push(app, "projectDialog.name", &r, "Name");
});
if ok_cancel(app, ui, &mut close) && !name.trim().is_empty() {
run = Some(("project.freeform.saveArrangement".into(), json!({"name": name, "bin": bin, "width": width})));
// the bin may have been deleted while the dialog was open
match bin.filter(|b| app.session.project.root.find_bin(filmcraft_project::BinId(*b)).is_none()) {
Some(b) => {
app.ui.status = format!("no bin {b}");
close = true;
}
None => run = Some(("project.freeform.saveArrangement".into(), json!({"name": name, "bin": bin, "width": width}))),
}
}
}
}
@@ -1713,3 +1713,22 @@ fn a_card_menu_stacks_only_the_selected_cards_of_its_bin() {
assert!(stack(&mut d, items[0]).is_some() && stack(&mut d, items[1]).is_some(), "this bin's cards are stacked");
assert_eq!(stack(&mut d, other), None, "the other bin's item is left alone");
}
/// #3774 review: Save Arrangement takes the bin of the canvas it was opened on; an unknown bin
/// is refused instead of saving to the root bin.
#[test]
fn save_arrangement_refuses_an_unknown_bin() {
let mut d = project_driver();
let (bin, _) = d.footage();
for bad in [json!(987_654), json!(-1), json!("footage"), json!(1.5)] {
let r = d.call("ui.menu.invoke", json!({"id": "projectPanel.saveArrangement", "params": {"bin": bad, "width": 400.0}}));
assert_eq!(r["ok"], json!(false), "bin {bad}: {r}");
assert!(d.app().ui.project_panel.dialog.is_none(), "no dialog for bin {bad}");
}
let root = d.app().session.project.root.id.0;
let saved = d.exec("project.freeform.arrangements", json!({"bin": root}));
assert_eq!(saved["arrangements"].as_array().map(Vec::len), Some(0), "nothing saved to the root: {saved}");
// a bin that exists opens the dialog
d.ok("ui.menu.invoke", json!({"id": "projectPanel.saveArrangement", "params": {"bin": bin, "width": 400.0}}));
assert!(d.app().ui.project_panel.dialog.is_some());
}