Merge pull request #5576 from dotCooCoo/fix/pattern-done-gives-back-lent-type-tool

Type: Done during a Cmd press does not save empty point type

crates/engine/src/tests/opacitymask.rs

@@ -6,7 +6,7 @@
 use serde_json::{Value, json};
 use vectorcraft_geom::Affine;
 use vectorcraft_render::{RenderOptions, Renderer};
-use vectorcraft_tools::{PointerEvent, PointerKind};
+use vectorcraft_tools::{Mods, PointerEvent, PointerKind};
 
 use crate::cmd::maskedit::MASK_EDIT_LAYER;
 use crate::tests::{history, select_with_a_step_to_redo};
@@ -234,6 +234,31 @@ fn empty_point_type_is_not_saved_into_the_mask() {
     }
 }
 
+/// The same while a Cmd/Ctrl press has lent the Selection tool to the Type tool: the command before
+/// the release gives the Type tool back first, so the empty point type is still discarded.
+#[test]
+fn empty_point_type_is_not_saved_into_the_mask_during_a_cmd_press() {
+    for cmd in ["transparency.stopEditingOpacityMask", "object.exitIsolation"] {
+        let mut s = session();
+        s.prefs.placeholder_text = false;
+        let obj = editing_mask(&mut s);
+        let v = ViewInfo::default();
+        s.select_tool("type", v).unwrap();
+        for kind in [PointerKind::Down, PointerKind::Up] {
+            s.pointer(&PointerEvent::new(kind, 250.0, 250.0), v).unwrap();
+        }
+        let press = Mods { cmd: true, ..Mods::default() };
+        s.pointer(&PointerEvent::new(PointerKind::Down, 280.0, 280.0).with_mods(press), v).unwrap();
+        assert_eq!(s.tool_id(), "selection", "{cmd}: the press lends the Selection tool");
+        s.execute(cmd, &json!({})).unwrap();
+        assert_eq!(s.tool_id(), "type", "{cmd}: the Type tool is back");
+        assert!(!s.tool_wants_text(), "{cmd}");
+        s.pointer(&PointerEvent::new(PointerKind::Up, 280.0, 280.0).with_mods(press), v).unwrap();
+        let art = &mask(&s, obj).unwrap().art;
+        assert!(art.children().is_none() && !matches!(art.kind, vectorcraft_doc::NodeKind::Text(_)), "{cmd}: the mask keeps only the square");
+    }
+}
+
 /// Releasing the masks of a group and of an object inside it puts each mask's art directly above
 /// its object, as releasing one mask at a time does.
 #[test]

crates/engine/src/tests/pattern.rs

@@ -5,7 +5,7 @@ use vectorcraft_color::Paint;
 use vectorcraft_doc::pattern::{PATTERN_EDIT_LAYER, RepeatKind, TileType};
 use vectorcraft_doc::{Node, NodeKind};
 use vectorcraft_geom::{Point, Rect};
-use vectorcraft_tools::{PointerEvent, PointerKind};
+use vectorcraft_tools::{Mods, PointerEvent, PointerKind};
 
 use crate::tests_menucmds::place_undecodable_png;
 use crate::*;
@@ -134,6 +134,72 @@ fn empty_point_type_is_not_saved_into_the_pattern() {
     }
 }
 
