collective

Browse / github.com/storytold/pdfcraft / patch

fix(ui): do not prompt to save unmodified imported images on tab close (#4942)

[Bugg] - Ask to save changes when no changes were made DESCRIPTION: When using pdfcraft 0.14.0 portable on windows 11 and opening up an image and closing the tab with the image it asks you to save changes even though no changes were made STEPS TO REPRODUCE: Open a jpg using pdfcraft - close the tab with the image ACTUAL BEHAVIOR: Asks to save changes EXPECTED BEHAVIOR: To just exit the tab when no changes were made

Status
open
Score
0.00
Adopters
0 other operators
Author
YusufMntm (imported from pull request #4998, open)
Base
1dd2737b47a9c1c9562f0af563053d1bf510e4bf
Size
+52 −9 in 6 files
Review
pending
Shared
2026-10-28
Id
a8f623f8c3834cddc8e2bb84

How it went

No other operator has reported using it yet.

Diff

raw · crates/ui-egui/src/canvas.rs crates/ui-egui/src/chrome.rs crates/ui-egui/src/editing.rs crates/ui-egui/src/editing/save.rs crates/ui-egui/src/workflows/create_ui.rs crates/ui-egui/tests/it/create.rs

diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs--- a/crates/ui-egui/src/canvas.rs+++ b/crates/ui-egui/src/canvas.rs@@ -296,6 +296,8 @@ pub struct DocView {     pub close_requested: bool,     /// A historical snapshot of another document (see [`Snapshot`]): `None` for real documents.     pub snapshot: Option<Snapshot>,+    /// Baseline bytes of an imported image document used to detect user edits independently of undo history (#4942).+    pub(crate) imported_image_baseline: Option<std::sync::Arc<Vec<u8>>>,     /// Two-page view: show the first page alone, as a cover (View ▸ Page display).     pub cover: bool,     /// Previous view / Next view: pages visited before (and after, once going back).@@ -614,6 +616,7 @@ impl DocView {             disk_changed: false,             close_requested: false,             snapshot: None,+            imported_image_baseline: None,             cover: false,             back: Vec::new(),             forward: Vec::new(),diff --git a/crates/ui-egui/src/chrome.rs b/crates/ui-egui/src/chrome.rs--- a/crates/ui-egui/src/chrome.rs+++ b/crates/ui-egui/src/chrome.rs@@ -129,7 +129,7 @@ pub fn tab_strip(app: &mut PdfCraftApp, ui: &mut egui::Ui) {                             for i in 0..app.views.len() {                                 let Some(doc) = app.session.get(app.views[i].id) else { continue };                                 let Some(name) = crate::canvas::tab_label(app, i) else { continue };-                                let dirty = doc.dirty;+                                let dirty = app.has_unsaved_work(i);                                 // A tab showing the document's title names its file on hover.                                 let file = (name != doc.name).then_some(doc.name.as_str());                                 let response = tab(diff --git a/crates/ui-egui/src/editing.rs b/crates/ui-egui/src/editing.rs--- a/crates/ui-egui/src/editing.rs+++ b/crates/ui-egui/src/editing.rs@@ -617,10 +617,15 @@ impl PdfCraftApp {     }      /// Whether tab `index` has work that isn't saved: edits, or form typing not committed yet.-    pub(crate) fn has_unsaved_work(&self, index: usize) -> bool {+    pub fn has_unsaved_work(&self, index: usize) -> bool {         let Some(v) = self.views.get(index) else { return false };         let Some(doc) = self.session.get(v.id) else { return false };-        doc.dirty+        let dirty = if let Some(baseline) = &v.imported_image_baseline {+            !std::sync::Arc::ptr_eq(&doc.bytes, baseline) && doc.bytes.as_slice() != baseline.as_slice()+        } else {+            doc.dirty+        };+        dirty             || v.pending_edit.is_some()             || v.content.blocked().is_some()             || v.forms.focus.as_ref().is_some_and(|f| crate::forms_ui::draft_edit(f, &doc.form).is_some())diff --git a/crates/ui-egui/src/editing/save.rs b/crates/ui-egui/src/editing/save.rs--- a/crates/ui-egui/src/editing/save.rs+++ b/crates/ui-egui/src/editing/save.rs@@ -96,6 +96,7 @@ impl PdfCraftApp {                         && let Some(view) = self.views.get_mut(index)                     {                         view.document_changed(&doc.info);+                        view.imported_image_baseline = None;                     }                     // The host reports where the file went (and any failure) itself.                     if !to_host {@@ -198,6 +199,7 @@ impl PdfCraftApp {                     view.document_changed(&doc.info);                     // Saving over a version that changed on disk keeps this one: nothing to ask.                     view.disk_changed = false;+                    view.imported_image_baseline = None;                 }                 self.notify_fmt("Saved {name}", &[("name", &short_name(dest))]);                 #[cfg(target_os = "macos")]diff --git a/crates/ui-egui/src/workflows/create_ui.rs b/crates/ui-egui/src/workflows/create_ui.rs--- a/crates/ui-egui/src/workflows/create_ui.rs+++ b/crates/ui-egui/src/workflows/create_ui.rs@@ -90,20 +90,25 @@ impl PdfCraftApp {     /// something Create understands (the caller then tries to open it as a PDF).     pub(crate) fn open_converted(&mut self, name: &str, bytes: &[u8]) -> Option<Result<(), String>> {         use pdfcraft_engine::SourceKind;-        if !matches!(pdfcraft_engine::source_kind(name, bytes), Some(SourceKind::Image | SourceKind::Text)) {+        let kind = pdfcraft_engine::source_kind(name, bytes);+        if !matches!(kind, Some(SourceKind::Image | SourceKind::Text)) {             return None;         }+        let is_image = matches!(kind, Some(SourceKind::Image));         let created = self.session.convert_to_pdf(name, &Arc::new(bytes.to_vec())).map(|(_, pdf)| pdf);-        Some(self.open_created_bytes(&format!("{}.pdf", stem(name)), created.map_err(|e| e.to_string())))+        Some(self.open_created_bytes(&format!("{}.pdf", stem(name)), created.map_err(|e| e.to_string()), is_image))     } -    fn open_created_bytes(&mut self, name: &str, created: Result<Arc<Vec<u8>>, String>) -> Result<(), String> {+    fn open_created_bytes(&mut self, name: &str, created: Result<Arc<Vec<u8>>, String>, is_image: bool) -> Result<(), String> {         let bytes = created?;         let id = self.session.open_new(name, bytes).map_err(|e| e.to_string())?;         // The reading-colours preference is app-wide: the newly created document starts with it.         self.session.set_reading_colors(id, self.reading_colors);         let info = &self.session.get(id).ok_or("the new document could not be opened")?.info;         let mut view = crate::DocView::new(id, info, self.view_defaults);+        if is_image {+            view.imported_image_baseline = self.session.get(id).map(|d| d.bytes.clone());+        }         if self.full_screen {             view.enter_presentation();         }@@ -138,13 +143,13 @@ impl PdfCraftApp {             }             Clip::Text(t) => self.session.create_from_text("Clipboard", &t),         };-        self.open_created_bytes("Clipboard.pdf", created.map_err(|e| e.to_string()))+        self.open_created_bytes("Clipboard.pdf", created.map_err(|e| e.to_string()), false)     }      /// Create ▸ Blank page: a new untitled US Letter document.     pub(crate) fn create_blank(&mut self) {         let created = self.session.create_blank(612.0, 792.0, 1).map_err(|e| e.to_string());-        if let Err(e) = self.open_created_bytes("Untitled.pdf", created) {+        if let Err(e) = self.open_created_bytes("Untitled.pdf", created, false) {             self.notify_fmt("Couldn't create a PDF: {e}", &[("e", &e.to_string())]);         }     }@@ -222,7 +227,7 @@ impl PdfCraftApp {         let name =             if let Some((name, _)) = images.first().filter(|_| images.len() == 1) { format!("{}.pdf", stem(name)) } else { "Images.pdf".to_string() };         let created = self.session.create_from_images_with_resolution(&images, resolution).map_err(|e| e.to_string());-        if let Err(e) = self.open_created_bytes(&name, created) {+        if let Err(e) = self.open_created_bytes(&name, created, false) {             self.notify_fmt("Couldn't create a PDF: {e}", &[("e", &e.to_string())]);         }     }diff --git a/crates/ui-egui/tests/it/create.rs b/crates/ui-egui/tests/it/create.rs--- a/crates/ui-egui/tests/it/create.rs+++ b/crates/ui-egui/tests/it/create.rs@@ -374,3 +374,31 @@ fn multiple_files_past_a_thousand_are_all_used() {     let doc = app.session.get(app.views[0].id).unwrap();     assert_eq!(doc.info.pages.len(), 1001, "every picked file is used, in order"); }++/// Opening an imported image and closing its tab without making any changes does not+/// trigger a "save changes" confirmation prompt (#4942).+#[test]+fn unmodified_imported_image_closes_without_save_prompt() {+    let mut app = PdfCraftApp::new();+    app.set_option("language", "en").unwrap();+    app.open_bytes("photo.png", None, png()).unwrap();+    assert_eq!(app.views.len(), 1);+    assert!(!app.has_unsaved_work(0), "freshly imported image has no unsaved user edits");+    app.request_close_tab(0);+    assert!(app.close_request.is_none(), "no save confirmation prompt asked on close");+    assert!(app.views.is_empty(), "tab closed cleanly");+}++/// An imported image that receives a user edit prompts to save on tab close (#4942).+#[test]+fn modified_imported_image_prompts_to_save_on_close() {+    let mut app = PdfCraftApp::new();+    app.set_option("language", "en").unwrap();+    app.open_bytes("photo.png", None, png()).unwrap();+    assert_eq!(app.views.len(), 1);+    app.apply_edit(pdfcraft_engine::Edit::RotatePages { pages: vec![0], degrees: 90 });+    assert!(app.has_unsaved_work(0), "edited imported image has unsaved work");+    app.request_close_tab(0);+    assert!(app.close_request.is_some(), "save prompt is presented when closing edited image");+    assert_eq!(app.views.len(), 1, "tab stays open pending user decision");+}