Address CodeRabbit review: close the sampler menu when its sampler changes

The Color Sampler menu kept only the sampler's index. If a control
command deleted an earlier sampler while the menu was open, the index
named another sampler and Delete removed that one. The menu now also
remembers the active document and the sampler's position; when either
no longer matches, Delete greys out and the menu closes.

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

crates/ui-egui/src/canvas/canvas_tool_menu.rs

@@ -50,6 +50,11 @@ pub struct CanvasToolMenu {
     /// `None` off a sampler, where every row is greyed.
     #[serde(default)]
     pub sampler: Option<usize>,
+    /// Where that sampler was when the menu opened: the active document's index and the sampler's
+    /// position. Once a control command deletes, moves or renumbers samplers, or another document
+    /// becomes active, the index may name a different sampler, so the menu closes and Delete greys.
+    #[serde(default)]
+    pub sampler_at: Option<(usize, [f64; 2])>,
 }
 
 /// A misspelled word in the type layer being edited (`edit.checkSpelling`'s `list` entry).
@@ -308,6 +313,7 @@ pub fn open_crop(app: &mut PhotocraftApp, ctx: &Context, tool: Tool, pos: [f32;
         spelling: None,
         crop: Some(users),
         sampler: None,
+        sampler_at: None,
     });
     crate::canvas_popup::mark_open(ctx, menu_id());
     true
@@ -478,6 +484,14 @@ pub const SAMPLER_MENU: &[Row] = &[
     Some(("Total Ink", "colorSampler.readout.totalInk")),
 ];
 
+/// Does the Color Sampler menu's index still name the sampler it opened on, in the same document?
+/// `true` off a sampler, where there is nothing to go stale.
+fn sampler_current(app: &PhotocraftApp, menu: &CanvasToolMenu) -> bool {
+    let Some(i) = menu.sampler else { return true };
+    let now = app.session.active_index().zip(app.session.active().and_then(|d| d.color_samplers.get(i).copied()));
+    now.is_some() && now == menu.sampler_at
+}
+
 /// Is `menu` the Color Sampler's own menu (not the pasteboard, guide or transform menu opened
 /// with it)?
 fn sampler_menu(menu: &CanvasToolMenu) -> bool {
@@ -693,6 +707,7 @@ pub fn open_transform(app: &mut PhotocraftApp, ctx: &Context, pos: [f32; 2]) ->
         spelling: None,
         crop: None,
         sampler: None,
+        sampler_at: None,
     });
     crate::canvas_popup::mark_open(ctx, menu_id());
     true
@@ -775,6 +790,7 @@ pub fn open_pasteboard(app: &mut PhotocraftApp, ctx: &Context, tool: Tool, pos:
         spelling: None,
         crop: None,
         sampler: None,
+        sampler_at: None,
     });
     crate::canvas_popup::mark_open(ctx, menu_id());
     true
@@ -807,6 +823,7 @@ pub fn open_guide(app: &mut PhotocraftApp, ctx: &Context, tool: Tool, mods: egui
         spelling: None,
         crop: None,
         sampler: None,
+        sampler_at: None,
     });
     crate::canvas_popup::mark_open(ctx, menu_id());
     true
