From a10e288a581d2fda01275b0a07c2f148852f5f76 Mon Sep 17 00:00:00 2001 From: texjer Date: Tue, 3 Nov 2026 21:54:43 -0500 Subject: [PATCH 1/8] Place: dropped images load the place cursor, in the order they came Split out of #2073. Port of #1307's drops and #1666. - Several images dropped together load the place cursor in the order they came, each run of them where it comes among the other files, after the images it already holds; click to place them one by one. - One image dropped off a frame that takes it loads the place cursor instead of being placed at once; on such a frame it still fills it. - A drop is handled in its order, each file at its own pointer, and a dialog (a multi-page PDF's Import Options, an alert) cancels the rest. - On the web, files dropped together are read as one batch (none if one can't be read), and a single dropped image loads the place cursor. --- ROADMAP.md | 1 + apps/designcraft-web/src/browser_policy.rs | 39 ++++++- apps/designcraft-web/src/web.rs | 23 +++- crates/ui-egui/src/app/files.rs | 97 ++++++++++++---- crates/ui-egui/src/app/input.rs | 9 +- crates/ui-egui/src/tests.rs | 128 ++++++++++++++++++++- docs/multi-image-placement.md | 10 +- 7 files changed, 269 insertions(+), 38 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 3cef01494..8aedbe1f0 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -405,6 +405,7 @@ Newest first. - **2026-10-30** · Import warnings about a story have a Show button: it closes the warnings (the status line keeps them) and selects the story's first frame, fitted in the window. `file.open` returns `warningItems`, the frame each IDML or InDesign warning is about (or null), matching the converter's INDD story numbers (`story 30892`, written to IDML as `u78ac`) and IDML `Self` names to the imported stories. No readiness percentage change. - **2026-10-30** · Parent pages: a running-header variable shows its name in angle brackets on the parent itself (it was blank), as InDesign shows it; on the document pages, and in text with no page, it shows its text as before. No readiness percentage change. - **2026-10-30** · Without facing pages, a page of a multi-page spread shows the parent page in its place, as in InDesign: the second page of a two-page spread shows the parent's second page. Every single-sided page showed the parent's first page, so an InDesign book set up without facing pages but laid out in two-page spreads repeated the left parent page's items (a black heading bar) on its right pages and lost the right parent page's. No readiness percentage change. +- **2026-10-30** · Dropping images from the file manager works as in InDesign: images dropped together load the place cursor in the order they came, and one image dropped off a frame that takes it loads the place cursor instead of being placed at once (on a frame it still fills it). Documents and text files dropped keep opening and flowing as before. Ported from #1307 and #1666; see [multi-image placement](docs/multi-image-placement.md). No readiness percentage change. - **2026-10-30** · Placing several images works as in InDesign. File › Place takes several files, and they load the place cursor in the order chosen: it shows the first one's thumbnail with how many are loaded ("(6)"), each click places the next, arrow keys while dragging lay out a grid (`place.grid`), and Escape unloads the rest (`place.cancel`). A drag with the cursor draws the frame at the graphic's proportions, with its scale. Ported from #1307, #1392 and #1432 (multi-image placement, cursor thumbnail and count); see [multi-image placement](docs/multi-image-placement.md). No readiness percentage change. - **2026-10-30** · macOS: a file dragged in from the Finder fills the frame it is dropped on. winit sends no pointer events during such a drag, so the drop asks the system where the pointer is (`Services::pointer`, from `NSEvent.mouseLocation`), at any UI scale. No readiness percentage change. - **2026-10-30** · File > Package opens a dialog, as the reference app's: a Summary of fonts, links and images and inks, pages for each, a Printing Instructions step, and a PDF made with the options of a preset (#1514, #1515). diff --git a/apps/designcraft-web/src/browser_policy.rs b/apps/designcraft-web/src/browser_policy.rs index 524df6cc5..ca82c71ca 100644 --- a/apps/designcraft-web/src/browser_policy.rs +++ b/apps/designcraft-web/src/browser_policy.rs @@ -1,4 +1,4 @@ -//! Pure browser startup and download metadata policies. +//! Browser startup, download metadata, and atomic import policies. /// Query flags are case-sensitive decoded keys. Values are ignored: `?sample=false` /// still enables sample startup, and repeated keys have the same presence semantics. @@ -65,3 +65,40 @@ mod tests { } } } + +/// Read a drop in order, publishing nothing unless every file can be read. +pub(crate) async fn read_batch(files: impl IntoIterator, mut read: F) -> Result, E> +where + F: FnMut(T) -> Fut, + Fut: std::future::Future>, +{ + let mut result = Vec::new(); + for file in files { + result.push(read(file).await?); + } + Ok(result) +} + +#[cfg(test)] +mod batch_tests { + #[test] + fn failed_read_aborts_the_whole_batch() { + use std::{ + future::Future, + task::{Context, Poll, Waker}, + }; + let mut seen = Vec::new(); + let result = { + let future = super::read_batch([1, 2, 3], |n| { + seen.push(n); + std::future::ready(if n == 2 { Err("read failed") } else { Ok(n) }) + }); + let mut future = std::pin::pin!(future); + future.as_mut().poll(&mut Context::from_waker(Waker::noop())) + }; + assert_eq!(result, Poll::Ready(Err("read failed"))); + assert_eq!(seen, [1, 2]); + let mut success = std::pin::pin!(super::read_batch([3, 1], |n| std::future::ready(Ok::<_, ()>(n)))); + assert_eq!(success.as_mut().poll(&mut Context::from_waker(Waker::noop())), Poll::Ready(Ok(vec![3, 1]))); + } +} diff --git a/apps/designcraft-web/src/web.rs b/apps/designcraft-web/src/web.rs index 8b23ef6b7..c87581e32 100644 --- a/apps/designcraft-web/src/web.rs +++ b/apps/designcraft-web/src/web.rs @@ -357,18 +357,29 @@ impl eframe::App for WebShell { let _ = self.graphics.run_if_healthy(|| { // Dropped files belong to this host iteration's raw input, not to stale logic input: // read them here once, before the batch is saved as pending for a hidden pass. - for f in std::mem::take(&mut raw.dropped_files) { + // Files dropped together arrive together, in their order, so several images load the + // place cursor as one batch. + let dropped = std::mem::take(&mut raw.dropped_files); + if !dropped.is_empty() { let request = self.app.import_request("drop"); let inbox = self.inbox.clone(); let ctx = ctx.clone(); wasm_bindgen_futures::spawn_local(async move { - let name = f.path().file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_else(|| "dropped".into()); - match f.bytes_async().await { - Ok(bytes) => { - inbox.lock().unwrap_or_else(|e| e.into_inner()).push(ImportedFile { request, name, bytes }); + let files = crate::browser_policy::read_batch(dropped, |f| { + let request = request.clone(); + async move { + let name = f.path().file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_else(|| "dropped".into()); + let bytes = f.bytes_async().await.map_err(|e| format!("couldn't read dropped file {name}: {e}"))?; + Ok::<_, String>(ImportedFile { request, name, bytes }) + } + }) + .await; + match files { + Ok(files) => { + inbox.lock().unwrap_or_else(|e| e.into_inner()).extend(files); ctx.request_repaint(); } - Err(e) => log::error!("couldn't read dropped file {name}: {e}"), + Err(error) => log::error!("{error}"), } }); } diff --git a/crates/ui-egui/src/app/files.rs b/crates/ui-egui/src/app/files.rs index 3df85b504..bb0a20661 100644 --- a/crates/ui-egui/src/app/files.rs +++ b/crates/ui-egui/src/app/files.rs @@ -162,7 +162,7 @@ impl DesignApp { let Some(inbox) = self.services.inbox.clone() else { return }; let files = std::mem::take(&mut *inbox.lock().unwrap_or_else(std::sync::PoisonError::into_inner)); let cursor_image = |file: &ImportedFile| { - file.request.purpose == "place" + matches!(file.request.purpose.as_str(), "place" | "drop") && !file.name.to_ascii_lowercase().ends_with(".pdf") && (is_cursor_image(&file.name) || designcraft_render::image_size(&file.bytes).is_some()) }; @@ -175,7 +175,7 @@ impl DesignApp { shortcut_import::import_delivered(self, &file.bytes); continue; } - let placing = file.request.purpose == "place"; + let placing = matches!(file.request.purpose.as_str(), "place" | "drop"); let mut group = vec![]; if cursor_image(&file) { while let Some(next) = @@ -184,8 +184,9 @@ impl DesignApp { group.push(next); } } - // Several images: into the place cursor. - if !group.is_empty() { + // Several images, or one dropped (a browser drop has no position to place it at or + // frame to fill): into the place cursor. + if !group.is_empty() || (file.request.purpose == "drop" && cursor_image(&file)) { if self.ui.dialog.is_some() { self.status(crate::i18n::tr(&self.ui.language, "Image import cancelled while a dialog is open")); continue; @@ -262,9 +263,9 @@ impl DesignApp { } /// A file dropped on the window: documents open, the swatches in a swatch exchange (.ase) file - /// are loaded, and anything else is placed on the spread in view (a parent spread while parents - /// are being edited). A file without an extension opens when its content is a DesignCraft or - /// IDML document. + /// are loaded, an image loads into the place cursor, and anything else is placed on the spread + /// in view (a parent spread while parents are being edited). A file without an extension + /// opens when its content is a DesignCraft or IDML document. pub fn open_dropped(&mut self, path: &str) -> Result { self.open_dropped_at(path, None) } @@ -273,18 +274,48 @@ impl DesignApp { /// window's coordinates. #[cfg(not(target_arch = "wasm32"))] pub(crate) fn open_dropped_files(&mut self, files: &[egui::DroppedFileHandle], pointer: Option) { + let drops = files.iter().map(|f| f.path().to_string_lossy().to_string()).filter(|p| !p.is_empty()).map(|p| (p, pointer)).collect(); + self.open_dropped_batch(drops); + } + + /// Files dropped on the window together, each with the pointer it was dropped at, handled in + /// the order they came. Each that isn't an image opens or is placed as + /// [`Self::open_dropped_at`] does at its own pointer. One image is dropped as that does too + /// (filling the frame under the pointer, else loading the place cursor); several load the place + /// cursor, each run of them where it came in the order, after any images it already holds, + /// whatever is under the pointer: click to place them one by one, drag with the arrow keys for + /// a grid. The rest of the drop is cancelled once a file opens a dialog (a multi-page PDF's + /// Import Options, an alert), as File › Place with several chosen is. + pub fn open_dropped_batch(&mut self, drops: Vec<(String, Option)>) { + let several = drops.iter().filter(|(p, _)| is_cursor_image(p)).count() > 1; + // Only the first graphic of a drop fills the frame under the pointer. let mut frame_free = true; - for f in files { - let p = f.path().to_string_lossy().to_string(); - if !p.is_empty() { - let _ = self.place_dropped(&p, pointer, &mut frame_free); + let mut images = Vec::new(); + // `None` at the end flushes the images left. + for drop in drops.into_iter().map(Some).chain([None]) { + if several + && let Some((path, _)) = &drop + && is_cursor_image(path) + { + images.push(path.clone()); + continue; } + // A file that can't be placed has its alert, which cancels the rest below. + if !images.is_empty() { + let _ = self.place_paths(std::mem::take(&mut images)); + } + let Some((path, pointer)) = drop else { break }; + if self.ui.dialog.is_some() { + self.status(crate::i18n::tr(&self.ui.language, "Place cancelled while a dialog is open")); + return; + } + let _ = self.place_dropped(&path, pointer, &mut frame_free); } } /// [`Self::open_dropped`] at the pointer: a graphic dropped on an empty or graphic frame fills - /// it, elsewhere its top left lands where the file was dropped on the canvas; text goes to a - /// new frame on the document page there. + /// it; elsewhere an image loads into the place cursor, and another graphic's top left lands + /// where the file was dropped on the canvas; text goes to a new frame on the document page there. pub fn open_dropped_at(&mut self, path: &str, pointer: Option) -> Result { self.place_dropped(path, pointer, &mut true) } @@ -303,13 +334,22 @@ impl DesignApp { if document { return self.open_file("file.open", json!({"path": path})); } - let mut params = json!({"path": path}); - // A graphic dropped on an empty or graphic frame fills it, as File > Place does. Text keeps + // A graphic dropped on a frame that takes one fills it, as File > Place does. Text keeps // getting a frame of its own, so it never replaces the image in a graphic frame. let frame = if *frame_free && !designcraft_textimport::is_text_file(path) { canvas::drop_frame(self, pointer) } else { None }; - if let (Some(frame), Some(o)) = (frame, params.as_object_mut()) { - o.insert("frame".into(), json!(frame.0)); + if let Some(frame) = frame { + let placed = self.open_file("file.place", json!({"path": path, "frame": frame.0})); + if placed.is_ok() { + *frame_free = false; + } + return placed; } + // An image dropped anywhere else waits in the place cursor, after any it already holds. + if is_cursor_image(path) { + let append = self.session.loaded.is_some(); + return self.open_file("place.load", json!({"path": path, "append": append})); + } + let mut params = json!({"path": path}); if let (Some((sr, at)), Some(o)) = (canvas::drop_point(self, pointer), params.as_object_mut()) { o.insert("spread".into(), serde_json::to_value(sr).unwrap_or(Value::Null)); o.insert("x".into(), json!(at.x)); @@ -319,11 +359,7 @@ impl DesignApp { o.insert("page".into(), json!(st.doc.first_page_of_spread(si) + pi + 1)); } } - let placed = self.open_file("file.place", params); - if frame.is_some() && placed.is_ok() { - *frame_free = false; - } - placed + self.open_file("file.place", params) } /// Queue files the operating system asked the app to open. They can arrive before the window @@ -908,6 +944,23 @@ pub(crate) mod browser_import_tests { assert_eq!(loaded_names(&app), ["a.png", "one.pdf", "b.png"], "in the order dropped, all in the cursor"); } + /// A browser drop of one image (it has no position or frame under it) loads the place + /// cursor; images dropped together load it at once, after what it holds. + #[test] + fn browser_dropped_images_load_the_place_cursor() { + let mut app = app(); + let request = app.import_request("drop"); + deliver(&mut app, request, "one.png", png(200)); + assert_eq!(app.session.tool_id(), "placeGun"); + assert!(app.session.doc().unwrap().doc.spreads[0].items.is_empty(), "nothing placed yet"); + let request = app.import_request("drop"); + for (name, v) in [("two.png", 90), ("three.png", 60)] { + app.services.inbox.as_ref().unwrap().lock().unwrap().push(ImportedFile { request: request.clone(), name: name.into(), bytes: png(v) }); + } + app.drain_inbox(); + assert_eq!(loaded_names(&app), ["one.png", "two.png", "three.png"]); + } + #[test] fn batch_cancel_status_uses_the_interface_language() { let mut app = app(); diff --git a/crates/ui-egui/src/app/input.rs b/crates/ui-egui/src/app/input.rs index 2682478c4..2281d3643 100644 --- a/crates/ui-egui/src/app/input.rs +++ b/crates/ui-egui/src/app/input.rs @@ -154,13 +154,10 @@ impl DesignApp { if drops.is_empty() { return; } - // Each file keeps the pointer it was dropped at; the system is asked once, for those - // that came without one. + // Dropped together, they go together: several images load the place cursor at once. Each + // file keeps the pointer it was dropped at. let system = drops.iter().any(|d| d.pointer.is_none()).then(|| self.system_pointer(ctx)).flatten(); - let mut frame_free = true; - for drop in drops { - let _ = self.place_dropped(&drop.path, drop.pointer.or(system), &mut frame_free); - } + self.open_dropped_batch(drops.into_iter().map(|d| (d.path, d.pointer.or(system))).collect()); } /// Where the pointer is in the window, asked of the system ([`crate::Services::pointer`]), diff --git a/crates/ui-egui/src/tests.rs b/crates/ui-egui/src/tests.rs index e53e603a1..1f8cedf68 100644 --- a/crates/ui-egui/src/tests.rs +++ b/crates/ui-egui/src/tests.rs @@ -884,10 +884,20 @@ fn a_graphic_dropped_on_a_frame_fills_it() { let r = app.open_dropped_at(&path.to_string_lossy(), Some(inside)); let _ = std::fs::remove_file(&path); assert_eq!(r.unwrap()["id"].as_u64(), Some(frame), "the frame under the pointer is filled"); + assert!(app.session.loaded.is_none(), "nothing waits in the place cursor"); let d = &app.session.doc().unwrap().doc; assert_eq!(d.spreads[0].items.len(), 1, "no new object"); assert!(matches!(d.item(designcraft_doc::ItemId(frame)).unwrap().content, designcraft_doc::Content::Graphic(_))); assert!(canvas::drop_frame(&app, Some(outside)).is_none(), "an empty spot on the page is no frame"); + // Dropped again, it replaces the image there rather than loading the place cursor. + let path = dropped_png("dropframe-again"); + app.open_dropped_at(&path.to_string_lossy(), Some(inside)).unwrap(); + let _ = std::fs::remove_file(&path); + let d = &app.session.doc().unwrap().doc; + assert_eq!(d.spreads[0].items.len(), 1, "still no new object"); + let designcraft_doc::Content::Graphic(g) = &d.item(designcraft_doc::ItemId(frame)).unwrap().content else { panic!("a graphic frame") }; + assert!(d.assets[&g.asset].name.contains("dropframe-again"), "the frame shows the new image"); + assert!(app.session.loaded.is_none(), "nothing waits in the place cursor"); // Text dropped there gets a frame of its own and leaves the image in place. let text = std::env::temp_dir().join(format!("designcraft-ui-{}-dropframe.txt", std::process::id())); std::fs::write(&text, "Dropped words").unwrap(); @@ -920,6 +930,7 @@ fn a_file_dropped_without_pointer_events_fills_the_frame_under_the_system_pointe ctx.run_ui(raw, |ui| app.take_pending_drops(ui.ctx())).textures_delta.clear(); let _ = std::fs::remove_file(&path); assert!(app.pending_drops.is_empty()); + assert!(app.session.loaded.is_none(), "nothing waits in the place cursor at {zoom}"); let d = &app.session.doc().unwrap().doc; assert_eq!(d.spreads[0].items.len(), 1, "no new object at {zoom}"); let designcraft_doc::Content::Graphic(g) = &d.item(designcraft_doc::ItemId(frame)).unwrap().content else { @@ -940,7 +951,8 @@ fn a_locked_frame_is_not_a_drop_target() { r.unwrap(); let d = &app.session.doc().unwrap().doc; assert!(matches!(d.item(designcraft_doc::ItemId(frame)).unwrap().content, designcraft_doc::Content::Unassigned), "the locked frame stays empty"); - assert_eq!(d.spreads[0].items.len(), 2, "the graphic became a new object"); + assert_eq!(d.spreads[0].items.len(), 1, "no new object yet"); + assert!(app.session.loaded.is_some(), "the image waits in the place cursor"); } #[test] @@ -954,8 +966,10 @@ fn only_the_first_graphic_of_a_drop_fills_the_frame() { let _ = (std::fs::remove_file(&first), std::fs::remove_file(&second)); assert_eq!(a.unwrap()["id"].as_u64(), Some(frame)); assert_ne!(b.unwrap()["id"].as_u64(), Some(frame), "the second graphic doesn't replace the first"); + // An image dropped off a frame that takes it waits in the place cursor. + assert!(app.session.loaded.is_some(), "the second image waits in the place cursor"); let d = &app.session.doc().unwrap().doc; - assert_eq!(d.spreads[0].items.len(), 2, "the second graphic is a new object"); + assert_eq!(d.spreads[0].items.len(), 1, "no new object"); assert!(matches!(d.item(designcraft_doc::ItemId(frame)).unwrap().content, designcraft_doc::Content::Graphic(_))); } @@ -971,11 +985,121 @@ fn a_file_dropped_while_a_parent_is_open_lands_on_the_parent() { let r = app.open_dropped(&path.to_string_lossy()); let _ = std::fs::remove_file(&path); r.unwrap(); + // The image waits in the place cursor; a click on the parent spread places it there. + assert!(app.session.loaded.is_some()); + let st = app.session.active().unwrap(); + let at = designcraft_tools::CanvasLayout::new(&st.doc, st.editing_parents) + .slot(designcraft_doc::SpreadRef::Parent(0)) + .unwrap() + .to_canvas(designcraft_geom::Point::new(50.0, 50.0)); + let v = designcraft_engine::ViewInfo::at_zoom(1.0); + for kind in [designcraft_tools::PointerKind::Down, designcraft_tools::PointerKind::Up] { + app.session.pointer(&designcraft_tools::PointerEvent::new(kind, at.x, at.y), v).unwrap(); + } let d = &app.session.doc().unwrap().doc; assert_eq!(d.parents[0].items.len(), 1, "the graphic is on the parent spread"); assert!(d.spreads.iter().all(|s| s.items.is_empty()), "no document page gets it"); } +/// The names of the images in the place cursor, the one a click places first. +fn loaded_names(app: &DesignApp) -> Vec { + let doc = &app.session.doc().unwrap().doc; + app.session.loaded.iter().chain(app.session.loaded_queue.iter()).map(|g| doc.assets[&g.asset].name.clone()).collect() +} + +/// One image dropped where no frame can take it loads the place cursor instead of being placed: +/// the next click places it. A frame selected doesn't take it, and another image dropped +/// meanwhile queues after it. +#[test] +fn a_single_image_dropped_off_a_frame_loads_the_place_cursor() { + let (mut app, frame, _, outside) = app_with_drop_frame(); + app.run("selection.set", json!({"ids": []})).unwrap(); + let first = dropped_png("drop-loads-a"); + let second = dropped_png("drop-loads-b"); + app.open_dropped_at(&first.to_string_lossy(), Some(outside)).unwrap(); + assert_eq!(app.session.tool_id(), "placeGun"); + assert_eq!(loaded_names(&app).len(), 1); + assert_eq!(app.session.doc().unwrap().doc.spreads[0].items.len(), 1, "nothing placed yet"); + app.run("selection.set", json!({"ids": [frame]})).unwrap(); + app.open_dropped_at(&second.to_string_lossy(), Some(outside)).unwrap(); + let _ = (std::fs::remove_file(&first), std::fs::remove_file(&second)); + let names = loaded_names(&app); + assert_eq!(names.len(), 2, "{names:?}"); + assert!(names[0].contains("drop-loads-a") && names[1].contains("drop-loads-b"), "{names:?}"); + let d = &app.session.doc().unwrap().doc; + assert!(matches!(d.item(designcraft_doc::ItemId(frame)).unwrap().content, designcraft_doc::Content::Unassigned)); + app.run("place.drop", json!({"x": 300, "y": 300})).unwrap(); + assert_eq!(app.session.doc().unwrap().doc.spreads[0].items.len(), 2, "the click placed the first"); + assert_eq!(app.session.tool_id(), "placeGun", "the second is still loaded"); +} + +/// Several images dropped together load the place cursor in the order they came, even over a +/// frame; a file that isn't an image keeps its own drop behaviour (text gets a frame). +#[test] +fn several_dropped_images_load_the_place_cursor_in_order() { + let (mut app, frame, inside, _) = app_with_drop_frame(); + app.run("selection.set", json!({"ids": []})).unwrap(); + let a = dropped_png("drop-many-a"); + let b = std::env::temp_dir().join(format!("designcraft-ui-{}-drop-many-b.svg", std::process::id())); + std::fs::write(&b, br##""##).unwrap(); + let c = dropped_png("drop-many-c"); + let text = std::env::temp_dir().join(format!("designcraft-ui-{}-drop-many.txt", std::process::id())); + std::fs::write(&text, "Dropped words").unwrap(); + let paths = [&a, &text, &b, &c].map(|p| p.to_string_lossy().to_string()); + app.open_dropped_batch(paths.iter().map(|p| (p.clone(), Some(inside))).collect()); + for p in [&a, &b, &c, &text] { + let _ = std::fs::remove_file(p); + } + let names = loaded_names(&app); + assert_eq!(names.len(), 3, "{names:?}"); + for (name, want) in names.iter().zip(["drop-many-a", "drop-many-b", "drop-many-c"]) { + assert!(name.contains(want), "{names:?}"); + } + assert_eq!(app.session.tool_id(), "placeGun"); + let d = &app.session.doc().unwrap().doc; + assert!(matches!(d.item(designcraft_doc::ItemId(frame)).unwrap().content, designcraft_doc::Content::Unassigned), "the frame isn't filled"); + assert_eq!(d.spreads[0].items.len(), 2, "the text got a frame of its own"); + // Clicks place them one by one; Escape unloads the rest. + app.run("place.drop", json!({"x": 300, "y": 300})).unwrap(); + assert_eq!(app.session.loaded_queue.len(), 1); + let mods = designcraft_tools::Mods::default(); + app.session.tool_key(designcraft_tools::ToolKey::Escape, mods, designcraft_engine::ViewInfo::at_zoom(1.0)).unwrap(); + assert!(app.session.loaded.is_none() && app.session.loaded_queue.is_empty()); + assert_eq!(app.session.tool_id(), "selection"); +} + +/// A drop is handled in the order its files came, each at its own pointer: an image dropped on a +/// frame before a document fills that frame in the document that was open, not in the one the +/// drop opens after it. +#[test] +fn a_mixed_drop_keeps_its_order() { + let (mut app, frame, inside, outside) = app_with_drop_frame(); + let mut other = DesignApp::new(Session::new(), Services::default()); + other.run("file.new", json!({})).unwrap(); + let document = std::env::temp_dir().join(format!("designcraft-ui-{}-drop-mixed.designcraft", std::process::id())); + other.run("file.saveAs", json!({"path": document})).unwrap(); + let image = dropped_png("drop-mixed"); + let text = std::env::temp_dir().join(format!("designcraft-ui-{}-drop-mixed.txt", std::process::id())); + std::fs::write(&text, "Dropped words").unwrap(); + let drops = vec![ + (image.to_string_lossy().to_string(), Some(inside)), + (text.to_string_lossy().to_string(), Some(outside)), + (document.to_string_lossy().to_string(), Some(outside)), + ]; + app.open_dropped_batch(drops); + for p in [&image, &text, &document] { + let _ = std::fs::remove_file(p); + } + assert_eq!(app.session.documents().len(), 2, "the document opened last"); + let d = &app.session.documents()[0].doc; + assert!( + matches!(d.item(designcraft_doc::ItemId(frame)).unwrap().content, designcraft_doc::Content::Graphic(_)), + "the image filled the frame in the document open before the drop" + ); + assert_eq!(d.spreads[0].items.len(), 2, "the text got a frame of its own"); + assert!(app.session.loaded.is_none(), "nothing waits in the place cursor"); +} + #[test] fn file_open_failures_show_an_alert() { let path = unreadable_file("open.designcraft"); diff --git a/docs/multi-image-placement.md b/docs/multi-image-placement.md index 997414bcd..6281de637 100644 --- a/docs/multi-image-placement.md +++ b/docs/multi-image-placement.md @@ -1,6 +1,6 @@ # Multiple images -Choose **File > Place** (Cmd+D on macOS) and select several images. They load into a placement cursor in the order supplied by the file picker. The cursor shows the next image's thumbnail and, beside it, how many are loaded ("(6)"). +Choose **File > Place** (Cmd+D on macOS) and select several images, or drop a selection of images from your file manager onto the document. Multiple images load into a placement cursor in the order supplied by the file picker (or the drop). The cursor shows the next image's thumbnail and, beside it, how many are loaded ("(6)"). - Click to place the next image at its natural size. - Drag to draw its frame: it keeps the image's proportions, and shows the scale beside the pointer. @@ -10,6 +10,14 @@ Choose **File > Place** (Cmd+D on macOS) and select several images. They load in A single selected file retains the normal Place behavior, including filling a selected frame and PDF import options. Image batches support up to 1000 images. Rows and columns each range from 1 to 100. A batch containing an unreadable image fails without replacing the existing queue or changing the document. An invalid grid area also leaves the document and queue unchanged. +## Dropping files + +- One image dropped on an empty or graphic frame fills it, or replaces the image in it. +- One image dropped anywhere else loads into the place cursor, as File > Place does with nothing selected: the next click or drag places it. +- Several images dropped together load into the place cursor in the order they came, wherever they are dropped. +- Images dropped while the cursor already holds some queue after them. +- DesignCraft, IDML and InDesign documents open; text files (.txt, .docx, .rtf, .md) and spreadsheets go into a new frame where they are dropped; a PDF is placed where it is dropped (or fills the frame under it); swatch exchange (.ase) files load their swatches. + Images here are PNG, JPEG, GIF, WebP, BMP, TIFF, SVG, PSD and EPS files. ## Automation From 0ea2a72a091e96833783dee24669bc7ea6b3a0f4 Mon Sep 17 00:00:00 2001 From: texjer Date: Wed, 4 Nov 2026 11:05:45 -0500 Subject: [PATCH 2/8] Place: switching, opening or closing a document leaves the place cursor's tool Review fix for #2309: set_active, add_document and close_document emptied the place cursor but kept the placeGun tool, so the next canvas click failed with 'nothing loaded in the place cursor'. One helper, Session::unload_place_cursor, empties the cursor and its queue and goes back to the Selection tool when the place cursor's tool is active; validate_place_cursor uses it too. Test: changing_documents_unloads_the_place_cursor_and_its_tool. --- ROADMAP.md | 2 +- crates/engine/src/cmd/file/place.rs | 4 +--- crates/engine/src/lib.rs | 9 +++------ crates/engine/src/tests.rs | 27 +++++++++++++++++++++++++++ crates/engine/src/tooling.rs | 10 ++++++++++ docs/multi-image-placement.md | 1 + 6 files changed, 43 insertions(+), 10 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 8aedbe1f0..0395fb926 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -406,7 +406,7 @@ Newest first. - **2026-10-30** · Parent pages: a running-header variable shows its name in angle brackets on the parent itself (it was blank), as InDesign shows it; on the document pages, and in text with no page, it shows its text as before. No readiness percentage change. - **2026-10-30** · Without facing pages, a page of a multi-page spread shows the parent page in its place, as in InDesign: the second page of a two-page spread shows the parent's second page. Every single-sided page showed the parent's first page, so an InDesign book set up without facing pages but laid out in two-page spreads repeated the left parent page's items (a black heading bar) on its right pages and lost the right parent page's. No readiness percentage change. - **2026-10-30** · Dropping images from the file manager works as in InDesign: images dropped together load the place cursor in the order they came, and one image dropped off a frame that takes it loads the place cursor instead of being placed at once (on a frame it still fills it). Documents and text files dropped keep opening and flowing as before. Ported from #1307 and #1666; see [multi-image placement](docs/multi-image-placement.md). No readiness percentage change. -- **2026-10-30** · Placing several images works as in InDesign. File › Place takes several files, and they load the place cursor in the order chosen: it shows the first one's thumbnail with how many are loaded ("(6)"), each click places the next, arrow keys while dragging lay out a grid (`place.grid`), and Escape unloads the rest (`place.cancel`). A drag with the cursor draws the frame at the graphic's proportions, with its scale. Ported from #1307, #1392 and #1432 (multi-image placement, cursor thumbnail and count); see [multi-image placement](docs/multi-image-placement.md). No readiness percentage change. +- **2026-10-30** · Placing several images works as in InDesign. File › Place takes several files, and they load the place cursor in the order chosen: it shows the first one's thumbnail with how many are loaded ("(6)"), each click places the next, arrow keys while dragging lay out a grid (`place.grid`), and Escape unloads the rest (`place.cancel`). Switching, opening or closing a document unloads it too, back to the Selection tool. A drag with the cursor draws the frame at the graphic's proportions, with its scale. Ported from #1307, #1392 and #1432 (multi-image placement, cursor thumbnail and count); see [multi-image placement](docs/multi-image-placement.md). No readiness percentage change. - **2026-10-30** · macOS: a file dragged in from the Finder fills the frame it is dropped on. winit sends no pointer events during such a drag, so the drop asks the system where the pointer is (`Services::pointer`, from `NSEvent.mouseLocation`), at any UI scale. No readiness percentage change. - **2026-10-30** · File > Package opens a dialog, as the reference app's: a Summary of fonts, links and images and inks, pages for each, a Printing Instructions step, and a PDF made with the options of a preset (#1514, #1515). - **2026-10-30** · IDML import reads TOC separators and entry styles written with the `$ID/` prefix or as `Properties` children, and the TOC separator notation (`^t`, `^y`, `^m`, `^>`) is shared by the dialog and IDML, with a special-character menu on the separator field (#1249, #1255). diff --git a/crates/engine/src/cmd/file/place.rs b/crates/engine/src/cmd/file/place.rs index ba9e0827a..dd7313776 100644 --- a/crates/engine/src/cmd/file/place.rs +++ b/crates/engine/src/cmd/file/place.rs @@ -313,9 +313,7 @@ fn validate_place_cursor(s: &mut Session, command: &str) -> Result<()> { } let valid = s.loaded.iter().chain(s.loaded_queue.iter()).all(|image| s.doc().is_ok_and(|d| d.doc.assets.contains_key(&image.asset))); if !valid { - s.loaded = None; - s.loaded_queue.clear(); - s.set_tool("selection"); + s.unload_place_cursor(); return Err(bad(command, "the loaded graphic is no longer in the document")); } Ok(()) diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 397be3203..33af11c42 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -552,8 +552,7 @@ impl Session { } self.reset_find_scope(); self.stop_data_preview(); - self.loaded = None; - self.loaded_queue.clear(); + self.unload_place_cursor(); self.active = Some(i); } } @@ -563,8 +562,7 @@ impl Session { } self.reset_find_scope(); self.stop_data_preview(); - self.loaded = None; - self.loaded_queue.clear(); + self.unload_place_cursor(); self.docs.push(d); self.active = Some(self.docs.len() - 1); self.docs.len() - 1 @@ -587,8 +585,7 @@ impl Session { recovery::discard_document(self, i, recovery_dir.as_deref()); } if i < self.docs.len() { - self.loaded = None; - self.loaded_queue.clear(); + self.unload_place_cursor(); self.reset_find_scope(); self.docs.remove(i); self.active = if self.docs.is_empty() { None } else { Some(i.min(self.docs.len() - 1)) }; diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 564e42605..a2751de72 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2127,6 +2127,33 @@ fn undoing_loaded_assets_cancels_every_placement_cursor() { } } +/// Switching, opening or closing a document empties the place cursor and leaves its tool: a click +/// on the canvas then selects, instead of failing with nothing loaded. Another tool stays. +#[test] +fn changing_documents_unloads_the_place_cursor_and_its_tool() { + let changes: [fn(&mut Session); 3] = [ + |s| s.set_active(0), + |s| { + s.execute("file.new", &json!({})).unwrap(); + }, + |s| s.close_document(1), + ]; + for (n, change) in changes.into_iter().enumerate() { + let mut s = session(); + s.execute("file.new", &json!({})).unwrap(); + load_test_images(&mut s, 3); + assert_eq!(s.tool_id(), "placeGun"); + change(&mut s); + assert!(s.loaded.is_none() && s.loaded_queue.is_empty(), "change {n}"); + assert_eq!(s.tool_id(), "selection", "change {n}"); + // Holding another tool, the change keeps it. + s.execute("file.new", &json!({})).unwrap(); + s.set_tool("rectangle"); + s.set_active(0); + assert_eq!(s.tool_id(), "rectangle", "change {n}"); + } +} + /// Issue #1637: with several objects selected, a click on one of them (no drag) makes it the key /// object, and Align to Key Object leaves it in place. A second click on it drops the key. #[test] diff --git a/crates/engine/src/tooling.rs b/crates/engine/src/tooling.rs index 6dfd23703..9bde0acfd 100644 --- a/crates/engine/src/tooling.rs +++ b/crates/engine/src/tooling.rs @@ -87,6 +87,16 @@ impl Session { } } + /// Empty the place cursor (its graphic and the ones queued after it). Holding the place + /// cursor's tool, go back to the Selection tool: with nothing loaded, a click would only fail. + pub(crate) fn unload_place_cursor(&mut self) { + self.loaded = None; + self.loaded_queue.clear(); + if self.tool_id() == "placeGun" { + self.set_tool("selection"); + } + } + pub fn layout(&self) -> CanvasLayout { match self.active() { Some(st) => CanvasLayout::new(&st.doc, st.editing_parents), diff --git a/docs/multi-image-placement.md b/docs/multi-image-placement.md index 6281de637..13119cf64 100644 --- a/docs/multi-image-placement.md +++ b/docs/multi-image-placement.md @@ -7,6 +7,7 @@ Choose **File > Place** (Cmd+D on macOS) and select several images, or drop a se - Click an existing graphic frame to replace its contents. - While drawing, Right/Left add/remove columns and Up/Down add/remove rows. Release to place a grid in row order with 6-point gutters. Extra images remain in the cursor; unused grid cells create no empty frames. - Press Escape or select another tool to cancel the remaining placement. +- Switching to, opening or closing a document also cancels it, and goes back to the Selection tool. A single selected file retains the normal Place behavior, including filling a selected frame and PDF import options. Image batches support up to 1000 images. Rows and columns each range from 1 to 100. A batch containing an unreadable image fails without replacing the existing queue or changing the document. An invalid grid area also leaves the document and queue unchanged. From f02e13318b0c870e09f4c791047e69db7681f2b1 Mon Sep 17 00:00:00 2001 From: texjer Date: Wed, 4 Nov 2026 11:02:26 -0500 Subject: [PATCH 3/8] Place: a drop without pointer events keeps the system pointer of its own input Review fixes for #2304: - Each pointer-less drop asks the system where the pointer is when its raw input arrives, and converts it with that input's window rect and the UI scale, instead of one sample taken at the later UI pass for the whole batch. Drops with pointer events keep their own pointer, and the system is still only asked when a drop brought none. - system_pointer is cfg'd off on wasm with its only caller; Services::pointer's docs say a drop without a pointer lands in the middle of the view. - pointer.rs: the AppKit flip is a pure function with a test, documenting why it flips against the main display only (winit's window positions use the same origin). - Test: a filled compound path takes a dropped image through its ring, not its hole. --- apps/designcraft/src/pointer.rs | 29 +++++++++++- crates/ui-egui/src/app.rs | 2 +- crates/ui-egui/src/app/input.rs | 46 ++++++++++--------- crates/ui-egui/src/lib.rs | 4 ++ crates/ui-egui/src/services.rs | 2 +- crates/ui-egui/src/tests.rs | 78 ++++++++++++++++++++++++++++++--- 6 files changed, 132 insertions(+), 29 deletions(-) diff --git a/apps/designcraft/src/pointer.rs b/apps/designcraft/src/pointer.rs index 14bf9599a..3725cfbaa 100644 --- a/apps/designcraft/src/pointer.rs +++ b/apps/designcraft/src/pointer.rs @@ -11,6 +11,33 @@ pub fn screen_position() -> Option { // AppKit's origin is the bottom left of the first screen, the one with the menu bar. let height = NSScreen::screens(mtm).firstObject()?.frame().size.height; let at = NSEvent::mouseLocation(); - let (x, y) = (at.x, height - at.y); + from_appkit(at.x, at.y, height) +} + +/// A point in AppKit's global coordinates (origin at the bottom left of the main display, the one +/// with the menu bar, y up) in winit's (origin at its top left, y down), given that display's +/// height. Only the main display's height: winit flips window positions the same way +/// (`flip_window_screen_coordinates`, against `CGMainDisplayID`), whatever display the window is +/// on, so a display left of or above the main one gives negative coordinates in both. +fn from_appkit(x: f64, y: f64, main_height: f64) -> Option { + let y = main_height - y; (x.is_finite() && y.is_finite()).then(|| egui::pos2(x as f32, y as f32)) } + +#[cfg(test)] +mod tests { + use super::from_appkit; + + #[test] + fn appkit_points_flip_against_the_main_display_only() { + // The main display is 1000 points high: its top left is (0, 0), its bottom left (0, 1000). + assert_eq!(from_appkit(0.0, 1000.0, 1000.0), Some(egui::pos2(0.0, 0.0))); + assert_eq!(from_appkit(200.0, 100.0, 1000.0), Some(egui::pos2(200.0, 900.0))); + // A second display above the main one, and one to its left: negative, as winit reports a + // window there. + assert_eq!(from_appkit(300.0, 1200.0, 1000.0), Some(egui::pos2(300.0, -200.0))); + assert_eq!(from_appkit(-800.0, 500.0, 1000.0), Some(egui::pos2(-800.0, 500.0))); + assert_eq!(from_appkit(f64::NAN, 0.0, 1000.0), None); + assert_eq!(from_appkit(0.0, 0.0, f64::INFINITY), None); + } +} diff --git a/crates/ui-egui/src/app.rs b/crates/ui-egui/src/app.rs index b2b433b71..55a257dd3 100644 --- a/crates/ui-egui/src/app.rs +++ b/crates/ui-egui/src/app.rs @@ -206,7 +206,7 @@ impl DesignApp { // must not replay shortcuts or dropped files from stale input. if ctx.current_pass_index() == 0 { #[cfg(not(target_arch = "wasm32"))] - self.take_pending_drops(&ctx); + self.take_pending_drops(); menus::shortcuts(self, &ctx); } let t0 = now_ms(); diff --git a/crates/ui-egui/src/app/input.rs b/crates/ui-egui/src/app/input.rs index 2682478c4..27049a1a7 100644 --- a/crates/ui-egui/src/app/input.rs +++ b/crates/ui-egui/src/app/input.rs @@ -59,10 +59,13 @@ impl DesignApp { _ => None, }); let generation = self.input_generation(); + // A drop without pointer events (dragged in from another application) asks the + // system now, while the pointer is still where the files were let go. + let system = if pointer.is_none() && !raw.dropped_files.is_empty() { self.system_pointer(ctx, raw) } else { None }; for file in std::mem::take(&mut raw.dropped_files) { let path = file.path().to_string_lossy().to_string(); if !path.is_empty() { - self.pending_drops.push(PendingDrop { path, pointer, generation }); + self.pending_drops.push(PendingDrop { path, pointer, system, generation }); } } } @@ -144,37 +147,29 @@ impl DesignApp { /// Open or place the files dropped since the last UI pass, at the pointer. Files dropped /// without pointer events (dragged in from another application) are placed where the system - /// says the pointer is, so one dropped on a frame still fills it. + /// said the pointer was when they arrived, so one dropped on a frame still fills it. #[cfg(not(target_arch = "wasm32"))] - pub(crate) fn take_pending_drops(&mut self, ctx: &egui::Context) { + pub(crate) fn take_pending_drops(&mut self) { // Sample the generation once for the whole batch: placing the first valid file must not // suppress later files that belong to the same captured batch. let current = self.input_generation(); - let drops: Vec<_> = std::mem::take(&mut self.pending_drops).into_iter().filter(|d| d.generation == current).collect(); - if drops.is_empty() { - return; - } - // Each file keeps the pointer it was dropped at; the system is asked once, for those - // that came without one. - let system = drops.iter().any(|d| d.pointer.is_none()).then(|| self.system_pointer(ctx)).flatten(); let mut frame_free = true; - for drop in drops { - let _ = self.place_dropped(&drop.path, drop.pointer.or(system), &mut frame_free); + for drop in std::mem::take(&mut self.pending_drops) { + if drop.generation == current { + let _ = self.place_dropped(&drop.path, drop.pointer.or(drop.system), &mut frame_free); + } } } /// Where the pointer is in the window, asked of the system ([`crate::Services::pointer`]), /// for files dropped without pointer events: dragged in from another application, they bring - /// none. `None` when the system doesn't say or the window's place isn't known. - pub(crate) fn system_pointer(&self, ctx: &egui::Context) -> Option { + /// none. Converted with the window's place in `raw` (that input's own viewport info, else the + /// last pass's). `None` when the system doesn't say or the window's place isn't known. + #[cfg(not(target_arch = "wasm32"))] + fn system_pointer(&self, ctx: &egui::Context, raw: &egui::RawInput) -> Option { let at = (self.services.pointer.as_ref()?)()?; - // The system's points are unscaled; egui's (the window's rect too) are divided by the - // UI scale. - let zoom = ctx.zoom_factor(); - let at = if zoom.is_finite() && zoom > 0.0 { at / zoom } else { at }; - let inner = ctx.input(|i| i.viewport().inner_rect)?; - let local = at - inner.min.to_vec2(); - (local.x.is_finite() && local.y.is_finite()).then_some(local) + let inner = raw.viewports.get(&raw.viewport_id).and_then(|v| v.inner_rect).or_else(|| ctx.input(|i| i.viewport().inner_rect))?; + screen_to_window(at, ctx.zoom_factor(), inner) } /// The current state an injected batch belongs to. @@ -224,3 +219,12 @@ impl DesignApp { self.control_input_frame.store(false, std::sync::atomic::Ordering::Relaxed); } } + +/// A point on the screen in the system's unscaled points, in the window whose content is `inner` +/// (egui's points, divided by the UI scale `zoom`). `None` when the result isn't finite. +#[cfg(not(target_arch = "wasm32"))] +fn screen_to_window(at: egui::Pos2, zoom: f32, inner: egui::Rect) -> Option { + let at = if zoom.is_finite() && zoom > 0.0 { at / zoom } else { at }; + let local = at - inner.min.to_vec2(); + (local.x.is_finite() && local.y.is_finite()).then_some(local) +} diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index c780d632a..531a8dbe7 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -111,6 +111,10 @@ struct InputGeneration { struct PendingDrop { path: String, pointer: Option, + /// Where the system said the pointer was in the window when this drop's raw input arrived, + /// for a drop that brought no pointer events ([`crate::Services::pointer`]). Each drop keeps + /// its own, converted with the window's place and the UI scale of that input. + system: Option, generation: InputGeneration, } diff --git a/crates/ui-egui/src/services.rs b/crates/ui-egui/src/services.rs index 107e9e775..32d7e4599 100644 --- a/crates/ui-egui/src/services.rs +++ b/crates/ui-egui/src/services.rs @@ -47,6 +47,6 @@ pub struct Services { /// The pointer's place on the screen, asked when a file is dropped: a file dragged in from /// another application brings no pointer events with it (winit reports none during a drag on /// macOS), so without it a drop can't tell which frame it landed on. `None`: a drop without - /// pointer events loads the place cursor. + /// pointer events lands in the middle of the view (`canvas::drop_point`), filling no frame. pub pointer: Option, } diff --git a/crates/ui-egui/src/tests.rs b/crates/ui-egui/src/tests.rs index e53e603a1..46c3ce3e2 100644 --- a/crates/ui-egui/src/tests.rs +++ b/crates/ui-egui/src/tests.rs @@ -899,6 +899,15 @@ fn a_graphic_dropped_on_a_frame_fills_it() { assert!(matches!(d.item(designcraft_doc::ItemId(frame)).unwrap().content, designcraft_doc::Content::Graphic(_))); } +/// The raw input of a drop without pointer events, with the window's content at `inner`. +#[cfg(not(target_arch = "wasm32"))] +fn pointerless_drop(path: &std::path::Path, inner: egui::Rect) -> egui::RawInput { + let mut raw = egui::RawInput { screen_rect: Some(egui::Rect::from_min_size(egui::Pos2::ZERO, inner.size())), ..Default::default() }; + raw.viewports.insert(egui::ViewportId::ROOT, egui::ViewportInfo { inner_rect: Some(inner), ..Default::default() }); + raw.dropped_files.push(dropped_file(path)); + raw +} + /// A file dragged in from the Finder brings no pointer events (winit sends none during a drag on /// macOS): the drop asks the system where the pointer is, so one dropped on a frame fills it. #[cfg(not(target_arch = "wasm32"))] @@ -911,13 +920,13 @@ fn a_file_dropped_without_pointer_events_fills_the_frame_under_the_system_pointe let window = egui::vec2(300.0, 200.0); app.services.pointer = Some(Box::new(move || Some(((inside + window).to_vec2() * zoom).to_pos2()))); let path = dropped_png("drop-system-pointer"); - app.pending_drops.push(crate::PendingDrop { path: path.to_string_lossy().into_owned(), pointer: None, generation: app.input_generation() }); let ctx = egui::Context::default(); ctx.set_zoom_factor(zoom); - let rect = egui::Rect::from_min_size(egui::Pos2::ZERO + window, egui::vec2(1400.0, 900.0)); - let mut raw = egui::RawInput { screen_rect: Some(egui::Rect::from_min_size(egui::Pos2::ZERO, rect.size())), ..Default::default() }; - raw.viewports.insert(egui::ViewportId::ROOT, egui::ViewportInfo { inner_rect: Some(rect), ..Default::default() }); - ctx.run_ui(raw, |ui| app.take_pending_drops(ui.ctx())).textures_delta.clear(); + // The zoom takes effect from the next pass, as it has by the time the system drops a file. + ctx.run_ui(egui::RawInput::default(), |_| {}).textures_delta.clear(); + let mut raw = pointerless_drop(&path, egui::Rect::from_min_size(egui::Pos2::ZERO + window, egui::vec2(1400.0, 900.0))); + app.raw_input_hook(&ctx, &mut raw); + ctx.run_ui(raw, |_| app.take_pending_drops()).textures_delta.clear(); let _ = std::fs::remove_file(&path); assert!(app.pending_drops.is_empty()); let d = &app.session.doc().unwrap().doc; @@ -929,6 +938,65 @@ fn a_file_dropped_without_pointer_events_fills_the_frame_under_the_system_pointe } } +/// A drop without pointer events keeps the place the system gave when its input arrived, +/// converted with that input's window: the pointer and the window moving before the UI pass that +/// places it don't move the drop. +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn a_pointerless_drop_keeps_the_system_pointer_of_its_own_input() { + let (mut app, frame, inside, outside) = app_with_drop_frame(); + let at = std::rc::Rc::new(std::cell::Cell::new(inside)); + let reported = at.clone(); + app.services.pointer = Some(Box::new(move || Some(reported.get()))); + let ctx = egui::Context::default(); + let window = egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(1400.0, 900.0)); + let path = dropped_png("drop-own-input"); + let mut raw = pointerless_drop(&path, window); + app.raw_input_hook(&ctx, &mut raw); + // Before the pass that places it, the window moves and the pointer leaves the frame; a later + // input with no drop doesn't ask the system again. + let moved = window.translate(egui::vec2(50.0, 40.0)); + at.set(outside + moved.min.to_vec2()); + let mut later = egui::RawInput { screen_rect: Some(egui::Rect::from_min_size(egui::Pos2::ZERO, moved.size())), ..Default::default() }; + later.viewports.insert(egui::ViewportId::ROOT, egui::ViewportInfo { inner_rect: Some(moved), ..Default::default() }); + app.raw_input_hook(&ctx, &mut later); + ctx.run_ui(later, |_| app.take_pending_drops()).textures_delta.clear(); + let _ = std::fs::remove_file(&path); + let d = &app.session.doc().unwrap().doc; + assert_eq!(d.spreads[0].items.len(), 1, "no new object"); + let designcraft_doc::Content::Graphic(g) = &d.item(designcraft_doc::ItemId(frame)).unwrap().content else { + panic!("the file fills the frame it was dropped on") + }; + assert!(d.assets[&g.asset].name.contains("drop-own-input")); +} + +/// A filled compound path (Object > Compound Paths > Make) is a frame a dropped image fills, as +/// in InDesign, through its ring and not its hole. +#[test] +fn a_graphic_dropped_on_a_compound_path_fills_it() { + let (mut app, frame, _, _) = app_with_drop_frame(); + let hole = app.run("frame.create", json!({"rect": [66, 66, 106, 106]})).unwrap()["id"].as_u64().unwrap(); + assert_eq!(app.run("object.makeCompoundPath", json!({"ids": [frame, hole]})).unwrap()["id"].as_u64(), Some(frame)); + app.run("object.fill", json!({"ids": [frame], "swatch": "[Black]"})).unwrap(); + let (view, rect) = (app.view().unwrap(), app.canvas_rect.unwrap()); + let st = app.session.active().unwrap(); + let layout = designcraft_tools::CanvasLayout::new(&st.doc, false); + let slot = layout.slot(designcraft_doc::SpreadRef::Doc(0)).unwrap(); + let screen = |x, y| canvas::Xf::new(rect, view).to_screen(slot.to_canvas(designcraft_geom::Point::new(x, y))); + // On the ring, between the outer edge (36) and the hole (66); the hole's middle is no frame. + let ring = screen(50.0, 50.0); + assert!(canvas::drop_frame(&app, Some(screen(86.0, 86.0))).is_none(), "the hole"); + let path = dropped_png("drop-compound"); + let r = app.open_dropped_at(&path.to_string_lossy(), Some(ring)); + let _ = std::fs::remove_file(&path); + assert_eq!(r.unwrap()["id"].as_u64(), Some(frame), "the compound path under the pointer is filled"); + let d = &app.session.doc().unwrap().doc; + assert_eq!(d.spreads[0].items.len(), 1, "no new object"); + let it = d.item(designcraft_doc::ItemId(frame)).unwrap(); + assert!(matches!(it.content, designcraft_doc::Content::Graphic(_))); + assert_eq!(it.path.subpaths.len(), 2, "it keeps its hole"); +} + #[test] fn a_locked_frame_is_not_a_drop_target() { let (mut app, frame, inside, _) = app_with_drop_frame(); From 267917b8de223df327e336e79c4ca646eda4de1e Mon Sep 17 00:00:00 2001 From: texjer Date: Wed, 4 Nov 2026 19:56:22 -0500 Subject: [PATCH 4/8] Place: drops that arrive before one UI pass stay separate All pending drops went into one batch, so two single images counted as several: both queued and neither filled the frame it was dropped on. Files of one raw input now share a batch, and each batch is opened on its own. --- crates/ui-egui/src/app/input.rs | 13 +++++++------ crates/ui-egui/src/lib.rs | 3 +++ crates/ui-egui/src/tests.rs | 31 +++++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 6 deletions(-) diff --git a/crates/ui-egui/src/app/input.rs b/crates/ui-egui/src/app/input.rs index 465eeaaff..fe5fb673b 100644 --- a/crates/ui-egui/src/app/input.rs +++ b/crates/ui-egui/src/app/input.rs @@ -62,10 +62,11 @@ impl DesignApp { // A drop without pointer events (dragged in from another application) asks the // system now, while the pointer is still where the files were let go. let system = if pointer.is_none() && !raw.dropped_files.is_empty() { self.system_pointer(ctx, raw) } else { None }; + let batch = self.pending_drops.last().map_or(0, |d| d.batch.wrapping_add(1)); for file in std::mem::take(&mut raw.dropped_files) { let path = file.path().to_string_lossy().to_string(); if !path.is_empty() { - self.pending_drops.push(PendingDrop { path, pointer, system, generation }); + self.pending_drops.push(PendingDrop { path, pointer, system, batch, generation }); } } } @@ -154,12 +155,12 @@ impl DesignApp { // suppress later files that belong to the same captured batch. let current = self.input_generation(); let drops: Vec<_> = std::mem::take(&mut self.pending_drops).into_iter().filter(|d| d.generation == current).collect(); - if drops.is_empty() { - return; - } // Dropped together, they go together: several images load the place cursor at once. Each - // file keeps the pointer it was dropped at, or the system's from when it arrived. - self.open_dropped_batch(drops.into_iter().map(|d| (d.path, d.pointer.or(d.system))).collect()); + // file keeps the pointer it was dropped at, or the system's from when it arrived. Separate + // drops stay separate, so a single image still fills the frame it was dropped on. + for batch in drops.chunk_by(|a, b| a.batch == b.batch) { + self.open_dropped_batch(batch.iter().map(|d| (d.path.clone(), d.pointer.or(d.system))).collect()); + } } /// Where the pointer is in the window, asked of the system ([`crate::Services::pointer`]), diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 531a8dbe7..239127c3b 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -115,6 +115,9 @@ struct PendingDrop { /// for a drop that brought no pointer events ([`crate::Services::pointer`]). Each drop keeps /// its own, converted with the window's place and the UI scale of that input. system: Option, + /// Which drop this file came with: files of one raw input share it, so drops that arrive + /// before the same UI pass stay separate. + batch: u64, generation: InputGeneration, } diff --git a/crates/ui-egui/src/tests.rs b/crates/ui-egui/src/tests.rs index 5692ba7fe..39534b0eb 100644 --- a/crates/ui-egui/src/tests.rs +++ b/crates/ui-egui/src/tests.rs @@ -1011,6 +1011,37 @@ fn a_pointerless_drop_keeps_the_system_pointer_of_its_own_input() { assert!(d.assets[&g.asset].name.contains("drop-own-input")); } +/// Two drops that arrive before one UI pass stay separate: the first still fills the frame it was +/// dropped on, and the second, off any frame, loads the place cursor. Merged, they'd both queue. +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn separate_drops_before_one_pass_are_not_one_batch() { + let (mut app, frame, inside, outside) = app_with_drop_frame(); + let at = std::rc::Rc::new(std::cell::Cell::new(inside)); + let reported = at.clone(); + app.services.pointer = Some(Box::new(move || Some(reported.get()))); + let ctx = egui::Context::default(); + let window = egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(1400.0, 900.0)); + let (first, second) = (dropped_png("separate-drop-a"), dropped_png("separate-drop-b")); + let mut raw = pointerless_drop(&first, window); + app.raw_input_hook(&ctx, &mut raw); + at.set(outside); + let mut raw = pointerless_drop(&second, window); + app.raw_input_hook(&ctx, &mut raw); + assert_eq!(app.pending_drops.len(), 2); + ctx.run_ui(raw, |_| app.take_pending_drops()).textures_delta.clear(); + let _ = std::fs::remove_file(&first); + let _ = std::fs::remove_file(&second); + let d = &app.session.doc().unwrap().doc; + let designcraft_doc::Content::Graphic(g) = &d.item(designcraft_doc::ItemId(frame)).unwrap().content else { + panic!("the first drop fills the frame it was dropped on") + }; + assert!(d.assets[&g.asset].name.contains("separate-drop-a")); + let loaded = app.session.loaded.as_ref().expect("the second drop loads the place cursor"); + assert!(d.assets[&loaded.asset].name.contains("separate-drop-b")); + assert!(app.session.loaded_queue.is_empty()); +} + /// A filled compound path (Object > Compound Paths > Make) is a frame a dropped image fills, as /// in InDesign, through its ring and not its hole. #[test] From e2b21cb3db126feb09e4bc4f6253c7c895ea01eb Mon Sep 17 00:00:00 2001 From: texjer Date: Thu, 5 Nov 2026 14:13:55 -0500 Subject: [PATCH 5/8] Place: a PDF later in a batch joins the place cursor, whatever is selected With a graphic frame selected, `file.place` chose that frame before it looked at the place cursor, so a PDF confirmed with `append` filled the frame and left the images around it queued. An appended file now skips the selected-frame fallback; an explicit `frame` still applies, and a single placed file still fills the selected frame. --- crates/engine/src/cmd/file/place.rs | 9 ++++++--- crates/ui-egui/src/app/files.rs | 21 +++++++++++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/cmd/file/place.rs b/crates/engine/src/cmd/file/place.rs index 081e3bd27..5ecd31268 100644 --- a/crates/engine/src/cmd/file/place.rs +++ b/crates/engine/src/cmd/file/place.rs @@ -100,8 +100,11 @@ pub(super) fn file_place(s: &mut Session, p: &Value) -> Result { (nw, nh) } }; + // A file joining the place cursor's queue goes after the ones already there, whatever is selected. + let load_cursor_param = p.get("loadCursor").and_then(Value::as_bool) == Some(true); + let append = p.get("append").and_then(Value::as_bool) == Some(true); let target_frame = crate::cmd::id_param(p, "frame").or_else(|| { - if ["x", "y", "spread", "width"].iter().any(|key| p.get(*key).is_some_and(|value| !value.is_null())) { + if (load_cursor_param && append) || ["x", "y", "spread", "width"].iter().any(|key| p.get(*key).is_some_and(|value| !value.is_null())) { return None; } let st = s.active()?; @@ -115,8 +118,8 @@ pub(super) fn file_place(s: &mut Session, p: &Value) -> Result { let lid = s.doc()?.active_layer; let px = p.get("x").and_then(Value::as_f64); let py = p.get("y").and_then(Value::as_f64); - if target_frame.is_none() && px.is_none() && py.is_none() && p.get("loadCursor").and_then(Value::as_bool) == Some(true) { - return load_cursor(s, asset, (bw, bh), p.get("append").and_then(Value::as_bool) == Some(true)); + if target_frame.is_none() && px.is_none() && py.is_none() && load_cursor_param { + return load_cursor(s, asset, (bw, bh), append); } s.edit(|d, sel| { let fresh = AssetId(d.alloc()); diff --git a/crates/ui-egui/src/app/files.rs b/crates/ui-egui/src/app/files.rs index d98fba286..ba5b23105 100644 --- a/crates/ui-egui/src/app/files.rs +++ b/crates/ui-egui/src/app/files.rs @@ -1063,6 +1063,27 @@ pub(crate) mod browser_import_tests { assert_eq!(loaded_names(&app), ["a.svg", "one.pdf", "b.svg", "c.svg", "a.svg"]); } + /// A PDF later in a batch joins the place cursor with the images around it, even with a + /// graphic frame selected: it doesn't fill that frame and leave the others queued. + #[test] + fn a_pdf_in_a_batch_queues_rather_than_filling_the_selected_frame() { + let dir = tempfile::tempdir().unwrap(); + let path = |n: &str| dir.path().join(n).to_string_lossy().to_string(); + std::fs::write(path("a.svg"), image()).unwrap(); + std::fs::write(path("one.pdf"), pdf_pages(1)).unwrap(); + std::fs::write(path("b.svg"), image()).unwrap(); + let mut app = app(); + app.services.read = Some(Box::new(|p| std::fs::read(p).map_err(|e| e.to_string()))); + let f = designcraft_doc::ItemId( + app.run("frame.create", json!({"rect": [200, 200, 300, 300], "content": "graphic"})).unwrap()["id"].as_u64().unwrap(), + ); + assert!(app.session.active().unwrap().selection.items.contains(&f)); + app.place_paths(vec![path("a.svg"), path("one.pdf"), path("b.svg")]).unwrap(); + assert_eq!(loaded_names(&app), ["a.svg", "one.pdf", "b.svg"]); + let frame = app.session.active().unwrap().doc.item(f).unwrap(); + assert!(!matches!(frame.content, designcraft_doc::Content::Graphic(_)), "the selected frame stays empty"); + } + /// File › Place with several files chosen in the dialog loads them all into the place /// cursor, in the order chosen; with one, it is placed as before. #[test] From ba2237f8d5e13a910135bca16416ec8e4ce9bd59 Mon Sep 17 00:00:00 2001 From: texjer Date: Thu, 5 Nov 2026 14:54:26 -0500 Subject: [PATCH 6/8] Place: the 1000-image cap holds for the whole place cursor when appending Appending only checked each batch against the cap, and a single image or a PDF added with append wasn't checked at all, so repeated appends could grow the cursor without limit. Refused before decoding a batch or adding an asset. --- crates/engine/src/cmd/file/place.rs | 90 ++++++++++++++++++++++++++++- docs/multi-image-placement.md | 2 +- 2 files changed, 90 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/file/place.rs b/crates/engine/src/cmd/file/place.rs index 5ecd31268..5ac886618 100644 --- a/crates/engine/src/cmd/file/place.rs +++ b/crates/engine/src/cmd/file/place.rs @@ -253,6 +253,7 @@ pub(super) fn place_load(s: &mut Session, p: &Value) -> Result { if sources.is_empty() || sources.len() > MAX_LOADED_IMAGES { return Err(bad("place.load", format!("select between 1 and {MAX_LOADED_IMAGES} images"))); } + check_cursor_room(s, sources.len(), append, "place.load")?; let mut images = Vec::with_capacity(sources.len()); for source in sources { let (bytes, name, link) = read_source(source)?; @@ -280,9 +281,18 @@ pub(super) fn place_load(s: &mut Session, p: &Value) -> Result { Ok(json!({"remaining": loaded_count(s), "loaded": true})) } -/// Most images one `place.load` takes. +/// Most images the place cursor holds, and so most one `place.load` takes. const MAX_LOADED_IMAGES: usize = 1000; +/// Whether `incoming` images fit in the place cursor: appended, they go after the ones it holds. +fn check_cursor_room(s: &Session, incoming: usize, append: bool, command: &str) -> Result<()> { + let held = if append { loaded_count(s) } else { 0 }; + if held.saturating_add(incoming) > MAX_LOADED_IMAGES { + return Err(bad(command, format!("the place cursor holds at most {MAX_LOADED_IMAGES} images, and it has {held}"))); + } + Ok(()) +} + /// How many images the place cursor holds: the current one and those queued after it. fn loaded_count(s: &Session) -> usize { s.loaded_queue.len().saturating_add(usize::from(s.loaded.is_some())) @@ -291,6 +301,7 @@ fn loaded_count(s: &Session) -> usize { /// Add a decoded asset with a fresh id, replacing the previous placement queue (or, with /// `append` and an image already loaded, queued after it). fn load_cursor(s: &mut Session, mut asset: Asset, size: (f64, f64), append: bool) -> Result { + check_cursor_room(s, 1, append, "place.load")?; let name = asset.name.clone(); let (pw, ph) = asset.pixels.unwrap_or_default(); let aid = s.edit(|d, _| { @@ -1067,3 +1078,80 @@ mod nested_frame_tests { assert_eq!(d.spreads[0].items.len(), 1, "no new object"); } } + +#[cfg(test)] +mod cursor_cap_tests { + use serde_json::{Value, json}; + + use super::MAX_LOADED_IMAGES; + use crate::Session; + + fn png() -> String { + let png = designcraft_render::Rendered { width: 4, height: 4, pixels: [200, 90, 30, 255].repeat(16) }.to_png(); + super::super::base64_encode(&png) + } + + /// A session whose place cursor holds `held` images, loaded without decoding each of them. + fn holding(held: usize) -> Session { + let mut s = Session::new(); + s.execute("file.new", &json!({})).unwrap(); + s.execute("place.load", &json!({"base64": png(), "name": "first.png"})).unwrap(); + let first = s.loaded.unwrap(); + s.loaded_queue = std::iter::repeat_n(first, held - 1).collect(); + s + } + + fn count(s: &Session) -> usize { + super::loaded_count(s) + } + + /// Every way of adding to the cursor stops at the cap, and a refused add changes nothing. + #[test] + fn appending_stops_at_the_cap_on_the_whole_cursor() { + let image = png(); + let single = json!({"base64": image, "name": "a.png", "append": true}); + let placed = json!({"base64": image, "name": "a.png", "append": true, "loadCursor": true}); + let two = json!({"files": [{"base64": image, "name": "a.png"}, {"base64": image, "name": "b.png"}], "append": true}); + for (command, params) in [("place.load", &single), ("file.place", &placed), ("place.load", &two)] { + let mut s = holding(MAX_LOADED_IMAGES); + let assets = s.doc().unwrap().doc.assets.len(); + assert!(s.execute(command, params).is_err(), "{command} {params}"); + assert_eq!(count(&s), MAX_LOADED_IMAGES, "{command} {params}"); + assert_eq!(s.doc().unwrap().doc.assets.len(), assets, "{command} {params}"); + } + // One over, as a batch. + let mut s = holding(MAX_LOADED_IMAGES - 1); + assert!(s.execute("place.load", &two).is_err()); + assert_eq!(count(&s), MAX_LOADED_IMAGES - 1); + } + + /// The last places up to the cap load, and an unreadable batch reports the cap before decoding. + #[test] + fn appending_up_to_the_cap_still_loads() { + let image = png(); + for params in + [json!({"base64": image, "name": "a.png", "append": true}), json!({"files": [{"base64": image, "name": "a.png"}], "append": true})] + { + let mut s = holding(MAX_LOADED_IMAGES - 1); + s.execute("place.load", ¶ms).unwrap(); + assert_eq!(count(&s), MAX_LOADED_IMAGES, "{params}"); + } + let mut s = holding(MAX_LOADED_IMAGES); + let error = + s.execute("place.load", &json!({"files": [{"base64": "not an image", "name": "a.png"}], "append": true})).unwrap_err().to_string(); + assert!(error.contains(&MAX_LOADED_IMAGES.to_string()), "{error}"); + } + + /// Without `append` the new images replace the cursor, so what it holds doesn't count. + #[test] + fn replacing_a_full_cursor_is_not_capped() { + let image = png(); + let mut s = holding(MAX_LOADED_IMAGES); + let result: Value = s.execute("place.load", &json!({"base64": image, "name": "a.png"})).unwrap(); + assert_eq!(result["loaded"], true); + assert_eq!(count(&s), 1); + let mut s = holding(MAX_LOADED_IMAGES); + s.execute("place.load", &json!({"files": [{"base64": image, "name": "a.png"}]})).unwrap(); + assert_eq!(count(&s), 1); + } +} diff --git a/docs/multi-image-placement.md b/docs/multi-image-placement.md index 50061c8c0..478ad6755 100644 --- a/docs/multi-image-placement.md +++ b/docs/multi-image-placement.md @@ -9,7 +9,7 @@ Choose **File > Place** (Cmd+D on macOS) and select several images, or drop a se - Press Escape or select another tool to cancel the remaining placement. - Switching to, opening or closing a document also cancels it, and goes back to the Selection tool. -A single selected file retains the normal Place behavior, including filling a selected frame and PDF import options. A multi-page PDF in a batch stops the batch at its import options; once confirmed, it queues after the images loaded before it. Image batches support up to 1000 images. Rows and columns each range from 1 to 100. A batch containing an unreadable image fails without replacing the existing queue or changing the document. An invalid grid area also leaves the document and queue unchanged. +A single selected file retains the normal Place behavior, including filling a selected frame and PDF import options. A multi-page PDF in a batch stops the batch at its import options; once confirmed, it queues after the images loaded before it. The place cursor holds up to 1000 images, counting those already in it when a batch adds to them. Rows and columns each range from 1 to 100. A batch containing an unreadable image fails without replacing the existing queue or changing the document. An invalid grid area also leaves the document and queue unchanged. ## Dropping files From 3111d4086a0b216e01545529508b4e900476d2b7 Mon Sep 17 00:00:00 2001 From: texjer Date: Thu, 5 Nov 2026 15:00:33 -0500 Subject: [PATCH 7/8] Place: the files after a multi-page PDF in a batch wait for its options They were left behind in place_paths when the PDF's Import Options opened, so the PDF was placed alone and the rest of what was chosen never loaded. They now wait in the dialog and carry on when it is confirmed; Cancel ends the batch. --- crates/ui-egui/src/app/files.rs | 82 +++++++++++++++++++++++++++------ crates/ui-egui/src/dialogs.rs | 16 ++++++- docs/multi-image-placement.md | 2 +- 3 files changed, 84 insertions(+), 16 deletions(-) diff --git a/crates/ui-egui/src/app/files.rs b/crates/ui-egui/src/app/files.rs index ba5b23105..4cd29d214 100644 --- a/crates/ui-egui/src/app/files.rs +++ b/crates/ui-egui/src/app/files.rs @@ -52,18 +52,21 @@ impl DesignApp { self.place_file(if append { json!({"path": path, "append": true}) } else { json!({"path": path}) }) } - /// Place an ordered batch of files, as File › Place with several chosen. The batch stops when - /// a file opens a dialog (a multi-page PDF's Import Options, an alert), as the inbox's does; - /// every file that loads the cursor adds to what the earlier ones loaded, in the order the - /// files were chosen (images in a row load together, before the file after them). A file that - /// can't be placed gets an alert and stops the batch; its error is returned, so automation - /// doesn't carry on as if the batch had loaded. + /// Place an ordered batch of files, as File › Place with several chosen. A multi-page PDF's + /// Import Options pause the batch: the files after it wait in the dialog and carry on when it + /// is confirmed, and Cancel ends the batch. Any other dialog (an alert, one already open) stops + /// it, as the inbox's does. Every file that loads the cursor adds to what the earlier ones + /// loaded, in the order the files were chosen (images in a row load together, before the file + /// after them). A file that can't be placed gets an alert and stops the batch; its error is + /// returned, so automation doesn't carry on as if the batch had loaded. pub(crate) fn place_paths(&mut self, paths: Vec) -> Result<(), String> { let mut images = Vec::new(); // Images already in the place cursor stay: the batch queues after them. let mut loaded = self.session.loaded.is_some(); - // `None` at the end loads the images left. - for path in paths.into_iter().map(Some).chain([None]) { + let mut paths = paths.into_iter(); + loop { + // `None` at the end loads the images left. + let path = paths.next(); if let Some(p) = &path && is_cursor_image(p) { @@ -71,7 +74,7 @@ impl DesignApp { continue; } if path.is_none() && images.is_empty() { - break; + return Ok(()); } if self.ui.dialog.is_some() { self.status(crate::i18n::tr(&self.ui.language, "Place cancelled while a dialog is open")); @@ -81,12 +84,14 @@ impl DesignApp { let r = self.load_place_cursor(std::mem::take(&mut images), loaded)?; loaded |= r.get("loaded").and_then(Value::as_bool) == Some(true); } - if let Some(p) = path { - let r = self.place_path(&p, loaded)?; - loaded |= r.get("loaded").and_then(Value::as_bool) == Some(true); + let Some(p) = path else { return Ok(()) }; + let r = self.place_path(&p, loaded)?; + loaded |= r.get("loaded").and_then(Value::as_bool) == Some(true); + if let Some(d) = self.ui.dialog.as_mut().filter(|d| d.id == "pdfImport") { + d.fields.insert(dialogs::PDF_IMPORT_REST.into(), json!(paths.collect::>())); + return Ok(()); } } - Ok(()) } /// `place.load` of `files` (`{path}` or `{name, base64}` each), queued after the images @@ -996,6 +1001,57 @@ pub(crate) mod browser_import_tests { assert_eq!(loaded_names(&app), ["a.png", "b.png", "three.pdf"]); } + /// The files after a multi-page PDF in a native batch wait for its Import Options, through a + /// second such PDF too, and keep the order chosen. + #[test] + fn native_batch_continues_after_a_multi_page_pdf_is_confirmed() { + let dir = tempfile::tempdir().unwrap(); + let path = |n: &str| dir.path().join(n).to_string_lossy().to_string(); + for name in ["a.svg", "b.svg", "c.svg", "d.svg"] { + std::fs::write(path(name), image()).unwrap(); + } + std::fs::write(path("three.pdf"), pdf_pages(3)).unwrap(); + std::fs::write(path("two.pdf"), pdf_pages(2)).unwrap(); + let mut app = app(); + app.services.read = Some(Box::new(|p| std::fs::read(p).map_err(|e| e.to_string()))); + let batch = ["a.svg", "three.pdf", "b.svg", "c.svg", "two.pdf", "d.svg"].map(path).to_vec(); + app.place_paths(batch).unwrap(); + assert_eq!(loaded_names(&app), ["a.svg"], "waiting for the first PDF's options"); + dialogs::confirm(&mut app).unwrap(); + assert_eq!(app.ui.dialog.as_ref().map(|d| d.id.as_str()), Some("pdfImport"), "the second PDF asks in its turn"); + assert_eq!(loaded_names(&app), ["a.svg", "three.pdf", "b.svg", "c.svg"]); + dialogs::confirm(&mut app).unwrap(); + assert!(app.ui.dialog.is_none()); + assert_eq!(loaded_names(&app), ["a.svg", "three.pdf", "b.svg", "c.svg", "two.pdf", "d.svg"]); + } + + /// A refused page number keeps the rest of the batch waiting; Cancel ends the batch, and what + /// loaded before the PDF stays. + #[test] + fn native_batch_waits_through_a_refused_page_and_ends_at_cancel() { + let dir = tempfile::tempdir().unwrap(); + let path = |n: &str| dir.path().join(n).to_string_lossy().to_string(); + std::fs::write(path("a.svg"), image()).unwrap(); + std::fs::write(path("b.svg"), image()).unwrap(); + std::fs::write(path("three.pdf"), pdf_pages(3)).unwrap(); + let mut app = app(); + app.services.read = Some(Box::new(|p| std::fs::read(p).map_err(|e| e.to_string()))); + let batch = ["a.svg", "three.pdf", "b.svg"].map(path).to_vec(); + app.place_paths(batch.clone()).unwrap(); + app.ui.dialog.as_mut().unwrap().fields.insert("page".into(), json!("4-5")); + assert!(dialogs::confirm(&mut app).is_err()); + app.ui.dialog.as_mut().unwrap().fields.insert("page".into(), json!("2")); + dialogs::confirm(&mut app).unwrap(); + assert_eq!(loaded_names(&app), ["a.svg", "three.pdf", "b.svg"]); + + let mut app = self::app(); + app.services.read = Some(Box::new(|p| std::fs::read(p).map_err(|e| e.to_string()))); + app.place_paths(batch).unwrap(); + dialogs::cancel(&mut app); + assert!(app.ui.dialog.is_none()); + assert_eq!(loaded_names(&app), ["a.svg"]); + } + /// A single multi-page PDF still replaces what the cursor holds, as any single Place does. #[test] fn a_single_multi_page_pdf_replaces_the_cursor_when_confirmed() { diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 4633620f3..b104a0ae1 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -1562,6 +1562,9 @@ fn text_frame_params(app: &DesignApp, d: &Dialog) -> Result, Ok(p) } +/// In a native batch's `pdfImport` dialog: the paths chosen after its PDF, placed once it is confirmed. +pub(crate) const PDF_IMPORT_REST: &str = "rest"; + /// File › Document Setup, filled from the document. /// The active document's ruler unit (picas without a document). fn doc_unit(app: &DesignApp) -> Unit { @@ -4711,9 +4714,18 @@ fn confirm_part3(app: &mut DesignApp, d: Dialog) -> Result { if let Some(target) = d.fields.get("target").and_then(Value::as_u64) { app.activate_import_target(&crate::ImportRequest { purpose: "place".into(), target: Some(target) })?; } - app.place_file( + let placed = app.place_file( json!({"path": d.s("path"), "pdfPage": page, "pdfCrop": if crop.is_empty() { "crop".to_string() } else { crop }, "append": d.b("append")}), - ) + )?; + // The files chosen after this PDF, which waited for its options. + let rest: Vec = d + .fields + .get(PDF_IMPORT_REST) + .and_then(Value::as_array) + .map(|paths| paths.iter().filter_map(Value::as_str).map(str::to_string).collect()) + .unwrap_or_default(); + app.place_paths(rest)?; + Ok(placed) } "print" => { let pages = if d.s("range") == "range" { json!(d.s("pages")) } else { Value::Null }; diff --git a/docs/multi-image-placement.md b/docs/multi-image-placement.md index 478ad6755..e64a26fbc 100644 --- a/docs/multi-image-placement.md +++ b/docs/multi-image-placement.md @@ -9,7 +9,7 @@ Choose **File > Place** (Cmd+D on macOS) and select several images, or drop a se - Press Escape or select another tool to cancel the remaining placement. - Switching to, opening or closing a document also cancels it, and goes back to the Selection tool. -A single selected file retains the normal Place behavior, including filling a selected frame and PDF import options. A multi-page PDF in a batch stops the batch at its import options; once confirmed, it queues after the images loaded before it. The place cursor holds up to 1000 images, counting those already in it when a batch adds to them. Rows and columns each range from 1 to 100. A batch containing an unreadable image fails without replacing the existing queue or changing the document. An invalid grid area also leaves the document and queue unchanged. +A single selected file retains the normal Place behavior, including filling a selected frame and PDF import options. A multi-page PDF in a batch pauses the batch at its import options; once confirmed, it queues after the images loaded before it and the rest of the batch follows. Cancelling them ends the batch, and the images already loaded stay. The place cursor holds up to 1000 images, counting those already in it when a batch adds to them. Rows and columns each range from 1 to 100. A batch containing an unreadable image fails without replacing the existing queue or changing the document. An invalid grid area also leaves the document and queue unchanged. ## Dropping files From a66355b6ca2f44fa526b01b2b547a2636e6d1e49 Mon Sep 17 00:00:00 2001 From: texjer Date: Thu, 5 Nov 2026 15:45:45 -0500 Subject: [PATCH 8/8] Place: say what a dropped batch does at a multi-page PDF MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit File › Place now carries on after the PDF's options; a drop still cancels the rest. --- crates/ui-egui/src/app/files.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/app/files.rs b/crates/ui-egui/src/app/files.rs index 4cd29d214..730b6e222 100644 --- a/crates/ui-egui/src/app/files.rs +++ b/crates/ui-egui/src/app/files.rs @@ -295,7 +295,8 @@ impl DesignApp { /// cursor, each run of them where it came in the order, after any images it already holds, /// whatever is under the pointer: click to place them one by one, drag with the arrow keys for /// a grid. The rest of the drop is cancelled once a file opens a dialog (a multi-page PDF's - /// Import Options, an alert), as File › Place with several chosen is. + /// Import Options, an alert): unlike File › Place with several chosen, nothing waits for a + /// PDF's options to be confirmed. pub fn open_dropped_batch(&mut self, drops: Vec<(String, Option)>) { let several = drops.iter().filter(|(p, _)| is_cursor_image(p)).count() > 1; // Only the first graphic of a drop fills the frame under the pointer.