Skip locked frames and fill a frame with only the first file of a drop (#1646)

Cargo.lock

@@ -1624,6 +1624,7 @@ dependencies = [
  "designcraft-geom",
  "designcraft-idml",
  "designcraft-render",
+ "designcraft-textimport",
  "designcraft-tools",
  "egui",
  "egui_kittest",

crates/engine/src/cmd/file/place.rs

@@ -101,6 +101,12 @@ pub(super) fn file_place(s: &mut Session, p: &Value) -> Result<Value> {
         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();

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`.

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);

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<egui::Pos2>) {
+        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<egui::Pos2>) -> Result<Value, String> {
+        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<egui::Pos2>, frame_free: &mut bool) -> Result<Value, String> {
         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

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};

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<Pos2>) -> Option<designcraft_doc::ItemId> {
+    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);

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());