Merge PR #4155: M4: a save prompt that opens in the background asks for the user's attention

crates/ui-egui/src/editing.rs

@@ -902,10 +902,15 @@ impl PdfCraftApp {
     /// Close tab `index` for a close request, then ask about the next dirty document, or quit.
     fn close_and_continue(&mut self, ctx: &egui::Context, index: usize, req: CloseRequest) {
         self.close_tab(index);
-        // A newer prompt wins: the user may have started another close while a save was pending.
+        // A newer prompt wins: the user may have started another close while a save was pending,
+        // and seen its question, which keeps counting as seen.
         if (req == CloseRequest::Quit || req == CloseRequest::All) && self.close_request.is_none() {
             match self.first_dirty() {
-                Some(_) => self.close_request = Some(req),
+                Some(_) => {
+                    // A question about the next document is a new one.
+                    self.save_prompt_seen = false;
+                    self.close_request = Some(req);
+                }
                 None if req == CloseRequest::Quit => self.quit(ctx),
                 None => {}
             }
@@ -962,6 +967,32 @@ impl PdfCraftApp {
             self.shutdown_recovery();
         }
     }
+
+    /// Ask for the user's attention when the "save changes?" question opens while PdfCraft is in
+    /// the background (Quit in the Dock menu, logging out, or an agent quitting), and withdraw the
+    /// request once the user comes back or the question is answered.
+    ///
+    /// The question holds up the quit the user asked for, so the request is critical, as AppKit
+    /// makes it for a modal panel that an inactive app presents: the Dock icon bounces until
+    /// PdfCraft is active, Windows flashes the taskbar button until the window is in front, and
+    /// X11 sets the urgency hint, which stays until it is reset. A question that opens in front
+    /// asks nothing, and neither does leaving it open: the user has seen it. Each document asked
+    /// about when quitting is a new question, so one that opens in the background asks even after
+    /// a question the user saw; a request that still stands carries over to it.
+    pub(crate) fn attend_save_prompt(&mut self, ctx: &egui::Context) {
+        let open = self.close_request.is_some();
+        let focused = ctx.input(|i| i.focused);
+        if self.save_prompt_attention {
+            if focused || !open {
+                ctx.send_viewport_cmd(egui::ViewportCommand::RequestUserAttention(egui::UserAttentionType::Reset));
+                self.save_prompt_attention = false;
+            }
+        } else if open && !self.save_prompt_seen && !focused {
+            ctx.send_viewport_cmd(egui::ViewportCommand::RequestUserAttention(egui::UserAttentionType::Critical));
+            self.save_prompt_attention = true;
+        }
+        self.save_prompt_seen = open;
+    }
 }
 
 /// The single page a content edit changes (`None` for other edits). Like comment edits,
@@ -1681,3 +1712,45 @@ mod content_page_tests {
         assert_eq!(content_page(&Edit::SetInfo { key: "Title".into(), value: "x".into() }), None);
     }
 }
+
+#[cfg(test)]
+mod attention_tests {
+    use super::CloseRequest;
+    use crate::PdfCraftApp;
+
+    /// The attention requests one frame of `attend_save_prompt` sends, with the window `focused`
+    /// or not.
+    fn attend(app: &mut PdfCraftApp, ctx: &egui::Context, focused: bool) -> Vec<egui::UserAttentionType> {
+        let mut out = ctx.run_ui(egui::RawInput { focused, ..Default::default() }, |ui| app.attend_save_prompt(ui.ctx()));
+        // The frame's font texture isn't uploaded anywhere in a unit test.
+        out.textures_delta.clear();
+        out.viewport_output
+            .values()
+            .flat_map(|v| v.commands.iter())
+            .filter_map(|c| if let egui::ViewportCommand::RequestUserAttention(a) = c { Some(*a) } else { None })
+            .collect()
+    }
+
+    /// A save that finishes after a newer "save changes?" question opened in front, and after the
+    /// user went to another app, asks for no attention: the user saw that question.
+    #[test]
+    fn a_save_finishing_behind_a_newer_question_the_user_saw_asks_for_nothing() {
+        let ctx = egui::Context::default();
+        let mut app = PdfCraftApp::new();
+        for name in ["a.pdf", "b.pdf", "c.pdf"] {
+            let bytes = app.session.create_blank(200.0, 300.0, 1).unwrap();
+            app.open_bytes(name, None, bytes.as_ref().clone()).unwrap();
+            app.active = Some(app.views.len() - 1);
+            assert!(app.apply_edit(pdfcraft_engine::Edit::RotatePages { pages: vec![0], degrees: 90 }));
+        }
+        // a.pdf was answered with Save while quitting. While that save is pending, the user closes
+        // c.pdf's tab, and its question opens in front.
+        let c = app.views[2].id;
+        app.close_request = Some(CloseRequest::Tab(c));
+        assert!(attend(&mut app, &ctx, true).is_empty());
+        // a.pdf's save finishes once the user has gone to another app.
+        app.close_and_continue(&ctx, 0, CloseRequest::Quit);
+        assert_eq!(app.close_request, Some(CloseRequest::Tab(c)), "the newer question stands");
+        assert!(attend(&mut app, &ctx, false).is_empty(), "the user has seen it");
+    }
+}

