Combine files: add a drop as one step, in order, and name what it can't take

Files dropped on the Combine files tab are handled as one batch: they join
the end of the list in the order dropped, as one undo step, from their path
or, when a drop carries only bytes, from those. A folder still adds the
files in it and its subfolders. Files of a kind Combine doesn't take are
left out and named in a notice, instead of joining the list as files that
can't be read. In a browser the dropped files are read one after another,
so they keep their order too.

crates/ui-egui/src/app.rs

@@ -520,11 +520,11 @@ impl eframe::App for PdfCraftApp {
         let dropped = self.drop_on_signature_pad(dropped);
         #[cfg(not(target_arch = "wasm32"))]
         let dropped = self.drop_on_grid(dropped, ctx);
-        for f in dropped {
-            // Files dropped on the Combine files tab join its list instead of opening.
-            if self.combine_showing() {
-                self.drop_into_combine(f, ctx);
-            } else {
+        // Files dropped on the Combine files tab join its list, all at once, instead of opening.
+        if self.combine_showing() && !dropped.is_empty() {
+            self.drop_into_combine(dropped, ctx);
+        } else {
+            for f in dropped {
                 self.open_dropped(f, ctx);
             }
         }

crates/ui-egui/src/app/input.rs

@@ -19,7 +19,7 @@ impl PdfCraftApp {
             self.open_path(&p);
             return;
         }
-        let name = f.path().file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| "dropped.pdf".into());
+        let name = crate::files::dropped_name(&f);
         match f.bytes() {
             Ok(bytes) => {
                 if self.ctx.is_some() && !self.run_inline {
@@ -76,7 +76,7 @@ impl PdfCraftApp {
         };
         let mut files = Vec::new();
         for f in &dropped {
-            let name = f.path().file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| "dropped.pdf".into());
+            let name = crate::files::dropped_name(f);
             if f.path().is_absolute() {
                 files.push((name, Err(f.path().to_path_buf())));
                 continue;
@@ -130,7 +130,7 @@ impl PdfCraftApp {
         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().into_owned()).unwrap_or_else(|| "dropped.pdf".into());
+            let name = crate::files::dropped_name(&f);
             if let Ok(bytes) = f.bytes_async().await
                 && let Ok(mut q) = inbox.lock()
             {

crates/ui-egui/src/workflows/combine_ui/files.rs

@@ -274,6 +274,13 @@ pub(crate) struct Incoming {
     pub open_doc: Option<pdfcraft_engine::DocId>,
 }
 
+impl Incoming {
+    /// A file known only by its name and bytes.
+    pub(crate) fn new(name: String, bytes: Vec<u8>) -> Self {
+        Incoming { name, bytes: Arc::new(bytes), modified: None, note: None, open_doc: None }
+    }
+}
+
 /// A new value for [`CombineFile::auth`]: counted for the whole run, outside the undo history.
 fn next_auth() -> u64 {
     static NEXT: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(1);

crates/ui-egui/src/workflows/combine_ui/parts.rs

@@ -38,6 +38,12 @@ fn combinable(ext: &str) -> bool {
     std::iter::once("pdf").chain(pdfcraft_engine::CONVERTIBLE).any(|c| ext.eq_ignore_ascii_case(c))
 }
 
+/// Whether Combine takes a file by what it is: a PDF (by its header or its name) or an image or
+/// text file it converts, as `Session::as_pdf` tells them apart.
+pub(super) fn takes(name: &str, bytes: &[u8]) -> bool {
+    pdfcraft_engine::source_kind(name, bytes).is_some() || name.to_ascii_lowercase().ends_with(".pdf")
+}
+
 /// A file's bookmark in the combined PDF: its name without ".pdf", or without the extension of
 /// the image or text file it was made from.
 pub(super) fn title_of(name: &str) -> &str {

crates/ui-egui/src/workflows/combine_ui/session.rs

@@ -3,7 +3,7 @@ use super::MAX_FOLDER_FILES;
 use super::files::{AssessError, Assessed, CombineFile, Incoming, Secret, Snapshot, SortKey, UnlockPrompt, assess};
 #[cfg(not(target_arch = "wasm32"))]
 use super::parts::combinable_in;
-use super::parts::{Part, items_of, moved_order, natural, parts_of, range_of, title_of};
+use super::parts::{Part, items_of, moved_order, natural, parts_of, range_of, takes, title_of};
 use super::{Arc, BTreeSet, CombineTab, HISTORY, Modifiers, PdfCraftApp};
 
 impl PdfCraftApp {
@@ -246,9 +246,21 @@ impl PdfCraftApp {
 
     /// Add picked files to the Combine files list (and show it).
     pub(crate) fn stage_combine(&mut self, files: Vec<(String, Vec<u8>)>) {
-        let incoming =
-            files.into_iter().map(|(name, bytes)| Incoming { name, bytes: Arc::new(bytes), modified: None, note: None, open_doc: None }).collect();
-        self.stage_combine_with(incoming);
+        self.stage_combine_with(files.into_iter().map(|(name, bytes)| Incoming::new(name, bytes)).collect());
+    }
+
+    /// Files dropped on the Combine files tab join the end of its list in the order dropped, as
+    /// one undo step. A drop isn't filtered like a pick, so files of a kind Combine doesn't take
+    /// are left out, and named.
+    pub(crate) fn add_dropped_to_combine(&mut self, files: Vec<Incoming>) {
+        let (taken, refused): (Vec<_>, Vec<_>) = files.into_iter().partition(|f| takes(&f.name, &f.bytes));
+        if !refused.is_empty() {
+            let names: Vec<&str> = refused.iter().map(|f| f.name.as_str()).collect();
+            self.notify_fmt("Not added: {names}. Combine takes PDFs, images and text files.", &[("names", &names.join(", "))]);
+        }
+        if !taken.is_empty() {
+            self.stage_combine_with(taken);
+        }
     }
 
     /// Add files to the Combine files list, with what can be known about them up front.

crates/ui-egui/src/workflows/files.rs

@@ -149,6 +149,8 @@ pub struct FileRequest {
     purpose: FilePurpose,
     target: Option<PickTarget>,
     files: Vec<(String, Vec<u8>)>,
+    /// Dropped on the Combine files tab rather than picked: what Combine can't take is left out.
+    dropped: bool,
 }
 
 /// Files picked asynchronously (web), waiting to be used.
@@ -310,6 +312,11 @@ fn read_named(path: &std::path::Path) -> Result<(String, Vec<u8>), (String, Stri
     }
 }
 
+/// The name of a file dropped on the window ("dropped.pdf" when it has none).
+pub(crate) fn dropped_name(f: &egui::DroppedFileHandle) -> String {
+    f.path().file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| "dropped.pdf".into())
+}
+
 impl PdfCraftApp {
     /// Ask for files to add to the Combine files list (its Add files… button).
     pub fn combine_dialog(&mut self) {
@@ -451,37 +458,53 @@ impl PdfCraftApp {
         }
     }
 
-    /// A file dropped on the Combine files tab: added to its list.
+    /// Files dropped on the Combine files tab join its list in the order dropped (see
+    /// [`Self::add_dropped_to_combine`]); a folder adds the files in it and its subfolders.
     #[cfg(not(target_arch = "wasm32"))]
-    pub(crate) fn drop_into_combine(&mut self, f: egui::DroppedFileHandle, _ctx: &egui::Context) {
-        // A folder: the PDFs in it and in the folders inside it.
-        if f.path().is_absolute() && f.path().is_dir() {
-            self.add_folder_to_combine(f.path(), true);
-            return;
-        }
-        if f.path().is_absolute() {
-            self.use_paths(FilePurpose::Combine, &[f.path().to_path_buf()]);
-            return;
-        }
-        let name = f.path().file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| "dropped.pdf".into());
-        match f.bytes() {
-            Ok(bytes) => self.use_files(FilePurpose::Combine, vec![(name, bytes)]),
-            Err(e) => self.notify_fmt("Couldn't read {name}: {e}", &[("name", &name), ("e", &e.to_string())]),
+    pub(crate) fn drop_into_combine(&mut self, dropped: Vec<egui::DroppedFileHandle>, _ctx: &egui::Context) {
+        let mut files = Vec::new();
+        for f in dropped {
+            let path = f.path();
+            if path.is_absolute() && path.is_dir() {
+                // After the files dropped before it.
+                self.add_dropped_to_combine(std::mem::take(&mut files));
+                self.add_folder_to_combine(path, true);
+                continue;
+            }
+            let name = dropped_name(&f);
+            // A file on disk is read from there, with when it was last modified; a drop that
+            // carries only bytes (no path) as it is.
+            let (read, modified) = if path.is_absolute() {
+                (std::fs::read(path).map_err(|e| e.to_string()), std::fs::metadata(path).and_then(|m| m.modified()).ok())
+            } else {
+                (f.bytes(), None)
+            };
+            match read {
+                Ok(bytes) => files.push(crate::combine_ui::Incoming { modified, ..crate::combine_ui::Incoming::new(name, bytes) }),
+                Err(e) => self.notify_fmt("Couldn't read {name}: {e}", &[("name", &name), ("e", &e)]),
+            }
         }
+        self.add_dropped_to_combine(files);
     }
 
     /// Browsers read dropped files asynchronously; they join the Combine files list next frame.
     #[cfg(target_arch = "wasm32")]
-    pub(crate) fn drop_into_combine(&mut self, f: egui::DroppedFileHandle, ctx: &egui::Context) {
+    pub(crate) fn drop_into_combine(&mut self, dropped: Vec<egui::DroppedFileHandle>, ctx: &egui::Context) {
         let requests = self.requests.clone();
-        let request = self.file_request(FilePurpose::Combine, Vec::new());
+        let request = FileRequest { dropped: true, ..self.file_request(FilePurpose::Combine, Vec::new()) };
         let ctx = ctx.clone();
         wasm_bindgen_futures::spawn_local(async move {
-            let name = f.path().file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| "dropped.pdf".into());
-            if let Ok(bytes) = f.bytes_async().await
+            // One after another, so they join the list in the order dropped.
+            let mut files = Vec::new();
+            for f in dropped {
+                if let Ok(bytes) = f.bytes_async().await {
+                    files.push((dropped_name(&f), bytes));
+                }
+            }
+            if !files.is_empty()
                 && let Ok(mut q) = requests.lock()
             {
-                q.push(FileRequest { files: vec![(name, bytes)], ..request });
+                q.push(FileRequest { files, ..request });
                 ctx.request_repaint();
             }
         });
@@ -492,7 +515,7 @@ impl PdfCraftApp {
     /// `requests` once the files are read; it is used on a later frame.
     pub fn file_request(&self, purpose: FilePurpose, files: Vec<(String, Vec<u8>)>) -> FileRequest {
         let target = if purpose.edits_active_document() { self.pick_target(self.active_ids().map(|(_, id)| id)) } else { None };
-        FileRequest { purpose, target, files }
+        FileRequest { purpose, target, files, dropped: false }
     }
 
     /// `doc` as it is now, for a pick that will edit it.
@@ -516,12 +539,19 @@ impl PdfCraftApp {
     /// Handle files picked asynchronously.
     pub(crate) fn process_file_requests(&mut self) {
         let pending: Vec<_> = self.requests.lock().map(|mut q| std::mem::take(&mut *q)).unwrap_or_default();
-        for FileRequest { purpose, target, files } in pending {
+        for FileRequest { purpose, target, files, dropped } in pending {
             if !self.still_pick_target(target) {
                 continue;
             }
             // Last-resort guard (AGENTS.md §4), as for desktop picks and commands.
-            if let Err(m) = pdfcraft_engine::guard(|| self.use_files(purpose, files)) {
+            let used = pdfcraft_engine::guard(|| {
+                if dropped {
+                    self.add_dropped_to_combine(files.into_iter().map(|(name, bytes)| crate::combine_ui::Incoming::new(name, bytes)).collect());
+                } else {
+                    self.use_files(purpose, files);
+                }
+            });
+            if let Err(m) = used {
                 self.notify_fmt("That didn't work: an internal error stopped it ({m}).", &[("m", m.as_str())]);
             }
         }

crates/ui-egui/tests/it/ui.rs

@@ -1166,6 +1166,86 @@ fn a_folder_dropped_on_the_combine_tab_adds_its_pdfs() {
     assert_eq!(names, ["two.pdf", "one.pdf"], "subfolders included, in path order");
 }
 
+/// The Combine files tab, showing, with one file in its list already.
+fn combine_tab_with_one_file() -> Harness<'static, PdfCraftApp> {
+    let mut h = Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_eframe(|_cc| {
+        let mut app = PdfCraftApp::new();
+        app.set_option("language", "en").unwrap();
+        app
+    });
+    h.run_steps(3);
+    h.state_mut().set_option("combine-view", "list").unwrap();
+    h.state_mut().execute("page.combine");
+    h.state_mut().use_files(pdfcraft_ui_egui::FilePurpose::Combine, vec![("first.pdf".into(), FIXTURE.to_vec())]);
+    h.run_steps(2);
+    h
+}
+
+fn combine_names(h: &Harness<'_, PdfCraftApp>) -> Vec<String> {
+    h.state().combine_draft.iter().map(|f| f.name.clone()).collect()
+}
+
+#[test]
+fn files_dropped_on_the_combine_tab_join_the_end_of_its_list_in_the_order_dropped() {
+    let mut h = combine_tab_with_one_file();
+    let dir = std::env::temp_dir().join(format!("pdfcraft-combine-drop-order-{}", std::process::id()));
+    let _ = std::fs::remove_dir_all(&dir);
+    std::fs::create_dir_all(&dir).unwrap();
+    // Files on disk come with their path (and no bytes); browsers hand over only a name and bytes.
+    let on_disk = |name: &str| -> egui::DroppedFileHandle {
+        std::fs::write(dir.join(name), FIXTURE).unwrap();
+        std::sync::Arc::new(Dropped { path: dir.join(name), bytes: Vec::new() })
+    };
+    let in_memory =
+        |name: &str, bytes: &[u8]| -> egui::DroppedFileHandle { std::sync::Arc::new(Dropped { path: name.into(), bytes: bytes.to_vec() }) };
+    let dropped = [on_disk("c.pdf"), in_memory("a.pdf", FIXTURE), in_memory("notes.txt", b"Some notes"), on_disk("b.pdf")];
+    h.input_mut().dropped_files.extend(dropped);
+    h.run_steps(3);
+    let _ = std::fs::remove_dir_all(&dir);
+    assert!(h.state().views.is_empty(), "nothing opens");
+    assert_eq!(combine_names(&h), ["first.pdf", "c.pdf", "a.pdf", "notes.txt", "b.pdf"]);
+    let draft = &h.state().combine_draft;
+    assert!(draft.iter().all(|f| f.problem.is_none()), "every file can be combined");
+    assert!(draft[3].pages == 1, "the text file became a page");
+    assert!(draft[1].modified.is_some() && draft[2].modified.is_none());
+    // One drop, one step to undo.
+    h.state_mut().execute("edit.undo");
+    h.run_steps(2);
+    assert_eq!(combine_names(&h), ["first.pdf"]);
+}
+
+#[test]
+fn a_file_combine_does_not_take_is_left_out_of_a_drop_and_named() {
+    let mut h = combine_tab_with_one_file();
+    let path = std::env::temp_dir().join(format!("pdfcraft-combine-drop-{}.zip", std::process::id()));
+    std::fs::write(&path, b"PK\x03\x04").unwrap();
+    let dropped: [egui::DroppedFileHandle; 4] = [
+        std::sync::Arc::new(Dropped { path: "report.docx".into(), bytes: b"PK\x03\x04".to_vec() }),
+        std::sync::Arc::new(Dropped { path: "one.pdf".into(), bytes: FIXTURE.to_vec() }),
+        std::sync::Arc::new(Dropped { path: path.clone(), bytes: Vec::new() }),
+        // A PDF without the name of one is still a PDF.
+        std::sync::Arc::new(Dropped { path: "scan".into(), bytes: FIXTURE.to_vec() }),
+    ];
+    h.input_mut().dropped_files.extend(dropped);
+    h.run_steps(3);
+    std::fs::remove_file(&path).ok();
+    assert_eq!(combine_names(&h), ["first.pdf", "one.pdf", "scan"]);
+    let zip = path.file_name().unwrap().to_string_lossy().into_owned();
+    h.get_by_label(&format!("Not added: report.docx, {zip}. Combine takes PDFs, images and text files."));
+}
+
+#[test]
+fn a_pdf_dropped_on_a_document_opens_while_the_combine_tab_waits_behind_it() {
+    let mut h = combine_tab_with_one_file();
+    h.state_mut().open_bytes("open.pdf", None, FIXTURE.to_vec()).unwrap();
+    h.run_steps(2);
+    assert!(!h.state().combine_showing() && h.state().combine_tab.open);
+    let file: egui::DroppedFileHandle = std::sync::Arc::new(Dropped { path: "dropped.pdf".into(), bytes: FIXTURE.to_vec() });
+    h.input_mut().dropped_files.push(file);
+    support::wait_until(&mut h, "dropped document", |app| app.views.len() == 2);
+    assert_eq!(combine_names(&h), ["first.pdf"], "the Combine files list is left as it was");
+}
+
 #[test]
 fn files_and_quit_from_the_operating_system() {
     // #73: macOS hands Finder double-clicks, Open With and Dock drops over as Apple events.