Place: an import dialog a drop opened places in the document the file was dropped on

A file dropped on a canvas that asks for import options (a layered PSD,
a PDF with several pages, a DXF drawing, a text file) kept only its
placement params in the dialog. The dialog's OK then placed the file in
whichever document was active at that moment, and placed it even when
the dropped-on document had been reverted since.

The drop now carries its target document and revert count in the place
params, as the place cursor's import options carry their token. Placing
checks them on the drop and again on the dialog's OK: the file goes to
the document it was dropped on, which becomes active again, and is not
placed when that document has been closed or reverted. The dialog then
stays open and shows the reason. The checks drop_files ran itself now
run in that one place.

crates/ui-egui/src/dialogs/psd_import.rs

@@ -322,6 +322,49 @@ mod tests {
         assert!(matches!(st.doc.node(id).unwrap().kind, NodeKind::Image(_)));
     }
 
+    /// The OK of the dialog a drop on a canvas opened places in the document the file was dropped
+    /// on, made active again, even when another document became active meanwhile; it doesn't
+    /// place when that document has been reverted or closed since.
+    #[test]
+    fn a_dropped_files_ok_places_in_the_document_it_was_dropped_on() {
+        use crate::place::{DropAt, DropTarget};
+        let psd = layered();
+        let mut app = app();
+        for _ in 0..2 {
+            app.session.execute("file.new", &json!({"width": 100, "height": 100})).unwrap();
+        }
+        let drop_on_first = |app: &mut VectorcraftApp| {
+            app.session.set_active(0);
+            let st = app.session.doc().unwrap();
+            let target = DropTarget::Place(DropAt { doc: st.uid, reverts: st.reverts, at: vectorcraft_geom::Point::new(50.0, 50.0), embed: true });
+            crate::place::drop_files(app, vec![(target, ("poster.psd".into(), None, psd.clone()))]);
+            assert_eq!(app.ui.dialog.as_ref().map(|d| d.kind.as_str()), Some(KIND));
+            // Another document becomes active while the dialog is open.
+            app.session.set_active(1);
+        };
+        let objects =
+            |app: &VectorcraftApp, i: usize| app.session.documents()[i].doc.layers.iter().map(|l| l.children().map_or(0, Vec::len)).sum::<usize>();
+        drop_on_first(&mut app);
+        super::super::confirm(&mut app).unwrap();
+        assert!(app.ui.dialog.is_none());
+        assert_eq!((objects(&app, 0), objects(&app, 1)), (1, 0), "placed in the document it was dropped on");
+        assert_eq!(app.session.active_index(), Some(0), "which is active again");
+        // Reverted meanwhile: the dialog stays open with the reason, and nothing is placed.
+        drop_on_first(&mut app);
+        let first = app.session.documents()[0].uid;
+        app.session.document_mut(first).unwrap().reverts += 1;
+        let e = super::super::confirm(&mut app).unwrap_err();
+        assert!(e.contains("was reverted"), "{e}");
+        assert_eq!((objects(&app, 0), objects(&app, 1)), (1, 0));
+        // Closed meanwhile.
+        app.ui.dialog = None;
+        drop_on_first(&mut app);
+        app.session.close_document(0);
+        let e = super::super::confirm(&mut app).unwrap_err();
+        assert!(e.contains("was closed"), "{e}");
+        assert_eq!(objects(&app, 0), 0);
+    }
+
     #[test]
     fn several_files_placed_without_link_take_the_last_choices() {
         let dir = std::env::temp_dir().join(format!("vc-psd-import-queue-{}", std::process::id()));

crates/ui-egui/src/dialogs/text_import.rs

@@ -165,4 +165,25 @@ mod tests {
         assert_eq!(t.plain_text(), "one two\nthree", "one paragraph per block");
         assert_eq!(st.history.undo.len(), 1);
     }
+
+    /// A text file dropped on a canvas is placed by the dialog's OK in the document it was dropped
+    /// on, even when another document became active meanwhile.
+    #[test]
+    fn a_dropped_text_files_ok_places_in_the_document_it_was_dropped_on() {
+        use crate::place::{DropAt, DropTarget};
+        let mut app = VectorcraftApp::new(Session::new(), Default::default());
+        for _ in 0..2 {
+            app.run("file.new", json!({"width": 300, "height": 200})).unwrap();
+        }
+        app.session.set_active(0);
+        let st = app.session.doc().unwrap();
+        let target = DropTarget::Place(DropAt { doc: st.uid, reverts: st.reverts, at: vectorcraft_geom::Point::new(100.0, 100.0), embed: true });
+        crate::place::drop_files(&mut app, vec![(target, ("notes.txt".into(), None, b"one".to_vec()))]);
+        assert_eq!(app.ui.dialog.as_ref().map(|d| d.kind.as_str()), Some(KIND));
+        app.session.set_active(1);
+        super::super::confirm(&mut app).unwrap();
+        let objects = |i: usize| app.session.documents()[i].doc.layers.iter().map(|l| l.children().map_or(0, Vec::len)).sum::<usize>();
+        assert_eq!((objects(0), objects(1)), (1, 0), "placed in the document it was dropped on");
+        assert_eq!(app.session.active_index(), Some(0));
+    }
 }

