Batch: picking the Crop tool again keeps a committed crop's frame hidden

Measured in the reference application: after ↵ commits a crop, the Crop
tool shows no frame; pressing C again, or another tool and back, keeps it
hidden, and only a press on the canvas brings the frame back. A crop
committed by leaving the tool is different: back on the Crop tool the idle
frame shows at once. #10642 brought the frame back on re-picking; picking
the tool no longer ends the hidden state, and a commit on leaving the tool
does not start it. Tests follow.

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

crates/ui-egui/src/tool_memory.rs

@@ -26,10 +26,6 @@ pub(crate) fn select(app: &mut PhotocraftApp, tool: Tool) {
     app.stylus.tool_before_eraser = None;
     app.ui.spring = None;
     app.ui.tool = tool;
-    // Picking the Crop tool, even the one already picked, brings back a frame a commit hid.
-    if tool == Tool::Crop {
-        crate::crop_ui::show_frame(app);
-    }
 }
 
 /// Temporary canvas overrides do not change `ui.tool`. Spring-loaded letters and the pen's

crates/ui-egui/src/tools/crop_mode/tests.rs

@@ -579,9 +579,9 @@ fn p_toggles_classic_mode_while_a_crop_is_in_progress() {
     assert!(!classic(&app));
 }
 
-/// After a commit the tool shows no frame until the next press or re-picking the tool, which
-/// brings its untouched default frame back: H and P are the Hand and Pen keys there, as when the
-/// tool is picked, not Show Cropped Area and Classic Mode (#9731, #6536).
+/// After a commit the tool shows no frame until the next press, which brings its untouched default
+/// frame back: H and P are the Hand and Pen keys there, as when the tool is picked, not Show
+/// Cropped Area and Classic Mode (#9731, #6536).
 #[test]
 fn h_and_p_pick_their_tools_over_the_default_frame_after_a_commit() {
     for (k, tool) in [(egui::Key::H, Tool::Hand), (egui::Key::P, Tool::Pen)] {
@@ -591,9 +591,8 @@ fn h_and_p_pick_their_tools_over_the_default_frame_after_a_commit() {
         assert_eq!(app.session.active().map(|st| st.doc.size), Some(Size::new(60, 40)), "the crop committed");
         crate::crop_ui::ensure_frame(&mut app);
         assert!(app.ui.crop_rect.is_none(), "the commit hides the frame");
-        crate::tool_memory::select(&mut app, Tool::Crop);
-        crate::crop_ui::ensure_frame(&mut app);
-        assert!(app.ui.crop_rect.is_some() && app.crop.default_frame, "re-picking the tool brings the default frame back");
+        crate::crop_ui::show_frame(&mut app);
+        assert!(app.ui.crop_rect.is_some() && app.crop.default_frame, "a press brings the default frame back");
         key(&mut app, k, NONE);
         assert_eq!(app.ui.tool, tool, "{k:?}");
         let s = &app.ui.tool_options.crop_shield;
@@ -740,11 +739,8 @@ fn a_recrop_handle_snaps_exactly_to_the_hidden_pixels_edge() {
                     let v = &mut app.ui.views[0];
                     (v.zoom, v.center, v.fit_pending, v.rotation) = (1.0, [50.0, 25.0], false, 0.0);
                     app.ui.extras.snap = true;
-                    // Re-picking the tool brings back the default frame the commit hid.
-                    app.ui.tool = Tool::Move;
-                    crate::crop_ui::ensure_frame(&mut app);
-                    app.ui.tool = Tool::Crop;
-                    crate::crop_ui::ensure_frame(&mut app);
+                    // A press brings back the default frame the commit hid.
+                    crate::crop_ui::show_frame(&mut app);
                     assert_eq!(app.ui.crop_rect, Some([0.0, 0.0, 100.0, 50.0]));
                     if ratio {
                         app.ui.tool_options.crop_ratio = "original".into();

crates/ui-egui/src/tools/crop_ui.rs

@@ -381,7 +381,6 @@ pub fn ensure_frame(app: &mut PhotocraftApp) {
 fn frame(app: &mut PhotocraftApp) {
     if app.ui.tool != Tool::Crop {
         app.crop.editing = false;
-        app.crop.dismissed = None;
         crate::crop_straighten::reset(app);
         // Leaving the tool drops an untouched default frame and, as in Photoshop, commits a
         // drawn one (#3368).
@@ -449,6 +448,9 @@ fn commit_on_leave(app: &mut PhotocraftApp) {
         cancel(app);
     }
     app.ui.tool = tool;
+    // Committed by leaving the tool: back on it the default frame shows at once, as in Photoshop
+    // (only a commit with the Crop tool still picked hides it).
+    app.crop.dismissed = None;
 }
 
 /// A crop gesture is in progress: Space repositions the frame rather than panning.
@@ -471,8 +473,9 @@ pub fn cancel(app: &mut PhotocraftApp) {
 }
 
 /// After a committed crop, with the Crop tool picked: no new default frame for the cropped
-/// document until a press on the canvas or re-picking the tool, so its corners don't make the
-/// applied crop look still pending.
+/// document until a press on the canvas (or a ratio), so its corners don't make the applied crop
+/// look still pending. Picking the tool again, also after another tool, keeps it hidden, as in
+/// Photoshop.
 pub fn hide_frame(app: &mut PhotocraftApp) {
     if app.ui.tool == Tool::Crop {
         app.crop.dismissed = dismiss_key(app);

crates/ui-egui/src/tools/crop_ui/tests.rs

@@ -122,14 +122,16 @@ fn enter_on_the_selection_frame_crops_to_it_at_8_and_16_bit() {
         assert_eq!(doc.size, Size::new(30, 40), "{depth:?}");
         assert_eq!(doc.depth, depth);
         assert!(doc.selection.is_none(), "cropping to the selection deselects, as in Photoshop");
-        // No frame until a press or re-picking the tool; then the default one is the whole
-        // (cropped) canvas, and ↵ on it does nothing.
+        // No frame until a press, also after another tool and back; then the default one is the
+        // whole (cropped) canvas, and ↵ on it does nothing.
         ensure_frame(&mut app);
         assert_eq!(app.ui.crop_rect, None);
         app.ui.tool = Tool::Move;
         ensure_frame(&mut app);
-        app.ui.tool = Tool::Crop;
+        crate::tool_memory::select(&mut app, Tool::Crop);
         ensure_frame(&mut app);
+        assert_eq!(app.ui.crop_rect, None, "picking the tool again keeps it hidden");
+        show_frame(&mut app);
         assert_eq!(app.ui.crop_rect, Some([0.0, 0.0, 30.0, 40.0]));
         let steps = app.session.active().unwrap().history.past_len();
         crate::canvas::commit_crop(&mut app);
@@ -138,10 +140,10 @@ fn enter_on_the_selection_frame_crops_to_it_at_8_and_16_bit() {
     }
 }
 
-/// A commit hides the frame; picking the Crop tool again, a ratio, or a press brings back the
-/// default frame, and the press's crop keeps its box Undo and Redo.
+/// A commit hides the frame; picking the Crop tool again keeps it hidden, as in Photoshop; a ratio
+/// or a press brings back the default frame, and the press's crop keeps its box Undo and Redo.
 #[test]
-fn a_hidden_frame_comes_back_on_re_picking_a_ratio_or_a_press_with_redo_intact() {
+fn a_hidden_frame_comes_back_on_a_ratio_or_a_press_with_redo_intact() {
     let committed = || {
         let mut app = app(SampleType::U8);
         app.ui.crop_rect = Some([20.0, 10.0, 120.0, 60.0]);
@@ -151,9 +153,17 @@ fn a_hidden_frame_comes_back_on_re_picking_a_ratio_or_a_press_with_redo_intact()
         app
     };
     let whole = Some([0.0, 0.0, 100.0, 50.0]);
-    // Picking the Crop tool while it is still picked.
+    // Picking the Crop tool while it is still picked, or after another tool: still hidden.
     let mut app = committed();
     crate::tool_memory::select(&mut app, Tool::Crop);
+    ensure_frame(&mut app);
+    assert_eq!(app.ui.crop_rect, None, "re-picked");
+    crate::tool_memory::select(&mut app, Tool::Move);
+    ensure_frame(&mut app);
+    crate::tool_memory::select(&mut app, Tool::Crop);
+    ensure_frame(&mut app);
+    assert_eq!(app.ui.crop_rect, None, "picked after another tool");
+    show_frame(&mut app);
     assert_eq!(app.ui.crop_rect, whole);
     assert!(app.crop.default_frame);
     // A ratio fits the frame it brings back.
@@ -1283,7 +1293,7 @@ fn info_size_is_shown_in_the_ruler_unit() {
 /// #3145 review: after the crop is cancelled or committed, or the tool is left, W and H are the
 /// selection's again. With the Crop tool still picked, the new default frame is the selection's
 /// bounds (or the canvas), as in Photoshop, so the panel shows the same size. After a commit that
-/// frame waits for the next press or re-picking the tool.
+/// frame waits for the next press.
 #[test]
 fn info_size_returns_to_the_selection_after_cancel_or_commit() {
     let mut app = app(SampleType::U8);
@@ -1306,8 +1316,8 @@ fn info_size_returns_to_the_selection_after_cancel_or_commit() {
     assert_eq!(info_size(&app), None, "committed: no frame");
     ensure_frame(&mut app);
     assert_eq!(info_size(&app), None, "committed: the frame stays hidden");
-    crate::tool_memory::select(&mut app, Tool::Crop);
-    ensure_frame(&mut app);
+    // A press brings it back.
+    show_frame(&mut app);
     assert_eq!(info_size(&app), Some([40.0, 30.0]), "the new frame is the cropped canvas");
     // Leaving the tool: the panel shows the selection, whatever the frame.
     app.ui.tool = Tool::Move;