From 9382b0ab0ac5ff3549ecb751e2a90e66d5cdbfa4 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sun, 15 Nov 2026 08:30:35 -0800 Subject: [PATCH] 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 | 43 +++++++++++++++++++++++ crates/ui-egui/src/dialogs/text_import.rs | 21 +++++++++++ crates/ui-egui/src/place.rs | 41 +++++++++++++-------- 3 files changed, 91 insertions(+), 14 deletions(-) diff --git a/crates/ui-egui/src/dialogs/psd_import.rs b/crates/ui-egui/src/dialogs/psd_import.rs index 75fd0e0f11..950b15c763 100644 --- a/crates/ui-egui/src/dialogs/psd_import.rs +++ b/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::(); + 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())); diff --git a/crates/ui-egui/src/dialogs/text_import.rs b/crates/ui-egui/src/dialogs/text_import.rs index a52e9f47fe..99128cc321 100644 --- a/crates/ui-egui/src/dialogs/text_import.rs +++ b/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::(); + assert_eq!((objects(0), objects(1)), (1, 0), "placed in the document it was dropped on"); + assert_eq!(app.session.active_index(), Some(0)); + } } diff --git a/crates/ui-egui/src/place.rs b/crates/ui-egui/src/place.rs index 80c2464f62..0ad9fbc3ed 100644 --- a/crates/ui-egui/src/place.rs +++ b/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 { +fn place_file(app: &mut VectorcraftApp, p: &Value, mut read: Read) -> Result { + // 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 { @@ -619,29 +641,20 @@ pub type DropFile = (String, Option, Vec); /// 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() {