From 74989b6f254e54ac77b5daed13fb59987c284533 Mon Sep 17 00:00:00 2001 From: "sorokin.oleg.edu" Date: Sun, 6 Sep 2026 04:40:08 +0300 Subject: [PATCH] Keep nested data field names visible in compact pickers --- crates/oa-desktop/src/data_values.rs | 150 ++++++++++++++------- crates/oa-desktop/src/forms/data_fields.rs | 7 + 2 files changed, 112 insertions(+), 45 deletions(-) diff --git a/crates/oa-desktop/src/data_values.rs b/crates/oa-desktop/src/data_values.rs index 12736cad..64241d40 100644 --- a/crates/oa-desktop/src/data_values.rs +++ b/crates/oa-desktop/src/data_values.rs @@ -11,10 +11,11 @@ pub struct FieldPathKey { pub struct FieldPathChoice { pub key: FieldPathKey, pub label: String, + pub picker_label: String, } impl std::fmt::Display for FieldPathChoice { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.write_str(&self.label) + f.write_str(&self.picker_label) } } @@ -49,8 +50,7 @@ fn named_choice(project: &Project, entry: data::CarriedFieldPath) -> FieldPathCh .map(|e| format!("{} (@{})", e.name, e.alias)) .unwrap_or_else(|| id.to_string()) }) - .collect::>() - .join(" → "); + .collect::>(); let direction = if data::item_mechanism(project, entry.item) == Some(ExchangeMechanism::Operation) { match entry.direction { @@ -63,54 +63,83 @@ fn named_choice(project: &Project, entry: data::CarriedFieldPath) -> FieldPathCh } else { "" }; + let terminal = qualified + .last() + .expect("A field path always has a terminal"); + let owner_path = qualified[..qualified.len() - 1].join(" → "); + let picker_label = format!( + "{terminal}{direction} — {}{}", + project.named(entry.item), + if owner_path.is_empty() { + String::new() + } else { + format!(" / {owner_path}") + }, + ); FieldPathChoice { key: FieldPathKey { path: entry.path, field: entry.field, }, - label: format!("{} / {qualified}{direction}", project.named(entry.item)), + label: format!( + "{} / {}{direction}", + project.named(entry.item), + qualified.join(" → ") + ), + picker_label, } } +// A bounded picker must expose its distinguishing field before a long path. +// Keep the full address separately for the wrapping detail text. +fn sorted_choices(mut choices: Vec) -> Vec { + choices.sort_by_cached_key(|choice| choice.picker_label.to_lowercase()); + choices +} + pub fn choices( project: &Project, exchange: &Relation, phase: &MessageKind, ) -> Result, String> { - Ok(data::try_carried_field_paths(project, exchange)? - .into_iter() - .filter(|entry| phase_allows(project, entry, phase)) - .map(|entry| named_choice(project, entry)) - .collect()) + Ok(sorted_choices( + data::try_carried_field_paths(project, exchange)? + .into_iter() + .filter(|entry| phase_allows(project, entry, phase)) + .map(|entry| named_choice(project, entry)) + .collect(), + )) } /// Direct carried fields only: an unrelated deep branch must not prevent /// browsing a root field. Preserve the core resolver's first carried owner. pub fn roots(project: &Project, exchange: &Relation, phase: &MessageKind) -> Vec { let mut seen = std::collections::BTreeSet::new(); - exchange - .body - .items() - .iter() - .flat_map(|item| { - data::effective_fields(project, *item) - .into_iter() - .filter_map(|field| { - let SharedBody::DataField { direction, .. } = field.shared()? else { - return None; - }; - Some(data::CarriedFieldPath { - item: *item, - path: vec![], - field: field.id, - direction: *direction, + sorted_choices( + exchange + .body + .items() + .iter() + .flat_map(|item| { + data::effective_fields(project, *item) + .into_iter() + .filter_map(|field| { + let SharedBody::DataField { direction, .. } = field.shared()? else { + return None; + }; + Some(data::CarriedFieldPath { + item: *item, + path: vec![], + field: field.id, + direction: *direction, + }) }) - }) - }) - .filter(|entry| seen.insert(entry.field)) - .filter(|entry| phase_allows(project, entry, phase)) - .map(|entry| named_choice(project, entry)) - .collect() + }) + .filter(|entry| seen.insert(entry.field)) + .filter(|entry| phase_allows(project, entry, phase)) + .map(|entry| named_choice(project, entry)) + .collect(), + ) } /// Resolve authored finite paths as well as the finite representative picker list. @@ -166,20 +195,22 @@ pub fn children( } let mut path = key.path.clone(); path.push(key.field); - Ok(data::effective_fields(project, *record) - .into_iter() - .map(|field| { - named_choice( - project, - data::CarriedFieldPath { - item: entry.item, - path: path.clone(), - field: field.id, - direction: entry.direction, - }, - ) - }) - .collect()) + Ok(sorted_choices( + data::effective_fields(project, *record) + .into_iter() + .map(|field| { + named_choice( + project, + data::CarriedFieldPath { + item: entry.item, + path: path.clone(), + field: field.id, + direction: entry.direction, + }, + ) + }) + .collect(), + )) } #[cfg(test)] @@ -482,4 +513,33 @@ pub(crate) mod tests { .contains("128") ); } + #[test] + fn recursive_picker_starts_with_distinct_leaf_names_and_sorts_them() { + let (f, next) = recursive_payload(); + let exchange = &f.project.relations[&f.exchange]; + let context = FieldPathKey { + path: vec![f.origin, next, next], + field: next, + }; + let visible = children(&f.project, exchange, &MessageKind::Call, &context).unwrap(); + let leaf = visible + .iter() + .find(|choice| choice.key.field == f.leaf) + .unwrap(); + assert!(leaf.to_string().starts_with("latitude (@latitude)")); + assert!(leaf.label.contains("origin (@origin) → next (@next)")); + assert!( + visible.windows(2).all( + |pair| pair[0].to_string().to_lowercase() <= pair[1].to_string().to_lowercase() + ) + ); + assert_ne!( + visible[0].to_string().chars().take(24).collect::(), + visible[1].to_string().chars().take(24).collect::() + ); + assert_eq!( + resolve(&f.project, exchange, &MessageKind::Call, &leaf.key), + Some(leaf.clone()) + ); + } } diff --git a/crates/oa-desktop/src/forms/data_fields.rs b/crates/oa-desktop/src/forms/data_fields.rs index 97b8afbe..05f9f1e8 100644 --- a/crates/oa-desktop/src/forms/data_fields.rs +++ b/crates/oa-desktop/src/forms/data_fields.rs @@ -363,6 +363,10 @@ fn message_value<'a>( { out = out.push(text("Current address needs review. Choose an explicit path to resolve a legacy, ambiguous or incompatible value.").size(11).color(style::MUTED)); let retained = crate::data_values::FieldPathChoice { + picker_label: format!( + "{} · current address needs review", + project.named(key.field) + ), label: format!( "Current address · {} / {}", key.path @@ -377,6 +381,9 @@ fn message_value<'a>( selected = Some(retained.clone()); choices.push(retained); } + if let Some(choice) = &selected { + out = out.push(text(choice.label.clone()).size(11).color(style::MUTED)); + } let owned = path.to_owned(); out = out.push( pick_list(choices, selected, move |choice| {