From e4f82e243eaa96591f3d88cb00b341a1b42ef0b3 Mon Sep 17 00:00:00 2001 From: texjer Date: Wed, 4 Nov 2026 11:05:45 -0500 Subject: [PATCH] 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 df9b5fea0..b080dfc9c 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -408,7 +408,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** · 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 ddad5ba96..081e3bd27 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 da8f180bf..ca0d38415 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. They load in - 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. A drag too small for the grid's cells shows and places one frame instead, as an ordinary drag does. - 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.