+/// The same while a Cmd/Ctrl press has lent the Selection tool to the Type tool: Done or Exit
+/// Isolation Mode before the release gives the Type tool back first, so the empty point type is
+/// still discarded, and the release that follows leaves the tile as it is.
+#[test]
+fn empty_point_type_is_not_saved_into_the_pattern_during_a_cmd_press() {
+    for cmd in ["object.pattern.done", "object.exitIsolation"] {
+        let mut s = session();
+        s.prefs.placeholder_text = false;
+        let r = rect(&mut s, 0.0, 0.0, 10.0, 10.0);
+        sel(&mut s, &[r]);
+        s.execute("object.pattern.make", &json!({"name": "P"})).unwrap();
+        let v = ViewInfo::default();
+        s.select_tool("type", v).unwrap();
+        for kind in [PointerKind::Down, PointerKind::Up] {
+            s.pointer(&PointerEvent::new(kind, 200.0, 200.0), v).unwrap();
+        }
+        let press = Mods { cmd: true, ..Mods::default() };
+        s.pointer(&PointerEvent::new(PointerKind::Down, 300.0, 300.0).with_mods(press), v).unwrap();
+        assert_eq!(s.tool_id(), "selection", "{cmd}: the press lends the Selection tool");
+        s.execute(cmd, &json!({})).unwrap();
+        assert_eq!(s.tool_id(), "type", "{cmd}: the Type tool is back");
+        assert!(!s.tool_wants_text(), "{cmd}");
+        s.pointer(&PointerEvent::new(PointerKind::Up, 300.0, 300.0).with_mods(press), v).unwrap();
+        let art = &doc(&s).pattern("P").unwrap().art;
+        assert_eq!(art.len(), 1, "{cmd}: the tile keeps only the square");
+        assert!(!matches!(art[0].kind, NodeKind::Text(_)), "{cmd}");
+    }
+}
+
+/// Done during a drag of the lent Selection tool keeps the drag as its own step before the tile is
+/// saved, so no drag is left open for the release to end. With Real-time Drawing and Editing off,
+/// the drag's last position is applied first, as the release would apply it.
+#[test]
+fn done_during_a_lent_selection_drag_keeps_the_drag_as_its_own_step() {
+    for real_time in [true, false] {
+        let mut s = session();
+        s.prefs.placeholder_text = false;
+        s.execute("prefs.set", &json!({"key": "realTimeDrawing", "value": real_time})).unwrap();
+        let r = rect(&mut s, 0.0, 0.0, 10.0, 10.0);
+        sel(&mut s, &[r]);
+        s.execute("object.pattern.make", &json!({"name": "P"})).unwrap();
+        let st = s.doc().unwrap();
+        let b = st.doc.bounds_of(&st.selection.objects, false).expect("the tile art is selected");
+        let c = b.center();
+        let v = ViewInfo::default();
+        s.select_tool("type", v).unwrap();
+        for kind in [PointerKind::Down, PointerKind::Up] {
+            s.pointer(&PointerEvent::new(kind, 200.0, 200.0), v).unwrap();
+        }
+        let press = Mods { cmd: true, ..Mods::default() };
+        s.pointer(&PointerEvent::new(PointerKind::Down, c.x, c.y).with_mods(press), v).unwrap();
+        s.pointer(&PointerEvent::new(PointerKind::Drag, c.x + 20.0, c.y).with_mods(press), v).unwrap();
+        assert!(s.in_interaction(), "{real_time}: the lent Selection tool drags the square");
+        s.execute("object.pattern.done", &json!({})).unwrap();
+        assert!(!s.in_interaction(), "{real_time}: the drag is kept before the tile is saved");
+        assert_eq!(s.tool_id(), "type", "{real_time}");
+        s.pointer(&PointerEvent::new(PointerKind::Up, c.x + 20.0, c.y).with_mods(press), v).unwrap();
+        let labels = s.doc().unwrap().history.undo.iter().map(|e| e.label.as_str()).collect::<Vec<_>>();
+        assert_eq!(labels.last(), Some(&"Done Editing Pattern"), "{real_time}: {labels:?}");
+        assert!(labels.contains(&"Move"), "{real_time}: {labels:?}");
+        let art = &doc(&s).pattern("P").unwrap().art;
+        assert_eq!(art.len(), 1, "{real_time}: the tile keeps only the square");
+        assert!(!matches!(art[0].kind, NodeKind::Text(_)), "{real_time}");
+    }
+}
+
 #[test]
 fn cancel_new_pattern_removes_it() {
     let mut s = session();

crates/engine/src/tooling.rs

@@ -861,9 +861,22 @@ impl Session {
 
     /// End the Type tool's text editing before a command saves the art of an editing layer (pattern
     /// editing's Done, Stop Editing Opacity Mask): point type its click placed that is still empty is
-    /// discarded first, so it is not saved into the pattern tile or the mask.
+    /// discarded first, so it is not saved into the pattern tile or the mask. A selection tool that a
+    /// Cmd press lent is given back first, so the Type tool behind it ends its edit too; a drag of the
+    /// lent tool is kept as its own step, as its release would keep it.
     pub(crate) fn end_text_edit(&mut self) {
-        if self.in_tool_actions || !self.tool.wants_text() {
+        if self.in_tool_actions {
+            return;
+        }
+        if self.lender.is_some() {
+            if let Err(e) = self.apply_actions(vec![Action::Commit]) {
+                log::warn!("keeping the lent tool's drag before ending the text edit: {e}");
+            }
+            if let Err(e) = self.give_back_tool(self.last_view) {
+                log::warn!("giving back the lent tool before ending the text edit: {e}");
+            }
+        }
+        if !self.tool.wants_text() {
             return;
         }
         let acts = self.with_tool_cx(self.last_view, |t, cx| t.deactivate(cx));