From 72d6655b28c174cfd6ed7e01ef6f6f5c46b2eb1e Mon Sep 17 00:00:00 2001 From: Marco Arnulfo <103679419+marcoarnulfo@users.noreply.github.com> Date: Thu, 15 Oct 2026 08:50:13 +0200 Subject: [PATCH] fix(forms): open a text field on press so dragging over it selects text (fixes #2213) (#2226) --- crates/ui-egui/src/editing.rs | 1 + crates/ui-egui/src/forms_ui.rs | 58 +++++++++++++++++++++-- crates/ui-egui/tests/forms.rs | 85 ++++++++++++++++++++++++++++++++++ 3 files changed, 141 insertions(+), 3 deletions(-) diff --git a/crates/ui-egui/src/editing.rs b/crates/ui-egui/src/editing.rs index 08c7a944e..608df92cb 100644 --- a/crates/ui-egui/src/editing.rs +++ b/crates/ui-egui/src/editing.rs @@ -1367,6 +1367,7 @@ mod revert_draft_tests { picked: Vec::new(), request_focus: false, select_all: false, + press: None, calendar: None, calendar_rect: None, } diff --git a/crates/ui-egui/src/forms_ui.rs b/crates/ui-egui/src/forms_ui.rs index bcaeac220..8284c55aa 100644 --- a/crates/ui-egui/src/forms_ui.rs +++ b/crates/ui-egui/src/forms_ui.rs @@ -31,6 +31,9 @@ pub struct Focus { pub request_focus: bool, /// Select the whole value when the editor opens (entered with Tab), so typing replaces it. pub select_all: bool, + /// Where the press that opened the editor landed: the editor puts its caret there and takes + /// over the drag, so press-and-drag selects text in a field that was not focused (#2213). + pub press: Option, /// Date fields: the month the calendar shows (year, month 1–12), and where it was drawn. pub calendar: Option<(i32, u32)>, pub calendar_rect: Option, @@ -42,6 +45,10 @@ pub struct FormView { /// The draft whose edit is queued (`DocView::pending_edit`): if the field refuses the value, /// the editor reopens with it rather than losing the typing. pub(crate) committed: Option, + /// A press on a text field committed the field before it (#2213). Until the button is + /// released, that field losing focus to the same gesture must not commit it again: a + /// refused value would be validated (and alerted) twice. + pub(crate) pressed: bool, /// Fields modified by the user whose appearance stream has not been rasterized by the render worker yet. /// Tracked as (field_name, page_index) so committed edits stay immediately visible without flicker. pub unbaked: HashSet<(String, usize)>, @@ -79,6 +86,7 @@ fn open_focus(f: &FormField, widget: usize) -> Focus { picked: f.value.clone(), request_focus: true, select_all: false, + press: None, calendar: None, calendar_rect: None, } @@ -128,6 +136,14 @@ pub(crate) fn page_input( (true, FormFieldKind::Text) => egui::CursorIcon::Text, _ => egui::CursorIcon::PointingHand, }); + // A text field opens on the press rather than the click, so a drag that starts on it + // selects its text (#2213). + let pressed = f.kind == FormFieldKind::Text && usable && resp.is_pointer_button_down_on() && ui.input(|i| i.pointer.primary_pressed()); + if pressed && view.forms.focus.as_ref().is_none_or(|x| x.name != f.name || x.widget != wi) { + commit(view, form); + view.forms.focus = Some(Focus { press: Some(p), ..open_focus(f, wi) }); + view.forms.pressed = true; + } if !resp.clicked() { return if resp.is_pointer_button_down_on() { FieldPointer::Owned } else { FieldPointer::Over }; } @@ -155,6 +171,16 @@ pub(crate) fn page_input( view.pending_edit = Some(Edit::SetFieldValue { name: f.name.clone(), value: FieldValue::Radio(value) }); } } + // The press already committed the previous field and opened this one (above), unless + // press and release came in one frame. Committing again would re-run a validation that + // refused the value on the press. + FormFieldKind::Text if !ui.input(|i| i.pointer.primary_pressed()) => { + // An editor opened by the press cannot be hit until a frame has laid it out: the + // release lands on the page and would take its focus away. + if let Some(x) = view.forms.focus.as_mut().filter(|x| x.name == f.name && x.widget == wi) { + x.request_focus = true; + } + } FormFieldKind::Text | FormFieldKind::Combo | FormFieldKind::List => { if view.forms.focus.as_ref().is_none_or(|x| x.name != f.name || x.widget != wi) { commit(view, form); @@ -372,6 +398,10 @@ fn paint_unbaked_widget(painter: &egui::Painter, xf: &PageXform, f: &FormField, /// The in-place editor or option list for the focused field. Returns an edit to apply. pub(crate) fn overlay(ctx: &egui::Context, view: &mut DocView, info: &DocInfo, form: &[FormField], today: (i64, u32, u32)) -> Option { + let pressed = view.forms.pressed; + if !ctx.input(|i| i.pointer.primary_down()) { + view.forms.pressed = false; + } let focus = view.forms.focus.clone()?; let Some(f) = form.iter().find(|f| f.name == focus.name) else { view.forms.focus = None; @@ -413,7 +443,24 @@ pub(crate) fn overlay(ctx: &egui::Context, view: &mut DocView, info: &DocInfo, f if multiline { te = te.desired_rows(((rect.height() / (font * 1.3)).floor() as usize).max(1)); } - let r = ui.add_sized(rect.size(), te); + let mut laid_out = None; + let r = ui.add_sized(rect.size(), |ui: &mut egui::Ui| { + let out = te.show(ui); + laid_out = Some((out.galley, out.galley_pos)); + out.response.response + }); + if let Some(press) = fx.press.take() + && let Some((galley, pos)) = laid_out + && let Some(mut st) = egui::TextEdit::load_state(ui.ctx(), r.id) + { + // The press went to the page, not to this editor, which did not exist yet. + let at = galley.cursor_from_pos(press - pos + vec2(galley.rect.left(), 0.0)); + st.cursor.set_char_range(Some(egui::text::CCursorRange::one(at))); + st.store(ui.ctx(), r.id); + if ui.input(|i| i.pointer.primary_down()) { + ui.ctx().set_dragged_id(r.id); + } + } if fx.request_focus { r.request_focus(); fx.request_focus = false; @@ -435,8 +482,13 @@ pub(crate) fn overlay(ctx: &egui::Context, view: &mut DocView, info: &DocInfo, f && (!multiline || !enter) && !fx.calendar_rect.is_some_and(|c| ui.input(|i| i.pointer.interact_pos().is_some_and(|p| c.contains(p)))) { - // Enter in a single-line field, or clicking elsewhere, commits. - commit(view, form); + if pressed { + // The field reopened with a refused value; keep it open and focused. + fx.request_focus = true; + } else { + // Enter in a single-line field, or clicking elsewhere, commits. + commit(view, form); + } } }); // Date fields: a calendar under the field (Acrobat's date picker). diff --git a/crates/ui-egui/tests/forms.rs b/crates/ui-egui/tests/forms.rs index 9fb675471..9cbed3ed7 100644 --- a/crates/ui-egui/tests/forms.rs +++ b/crates/ui-egui/tests/forms.rs @@ -122,6 +122,45 @@ fn tabbing_into_a_filled_field_selects_it_so_typing_replaces() { assert_eq!(value(&h, "city"), ["Lyon"]); } +#[test] +fn dragging_over_another_fields_text_focuses_it_and_selects_from_the_press() { + // Issue #2213: with the caret in one field, a drag over another field's text selected + // nothing; the field opened only on a click. + let mut h = harness(); + click_field(&mut h, "name", 0); + h.event(egui::Event::Text("Ada Lovelace".into())); + h.run_steps(1); + h.key_press(egui::Key::Enter); + h.run_steps(3); + click_field(&mut h, "city", 0); + assert_eq!(h.state().views[0].forms.focus.as_ref().map(|f| f.name.as_str()), Some("city")); + let (rect, id) = { + let s = h.state(); + let doc = s.session.get(s.views[0].id).unwrap(); + let f = doc.form.iter().find(|f| f.name == "name").unwrap(); + let rect = pdfcraft_ui_egui::forms_ui::field_screen_rect(&s.views[0], &doc.info, f, 0).expect("on screen"); + (rect, egui::Id::new(("form-field", s.views[0].id.0, "name", 0usize))) + }; + // From inside the first word to past the end of the text. + let (from, to) = (egui::pos2(rect.left() + 30.0, rect.center().y), egui::pos2(rect.right() - 8.0, rect.center().y)); + h.hover_at(from); + h.run_steps(1); + h.drag_at(from); + h.run_steps(1); + for k in 1..=4 { + h.hover_at(from.lerp(to, k as f32 / 4.0)); + h.run_steps(1); + } + h.drop_at(to); + h.run_steps(3); + assert_eq!(h.state().views[0].forms.focus.as_ref().map(|f| f.name.as_str()), Some("name"), "the press moved the focus"); + let n = "Ada Lovelace".chars().count(); + let range = egui::TextEdit::load_state(&h.ctx, id).and_then(|s| s.cursor.char_range()).expect("a selection in the field"); + // "Ad|a": not the caret at the end that a newly focused editor gets on its own. + assert_eq!(range.secondary.index.0, 2, "anchored where the drag started: {range:?}"); + assert_eq!(range.primary.index.0, n, "and extended to where it ended: {range:?}"); +} + #[test] fn date_fields_offer_a_calendar() { use egui_kittest::kittest::Queryable; @@ -396,6 +435,52 @@ fn validated_form() -> Vec { ]) } +#[test] +fn pressing_another_text_field_validates_the_refused_value_once() { + // The press commits `qty`; its release must not commit (and alert) a second time. + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(|_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("order.pdf", None, validated_form_with_note()).unwrap(); + app.set_option("left", "closed").unwrap(); + app.set_option("zoom", "150").unwrap(); + app + }); + h.run_steps(6); + click_field(&mut h, "qty", 0); + h.event(egui::Event::Text("500".into())); + h.run_steps(2); + let p = { + let s = h.state(); + let doc = s.session.get(s.views[0].id).unwrap(); + let f = doc.form.iter().find(|f| f.name == "note").unwrap(); + pdfcraft_ui_egui::forms_ui::field_screen_rect(&s.views[0], &doc.info, f, 0).expect("on screen").center() + }; + h.hover_at(p); + h.run_steps(1); + h.drag_at(p); + h.run_steps(2); + assert!(h.state().toast.is_some(), "the press committed 4500 and the script refused it"); + h.state_mut().toast = None; + h.drop_at(p); + h.run_steps(3); + assert_eq!(h.state().toast, None, "the release did not validate it again"); + let focus = h.state().views[0].forms.focus.clone().expect("the refused field stays open"); + assert_eq!((focus.name.as_str(), focus.text.as_str()), ("qty", "4500")); +} + +/// `validated_form` plus a plain text field, `note`, under `qty`. +fn validated_form_with_note() -> Vec { + pdf_from_objects(&[ + "<< /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [4 0 R 6 0 R] /DA (/Helv 0 Tf 0 g) /DR << /Font << /Helv 5 0 R >> >> >> >>", + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 300 400] >>", + "<< /Type /Page /Parent 2 0 R /Annots [4 0 R 6 0 R] >>", + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (qty) /V (4) /Rect [20 350 280 372] /P 3 0 R /F 4 /AA << /V << /S /JavaScript /JS (if (event.value > 100) { app.alert('At most 100'); event.rc = false; }) >> >> >>", + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>", + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (note) /Rect [20 300 280 322] /P 3 0 R /F 4 >>", + ]) +} + fn pdf_from_objects(objs: &[&str]) -> Vec { let mut out = b"%PDF-1.7\n".to_vec(); let mut offsets = Vec::new();