From 48c6a85cee11b864c0e48c40a8818c0fc2139b8d Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Mon, 2 Nov 2026 10:47:28 +0800 Subject: [PATCH] M6: arrow keys in a same-name twin list box step through its own rows The focus now carries the focused field's object, so the overlay, the draft commit and the focus frame use the twin that has the focus rather than the first field with its name. The arrow keys pick by position through the same twin mapping a click uses (list_pick_edit), so an arrow in a twin listing its options in another order picks its own next row. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/ui-egui/src/editing.rs | 1 + crates/ui-egui/src/tools/forms_ui.rs | 68 +++++++++++++++++++--------- crates/ui-egui/tests/it/forms.rs | 8 ++++ 3 files changed, 55 insertions(+), 22 deletions(-) diff --git a/crates/ui-egui/src/editing.rs b/crates/ui-egui/src/editing.rs index 8def569f58..f18d1c64b2 100644 --- a/crates/ui-egui/src/editing.rs +++ b/crates/ui-egui/src/editing.rs @@ -937,6 +937,7 @@ mod revert_draft_tests { fn focus(text: &str) -> Focus { Focus { name: "name".into(), + obj: None, widget: 0, text: text.into(), picked: Vec::new(), diff --git a/crates/ui-egui/src/tools/forms_ui.rs b/crates/ui-egui/src/tools/forms_ui.rs index 8219ee3e5b..51a8270555 100644 --- a/crates/ui-egui/src/tools/forms_ui.rs +++ b/crates/ui-egui/src/tools/forms_ui.rs @@ -10,6 +10,7 @@ use std::collections::HashSet; use egui::{Color32, CornerRadius, Pos2, Rect, Stroke, vec2}; +use pdfcraft_cos::ObjRef; use pdfcraft_engine::{Edit, FieldValue, FormField, FormFieldKind, FormWidget, field_flags, form_appearance}; use pdfcraft_render::DocInfo; @@ -26,6 +27,10 @@ const FIELD_FILL: Color32 = Color32::from_rgb(0xFF, 0xFF, 0xF4); #[derive(Clone, Debug, PartialEq)] pub struct Focus { pub name: String, + /// The focused field's object. Same-name twins are one field (ISO 32000-2 ยง12.7.3.1) but come + /// as separate fields here, each with its own widgets and option order, and this says which + /// one has the focus. `None` goes by the name: the first field with it. + pub obj: Option, /// Which of the field's widgets (a field can appear on several pages). pub widget: usize, /// Text being typed (text fields). @@ -117,9 +122,38 @@ pub(crate) fn draft_text(f: &FormField) -> String { value } +impl Focus { + /// Whether this is the focus of widget `widget` of `f` (of that twin, for same-name fields). + fn is_on(&self, f: &FormField, widget: usize) -> bool { + self.name == f.name && self.widget == widget && self.obj.is_none_or(|o| o == f.obj) + } +} + +/// The field `focus` is on: the twin with its field object, or else the first field with its name. +pub(crate) fn focused_field<'a>(focus: &Focus, form: &'a [FormField]) -> Option<&'a FormField> { + focus.obj.and_then(|o| form.iter().find(|f| f.obj == o && f.name == focus.name)).or_else(|| form.iter().find(|f| f.name == focus.name)) +} + +/// The edit picking the options at `picked` (positions in `f`'s options) in the list box `f`. +/// The edit names the field, and positions are read in the first field of that name: a same-name +/// twin listing its options in another order gives its picks as positions there. One option +/// missing there goes by export value. +fn list_pick_edit(form: &[FormField], f: &FormField, picked: Vec) -> Edit { + let first = form.iter().find(|x| x.name == f.name).filter(|x| x.obj != f.obj); + let value = match first { + None => FieldValue::ChoiceIndices(picked), + Some(first) => match f.positions_in(first, &picked) { + Some(there) => FieldValue::ChoiceIndices(there), + None => FieldValue::Choice(picked.iter().filter_map(|&i| f.options.get(i)).map(|(e, _)| e.clone()).collect()), + }, + }; + Edit::SetFieldValue { name: f.name.clone(), value } +} + fn open_focus(f: &FormField, widget: usize) -> Focus { Focus { name: f.name.clone(), + obj: Some(f.obj), widget, text: draft_text(f), picked: f.value.clone(), @@ -206,7 +240,7 @@ pub(crate) fn page_input(cx: PageToolContext<'_>, form: &[FormField], allowed: b if f.kind == FormFieldKind::Text && usable && let Some(at) = ptr.pressed() - && view.forms.focus.as_ref().is_none_or(|x| x.name != f.name || x.widget != wi) + && view.forms.focus.as_ref().is_none_or(|x| !x.is_on(f, wi)) { commit(view, form); view.forms.focus = Some(Focus { press: Some(at), ..open_focus(f, wi) }); @@ -232,7 +266,7 @@ pub(crate) fn page_input(cx: PageToolContext<'_>, form: &[FormField], allowed: b FormFieldKind::CheckBox | FormFieldKind::Radio => { // The box takes the focus, as in Acrobat, so Tab goes on from it (#2223). The overlay // toggles it, after the edit for any text this commits. - if view.forms.focus.as_ref().is_none_or(|x| x.name != f.name || x.widget != wi) { + if view.forms.focus.as_ref().is_none_or(|x| !x.is_on(f, wi)) { commit(view, form); view.forms.focus = Some(open_focus(f, wi)); } @@ -244,12 +278,12 @@ pub(crate) fn page_input(cx: PageToolContext<'_>, form: &[FormField], allowed: b FormFieldKind::Text if ptr.pressed().is_none() => { // 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) { + if let Some(x) = view.forms.focus.as_mut().filter(|x| x.is_on(f, wi)) { x.request_focus = true; } } FormFieldKind::Text | FormFieldKind::Combo => { - if view.forms.focus.as_ref().is_none_or(|x| x.name != f.name || x.widget != wi) { + if view.forms.focus.as_ref().is_none_or(|x| !x.is_on(f, wi)) { commit(view, form); view.forms.focus = Some(open_focus(f, wi)); } @@ -295,18 +329,7 @@ pub(crate) fn page_input(cx: PageToolContext<'_>, form: &[FormField], allowed: b // A single-select list holding an array is written as one value even when its shown // row is clicked again. if picked != in_order || f.single_value_is_array { - // The edit names the field, and positions are read in the first field of that - // name: a same-name twin listing its options in another order gives its own - // picks as positions there. One option missing there goes by export value. - let first = form.iter().find(|x| x.name == f.name).filter(|x| x.obj != f.obj); - let value = match first { - None => FieldValue::ChoiceIndices(picked), - Some(first) => match f.positions_in(first, &picked) { - Some(there) => FieldValue::ChoiceIndices(there), - None => FieldValue::Choice(picked.iter().filter_map(|&i| f.options.get(i)).map(|(e, _)| e.clone()).collect()), - }, - }; - view.pending_edit = Some(Edit::SetFieldValue { name: f.name.clone(), value }); + view.pending_edit = Some(list_pick_edit(form, f, picked)); } } } @@ -316,7 +339,7 @@ pub(crate) fn page_input(cx: PageToolContext<'_>, form: &[FormField], allowed: b /// The edit the focused field's draft makes, if it differs from the field's value. pub(crate) fn draft_edit(focus: &Focus, form: &[FormField]) -> Option { - let f = form.iter().find(|f| f.name == focus.name)?; + let f = focused_field(focus, form)?; let value = match f.kind { FormFieldKind::Text if f.value.first().map(String::as_str).unwrap_or("") != focus.text => FieldValue::Text(focus.text.clone()), // Typed into an editable combo box: a shown text picks its option, anything else is the @@ -525,7 +548,7 @@ pub(crate) fn paint_page( for f in form { for (wi, w) in f.widgets.iter().enumerate().filter(|(_, w)| w.page == Some(page) && !w.hidden) { let r = widget_rect(xf, info, page, w.rect); - let focused = view.forms.focus.as_ref().is_some_and(|x| x.name == f.name && x.widget == wi); + let focused = view.forms.focus.as_ref().is_some_and(|x| x.is_on(f, wi)); if focused { painter.rect_stroke(r.expand(1.0), CornerRadius::same(2), Stroke::new(2.0, FOCUS_BLUE), egui::StrokeKind::Outside); } @@ -738,7 +761,7 @@ fn tab_to_next_field(view: &mut DocView, form: &[FormField], f: &FormField, widg .collect(); order.sort_by_key(|o| o.0); if let Some(pos) = - order.iter().position(|(_, x, wi)| x.name == f.name && *wi == widget).or_else(|| order.iter().position(|(_, x, _)| x.name == f.name)) + order.iter().position(|(_, x, wi)| x.obj == f.obj && *wi == widget).or_else(|| order.iter().position(|(_, x, _)| x.name == f.name)) { let k = if forward { (pos + 1) % order.len() } else { (pos + order.len() - 1) % order.len() }; let (_, target, wi) = order[k]; @@ -790,7 +813,7 @@ pub(crate) fn overlay( let toggle = std::mem::take(&mut view.forms.toggle); view.forms.keys = None; let focus = view.forms.focus.clone()?; - let Some(f) = form.iter().find(|f| f.name == focus.name) else { + let Some(f) = focused_field(&focus, form) else { view.forms.focus = None; return None; }; @@ -950,8 +973,9 @@ pub(crate) fn overlay( let current_idx = f.picked_in_order().last().copied().unwrap_or(0); let next_idx = if up { current_idx.saturating_sub(1) } else { current_idx.saturating_add(1).min(f.options.len().saturating_sub(1)) }; if next_idx != current_idx && next_idx < f.options.len() { - // By position, so the arrows step through options that share an export value. - edit = Some(Edit::SetFieldValue { name: f.name.clone(), value: FieldValue::ChoiceIndices(vec![next_idx]) }); + // By position, so the arrows step through options that share an export value, + // in the focused twin's own order. + edit = Some(list_pick_edit(form, f, vec![next_idx])); } } if clicked_outside() { diff --git a/crates/ui-egui/tests/it/forms.rs b/crates/ui-egui/tests/it/forms.rs index 781f23150b..91e06d9c0b 100644 --- a/crates/ui-egui/tests/it/forms.rs +++ b/crates/ui-egui/tests/it/forms.rs @@ -1389,6 +1389,14 @@ fn a_click_in_a_twin_list_box_with_options_in_another_order_picks_the_row_clicke click_at(&mut h, egui::pos2(r.center().x, r.min.y + 8.0 * zoom)); assert_eq!(value(&h, "dup"), ["1"]); assert_eq!(twins(&h), [vec![1], vec![0]], "One-B in both: second in the first, first in the twin"); + // The arrows step through the focused twin's own rows: โ†“ from One-B there is One-A, which is + // first in the first field, not Two (the row after One-B in the first field). + h.key_press(egui::Key::ArrowDown); + h.run_steps(3); + assert_eq!(twins(&h), [vec![0], vec![1]], "One-A in both after โ†“ in the twin"); + h.key_press(egui::Key::ArrowUp); + h.run_steps(3); + assert_eq!(twins(&h), [vec![1], vec![0]], "back to One-B after โ†‘ in the twin"); } /// ISO 32000-2 Table 233: a list box without MultiSelect selects "at most one item". Given