aleleba/pdfcraft
1
0
Fork 0
mirror of https://github.com/storytold/pdfcraft.git synced 2026-11-02 22:41:47 -06:00
Code Issues Packages Projects Releases Wiki Activity

M8: typing the last page of a range keeps what is typed (#5574) (#5579)

Browse Source 
* M8: typing the last page of a range keeps what is typed (#5574)

The page-range dialogs (Redact pages, Rotate pages, Duplicate field,
Number pages) raised the end to the start on every frame, so the
half-typed "1" of "12" became the start before the "2" arrived.
The end is now ordered only once its field is no longer being typed
into. Number pages no longer lowers the start to a half-typed end, and
its preview cannot underflow while the end is below the start.

* M8: a test types the last page of Rotate Pages' range (#5574)

Rotate Pages' From and to fields are labelled, and a test types "12" after a
first page of 5 (pages 5-12 are turned) and "3" left by clicking OK (page 5
alone), as croquete1 asked on #5579.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GbuSKAS2k5248LGFEapMUR

* M8: page ranges stay typed while the window is in the background, and OK orders them (#5574)

Review follow-up. The end field's typing check used Response::has_focus,
which is false while another window is in front, so switching apps
halfway through "12" still raised the typed "1" to the start. It now
asks egui's memory (widgets::typing_into). OK pressed while the end is
still focused (a screen reader's click) orders the range first in Rotate
pages, Duplicate field and Redact pages, and Number pages builds its edit
from the ordered end, so a reversed range never reaches the edit.

---------

Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
Livio Oswald
2026-11-01 15:05:01 -05:00
committed by GitHub
co-authored by Claude
parent 901050a31f
commit eafe9e2c0b
4 changed files with 198 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/ui-egui/src/dialogs.rs
+30 -19
View File
@@ -1086,6 +1086,7 @@ fn dialog_rotate_pages(ui: &mut egui::Ui, app: &mut PdfCraftApp, req: &mut Pendi
use pdfcraft_engine::{PageOrientation as O, PageParity as P};
let n = app.active_ids().and_then(|(_, id)| app.session.get(id)).map_or(1, |d| d.info.pages.len());
let d = &mut app.rotate_draft;
let mut typing_to = false;
ui.label(egui::RichText::new(tl!("Rotate Pages")).font(theme::semibold(18.0)));
ui.add_space(8.0);
egui::Grid::new("rotate-pages").num_columns(2).spacing([12.0, 8.0]).show(ui, |ui| {
@@ -1107,10 +1108,11 @@ fn dialog_rotate_pages(ui: &mut egui::Ui, app: &mut PdfCraftApp, req: &mut Pendi
widgets::radio_value(ui, &mut d.which, 0, tl!("All"));
widgets::radio_value(ui, &mut d.which, 1, tl!("Selection"));
ui.horizontal(|ui| {
widgets::radio_value(ui, &mut d.which, 2, tl!("From"));
ui.add_enabled(d.which == 2, egui::DragValue::new(&mut d.from).range(1..=n));
ui.label(tl!("to"));
ui.add_enabled(d.which == 2, egui::DragValue::new(&mut d.to).range(1..=n));
let from_label = widgets::radio_value(ui, &mut d.which, 2, tl!("From"));
ui.add_enabled(d.which == 2, egui::DragValue::new(&mut d.from).range(1..=n)).labelled_by(from_label.id);
let to_label = ui.label(tl!("to"));
typing_to =
widgets::typing_into(&ui.add_enabled(d.which == 2, egui::DragValue::new(&mut d.to).range(1..=n)).labelled_by(to_label.id));
ui.label(crate::i18n::fmt(tl!("of {name}"), &[("name", &n.to_string())]));
});
});
@@ -1142,10 +1144,12 @@ fn dialog_rotate_pages(ui: &mut egui::Ui, app: &mut PdfCraftApp, req: &mut Pendi
});
ui.end_row();
});
d.to = d.to.max(d.from);
widgets::keep_range_ordered(d.from, &mut d.to, typing_to);
ui.add_space(12.0);
ui.with_layout(Layout::right_to_left(Align::Center), |ui| {
if widgets::pill_button(ui, tl!("OK"), true).clicked() {
// OK while the end is still being typed (a screen reader can press it).
widgets::keep_range_ordered(d.from, &mut d.to, false);
req.rotate_now = true;
req.close = true;
}
@@ -1167,17 +1171,21 @@ fn dialog_duplicate_field(ui: &mut egui::Ui, app: &mut PdfCraftApp, req: &mut Pe
ui.add_space(8.0);
ui.label(crate::i18n::fmt(tl!("Duplicate \"{name}\" onto:"), &[("name", &d.name)]));
widgets::radio_value(ui, &mut d.all, true, tl!("All pages"));
ui.horizontal(|ui| {
widgets::radio_value(ui, &mut d.all, false, tl!("From"));
ui.add_enabled(!d.all, egui::DragValue::new(&mut d.from).range(1..=n));
ui.label(tl!("to"));
ui.add_enabled(!d.all, egui::DragValue::new(&mut d.to).range(1..=n));
ui.label(crate::i18n::fmt(tl!("of {name}"), &[("name", &n.to_string())]));
});
d.to = d.to.max(d.from);
let typing_to = ui
.horizontal(|ui| {
widgets::radio_value(ui, &mut d.all, false, tl!("From"));
ui.add_enabled(!d.all, egui::DragValue::new(&mut d.from).range(1..=n));
ui.label(tl!("to"));
let typing_to = widgets::typing_into(&ui.add_enabled(!d.all, egui::DragValue::new(&mut d.to).range(1..=n)));
ui.label(crate::i18n::fmt(tl!("of {name}"), &[("name", &n.to_string())]));
typing_to
})
.inner;
widgets::keep_range_ordered(d.from, &mut d.to, typing_to);
ui.add_space(12.0);
ui.with_layout(Layout::right_to_left(Align::Center), |ui| {
if widgets::pill_button(ui, tl!("OK"), true).clicked() {
widgets::keep_range_ordered(d.from, &mut d.to, false);
let pages: Vec<usize> = if d.all { (0..n).collect() } else { (d.from - 1..d.to.min(n)).collect() };
req.duplicate_now = Some(Edit::DuplicateField { name: d.name.clone(), pages });
req.close = true;
@@ -1376,13 +1384,14 @@ fn dialog_number_pages(ui: &mut egui::Ui, app: &mut PdfCraftApp, req: &mut Pendi
let n = app.session.get(id).map(|d| d.info.pages.len()).unwrap_or(1).max(1);
let d = &mut app.number_draft;
d.to = d.to.clamp(1, n);
d.from = d.from.clamp(1, d.to);
d.from = d.from.clamp(1, n);
let mut typing_to = false;
egui::Grid::new("number_pages").num_columns(2).spacing([12.0, 8.0]).show(ui, |ui| {
ui.label(tl!("Pages"));
ui.horizontal(|ui| {
ui.add(egui::DragValue::new(&mut d.from).range(1..=n));
ui.label(tl!("to"));
ui.add(egui::DragValue::new(&mut d.to).range(1..=n));
typing_to = widgets::typing_into(&ui.add(egui::DragValue::new(&mut d.to).range(1..=n)));
ui.label(egui::RichText::new(crate::i18n::fmt(tl!("of {name}"), &[("name", &n.to_string())])).color(t.text_muted));
});
ui.end_row();
@@ -1409,18 +1418,20 @@ fn dialog_number_pages(ui: &mut egui::Ui, app: &mut PdfCraftApp, req: &mut Pendi
ui.add(egui::DragValue::new(&mut d.start).range(1..=99_999));
ui.end_row();
});
d.to = d.to.max(d.from);
widgets::keep_range_ordered(d.from, &mut d.to, typing_to);
let label = |k: u32| format!("{}{}", d.prefix, d.style.format(k));
ui.add_space(8.0);
let preview = if d.from == d.to {
// `to` can be below `from` while it is being typed.
let preview = if d.to <= d.from {
label(d.start)
} else {
format!("{}, {} … {}", label(d.start), label(d.start + 1), label(d.start + (d.to - d.from) as u32))
format!("{}, {} … {}", label(d.start), label(d.start + 1), label(d.start.saturating_add(u32::try_from(d.to - d.from).unwrap_or(u32::MAX))))
};
ui.label(
egui::RichText::new(crate::i18n::fmt(tl!("Labels: {preview}. Later pages keep their labels."), &[("preview", &preview)])).color(t.text_muted),
);
req.number_now = Some(Edit::NumberPages { from: d.from - 1, to: d.to - 1, style: d.style, prefix: d.prefix.clone(), first: d.start });
// Ordered here too: the footer's OK can be pressed while the end is still being typed.
req.number_now = Some(Edit::NumberPages { from: d.from - 1, to: d.to.max(d.from) - 1, style: d.style, prefix: d.prefix.clone(), first: d.start });
true
}
crates/ui-egui/src/tools/redact_ui.rs
+120 -9
View File
@@ -360,16 +360,24 @@ pub(crate) fn pages_body(ui: &mut egui::Ui, d: &mut PagesDraft, pages: usize, _t
ui.label(egui::RichText::new(tl!("Mark Page Range")).font(crate::theme::semibold(18.0)));
ui.add_space(8.0);
widgets::radio_value(ui, &mut d.current, true, tl!("Current page"));
ui.horizontal(|ui| {
widgets::radio_value(ui, &mut d.current, false, tl!("Pages from"));
ui.add_enabled(!d.current, egui::DragValue::new(&mut d.from).range(1..=pages));
ui.label(tl!("to"));
ui.add_enabled(!d.current, egui::DragValue::new(&mut d.to).range(1..=pages));
ui.label(crate::i18n::fmt(tl!("of {name}"), &[("name", &pages.to_string())]));
});
d.to = d.to.max(d.from);
let to_field = ui
.horizontal(|ui| {
let from_label = widgets::radio_value(ui, &mut d.current, false, tl!("Pages from"));
ui.add_enabled(!d.current, egui::DragValue::new(&mut d.from).range(1..=pages)).labelled_by(from_label.id);
let to_label = ui.label(tl!("to"));
let to_field = ui.add_enabled(!d.current, egui::DragValue::new(&mut d.to).range(1..=pages)).labelled_by(to_label.id);
ui.label(crate::i18n::fmt(tl!("of {name}"), &[("name", &pages.to_string())]));
to_field
})
.inner;
widgets::keep_range_ordered(d.from, &mut d.to, widgets::typing_into(&to_field));
ui.add_space(12.0);
buttons(ui, "OK", true)
let (ok, cancel) = buttons(ui, "OK", true);
if ok {
// OK while the end is still being typed (a screen reader can press it).
widgets::keep_range_ordered(d.from, &mut d.to, false);
}
(ok, cancel)
}
/// Find text and redact. Returns (search, cancel, import a word list).
@@ -675,3 +683,106 @@ pub(crate) fn sanitize_body(ui: &mut egui::Ui, t: &Tokens) -> (bool, bool) {
ui.add_space(12.0);
buttons(ui, "Sanitize", true)
}
#[cfg(test)]
mod tests {
use super::*;
use egui::accesskit::Role;
use egui_kittest::kittest::Queryable;
fn key(key: egui::Key, modifiers: egui::Modifiers) -> egui::Event {
egui::Event::Key { key, physical_key: None, pressed: true, repeat: false, modifiers }
}
/// #5574: typing "12" into the end of 5 to 5 came out as 5. The end was raised to the start
/// after every keystroke, so the half-typed "1" was replaced before the "2" arrived.
#[test]
fn typing_the_last_page_keeps_what_is_typed() {
let mut fonts = false;
let mut h = egui_kittest::Harness::new_ui_state(
move |ui, d: &mut PagesDraft| {
// The app's fonts (the semibold heading) take effect from the next frame.
if !fonts {
ui.ctx().set_fonts(crate::theme::font_definitions());
fonts = true;
return;
}
let t = Tokens::get(ui.ctx());
pages_body(ui, d, 20, &t);
},
PagesDraft { current: false, from: 5, to: 5 },
);
h.run();
for (typed, expected) in [("12", 12), ("3", 5)] {
h.get_by_role_and_label(Role::SpinButton, "to").click();
h.run();
h.event(key(egui::Key::A, egui::Modifiers::COMMAND));
for c in typed.chars() {
h.event(egui::Event::Text(c.to_string()));
h.run_steps(2);
}
h.event(key(egui::Key::Enter, egui::Modifiers::NONE));
h.run_steps(2);
assert_eq!(*h.state(), PagesDraft { current: false, from: 5, to: expected }, "typed {typed:?}");
}
}
/// The dialog with its OK result kept: (draft, OK pressed).
fn dialog() -> egui_kittest::Harness<'static, (PagesDraft, bool)> {
let mut fonts = false;
let mut h = egui_kittest::Harness::new_ui_state(
move |ui, (d, ok): &mut (PagesDraft, bool)| {
if !fonts {
ui.ctx().set_fonts(crate::theme::font_definitions());
fonts = true;
return;
}
let t = Tokens::get(ui.ctx());
*ok |= pages_body(ui, d, 20, &t).0;
},
(PagesDraft { current: false, from: 5, to: 5 }, false),
);
h.run();
h
}
fn type_last_page(h: &mut egui_kittest::Harness<'static, (PagesDraft, bool)>, typed: &str) {
h.get_by_role_and_label(Role::SpinButton, "to").click();
h.run();
h.event(key(egui::Key::A, egui::Modifiers::COMMAND));
for c in typed.chars() {
h.event(egui::Event::Text(c.to_string()));
h.run_steps(2);
}
}
/// OK pressed while the end is still being typed (a screen reader's click leaves the field
/// focused) orders the range, so a reversed one never reaches the edit.
#[test]
fn ok_while_the_last_page_is_typed_orders_the_range() {
let mut h = dialog();
type_last_page(&mut h, "3");
assert_eq!(h.state().0.to, 3, "still typing");
h.get_by_label("OK").click_accesskit();
h.run_steps(2);
assert_eq!(*h.state(), (PagesDraft { current: false, from: 5, to: 5 }, true));
}
/// Switching to another window halfway through "12" leaves the typed "1" alone: the field
/// still has the keyboard when the window comes back.
#[test]
fn switching_windows_mid_number_keeps_what_is_typed() {
let mut h = dialog();
type_last_page(&mut h, "1");
h.input_mut().focused = false;
h.run_steps(2);
assert_eq!(h.state().0.to, 1, "the typed 1 stays while the window is in the background");
h.input_mut().focused = true;
h.run_steps(2);
h.event(egui::Event::Text("2".into()));
h.run_steps(2);
h.event(key(egui::Key::Enter, egui::Modifiers::NONE));
h.run_steps(2);
assert_eq!(h.state().0.to, 12);
}
}
crates/ui-egui/src/widgets.rs
+17
View File
@@ -254,6 +254,23 @@ pub fn radio_value<'a, V: PartialEq>(ui: &mut egui::Ui, current: &mut V, alterna
response
}
/// Keeps the end of a "from … to …" page range at or after its start, but not while `typing` into the
/// end's `DragValue` ([`typing_into`]): the half-typed "1" of "12" is below `from`, and raising it
/// then replaced what the user was typing with `from` (#5574). A dialog's OK orders it whatever has
/// the keyboard (`typing` false), so a range left reversed never reaches the edit.
pub fn keep_range_ordered(from: usize, to: &mut usize, typing: bool) {
if !typing {
*to = (*to).max(from);
}
}
/// Whether `field` is being typed into: it has the keyboard focus in egui's memory. Unlike
/// `Response::has_focus` this stays true while the window is in the background, so switching to
/// another app mid-number doesn't end the typing.
pub fn typing_into(field: &Response) -> bool {
field.ctx.memory(|m| m.has_focus(field.id))
}
/// The box of a checkbox or the circle of a radio button. egui draws it with the widgets' visuals
/// and has no style of its own for it, so it is set in a scope around the widget, keeping what
/// the caller set (dialogs and the search panel already outline theirs):
crates/ui-egui/tests/it/editing.rs
+31
View File
@@ -3135,6 +3135,37 @@ fn extract_options_and_rotate_pages_dialog() {
assert_eq!(d.info.pages.iter().map(|p| p.rotation).collect::<Vec<_>>(), [90, 0]);
}
/// #5579: typing the last page of Rotate Pages' range keeps what is typed (croquete1): "12" after
/// a first page of 5 rotates pages 5–12, not page 5 alone. An end typed below the start and left
/// by clicking OK is raised to the start as the field lets go: page 5, not nothing.
#[test]
fn rotate_pages_keeps_a_typed_last_page() {
use egui::accesskit::Role;
for (typed, rotated) in [("12", 5..13), ("3", 5..6)] {
let mut h = organize(20);
h.state_mut().run_command("page.rotate_dialog");
h.run_steps(2);
let d = &mut h.state_mut().rotate_draft;
(d.which, d.from, d.to) = (2, 5, 5);
h.run_steps(1);
h.get_by_role_and_label(Role::SpinButton, "to").click();
h.run_steps(2);
// As events, not `key_press`: its `run` checks the dialog's unnamed combo boxes.
h.event(egui::Event::Key { key: egui::Key::A, physical_key: None, pressed: true, repeat: false, modifiers: egui::Modifiers::COMMAND });
for c in typed.chars() {
h.event(egui::Event::Text(c.to_string()));
h.run_steps(2);
}
// OK at once, the end still being typed.
h.get_by_label("OK").click();
h.run_steps(3);
let s = h.state();
let d = s.session.get(s.views[0].id).unwrap();
let turned: Vec<usize> = d.info.pages.iter().enumerate().filter(|(_, p)| p.rotation != 0).map(|(i, _)| i + 1).collect();
assert_eq!(turned, rotated.collect::<Vec<usize>>(), "typed {typed:?}");
}
}
#[test]
fn dragging_thumbnails_reorders_pages() {
let mut h = organize(4);