From 201097793c010a181d113bdec7c5bf7ec81eacf9 Mon Sep 17 00:00:00 2001 From: "sorokin.oleg.edu" Date: Thu, 24 Sep 2026 17:45:13 -0700 Subject: [PATCH] Fix native reorder preview clipping and result lifetime --- crates/oa-desktop/src/app/scenario_reorder.rs | 28 ++- .../src/app/scenario_reorder/summary_tests.rs | 182 ++++++++++++++++++ .../src/app/scenario_reorder/tests.rs | 58 ++++++ .../src/behavior/scenario/reorder_view.rs | 66 +++++-- .../src/behavior/scenario/workspace.rs | 10 +- 5 files changed, 322 insertions(+), 22 deletions(-) create mode 100644 crates/oa-desktop/src/app/scenario_reorder/summary_tests.rs diff --git a/crates/oa-desktop/src/app/scenario_reorder.rs b/crates/oa-desktop/src/app/scenario_reorder.rs index 555626d8..ee106219 100644 --- a/crates/oa-desktop/src/app/scenario_reorder.rs +++ b/crates/oa-desktop/src/app/scenario_reorder.rs @@ -16,6 +16,7 @@ pub(crate) struct State { pub(crate) query: String, pub(crate) page: usize, pub(crate) summary: Option<(scenario::Context, String)>, + summary_selection: Option, } #[derive(Debug, Clone)] @@ -99,6 +100,7 @@ impl App { self.scenario_reorder.query.clear(); self.scenario_reorder.page = 0; self.scenario_reorder.summary = None; + self.scenario_reorder.summary_selection = None; } pub(crate) fn scenario_reorder_reconcile(&mut self) { @@ -106,9 +108,27 @@ impl App { .scenario_reorder .summary .as_ref() - .is_some_and(|(context, _)| self.scenario_context() != Some(*context)) + .is_some_and(|(context, _)| { + // A committed result is presentation, not a live authoring packet. + // Resizing diagnostics or moving the camera retires input epochs + // without changing the result's document, view or selection. + let same_result = self.scenario_context().is_some_and(|mut current| { + current.epoch = context.epoch; + current == *context + }); + let same_selection = self + .scenario_reorder + .summary_selection + .as_ref() + .zip(self.diagram_selection.as_ref()) + .is_some_and(|(snapshot, selected)| { + selected.matches(snapshot, snapshot.context()) + }); + !same_result || !same_selection + }) { self.scenario_reorder.summary = None; + self.scenario_reorder.summary_selection = None; } let Some(session) = self.scenario_reorder.session.as_ref() else { return; @@ -149,6 +169,7 @@ impl App { self.scenario_reorder.query.clear(); self.scenario_reorder.page = 0; self.scenario_reorder.summary = None; + self.scenario_reorder.summary_selection = None; match Session::new( self.document.snapshot().decoded.project.clone().into(), context, @@ -179,6 +200,7 @@ impl App { } if matches!(&event.intent, Intent::Cancel) { self.scenario_reorder_cancel(); + self.status = "Reorder cancelled. Sequence unchanged.".into(); return Task::none(); } if !self.scenario_reorder_enabled() { @@ -283,6 +305,8 @@ impl App { } self.scenario_reorder.summary = self.scenario_context().map(|context| (context, summary)); + self.scenario_reorder.summary_selection = + self.multiselect_snapshot(); self.status = if matches!( completion.presentation_status, PresentationStatus::Fallback { .. } @@ -309,5 +333,7 @@ impl App { } } +#[cfg(test)] +mod summary_tests; #[cfg(test)] mod tests; diff --git a/crates/oa-desktop/src/app/scenario_reorder/summary_tests.rs b/crates/oa-desktop/src/app/scenario_reorder/summary_tests.rs new file mode 100644 index 00000000..93f4555e --- /dev/null +++ b/crates/oa-desktop/src/app/scenario_reorder/summary_tests.rs @@ -0,0 +1,182 @@ +use super::*; +use crate::behavior::scenario::{CanvasEvent, Slot, direct_tests}; +use crate::behavior::scenario_layout::Representation; +use crate::diagram_usage::Target; +use oa_core::{Diagram, DiagramLayout, Layer, OccurrenceBody, ProjectionKind, ScenarioLayout}; + +fn committed() -> (App, Id, Id, oa_core::SourceBundle) { + let f = direct_tests::fixture(); + let mut project = f.project; + let mut extra = project + .make_element(Kind::Occurrence, Layer::Oa, "Tail", Some(f.owner)) + .unwrap(); + if let Some(oa_core::SharedBody::Occurrence { body, .. }) = extra.shared_mut() { + *body = OccurrenceBody::Action { + action: None, + lifeline: None, + }; + } + let extra_id = extra.id; + project.elements.insert(extra_id, extra); + project + .elements + .get_mut(&f.owner) + .unwrap() + .steps_mut() + .unwrap() + .push(extra_id); + let diagram = Id::new_v4(); + project.diagrams.insert( + diagram, + Diagram { + id: diagram, + alias: "summary_view".into(), + name: "Summary view".into(), + description: String::new(), + layer: Layer::Oa, + projection: ProjectionKind::Scenario { scenario: f.owner }, + nodes: Default::default(), + routes: vec![], + view_options: Default::default(), + layout: Some(DiagramLayout::Scenario(ScenarioLayout::default())), + }, + ); + let mut app = App::with_project(project); + let _ = app.update(Message::Select(f.owner)); + let _ = app.update(Message::Diagram(Choice { + id: diagram, + label: "Summary view".into(), + })); + for (id, choice) in [ + (f.call, multiselect::Choice::Replace), + (f.reply, multiselect::Choice::Add), + ] { + let event = selection::Event { + context: app.scenario_context().unwrap(), + snapshot: app.multiselect_snapshot().unwrap(), + intent: selection::Intent::Choose(Target::ScenarioObject(id), choice), + }; + let _ = app.update(Message::ScenarioCanvas(CanvasEvent::Selection(Box::new( + event, + )))); + } + viewport(&mut app, 700.); + let baseline = app.document.bundle().clone(); + dispatch(&mut app, Intent::Begin); + let slot = Slot { + parent: f.owner, + before: None, + }; + dispatch(&mut app, Intent::Choose(slot)); + dispatch(&mut app, Intent::Finish(slot)); + assert_eq!( + app.project().elements[&f.owner].steps().unwrap(), + &[extra_id, f.call, f.reply] + ); + assert!(app.scenario_reorder.summary.is_some(), "{}", app.status); + (app, f.owner, f.call, baseline) +} + +fn dispatch(app: &mut App, intent: Intent) { + let event = Event { + context: app.scenario_context().unwrap(), + snapshot: app.multiselect_snapshot().unwrap(), + token: app + .scenario_reorder + .session + .as_ref() + .map_or_else(Id::new_v4, Session::token), + intent, + }; + let _ = app.update(Message::ScenarioReorder(Box::new(event))); +} + +fn viewport(app: &mut App, height: f32) { + let context = app.scenario_context().unwrap(); + let _ = app.update(Message::ScenarioCanvas(CanvasEvent::Viewport( + context, + iced::Size::new(900., height), + ))); +} + +#[test] +fn scenario_reorder_summary_survives_diagnostics_resize_and_camera_then_undo_clears() { + let (mut app, _, _, baseline) = committed(); + let summary = app.scenario_reorder.summary.clone(); + let changed = app.document.bundle().clone(); + let epoch = app.canvas_epoch; + viewport(&mut app, 660.); + assert_ne!( + app.canvas_epoch, epoch, + "real diagnostics-height viewport change must retire input" + ); + assert_eq!(app.scenario_reorder.summary, summary); + let context = app.scenario_context().unwrap(); + let camera = app.behavior.scenario.camera_boundary(); + let _ = app.update(Message::ScenarioCanvas(CanvasEvent::Camera( + context, + app.camera_epoch, + crate::behavior::canvas_workspace::Event::Pan(iced::Vector::new(24., 12.)), + ))); + assert!(app.behavior.scenario.camera_boundary() != camera); + assert_eq!(app.scenario_reorder.summary, summary); + assert_eq!(app.document.bundle(), &changed); + let _ = app.update(Message::Undo); + assert!(app.scenario_reorder.summary.is_none()); + assert!(app.scenario_reorder.summary_selection.is_none()); + assert_eq!(app.document.bundle(), &baseline); +} + +#[test] +fn scenario_reorder_summary_retires_on_view_selection_and_new_operation() { + for action in 0..3 { + let (mut app, owner, call, _) = committed(); + let source = app.document.bundle().clone(); + match action { + 0 => { + let context = app.scenario_context().unwrap(); + let _ = app.update(Message::ScenarioCanvas(CanvasEvent::Representation( + context, + Representation::Derived { owner }, + ))); + assert_eq!( + app.scenario_context().unwrap().representation, + Representation::Derived { owner } + ); + } + 1 => { + let event = selection::Event { + context: app.scenario_context().unwrap(), + snapshot: app.multiselect_snapshot().unwrap(), + intent: selection::Intent::Collapse(Target::ScenarioObject(call)), + }; + let _ = app.update(Message::ScenarioCanvas(CanvasEvent::Selection(Box::new( + event, + )))); + } + _ => { + dispatch(&mut app, Intent::Begin); + assert!(app.scenario_reorder.session.is_some()); + dispatch(&mut app, Intent::Cancel); + assert!(app.scenario_reorder.session.is_none()); + assert_eq!(app.status, "Reorder cancelled. Sequence unchanged."); + } + } + assert!(app.scenario_reorder.summary.is_none()); + assert!(app.scenario_reorder.summary_selection.is_none()); + assert_eq!(app.document.bundle(), &source); + } +} + +#[test] +fn scenario_reorder_summary_selection_generation_aba_is_not_a_camera_change() { + let (mut app, _, call, _) = committed(); + let captured = app.multiselect_snapshot().unwrap(); + let selected = app.diagram_selection.as_mut().unwrap(); + assert!(selected.set_primary(&Target::ScenarioObject(call))); + assert!(selected.set_primary(captured.primary().unwrap())); + assert_eq!(selected.targets(), captured.targets()); + assert_eq!(selected.primary(), captured.primary()); + app.scenario_reorder_reconcile(); + assert!(app.scenario_reorder.summary.is_none()); +} diff --git a/crates/oa-desktop/src/app/scenario_reorder/tests.rs b/crates/oa-desktop/src/app/scenario_reorder/tests.rs index fbb86346..66a660b7 100644 --- a/crates/oa-desktop/src/app/scenario_reorder/tests.rs +++ b/crates/oa-desktop/src/app/scenario_reorder/tests.rs @@ -946,3 +946,61 @@ async fn mounted_same_batch_arrangement_picker_owns_ctrl_enter_before_app_observ assert!(app.scenario_reorder.session.is_none()); assert_eq!(app.document.bundle(), &baseline.bundle); } + +#[tokio::test] +async fn mounted_preview_columns_reserve_scrollbar_gutter_for_long_aliases() { + use crate::behavior::canvas_workspace::tests::Mounted; + use iced::advanced::widget; + let (mut app, owner, ids) = setup(true); + let mut project = app.project().clone(); + for (index, id) in ids.into_iter().enumerate() { + let element = project.elements.get_mut(&id).unwrap(); + element.name = "Repeated report awaiting engineering review".into(); + element.alias = format!("report_{}_{}", "unbroken_identifier_".repeat(8), index); + } + app.document = Document::from_project(project, Default::default()).unwrap(); + app.sync(); + app.window_size = iced::Size::new(1440., 1000.); + let mut mounted = Mounted::new(&mut app).await; + let start = packet(&app, Intent::Begin); + let task = app.update(Message::ScenarioReorder(Box::new(start))); + mounted.perform(&mut app, task).await; + choose( + &mut app, + Slot { + parent: owner, + before: None, + }, + ); + mounted.settle(&mut app).await; + let token = app.scenario_reorder.session.as_ref().unwrap().token(); + for offset in [0., 1000.] { + mounted.scroll(&app, "scenario-reorder-body", offset); + let geometry = mounted.geometry(&app); + let viewport = geometry.scrolls[&widget::Id::new("scenario-reorder-body")].0; + let before = geometry.bounds("scenario-reorder-before"); + let after = geometry.bounds("scenario-reorder-after"); + assert!( + (before.width - after.width).abs() < 1., + "equal allocated preview columns" + ); + assert!(before.width > 150. && before.width < 240.); + assert!(before.x >= viewport.x); + assert!(after.x >= before.x + before.width + 9.); + assert!( + after.x + after.width <= viewport.x + viewport.width - 8., + "After column must end before the reserved scrollbar gutter: {after:?} / {viewport:?}" + ); + let query = geometry + .focused + .iter() + .find(|(id, _, _)| id.as_ref() == Some(&scenario::reorder_view::query_id(token))) + .unwrap() + .1; + assert!(query.x + query.width <= viewport.x + viewport.width - 8.); + assert!( + before.height > 100. && after.height > 100., + "long aliases must wrap into readable multiple lines" + ); + } +} diff --git a/crates/oa-desktop/src/behavior/scenario/reorder_view.rs b/crates/oa-desktop/src/behavior/scenario/reorder_view.rs index ddb9f0b1..9d9b48bf 100644 --- a/crates/oa-desktop/src/behavior/scenario/reorder_view.rs +++ b/crates/oa-desktop/src/behavior/scenario/reorder_view.rs @@ -176,12 +176,17 @@ pub(super) fn panel(app: &App, viewport: Option) -> Option>(); let pages = slots.len().div_ceil(PAGE_SIZE).max(1); let page = state.page.min(pages - 1); - let mut choices = column![].spacing(4); + let mut choices = column![].spacing(4).width(Fill); for (slot, title) in slots.iter().skip(page * PAGE_SIZE).take(PAGE_SIZE) { choices = choices.push( - button(text(title.clone()).size(12)) - .width(Fill) - .on_press(message(session, Intent::Choose(*slot))), + button( + text(title.clone()) + .size(12) + .width(Fill) + .wrapping(iced::widget::text::Wrapping::WordOrGlyph), + ) + .width(Fill) + .on_press(message(session, Intent::Choose(*slot))), ); } if slots.is_empty() { @@ -194,7 +199,10 @@ pub(super) fn panel(app: &App, viewport: Option) -> Option) -> Option) -> Option reason.clone(), @@ -269,7 +295,11 @@ pub(super) fn panel(app: &App, viewport: Option) -> Option( for diagnostic in diagnostics.iter().take(3) { let explanation = match diagnostic { Diagnostic::SourceOrderCrossing { before, after, .. } => format!( - "Saved spacing places '{}' below '{}'. The Scenario order is unchanged.", - p.named(*before), - p.named(*after) + "Saved positions place @{} below @{}, contrary to execution order.", + p.elements + .get(before) + .map_or("?", |element| element.alias.as_str()), + p.elements + .get(after) + .map_or("?", |element| element.alias.as_str()) ), Diagnostic::FrameExtendsPastFollowingAnchor { frame, following } => format!( "Saved spacing makes '{}' overlap '{}'. Both keep their saved positions.",