diff --git a/crates/ui-egui/src/editing.rs b/crates/ui-egui/src/editing.rs index 2fdc2e9d0..8d341dcff 100644 --- a/crates/ui-egui/src/editing.rs +++ b/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 { + 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"); + } +} diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index af9912f45..edfcb3b71 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -642,6 +642,12 @@ pub struct PdfCraftApp { pub close_request: Option, /// 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, @@ -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) { diff --git a/crates/ui-egui/tests/it/control.rs b/crates/ui-egui/tests/it/control.rs index 54bfe2fa3..8bd7b5dd4 100644 --- a/crates/ui-egui/tests/it/control.rs +++ b/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 { + 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 { + 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] { diff --git a/crates/ui-egui/tests/it/editing.rs b/crates/ui-egui/tests/it/editing.rs index 73464d33e..8f42564c8 100644 --- a/crates/ui-egui/tests/it/editing.rs +++ b/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 { + 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); diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index b0a2ee804..2fc400539 100644 --- a/parity/acrobat-features.toml +++ b/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"