From f976adeb17248390699a227db1be6dd44734b41a Mon Sep 17 00:00:00 2001 From: Harrison Booth Date: Fri, 16 Oct 2026 11:20:11 -0500 Subject: [PATCH] M3: the "Save changes?" prompt opens on Save and moves between buttons by keyboard Save is chosen when the prompt opens, so Enter saves as before. The arrow keys 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. Before, Enter always saved, even with another button reached by Tab. Co-Authored-By: Claude Sonnet 5.5 --- ROADMAP.md | 1 + crates/ui-egui/src/dialogs.rs | 115 +++++++++++++++++++++++++++----- crates/ui-egui/src/editing.rs | 2 + crates/ui-egui/src/lib.rs | 5 +- crates/ui-egui/src/widgets.rs | 10 +++ crates/ui-egui/tests/editing.rs | 84 +++++++++++++++++++++++ parity/acrobat-features.toml | 4 +- 7 files changed, 203 insertions(+), 18 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 259dbfec0..db5b05aa5 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -157,6 +157,7 @@ Ranked; detail and the full milestone table in [docs/roadmap.md](docs/roadmap.md | 8 | 1.0 polish: performance, right-to-left, missing languages, keyboard-only (M14) | 55–95 | ## Progress log +- **2026-10-16 (M3, close prompt keyboard):** The "Save changes?" prompt opens with Save chosen (Enter saves, as before) and the arrow keys and Tab / Shift+Tab move the choice to Cancel or Don't save, shown with a focus ring; Enter or Space press the chosen button and Escape still cancels. Before, Enter always saved whatever was Tab-focused. Estimates unchanged. - **2026-10-15 (M2, gap 12):** `cargo xtask text-oracle` scores reading order as well as word F1: the longest common subsequence of the two word sequences over the longer one. F1 is a bag of words and cannot see order at all -- measuring #2431, a change that moves text and nothing else, it called 26 of the 29 files whose output changed identical, where the order score separated 18 improvements from 6 regressions. Reports, does not gate; a disagreement with Poppler, not a verdict. Estimates unchanged. - **2026-10-15 (M2, #2305 follow-up):** WinAnsiEncoding 0xA0 and 0xAD and MacRomanEncoding 0xCA extract as the ordinary space and hyphen Annex D.2 names them, in the vendored hayro tables (patch 30) as well as `crates/fonts`, so search, copy and redaction see characters that were hidden behind U+00A0 and U+00AD. Across the pdf.js corpus the soft hyphens we extract fall 117 to 13 and the non-breaking spaces 30 to 26; the rest come from `/ToUnicode`. An explicit `/Differences` name keeps its glyph list value, as pdftotext does. Estimates unchanged. - **2026-10-13 (community, M3, #1569 review):** Pixel-only Wayland wheel, wheel-tilt and continuous input use standalone moved events, so Ctrl zoom does not remain latched after the modifier is released (#991); vendored winit patch (13), with a policy test. Estimates unchanged. diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 2d2407629..05209b866 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -1438,21 +1438,74 @@ fn draft_changes(app: &PdfCraftApp) -> Option> { Some(edits) } -/// The "Save changes?" prompt's keys (issue #8), read before anything else can take them: Enter -/// saves (the default button) and Escape cancels; Don't save takes ⌘D / Ctrl+D, the macOS -/// convention, or Alt+D / Alt+N, the mnemonics Windows and Linux desktops use for it. -pub(crate) fn save_prompt_key(ctx: &egui::Context) -> Option> { +/// The buttons of the "Save changes?" prompt, in the order they sit on screen, left to right. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub(crate) enum PromptButton { + DontSave, + Cancel, + #[default] + Save, +} + +impl PromptButton { + const ORDER: [PromptButton; 3] = [PromptButton::DontSave, PromptButton::Cancel, PromptButton::Save]; + + fn position(self) -> usize { + Self::ORDER.iter().position(|b| *b == self).unwrap_or(2) + } + + /// The button `by` places to the right (negative: left). Arrow keys stop at the ends. + fn step(self, by: isize) -> PromptButton { + let to = self.position().saturating_add_signed(by).min(Self::ORDER.len() - 1); + Self::ORDER.get(to).copied().unwrap_or(self) + } + + /// The button after (or, with `back`, before) this one, wrapping round: Tab and Shift+Tab. + fn cycle(self, back: bool) -> PromptButton { + let n = Self::ORDER.len(); + let to = if back { self.position().checked_sub(1).unwrap_or(n - 1) } else { (self.position() + 1) % n }; + Self::ORDER.get(to).copied().unwrap_or(self) + } + + /// What pressing the button answers. + fn answer(self) -> Option { + match self { + PromptButton::Save => Some(true), + PromptButton::DontSave => Some(false), + PromptButton::Cancel => None, + } + } +} + +/// The "Save changes?" prompt's keys (issue #8), read before anything else can take them. Save is +/// the button chosen when the prompt opens, so Enter saves; the arrow keys and Tab / Shift+Tab move +/// the choice, and Enter or Space press the chosen button. Escape cancels. Don't save also has +/// ⌘D / Ctrl+D, the macOS convention, and Alt+D / Alt+N, the mnemonics Windows and Linux desktops +/// use for it. Returns the answer once one is given. +pub(crate) fn save_prompt_key(ctx: &egui::Context, chosen: &mut PromptButton) -> Option> { use egui::{Key, Modifiers}; ctx.input_mut(|i| { - if i.consume_key(Modifiers::NONE, Key::Enter) { - Some(Some(true)) - } else if i.consume_key(Modifiers::NONE, Key::Escape) { - Some(None) - } else if [(Modifiers::COMMAND, Key::D), (Modifiers::ALT, Key::D), (Modifiers::ALT, Key::N)].into_iter().any(|(m, k)| i.consume_key(m, k)) { - Some(Some(false)) - } else { - None + if i.consume_key(Modifiers::NONE, Key::Enter) || i.consume_key(Modifiers::NONE, Key::Space) { + return Some(chosen.answer()); } + if i.consume_key(Modifiers::NONE, Key::Escape) { + return Some(None); + } + if [(Modifiers::COMMAND, Key::D), (Modifiers::ALT, Key::D), (Modifiers::ALT, Key::N)].into_iter().any(|(m, k)| i.consume_key(m, k)) { + return Some(Some(false)); + } + // Shift+Tab before Tab: a key press matches a pattern that asks for fewer modifiers. + if i.consume_key(Modifiers::SHIFT, Key::Tab) { + *chosen = chosen.cycle(true); + } else if i.consume_key(Modifiers::NONE, Key::Tab) { + *chosen = chosen.cycle(false); + } + for (key, by) in [(Key::ArrowLeft, -1), (Key::ArrowUp, -1), (Key::ArrowRight, 1), (Key::ArrowDown, 1)] { + if i.consume_key(Modifiers::NONE, key) { + *chosen = chosen.step(by); + } + } + None }) } @@ -1492,14 +1545,15 @@ fn save_prompt(app: &mut PdfCraftApp, ctx: &egui::Context) { ui.label(egui::RichText::new(tl!("Your changes will be lost if you don't save them.")).color(t.text_muted)); ui.add_space(14.0); ui.with_layout(Layout::right_to_left(Align::Center), |ui| { - if widgets::pill_button(ui, tl!("Save"), true).clicked() { + let chosen = app.save_prompt_button; + if widgets::pill_button_marked(ui, tl!("Save"), true, chosen == PromptButton::Save).clicked() { choice = Some(Some(true)); } - if widgets::pill_button(ui, tl!("Cancel"), false).clicked() { + if widgets::pill_button_marked(ui, tl!("Cancel"), false, chosen == PromptButton::Cancel).clicked() { choice = Some(None); } ui.add_space(24.0); - if widgets::pill_button(ui, tl!("Don't save"), false).clicked() { + if widgets::pill_button_marked(ui, tl!("Don't save"), false, chosen == PromptButton::DontSave).clicked() { choice = Some(Some(false)); } }); @@ -1748,3 +1802,34 @@ fn unlock(app: &mut PdfCraftApp, ctx: &egui::Context) { None => {} } } + +#[cfg(test)] +mod prompt_button_tests { + use super::PromptButton::{self, Cancel, DontSave, Save}; + + #[test] + fn opens_on_save_and_arrows_stop_at_the_ends() { + assert_eq!(PromptButton::default(), Save); + assert_eq!(Save.step(1), Save); + assert_eq!(Save.step(-1), Cancel); + assert_eq!(Cancel.step(-1), DontSave); + assert_eq!(DontSave.step(-1), DontSave); + assert_eq!(DontSave.step(1000), Save, "a long step clamps"); + assert_eq!(Save.step(isize::MIN), DontSave, "and so does a huge negative one"); + } + + #[test] + fn tab_wraps_both_ways() { + assert_eq!(Save.cycle(false), DontSave); + assert_eq!(DontSave.cycle(true), Save); + assert_eq!(Cancel.cycle(false), Save); + assert_eq!(Cancel.cycle(true), DontSave); + } + + #[test] + fn each_button_answers_as_labelled() { + assert_eq!(Save.answer(), Some(true)); + assert_eq!(DontSave.answer(), Some(false)); + assert_eq!(Cancel.answer(), None); + } +} diff --git a/crates/ui-egui/src/editing.rs b/crates/ui-egui/src/editing.rs index be1347d68..45ce08221 100644 --- a/crates/ui-egui/src/editing.rs +++ b/crates/ui-egui/src/editing.rs @@ -806,6 +806,8 @@ impl PdfCraftApp { /// Answer the save prompt: `Some(true)` save, `Some(false)` discard, `None` cancel. pub fn resolve_close(&mut self, ctx: &egui::Context, choice: Option) { let Some(req) = self.close_request.take() else { return }; + // The next question (another dirty document) starts on Save again. + self.save_prompt_button = Default::default(); let index = match req { CloseRequest::Tab(id) => match self.views.iter().position(|v| v.id == id) { Some(i) => i, diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 79464624c..f0f0cf9dc 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -619,6 +619,8 @@ pub struct PdfCraftApp { pub host_dirty: Option, /// A pending "save changes?" question (closing a dirty tab or quitting). 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, /// A save waiting behind [`Dialog::OverwriteChanged`] (the web build has no files to check). #[cfg(not(target_arch = "wasm32"))] pub pending_overwrite: Option, @@ -962,6 +964,7 @@ impl PdfCraftApp { host_save: None, host_dirty: None, close_request: None, + save_prompt_button: Default::default(), #[cfg(not(target_arch = "wasm32"))] pending_overwrite: None, save_override: None, @@ -2771,7 +2774,7 @@ impl PdfCraftApp { // properties; Escape cancels it rather than clearing a selection), and nothing may run // underneath it. They are read here, before the canvas can consume them. if self.close_request.is_some() { - if let Some(choice) = dialogs::save_prompt_key(ctx) { + if let Some(choice) = dialogs::save_prompt_key(ctx, &mut self.save_prompt_button) { self.resolve_close(ctx, choice); } return; diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index 4f5aaa79b..51ff47781 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -130,6 +130,12 @@ pub fn mode_tab(ui: &mut egui::Ui, label: &str, active: bool) -> Response { /// Rounded pill button; `primary` fills with the accent. pub fn pill_button(ui: &mut egui::Ui, label: &str, primary: bool) -> Response { + pill_button_marked(ui, label, primary, false) +} + +/// [`pill_button`] that, when `marked`, is the keyboard's choice: it takes egui focus (so +/// assistive tech follows it) and shows a ring in the accent colour around the pill. +pub fn pill_button_marked(ui: &mut egui::Ui, label: &str, primary: bool, marked: bool) -> Response { let t = Tokens::get(ui.ctx()); let font = theme::medium(12.5); let w = ui.fonts_mut(|f| f.layout_no_wrap(label.to_owned(), font.clone(), t.text).size().x); @@ -142,6 +148,10 @@ pub fn pill_button(ui: &mut egui::Ui, label: &str, primary: bool) -> Response { (press.fill(t.card, t.hover, t.pressed), Stroke::new(1.0, t.border), t.text) }; ui.painter().rect(rect, CornerRadius::same(14), fill, stroke, egui::StrokeKind::Inside); + if marked { + resp.request_focus(); + ui.painter().rect_stroke(rect.expand(2.0), CornerRadius::same(16), Stroke::new(2.0, t.accent), egui::StrokeKind::Outside); + } ui.painter().text(rect.translate(press.offset()).center(), Align2::CENTER_CENTER, label, font, text); theme::hand(resp) } diff --git a/crates/ui-egui/tests/editing.rs b/crates/ui-egui/tests/editing.rs index f8db52575..965867095 100644 --- a/crates/ui-egui/tests/editing.rs +++ b/crates/ui-egui/tests/editing.rs @@ -596,6 +596,90 @@ fn the_save_prompt_answers_to_the_keyboard() { let _ = std::fs::remove_file(out); } +/// An organize harness with a dirty document and the "Save changes?" prompt open. +fn dirty_prompt() -> Harness<'static, PdfCraftApp> { + let mut h = organize(2); + h.get_by_label("Rotate clockwise").click(); + h.run_steps(3); + h.key_press_modifiers(Modifiers::COMMAND, Key::W); + h.run_steps(3); + h.get_by_label_contains("Save changes to “doc.pdf”"); + h +} + +/// The prompt has Save chosen on arrival; `keys` then move the choice and Enter or Space presses it. +fn press_after(keys: &[(Modifiers, Key)], activate: Key) -> Harness<'static, PdfCraftApp> { + let mut h = dirty_prompt(); + for (m, k) in keys { + h.key_press_modifiers(*m, *k); + h.run_steps(2); + } + h.key_press(activate); + h.run_steps(3); + h +} + +#[test] +fn the_save_prompt_moves_between_buttons_with_the_keyboard() { + let none = Modifiers::NONE; + // Left from Save is Cancel: Enter or Space presses it, so nothing closes and nothing saves. + for activate in [Key::Enter, Key::Space] { + let h = press_after(&[(none, Key::ArrowLeft)], activate); + assert_eq!(h.state().views.len(), 1, "{activate:?} on Cancel keeps the document"); + assert!(h.state().close_request.is_none()); + assert!(dirty(&h), "{activate:?} on Cancel doesn't save"); + } + // Two left is Don't save, which closes without saving; Shift+Tab walks the same way. + for keys in [ + vec![(none, Key::ArrowLeft), (none, Key::ArrowLeft)], + vec![(Modifiers::SHIFT, Key::Tab), (Modifiers::SHIFT, Key::Tab)], + vec![(none, Key::ArrowLeft), (none, Key::ArrowLeft), (none, Key::ArrowLeft)], + vec![(none, Key::Tab)], + ] { + let h = press_after(&keys, Key::Enter); + assert!(h.state().views.is_empty(), "{keys:?} reaches Don't save, which closes the tab"); + } + // Tab from Save wraps to Don't save; arrows stop at the right end, so Save stays chosen. + let out = temp_path("arrows-stop.pdf"); + let mut h = dirty_prompt(); + h.state_mut().save_override = Some(out.to_string_lossy().into_owned()); + h.key_press(Key::ArrowRight); + h.run_steps(2); + h.key_press(Key::Enter); + h.run_steps(3); + assert!(h.state().views.is_empty(), "Right from Save stays on Save, which saves"); + assert!(out.exists()); + let _ = std::fs::remove_file(out); +} + +#[test] +fn the_save_prompt_escape_still_cancels_whatever_is_chosen() { + let h = press_after(&[(Modifiers::NONE, Key::ArrowLeft), (Modifiers::NONE, Key::ArrowLeft)], Key::Escape); + assert_eq!(h.state().views.len(), 1, "Escape cancels even with Don't save chosen"); + assert!(dirty(&h)); +} + +#[test] +fn the_save_prompt_starts_on_save_every_time_it_opens() { + // Move to Don't save, cancel with Escape, and ask again: the choice is back on Save. + let out = temp_path("reopened.pdf"); + let mut h = dirty_prompt(); + h.state_mut().save_override = Some(out.to_string_lossy().into_owned()); + h.key_press(Key::ArrowLeft); + h.key_press(Key::ArrowLeft); + h.run_steps(2); + h.key_press(Key::Escape); + h.run_steps(3); + assert_eq!(h.state().views.len(), 1); + h.key_press_modifiers(Modifiers::COMMAND, Key::W); + h.run_steps(3); + h.key_press(Key::Enter); + h.run_steps(3); + assert!(h.state().views.is_empty(), "Enter saved and closed"); + assert!(out.exists(), "the second prompt opened on Save"); + let _ = std::fs::remove_file(out); +} + #[test] fn keys_leave_the_document_under_a_dialog_alone() { // Issue #870: ⌘W with Preferences open closed the file underneath it, and in the page grid diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index c3d319f94..49ad3192a 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/editing.rs::closing_a_dirty_tab_asks_and_cancel_keeps_it", "crates/ui-egui/tests/editing.rs::closing_a_dirty_tab_can_save_first", "crates/ui-egui/tests/editing.rs::quitting_with_unsaved_changes_asks_for_each_document", "crates/ui-egui/tests/editing.rs::save_prompt_stays_inside_the_screen_for_a_long_filename"] -notes = "" +evidence = ["crates/ui-egui/tests/editing.rs::closing_a_dirty_tab_asks_and_cancel_keeps_it", "crates/ui-egui/tests/editing.rs::closing_a_dirty_tab_can_save_first", "crates/ui-egui/tests/editing.rs::quitting_with_unsaved_changes_asks_for_each_document", "crates/ui-egui/tests/editing.rs::save_prompt_stays_inside_the_screen_for_a_long_filename", "crates/ui-egui/tests/editing.rs::the_save_prompt_moves_between_buttons_with_the_keyboard", "crates/ui-egui/tests/editing.rs::the_save_prompt_escape_still_cancels_whatever_is_chosen", "crates/ui-egui/tests/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)." [[feature]] id = "core.dirty-indicator"