diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index aec79cf4f5..42bed83657 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -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 = 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 } diff --git a/crates/ui-egui/src/tools/redact_ui.rs b/crates/ui-egui/src/tools/redact_ui.rs index 96b544b4d0..e55dd8c5c3 100644 --- a/crates/ui-egui/src/tools/redact_ui.rs +++ b/crates/ui-egui/src/tools/redact_ui.rs @@ -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); + } +} diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index d8267caa9f..5e33f9cd9a 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -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): diff --git a/crates/ui-egui/tests/it/editing.rs b/crates/ui-egui/tests/it/editing.rs index 12935b6d0f..7b245a5a9e 100644 --- a/crates/ui-egui/tests/it/editing.rs +++ b/crates/ui-egui/tests/it/editing.rs @@ -3135,6 +3135,37 @@ fn extract_options_and_rotate_pages_dialog() { assert_eq!(d.info.pages.iter().map(|p| p.rotation).collect::>(), [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 = d.info.pages.iter().enumerate().filter(|(_, p)| p.rotation != 0).map(|(i, _)| i + 1).collect(); + assert_eq!(turned, rotated.collect::>(), "typed {typed:?}"); + } +} + #[test] fn dragging_thumbnails_reorders_pages() { let mut h = organize(4);