mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-10-30 04:58:34 -06:00
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5.5
parent
f0c9901c39
commit
f976adeb17
@@ -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.
|
||||
|
||||
+100
-15
@@ -1438,21 +1438,74 @@ fn draft_changes(app: &PdfCraftApp) -> Option<Vec<Edit>> {
|
||||
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<Option<bool>> {
|
||||
/// 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<bool> {
|
||||
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<Option<bool>> {
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<bool>) {
|
||||
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,
|
||||
|
||||
@@ -619,6 +619,8 @@ pub struct PdfCraftApp {
|
||||
pub host_dirty: Option<HostDirtyFn>,
|
||||
/// A pending "save changes?" question (closing a dirty tab or quitting).
|
||||
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,
|
||||
/// 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>,
|
||||
@@ -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;
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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"
|
||||
|
||||
Reference in New Issue
Block a user