@@ -861,8 +878,7 @@ pub fn entry_enabled(app: &PhotocraftApp, menu: &CanvasToolMenu, command: &str)
     }
     // Off a sampler every row is greyed. The readout rows wait for per-sampler readout modes.
     if sampler_menu(menu) {
-        let count = app.session.active().map_or(0, |d| d.color_samplers.len());
-        return command == SAMPLER_DELETE && menu.sampler.is_some_and(|i| i < count);
+        return command == SAMPLER_DELETE && menu.sampler.is_some() && sampler_current(app, menu);
     }
     if menu.transform || !path_menu(menu.tool) {
         return crate::menus::is_enabled(app, command);
@@ -925,6 +941,7 @@ pub fn open_at(app: &mut PhotocraftApp, ctx: &Context, tool: Tool, pos: [f32; 2]
     let path_name = crate::vector_ui::active_path_name(app);
     let spelling = if text { doc.and_then(|d| spelling_at(app, d)) } else { None };
     let sampler = if tool == Tool::ColorSampler { doc.and_then(|d| crate::analysis_ui::sampler_at(app, d)) } else { None };
+    let sampler_at = sampler.and_then(|i| Some((app.session.active_index()?, *app.session.active()?.color_samplers.get(i)?)));
     app.ui.canvas_tool_menu = Some(CanvasToolMenu {
         pos,
         tool,
@@ -940,6 +957,7 @@ pub fn open_at(app: &mut PhotocraftApp, ctx: &Context, tool: Tool, pos: [f32; 2]
         spelling,
         crop: None,
         sampler,
+        sampler_at,
     });
     crate::canvas_popup::mark_open(ctx, menu_id());
     true
@@ -968,6 +986,7 @@ pub fn open_view(app: &mut PhotocraftApp, ctx: &Context, tool: Tool, pos: [f32;
         spelling: None,
         crop: None,
         sampler: None,
+        sampler_at: None,
     });
     crate::canvas_popup::mark_open(ctx, menu_id());
     true
@@ -1213,6 +1232,7 @@ pub fn show(app: &mut PhotocraftApp, ctx: &Context) {
         || (menu.text && app.ui.text_edit.is_none())
         || (menu.type_layer && app.ui.text_edit.is_some())
         || (menu.crop.is_some() && !crate::crop_ui::pending(app))
+        || (sampler_menu(&menu) && !sampler_current(app, &menu))
     {
         app.ui.canvas_tool_menu = None;
         return;

crates/ui-egui/src/tools/analysis_ui/sampler_menu_tests.rs

@@ -112,6 +112,33 @@ fn delete_removes_the_sampler_under_the_pointer_and_renumbers_the_rest() {
     assert_eq!(listed["samplers"][1]["number"], json!(2));
 }
 
+#[test]
+fn delete_never_removes_another_sampler_after_the_list_changes_under_the_menu() {
+    let (mut app, ctx) = app();
+    app.run("view.colorSamplers.add", json!({"x": 180, "y": 90})).unwrap();
+    right_click(&mut app, &ctx, 120.0, 60.0);
+    assert_eq!(app.ui.canvas_tool_menu.as_ref().and_then(|m| m.sampler), Some(1));
+    // An agent deletes #1 while the menu is open: index 1 now names the old #3.
+    app.run("view.colorSamplers.delete", json!({"index": 0})).unwrap();
+    let r = control(&mut app, &ctx, "ui.context.choose", json!({"id": SAMPLER_DELETE}));
+    assert_eq!(r["ok"], json!(false), "Delete greys out: {r}");
+    assert_eq!(samplers(&app), [[120.0, 60.0], [180.0, 90.0]], "no other sampler is deleted");
+    // The menu itself closes on the next frame.
+    right_click(&mut app, &ctx, 120.0, 60.0);
+    app.run("view.colorSamplers.move", json!({"index": 0, "x": 30, "y": 30})).unwrap();
+    ctx.run_ui(Default::default(), |ui| crate::canvas_tool_menu::show(&mut app, ui.ctx())).textures_delta.clear();
+    assert!(app.ui.canvas_tool_menu.is_none(), "a moved sampler closes the menu");
+    // Another document with a sampler at the same index and spot: still not this menu's sampler.
+    right_click(&mut app, &ctx, 180.0, 90.0);
+    assert_eq!(app.ui.canvas_tool_menu.as_ref().and_then(|m| m.sampler), Some(1));
+    app.run("file.new", json!({"width": 200, "height": 100})).unwrap();
+    for (x, y) in [(20, 20), (180, 90)] {
+        app.run("view.colorSamplers.add", json!({"x": x, "y": y})).unwrap();
+    }
+    let menu = app.ui.canvas_tool_menu.clone().expect("the menu is still open");
+    assert!(!crate::canvas_tool_menu::entry_enabled(&app, &menu, SAMPLER_DELETE), "another document's sampler");
+}
+
 #[test]
 fn readout_rows_do_nothing_until_samplers_have_readout_modes() {
     let (mut app, ctx) = app();

crates/ui-egui/src/transform_type_tests.rs

@@ -57,6 +57,7 @@ fn type_layers_offer_free_transform_but_not_distort_or_perspective() {
         spelling: None,
         crop: None,
         sampler: None,
+        sampler_at: None,
     };
     assert!(crate::canvas_tool_menu::available(&app, &menu, "edit.freeTransform"));
     assert!(!crate::canvas_tool_menu::available(&app, &menu, "edit.transform.distort"));