diff --git a/crates/ui-egui/src/canvas/canvas_tool_menu.rs b/crates/ui-egui/src/canvas/canvas_tool_menu.rs index 098304d147..50059007ad 100644 --- a/crates/ui-egui/src/canvas/canvas_tool_menu.rs +++ b/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, + /// 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; diff --git a/crates/ui-egui/src/tools/analysis_ui/sampler_menu_tests.rs b/crates/ui-egui/src/tools/analysis_ui/sampler_menu_tests.rs index feef455427..9388a31f68 100644 --- a/crates/ui-egui/src/tools/analysis_ui/sampler_menu_tests.rs +++ b/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(); diff --git a/crates/ui-egui/src/transform_type_tests.rs b/crates/ui-egui/src/transform_type_tests.rs index c62e7cf87d..e179e4f6d5 100644 --- a/crates/ui-egui/src/transform_type_tests.rs +++ b/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"));