crates/ui-egui/src/place.rs

@@ -88,6 +88,11 @@ pub struct PlaceState {
 /// file is placed.
 const ASKED: &str = "__placeCursorAsk";
 
+/// The `file.place` param that keeps where a file dropped on a canvas goes (`{doc, reverts}`, as
+/// [`DropAt`]) through the import options dialog the drop opened, so its OK places the file in that
+/// document; hidden from the dialog's fields and removed before the file is placed.
+const DROPPED: &str = "__dropTarget";
+
 /// A place cursor thumbnail: decoded when the cursor is loaded, uploaded when first drawn (and
 /// again in a new graphics context).
 struct Thumb {
@@ -149,7 +154,23 @@ enum Read {
     ForUser { dropped: bool },
 }
 
-fn place_file(app: &mut VectorcraftApp, p: &Value, read: Read) -> Result<Value, String> {
+fn place_file(app: &mut VectorcraftApp, p: &Value, mut read: Read) -> Result<Value, String> {
+    // A file dropped on a canvas, and the OK of the import options its drop asked for, go to the
+    // document it was dropped on, which becomes active again; not when that document has closed or
+    // been reverted since.
+    if let Some(target) = p.get(DROPPED) {
+        let field = |k: &str| target.get(k).and_then(Value::as_u64);
+        let Some(i) = app.session.documents().iter().position(|st| Some(st.uid) == field("doc")) else {
+            return Err("the document it was dropped on was closed".into());
+        };
+        if app.session.documents().get(i).is_some_and(|st| Some(st.reverts) != field("reverts")) {
+            return Err("the document it was dropped on was reverted".into());
+        }
+        app.session.set_active(i);
+        if let Read::ForUser { .. } = read {
+            read = Read::ForUser { dropped: true };
+        }
+    }
     // The OK of import options a click of the place cursor asked for places within the roots the
     // cursor was loaded in. A caller with roots of its own places within those.
     let Some(token) = p.get(ASKED) else { return place_file_in(app, p, read) };
@@ -224,6 +245,7 @@ fn place_file_in(app: &mut VectorcraftApp, p: &Value, read: Read) -> Result<Valu
     }
     if let Some(o) = p.as_object_mut() {
         o.remove(ASKED);
+        o.remove(DROPPED);
     }
     match read {
         Read::ForUser { dropped } if crate::placing::available(app) => {
@@ -619,29 +641,20 @@ pub type DropFile = (String, Option<String>, Vec<u8>);
 /// Files dropped on the window, each placed on a canvas or opened (and added to Open Recent
 /// Files) as its target says: the placed ones first, so the last document opened is the one
 /// shown. A file for a canvas goes to the document it was dropped on, which becomes active again;
-/// it isn't placed when that document has closed or been reverted since.
+/// it isn't placed when that document has closed or been reverted since, and neither is it by the
+/// OK of an import options dialog the drop opened ([`DROPPED`]).
 pub fn drop_files(app: &mut VectorcraftApp, mut files: Vec<(DropTarget, DropFile)>) {
     files.sort_by_key(|(target, _)| *target == DropTarget::Open);
     for (target, (name, path, bytes)) in files {
-        if let DropTarget::Place(d) = target {
-            let Some(i) = app.session.documents().iter().position(|st| st.uid == d.doc) else {
-                app.status(format!("Couldn't place {name}: the document it was dropped on was closed"));
-                continue;
-            };
-            if app.session.documents().get(i).is_some_and(|st| st.reverts != d.reverts) {
-                app.status(format!("Couldn't place {name}: the document it was dropped on was reverted"));
-                continue;
-            }
-            app.session.set_active(i);
-        }
         let r = match target {
-            DropTarget::Place(DropAt { at, embed, .. }) => {
+            DropTarget::Place(DropAt { doc, reverts, at, embed }) => {
                 let mut p = match &path {
                     Some(path) => json!({ "path": path }),
                     None => json!({ "name": name, "dataBase64": vectorcraft_format::base64_encode(&bytes) }),
                 };
                 p["at"] = json!([at.x, at.y]);
                 p["link"] = json!(!embed);
+                p[DROPPED] = json!({ "doc": doc, "reverts": reverts });
                 place_file(app, &p, Read::ForUser { dropped: true }).map(|_| ())
             }
             DropTarget::Open => match path.as_deref() {