crates/ui-egui/src/lib.rs

@@ -642,6 +642,12 @@ pub struct PdfCraftApp {
     pub close_request: Option<CloseRequest>,
     /// The button of the "save changes?" question the keyboard has chosen; Save when it opens.
     pub(crate) save_prompt_button: dialogs::PromptButton,
+    /// `attend_save_prompt` has seen the pending "save changes?" question. It is false again once
+    /// the question is answered, so the next document's question is told apart from it.
+    save_prompt_seen: bool,
+    /// The "save changes?" question asked for the user's attention and the request still stands
+    /// (see `attend_save_prompt`).
+    save_prompt_attention: bool,
     /// A save waiting behind [`Dialog::OverwriteChanged`] (the web build has no files to check).
     #[cfg(not(target_arch = "wasm32"))]
     pub pending_overwrite: Option<editing::PendingOverwrite>,
@@ -1018,6 +1024,8 @@ impl PdfCraftApp {
             host_dirty: None,
             close_request: None,
             save_prompt_button: Default::default(),
+            save_prompt_seen: false,
+            save_prompt_attention: false,
             #[cfg(not(target_arch = "wasm32"))]
             pending_overwrite: None,
             save_override: None,
@@ -3146,6 +3154,8 @@ impl eframe::App for PdfCraftApp {
         storage.set_string("pdfcraft", self.persist());
     }
 
+    /// Each frame's work before drawing: opens, operating-system and control-channel requests,
+    /// the quit guard, shortcuts, and the background jobs' results.
     fn logic(&mut self, ctx: &egui::Context, _frame: &mut eframe::Frame) {
         self.ctx = Some(ctx.clone());
         #[cfg(not(target_arch = "wasm32"))]
@@ -3312,6 +3322,8 @@ impl eframe::App for PdfCraftApp {
         self.process_file_requests();
         #[cfg(not(target_arch = "wasm32"))]
         self.process_picked();
+        // Last, after everything that can open or answer the "save changes?" question.
+        self.attend_save_prompt(ctx);
     }
 
     fn ui(&mut self, ui: &mut egui::Ui, _frame: &mut eframe::Frame) {

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

@@ -254,6 +254,65 @@ fn control_quit_preserves_dirty_documents_and_cancel_keeps_them_open() {
     assert!(!close_requested(&h));
 }
 
+/// Run one frame and return the attention requests it sent to the window.
+fn step_attention(h: &mut Harness<'static, PdfCraftApp>) -> Vec<egui::UserAttentionType> {
+    h.step();
+    let commands = h.output().viewport_output.values().flat_map(|v| v.commands.iter());
+    commands.filter_map(|c| if let egui::ViewportCommand::RequestUserAttention(a) = c { Some(*a) } else { None }).collect()
+}
+
+/// Send a request, run frames until it is answered and three more, and collect every attention
+/// request those frames sent.
+fn attention_during(h: &mut Harness<'static, PdfCraftApp>, c: &ControlClient, method: &str, params: Value) -> Vec<egui::UserAttentionType> {
+    let rx = c.send(method, params);
+    let mut sent = Vec::new();
+    for _ in 0..30 {
+        sent.extend(step_attention(h));
+        if rx.try_recv().is_ok() {
+            (0..3).for_each(|_| sent.extend(step_attention(h)));
+            return sent;
+        }
+    }
+    panic!("{method}: no reply after 30 frames");
+}
+
+/// Open `name` with an unsaved change, in a tab of its own.
+fn open_unsaved(h: &mut Harness<'static, PdfCraftApp>, name: &str) {
+    let app = h.state_mut();
+    app.open_bytes(name, None, fixture(1)).unwrap();
+    app.active = Some(app.views.len() - 1);
+    app.apply_edit(pdfcraft_engine::Edit::RotatePages { pages: vec![0], degrees: 90 });
+}
+
+/// An agent quits while another app is in front, then answers the prompts itself. Quitting asks
+/// about each unsaved document in turn; the request made for the first question stands for the
+/// next, and answering the last withdraws it, so the taskbar or the window list stops flagging a
+/// window with nothing to ask. Each question is a new one: when the first opened in front, the
+/// next one asks when it opens in the background.
+#[test]
+fn a_save_prompt_answered_in_the_background_withdraws_its_attention_request() {
+    use egui::UserAttentionType::{Critical, Reset};
+    let (mut h, c) = harness();
+    ok(&mut h, &c, "ui.command", json!({"id": "page.insert_blank"}));
+    open_unsaved(&mut h, "second.pdf");
+    h.input_mut().focused = false;
+    assert_eq!(attention_during(&mut h, &c, "ui.quit", json!({})), [Critical]);
+    assert!(attention_during(&mut h, &c, "ui.click", json!({"label": "Don't save"})).is_empty(), "the request stands");
+    assert_eq!(attention_during(&mut h, &c, "ui.click", json!({"label": "Cancel"})), [Reset]);
+
+    open_unsaved(&mut h, "third.pdf");
+    h.input_mut().focused = true;
+    assert!(attention_during(&mut h, &c, "ui.quit", json!({})).is_empty(), "the first question opens in front");
+    h.input_mut().focused = false;
+    assert!(attention_during(&mut h, &c, "ui.state", json!({})).is_empty(), "leaving it open asks nothing");
+    assert_eq!(attention_during(&mut h, &c, "ui.click", json!({"label": "Don't save"})), [Critical], "the next opens in the background");
+    assert_eq!(attention_during(&mut h, &c, "ui.click", json!({"label": "Cancel"})), [Reset]);
+    let state = ok(&mut h, &c, "ui.state", json!({}));
+    assert_eq!(state["close_prompt"], false);
+    let left: Vec<_> = state["documents"].as_array().unwrap().iter().map(|d| (d["name"].clone(), d["dirty"].clone())).collect();
+    assert_eq!(left, [(json!("third.pdf"), json!(true))], "Don't save closed doc.pdf and second.pdf; Cancel kept third.pdf");
+}
+
 #[test]
 fn control_quit_clean_document_requests_normal_window_close() {
     for params in [json!({}), Value::Null] {

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

@@ -791,6 +791,50 @@ fn quitting_with_unsaved_changes_asks_for_each_document() {
     assert!(h.state().close_request.is_none());
 }
 
+/// Run `frames` frames and collect every attention request they sent to the window.
+fn attention_over(h: &mut Harness<'static, PdfCraftApp>, frames: usize) -> Vec<egui::UserAttentionType> {
+    let mut sent = Vec::new();
+    for _ in 0..frames {
+        h.step();
+        let commands = h.output().viewport_output.values().flat_map(|v| v.commands.iter());
+        sent.extend(commands.filter_map(|c| if let egui::ViewportCommand::RequestUserAttention(a) = c { Some(*a) } else { None }));
+    }
+    sent
+}
+
+/// Close the window as the operating system does: Quit in the Dock menu, or logging out.
+fn close_window(h: &mut Harness<'static, PdfCraftApp>) {
+    h.input_mut().viewports.entry(egui::ViewportId::ROOT).or_default().events.push(egui::ViewportEvent::Close);
+}
+
+/// Quit from the Dock while another app is in front: the prompt opens in a window the user can't
+/// see, so the Dock icon bounces (the taskbar button flashes) until they come back. A prompt that
+/// opens in front asks for nothing.
+#[test]
+fn a_save_prompt_opened_in_the_background_asks_for_attention_until_the_user_returns() {
+    use egui::UserAttentionType::{Critical, Reset};
+    let mut h = harness(2, |_| {});
+    h.state_mut().views[0].select_pages(&[0]);
+    h.state_mut().apply_edit(pdfcraft_engine::Edit::RotatePages { pages: vec![0], degrees: 90 });
+    h.input_mut().focused = false;
+    close_window(&mut h);
+    assert_eq!(attention_over(&mut h, 4), [Critical], "asked once, when the prompt opened");
+    assert_eq!(h.state().close_request, Some(CloseRequest::Quit));
+    h.input_mut().focused = true;
+    assert_eq!(attention_over(&mut h, 3), [Reset], "coming back withdraws the request");
+    h.get_by_label_contains("Save changes to “doc.pdf”");
+    h.get_by_label("Cancel").click();
+    assert!(attention_over(&mut h, 3).is_empty());
+    assert_eq!(h.state().views.len(), 1, "cancel keeps the document open");
+    // With PdfCraft in front, the prompt is in plain sight.
+    close_window(&mut h);
+    assert!(attention_over(&mut h, 4).is_empty());
+    h.get_by_label_contains("Save changes to “doc.pdf”");
+    // Leaving with the prompt open doesn't ask either: the user has seen it.
+    h.input_mut().focused = false;
+    assert!(attention_over(&mut h, 3).is_empty());
+}
+
 /// PDFs written to a fresh temp folder, removed when the test ends.
 struct TempPdfs(std::path::PathBuf);
 

parity/acrobat-features.toml

@@ -586,8 +586,8 @@ milestone = "M4"
 status = "shipped"
 commands = ["file.close"]
 tools = ["doc_close"]
-evidence = ["crates/ui-egui/tests/it/editing.rs::closing_a_dirty_tab_asks_and_cancel_keeps_it", "crates/ui-egui/tests/it/editing.rs::closing_a_dirty_tab_can_save_first", "crates/ui-egui/tests/it/editing.rs::quitting_with_unsaved_changes_asks_for_each_document", "crates/ui-egui/tests/it/editing.rs::save_prompt_stays_inside_the_screen_for_a_long_filename", "crates/ui-egui/tests/it/editing.rs::the_save_prompt_moves_between_buttons_with_the_keyboard", "crates/ui-egui/tests/it/editing.rs::the_save_prompt_escape_still_cancels_whatever_is_chosen", "crates/ui-egui/tests/it/editing.rs::the_save_prompt_starts_on_save_every_time_it_opens"]
-notes = "Save is chosen when the prompt opens, so Enter saves. Left/Right (and Up/Down) and Tab/Shift+Tab move the choice between Don't save, Cancel and Save, shown with a focus ring; Enter or Space press the chosen button; Escape cancels. Alt+D, Alt+N and Ctrl/Cmd+D still answer Don't save. Not yet: the same keys on the other confirmations (Revert, reload, overwrite)."
+evidence = ["crates/ui-egui/tests/it/editing.rs::closing_a_dirty_tab_asks_and_cancel_keeps_it", "crates/ui-egui/tests/it/editing.rs::closing_a_dirty_tab_can_save_first", "crates/ui-egui/tests/it/editing.rs::quitting_with_unsaved_changes_asks_for_each_document", "crates/ui-egui/tests/it/editing.rs::save_prompt_stays_inside_the_screen_for_a_long_filename", "crates/ui-egui/tests/it/editing.rs::the_save_prompt_moves_between_buttons_with_the_keyboard", "crates/ui-egui/tests/it/editing.rs::the_save_prompt_escape_still_cancels_whatever_is_chosen", "crates/ui-egui/tests/it/editing.rs::the_save_prompt_starts_on_save_every_time_it_opens", "crates/ui-egui/tests/it/editing.rs::a_save_prompt_opened_in_the_background_asks_for_attention_until_the_user_returns", "crates/ui-egui/tests/it/control.rs::a_save_prompt_answered_in_the_background_withdraws_its_attention_request"]
+notes = "Save is chosen when the prompt opens, so Enter saves. Left/Right (and Up/Down) and Tab/Shift+Tab move the choice between Don't save, Cancel and Save, shown with a focus ring; Enter or Space press the chosen button; Escape cancels. Alt+D, Alt+N and Ctrl/Cmd+D still answer Don't save. A prompt that opens while PdfCraft is in the background (Quit in the Dock menu, logging out, an agent's ui.quit) asks once for the user's attention, as a critical request: the Dock icon bounces until PdfCraft is active, the Windows taskbar button flashes until the window is in front, and X11 sets the urgency hint; coming back or answering withdraws it. Not yet: the same keys on the other confirmations (Revert, reload, overwrite); the other confirmations don't ask for attention; on macOS a prompt answered without coming back (over the control channel) leaves the Dock icon bouncing until PdfCraft is activated, because winit can't withdraw the request there."
 
 [[feature]]
 id = "core.dirty-indicator"