diff --git a/crates/ui-egui/src/browser.rs b/crates/ui-egui/src/browser.rs index 3703d7ce09..cb7c09c989 100644 --- a/crates/ui-egui/src/browser.rs +++ b/crates/ui-egui/src/browser.rs @@ -2824,7 +2824,9 @@ fn inline_rename(app: &mut SolveApp, ui: &mut egui::Ui, row: Rect, label_left: f if escape { // Dropping the state cancels the edit. } else if commit { - crate::context_menu::commit_rename(app, &rename); + if crate::context_menu::commit_rename(app, &rename).is_err() { + app.menu.rename = Some(rename); + } } else { app.menu.rename = Some(rename); } diff --git a/crates/ui-egui/src/browser_card_tests.rs b/crates/ui-egui/src/browser_card_tests.rs index 01cdbf6299..e0465f149e 100644 --- a/crates/ui-egui/src/browser_card_tests.rs +++ b/crates/ui-egui/src/browser_card_tests.rs @@ -106,6 +106,31 @@ fn f2_on_a_clicked_browser_group_renames_the_group() { } } +/// #10654: a refused inline rename keeps the editor open and says why, instead of closing +/// silently and leaving the old name. +#[test] +fn a_refused_inline_rename_keeps_the_editor_and_shows_the_error() { + let mut h = Harness::new(); + h.app.run("solid.box", json!({"length":10,"width":10,"height":10,"body_name":"A"})).unwrap(); + h.app.run("solid.box", json!({"length":10,"width":10,"height":10,"body_name":"B"})).unwrap(); + h.app.preferences.browser_bodies_folders = crate::prefs::BodiesFolders::Always; + h.settle(); + h.app.menu.rename = Some(crate::context_menu::Rename { + what: crate::context_menu::RenameWhat::Body("B".into()), + text: "A".into(), + at: egui::Pos2::ZERO, + focused: true, + inline: true, + }); + h.frames(2); + h.step(&json!({"key":"Enter"})).unwrap(); + h.frames(2); + assert!(h.app.menu.rename.is_some(), "the editor stays open after a refused rename"); + let (text, _, error) = h.app.status.clone().expect("the refusal is shown"); + assert!(error && text.contains("another body is called"), "{text}"); + assert!(h.app.session.world_state().body("B").is_some(), "the body keeps its name"); +} + #[test] fn folder_visibility_target_clears_when_the_user_moves_on() { for action in ["body click", "selection command", "empty click", "escape", "design tab", "model edit", "tool"] { diff --git a/crates/ui-egui/src/context_menu.rs b/crates/ui-egui/src/context_menu.rs index b1b10ee429..72d9b2cf12 100644 --- a/crates/ui-egui/src/context_menu.rs +++ b/crates/ui-egui/src/context_menu.rs @@ -2036,24 +2036,30 @@ pub fn start_rename(app: &mut SolveApp, what: RenameWhat, at: Pos2) { app.menu.rename = Some(Rename { what, text, at, focused: false, inline: false }); } -pub(crate) fn commit_rename(app: &mut SolveApp, r: &Rename) { +pub(crate) fn commit_rename(app: &mut SolveApp, r: &Rename) -> Result<(), String> { let name = r.text.trim().to_string(); if name.is_empty() { - return; + return Ok(()); } - match &r.what { + let result = match &r.what { RenameWhat::Body(old) => { // The command renames the body where it is hidden or locked too. - drop(app.run_heavy("body.rename", json!({ "body": old, "name": name }))); + app.run_heavy("body.rename", json!({ "body": old, "name": name })).map(|_| ()) } - RenameWhat::Feature(f) => drop(app.run("timeline.rename", json!({ "feature": f, "name": name }))), - RenameWhat::Component(c) => drop(app.run("component.rename", json!({ "component": c, "name": name }))), - RenameWhat::Group(g) => crate::browser::group_action(app, "ui.renameGroup", &json!({ "group": g, "name": name })), - RenameWhat::TimelineGroup(g) => drop(app.run("timeline.rename_group", json!({ "group": g, "name": name }))), - RenameWhat::Canvas(c) => drop(app.run("canvas.edit", json!({ "canvas": c, "name": name }))), - RenameWhat::View(v) => drop(app.run("view.rename", json!({ "view": v, "name": name }))), - RenameWhat::Joint(j) => drop(app.run("joint.edit", json!({ "joint": j, "name": name }))), - } + RenameWhat::Feature(f) => app.run("timeline.rename", json!({ "feature": f, "name": name })).map(|_| ()), + RenameWhat::Component(c) => app.run("component.rename", json!({ "component": c, "name": name })).map(|_| ()), + RenameWhat::Group(g) => { + crate::browser::group_action(app, "ui.renameGroup", &json!({ "group": g, "name": name })); + Ok(()) + } + RenameWhat::TimelineGroup(g) => app.run("timeline.rename_group", json!({ "group": g, "name": name })).map(|_| ()), + RenameWhat::Canvas(c) => app.run("canvas.edit", json!({ "canvas": c, "name": name })).map(|_| ()), + RenameWhat::View(v) => app.run("view.rename", json!({ "view": v, "name": name })).map(|_| ()), + RenameWhat::Joint(j) => app.run("joint.edit", json!({ "joint": j, "name": name })).map(|_| ()), + }; + // The command's own status line explains a refusal; the caller decides whether to keep the + // editor open on it. + result } // --------------------------------------------------------------------------------------------- @@ -2583,7 +2589,9 @@ fn show_rename(app: &mut SolveApp, ctx: &egui::Context) { done = Some(true); } match done { - Some(true) => commit_rename(app, &r), + Some(true) => { + let _ = commit_rename(app, &r); + } Some(false) => {} None => app.menu.rename = Some(r), } @@ -2596,7 +2604,7 @@ pub fn finish_rename(app: &mut SolveApp, text: Option<&str>, commit: bool) -> bo r.text = s.to_string(); } if commit { - commit_rename(app, &r); + let _ = commit_rename(app, &r); } true } diff --git a/crates/ui-egui/src/history_off_thread_tests.rs b/crates/ui-egui/src/history_off_thread_tests.rs index dd1b8bdf2c..00ee8812ba 100644 --- a/crates/ui-egui/src/history_off_thread_tests.rs +++ b/crates/ui-egui/src/history_off_thread_tests.rs @@ -1091,7 +1091,7 @@ fn a_body_renamed_in_the_browser_keeps_the_pattern_after_it() { let before = mesh(&h); // What Enter does, without the frames after it that would see the rename in. let rename = h.app.menu.rename.take().unwrap(); - crate::context_menu::commit_rename(&mut h.app, &rename); + let _ = crate::context_menu::commit_rename(&mut h.app, &rename); assert!(h.app.computing_since().is_some(), "computing"); assert_eq!(names(&h), ["Body1"], "the design is as it was while it computes"); until_settled(&mut h); diff --git a/crates/ui-egui/src/timeline_left.rs b/crates/ui-egui/src/timeline_left.rs index d7a9ec220a..c963816b88 100644 --- a/crates/ui-egui/src/timeline_left.rs +++ b/crates/ui-egui/src/timeline_left.rs @@ -1068,7 +1068,7 @@ fn timeline_group_name( if escape { // Dropping the shared state cancels the edit and leaves the old name intact. } else if commit { - crate::context_menu::commit_rename(app, &rename); + let _ = crate::context_menu::commit_rename(app, &rename); } else { app.menu.rename = Some(rename); }