From 84348b24c0773646f95cbdb097f915014deb857b Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Sun, 1 Nov 2026 07:40:55 +0100 Subject: [PATCH] Skip locked frames and fill a frame with only the first file of a drop (#1646) --- Cargo.lock | 1 + crates/engine/src/cmd/file/place.rs | 21 ++++++++ crates/ui-egui/Cargo.toml | 1 + crates/ui-egui/src/app.rs | 3 +- crates/ui-egui/src/app/files.rs | 27 ++++++++-- crates/ui-egui/src/canvas.rs | 2 +- crates/ui-egui/src/canvas/input.rs | 12 +++++ crates/ui-egui/src/tests.rs | 76 +++++++++++++++++++++++++++++ 8 files changed, 137 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c89a20002..95e21ace9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1624,6 +1624,7 @@ dependencies = [ "designcraft-geom", "designcraft-idml", "designcraft-render", + "designcraft-textimport", "designcraft-tools", "egui", "egui_kittest", diff --git a/crates/engine/src/cmd/file/place.rs b/crates/engine/src/cmd/file/place.rs index 638742885..2835845f5 100644 --- a/crates/engine/src/cmd/file/place.rs +++ b/crates/engine/src/cmd/file/place.rs @@ -101,6 +101,12 @@ pub(super) fn file_place(s: &mut Session, p: &Value) -> Result { let st = s.active()?; st.selection.items.iter().copied().find(|i| st.doc.item(*i).is_some_and(|it| matches!(it.content, Content::Unassigned | Content::Graphic(_)))) }); + // A frame named outright is not replaced while it is locked (the selected frame is the user's own choice). + if let Some(fid) = crate::cmd::id_param(p, "frame") + && s.doc()?.doc.item(fid).is_some_and(|it| it.locked) + { + return Err(bad("file.place", "the frame is locked")); + } // `"spread": null` counts as omitted, like the other placement parameters. let spread = match p.get("spread") { Some(Value::Null) => designcraft_doc::SpreadRef::Doc(0), @@ -428,6 +434,21 @@ pub(super) mod image_place_tests { } } + #[test] + fn a_locked_frame_named_outright_is_not_replaced() { + let image = image_file_with_color(30).unwrap(); + let mut session = Session::new(); + session.execute("file.new", &json!({})).unwrap(); + let frame = session.execute("frame.create", &json!({"rect": [36, 36, 136, 136]})).unwrap()["id"].as_u64().unwrap(); + session.execute("object.lock", &json!({"ids": [frame]})).unwrap(); + assert!(session.execute("file.place", &json!({"path": image.path(), "frame": frame})).is_err()); + let document = &session.doc().unwrap().doc; + assert!(matches!(document.item(ItemId(frame)).unwrap().content, Content::Unassigned), "the locked frame stays empty"); + session.execute("object.unlockAll", &json!({})).unwrap(); + session.execute("file.place", &json!({"path": image.path(), "frame": frame})).unwrap(); + assert!(matches!(session.doc().unwrap().doc.item(ItemId(frame)).unwrap().content, Content::Graphic(_))); + } + #[test] fn individual_placement_parameters_bypass_selected_frames() { let image = image_file().unwrap(); diff --git a/crates/ui-egui/Cargo.toml b/crates/ui-egui/Cargo.toml index ffe855f4a..9167a157d 100644 --- a/crates/ui-egui/Cargo.toml +++ b/crates/ui-egui/Cargo.toml @@ -21,6 +21,7 @@ designcraft-compose = { workspace = true } designcraft-render = { workspace = true } designcraft-tools = { workspace = true } designcraft-engine = { workspace = true } +designcraft-textimport = { workspace = true } craft-prepress = { workspace = true } # The UI draws monochrome icon glyphs from egui's bundled emoji faces (☰, ⚠, 📋); 0.37 moved # them behind this feature. They stay bundled copies of OSI/CC0 faces from `epaint_default_fonts`. diff --git a/crates/ui-egui/src/app.rs b/crates/ui-egui/src/app.rs index 383362594..c1b7a62e3 100644 --- a/crates/ui-egui/src/app.rs +++ b/crates/ui-egui/src/app.rs @@ -210,11 +210,12 @@ impl DesignApp { // 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 mut frame_free = true; for drop in std::mem::take(&mut self.pending_drops) { if drop.generation != current { continue; } - let _ = self.open_dropped_at(&drop.path, drop.pointer); + let _ = self.place_dropped(&drop.path, drop.pointer, &mut frame_free); } } menus::shortcuts(self, &ctx); diff --git a/crates/ui-egui/src/app/files.rs b/crates/ui-egui/src/app/files.rs index b5de53ab1..563a32a7e 100644 --- a/crates/ui-egui/src/app/files.rs +++ b/crates/ui-egui/src/app/files.rs @@ -178,17 +178,26 @@ impl DesignApp { /// window's coordinates. #[cfg(not(target_arch = "wasm32"))] pub(crate) fn open_dropped_files(&mut self, files: &[egui::DroppedFileHandle], pointer: Option) { + let mut frame_free = true; for f in files { let p = f.path().to_string_lossy().to_string(); if !p.is_empty() { - let _ = self.open_dropped_at(&p, pointer); + let _ = self.place_dropped(&p, pointer, &mut frame_free); } } } - /// [`Self::open_dropped`] at the pointer: a graphic's top left lands where the file was - /// dropped on the canvas, text goes to a new frame on the document page there. + /// [`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. pub fn open_dropped_at(&mut self, path: &str, pointer: Option) -> Result { + self.place_dropped(path, pointer, &mut true) + } + + /// [`Self::open_dropped_at`] for one file of a drop. `frame_free` is whether the frame under the + /// pointer can still be filled: the first graphic of a drop takes it (and clears the flag), so + /// the files after it land at the pointer instead of replacing that graphic. + pub(crate) fn place_dropped(&mut self, path: &str, pointer: Option, frame_free: &mut bool) -> Result { if path.to_ascii_lowercase().ends_with(".ase") { return self.run("swatch.load", json!({"path": path})); } @@ -200,6 +209,12 @@ impl DesignApp { 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 + // 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((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)); @@ -209,7 +224,11 @@ impl DesignApp { o.insert("page".into(), json!(st.doc.first_page_of_spread(si) + pi + 1)); } } - self.open_file("file.place", params) + let placed = self.open_file("file.place", params); + if frame.is_some() && placed.is_ok() { + *frame_free = false; + } + placed } /// Queue files the operating system asked the app to open. They can arrive before the window diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index ed207c34b..8aac73ae6 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -37,7 +37,7 @@ use guides::{guide_at, guide_drag_delta, guide_measure}; use ime::ime_canvas_pass; pub(crate) use ime::{CanvasIme, ime_observe}; pub(crate) use input::forwarded_keys; -pub use input::{begin_frame, context_menu_open}; +pub use input::{begin_frame, context_menu_open, drop_frame}; use input::{context_menu, draw_cursor_badge, handle_input, pointer_elsewhere}; #[cfg(test)] use input::{press_count, release_clicks}; diff --git a/crates/ui-egui/src/canvas/input.rs b/crates/ui-egui/src/canvas/input.rs index 4616ca11d..21f269e03 100644 --- a/crates/ui-egui/src/canvas/input.rs +++ b/crates/ui-egui/src/canvas/input.rs @@ -169,6 +169,18 @@ fn item_at(app: &DesignApp, p: Point, zoom: f64) -> Option<(designcraft_doc::Ite Some((id, sp)) } +/// The empty or graphic frame under a pointer over the canvas: a graphic dropped there fills it +/// instead of becoming a new object. `None` when the pointer isn't over the canvas or over such a +/// frame, and for a locked frame, which a drop never replaces. +pub fn drop_frame(app: &DesignApp, screen: Option) -> Option { + let rect = app.canvas_rect?; + let pos = screen.filter(|p| rect.contains(*p))?; + let v = app.view()?; + let (id, _) = item_at(app, Xf::new(rect, v).to_canvas(pos), v.zoom)?; + let it = app.session.active()?.doc.item(id)?; + (!it.locked && matches!(it.content, designcraft_doc::Content::Unassigned | designcraft_doc::Content::Graphic(_))).then_some(id) +} + /// Wheel, pinch and Alt/Ctrl/Cmd + wheel over the canvas: zoom at the pointer, else scroll. fn wheel_navigate(app: &mut DesignApp, ui: &egui::Ui, resp: &egui::Response, xf: &Xf) { let alt = wheel_alt(ui); diff --git a/crates/ui-egui/src/tests.rs b/crates/ui-egui/src/tests.rs index 36bac93a9..db52fbd72 100644 --- a/crates/ui-egui/src/tests.rs +++ b/crates/ui-egui/src/tests.rs @@ -853,6 +853,82 @@ fn dropping_an_ase_file_loads_its_swatches() { } } +/// A document with one empty frame at (36, 36)-(136, 136): the app, the frame's id and a screen point inside it +/// and one outside it. +fn app_with_drop_frame() -> (DesignApp, u64, egui::Pos2, egui::Pos2) { + let mut app = DesignApp::new(Session::new(), Services::default()); + app.run("file.new", json!({})).unwrap(); + let frame = app.run("frame.create", json!({"rect": [36, 36, 136, 136]})).unwrap()["id"].as_u64().unwrap(); + app.canvas_rect = Some(egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(1400.0, 900.0))); + let _ = app.view_mut(); + 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 xf = canvas::Xf::new(rect, view); + let inside = xf.to_screen(slot.to_canvas(designcraft_geom::Point::new(80.0, 80.0))); + let outside = xf.to_screen(slot.to_canvas(designcraft_geom::Point::new(300.0, 300.0))); + (app, frame, inside, outside) +} + +fn dropped_png(name: &str) -> std::path::PathBuf { + let path = std::env::temp_dir().join(format!("designcraft-ui-{}-{name}.png", std::process::id())); + std::fs::write(&path, designcraft_render::Rendered { width: 10, height: 10, pixels: vec![200; 400] }.to_png()).unwrap(); + path +} + +#[test] +fn a_graphic_dropped_on_a_frame_fills_it() { + let (mut app, frame, inside, outside) = app_with_drop_frame(); + let path = dropped_png("dropframe"); + 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"); + 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"); + // 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(); + let r = app.open_dropped_at(&text.to_string_lossy(), Some(inside)); + let _ = std::fs::remove_file(&text); + r.unwrap(); + let d = &app.session.doc().unwrap().doc; + assert_eq!(d.spreads[0].items.len(), 2, "the text has a new frame"); + assert!(matches!(d.item(designcraft_doc::ItemId(frame)).unwrap().content, designcraft_doc::Content::Graphic(_))); +} + +#[test] +fn a_locked_frame_is_not_a_drop_target() { + let (mut app, frame, inside, _) = app_with_drop_frame(); + app.run("object.lock", json!({"ids": [frame]})).unwrap(); + assert!(canvas::drop_frame(&app, Some(inside)).is_none(), "a locked frame is skipped"); + let path = dropped_png("droplocked"); + let r = app.open_dropped_at(&path.to_string_lossy(), Some(inside)); + let _ = std::fs::remove_file(&path); + 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"); +} + +#[test] +fn only_the_first_graphic_of_a_drop_fills_the_frame() { + let (mut app, frame, inside, _) = app_with_drop_frame(); + let (first, second) = (dropped_png("dropfirst"), dropped_png("dropsecond")); + let mut frame_free = true; + let a = app.place_dropped(&first.to_string_lossy(), Some(inside), &mut frame_free); + assert!(!frame_free, "the first graphic took the frame"); + let b = app.place_dropped(&second.to_string_lossy(), Some(inside), &mut frame_free); + 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"); + let d = &app.session.doc().unwrap().doc; + assert_eq!(d.spreads[0].items.len(), 2, "the second graphic is a new object"); + assert!(matches!(d.item(designcraft_doc::ItemId(frame)).unwrap().content, designcraft_doc::Content::Graphic(_))); +} + #[test] fn a_file_dropped_while_a_parent_is_open_lands_on_the_parent() { let mut app = DesignApp::new(Session::new(), Services::default());