Fix native reorder preview clipping and result lifetime

This commit is contained in:
2026-09-24 17:45:13 -07:00
parent 14df924b98
commit 201097793c
5 changed files with 322 additions and 22 deletions
+27 -1
View File
@@ -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<Snapshot>,
}
#[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;
@@ -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());
}
@@ -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"
);
}
}
@@ -176,12 +176,17 @@ pub(super) fn panel(app: &App, viewport: Option<Size>) -> Option<Element<'_, Mes
.collect::<Vec<_>>();
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<Size>) -> Option<Element<'_, Mes
session.snapshot().targets().len()
))
.size(17),
text(format!("Sequence: {}", label(p, session.parent()))).size(12),
text(format!("Sequence: {}", label(p, session.parent())))
.size(12)
.width(Fill)
.wrapping(iced::widget::text::Wrapping::WordOrGlyph),
text("Changes execution order; saved positions stay as authored.").size(12),
text_input("Find destination by name, alias or position", &state.query)
.id(query_id(session.token()))
@@ -214,7 +222,8 @@ pub(super) fn panel(app: &App, viewport: Option<Size>) -> Option<Element<'_, Mes
]
.spacing(8),
]
.spacing(8);
.spacing(8)
.width(Fill);
let mut apply = None;
let mut footer =
column![text("Enter previews a unique match. Ctrl+Enter applies; Esc cancels.").size(12)]
@@ -225,21 +234,38 @@ pub(super) fn panel(app: &App, viewport: Option<Size>) -> Option<Element<'_, Mes
"Destination: {}",
destination(p, session.before(), preview.slot)
))
.size(13),
.size(13)
.width(Fill)
.wrapping(iced::widget::text::Wrapping::WordOrGlyph),
);
let comparison = row![
column![
text("Before").size(13),
text(order(p, &preview.before, session.snapshot())).size(12)
]
.width(Fill),
column![
text("After • selected").size(13),
text(order(p, &preview.after, session.snapshot())).size(12)
]
container(
column![
text("Before").size(13),
text(order(p, &preview.before, session.snapshot()))
.size(12)
.width(Fill)
.wrapping(iced::widget::text::Wrapping::WordOrGlyph)
]
.width(Fill)
)
.width(Fill)
.id("scenario-reorder-before"),
container(
column![
text("After • selected").size(13),
text(order(p, &preview.after, session.snapshot()))
.size(12)
.width(Fill)
.wrapping(iced::widget::text::Wrapping::WordOrGlyph)
]
.width(Fill)
)
.width(Fill)
.id("scenario-reorder-after")
]
.spacing(10);
.spacing(10)
.width(Fill);
body = body.push(comparison);
let feedback = match &preview.outcome {
reorder_session::Outcome::Refused(reason) => reason.clone(),
@@ -269,7 +295,11 @@ pub(super) fn panel(app: &App, viewport: Option<Size>) -> Option<Element<'_, Mes
}
let height = viewport.map_or(500., |size| (size.height - 32.).max(100.));
let card = column![
scrollable(body).height(Fill),
scrollable(body)
.height(Fill)
.width(Fill)
.spacing(8)
.id("scenario-reorder-body"),
footer,
row![
button("Cancel (Esc)").on_press(message(session, Intent::Cancel)),
@@ -790,9 +790,13 @@ fn display_notice_parts<'a>(
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.",