diff --git a/crates/oa-desktop/src/app.rs b/crates/oa-desktop/src/app.rs index 2e29c001..e4a10bc7 100644 --- a/crates/oa-desktop/src/app.rs +++ b/crates/oa-desktop/src/app.rs @@ -3,6 +3,8 @@ pub(crate) mod architecture_authoring; #[cfg(test)] mod architecture_authoring_tests; mod checkpoints; +#[cfg(test)] +mod diagram_focus_tests; pub(crate) mod diagram_removal; #[cfg(test)] mod diagram_removal_tests; diff --git a/crates/oa-desktop/src/app/architecture_authoring/tests.rs b/crates/oa-desktop/src/app/architecture_authoring/tests.rs index 47a48055..564c2dc2 100644 --- a/crates/oa-desktop/src/app/architecture_authoring/tests.rs +++ b/crates/oa-desktop/src/app/architecture_authoring/tests.rs @@ -89,17 +89,8 @@ fn architecture_f2_routes_selected_node_to_inline_rename_and_exact_undo() { text: None, repeat: false, }); - assert!( - canvas - .update(&mut state, &key, bounds, mouse::Cursor::Unavailable) - .is_none() - ); - let _ = canvas.update( - &mut state, - &CanvasEvent::Mouse(mouse::Event::ButtonPressed(mouse::Button::Left)), - bounds, - mouse::Cursor::Available(iced::Point::new(150., 120.)), - ); + // Program receives keys already admitted by diagram::focus::Owner. Real + // unfocused/Inspector refusal is covered by app::diagram_focus_tests. let action = canvas .update(&mut state, &key, bounds, mouse::Cursor::Unavailable) .unwrap(); diff --git a/crates/oa-desktop/src/app/architecture_authoring_tests.rs b/crates/oa-desktop/src/app/architecture_authoring_tests.rs index 5246957e..2cd1ea84 100644 --- a/crates/oa-desktop/src/app/architecture_authoring_tests.rs +++ b/crates/oa-desktop/src/app/architecture_authoring_tests.rs @@ -321,16 +321,11 @@ fn architecture_f2_rename_preserves_identity_placement_and_atomic_history() { text: None, repeat: false, }); - assert!(canvas.update(&mut state, &key, bounds, cursor).is_none()); - let _ = canvas.update( - &mut state, - &iced::widget::canvas::Event::Mouse(mouse::Event::ButtonPressed(mouse::Button::Left)), - bounds, - cursor, - ); + // Program receives keys already admitted by diagram::focus::Owner. Real + // unfocused/Inspector refusal is covered by app::diagram_focus_tests. let message = canvas .update(&mut state, &key, bounds, cursor) - .expect("focused F2 opens the selected semantic name") + .expect("admitted F2 opens the selected semantic name") .into_inner() .0 .unwrap(); diff --git a/crates/oa-desktop/src/app/diagram_focus_tests.rs b/crates/oa-desktop/src/app/diagram_focus_tests.rs new file mode 100644 index 00000000..d029f783 --- /dev/null +++ b/crates/oa-desktop/src/app/diagram_focus_tests.rs @@ -0,0 +1,156 @@ +//! Keyboard ownership is exercised on the mounted App, not raw Canvas Program. +use super::*; +use crate::behavior::canvas_workspace::tests::{Mounted, key}; +use iced::advanced::widget::{Operation, operation}; +use iced::{Point, Rectangle, Size, Vector, keyboard::key::Named, mouse}; + +struct FocusField(Rectangle); +impl Operation for FocusField { + fn traverse(&mut self, visit: &mut dyn FnMut(&mut dyn Operation)) { + visit(self); + } + fn focusable( + &mut self, + _: Option<&iced::advanced::widget::Id>, + bounds: Rectangle, + state: &mut dyn operation::Focusable, + ) { + if bounds == self.0 { + state.focus(); + } else { + state.unfocus(); + } + } +} + +async fn f2(mounted: &mut Mounted, app: &mut App) -> Vec { + mounted + .batch(app, &[key(Named::F2)], mouse::Cursor::Unavailable) + .await +} + +fn rename_emitted(messages: &[Message]) -> bool { + messages.iter().any(|message| { + matches!( + message, + Message::Architecture(architecture_authoring::Event::Rename(..)) + ) + }) +} + +#[tokio::test] +async fn mounted_f2_requires_canvas_focus_and_yields_to_clean_inspector() { + for layer in [Layer::Oa, Layer::Sa] { + for programmatic in [true, false] { + let mut project = Project::empty(); + let object = project + .add_element( + if layer == Layer::Oa { + Kind::Entity + } else { + Kind::System + }, + layer, + "Original team", + None, + ) + .unwrap(); + let diagram = project.ensure_diagram(layer); + project.diagrams.get_mut(&diagram).unwrap().projection = ProjectionKind::Overview; + let node = project.place(diagram, object, Position { x: 80., y: 80. }); + let mut app = App::with_project(project); + app.page = Page::Diagram; + app.layer = layer; + app.active_diagram = Some(diagram); + app.window_size = Size::new(2000., 1400.); + let _ = app.update(Message::Select(object)); + let before = app.document.snapshot(); + let mut mounted = Mounted::new(&mut app).await; + app.zoom = 1.; + app.pan = Vector::ZERO; + app.fit_to_view = false; + mounted.settle(&mut app).await; + + // Selection alone does not grant the canvas keyboard ownership. + assert!(!rename_emitted(&f2(&mut mounted, &mut app).await)); + assert!(app.architecture.is_none()); + assert_eq!(app.document.bundle(), &before.bundle); + + let bounds = mounted + .geometry(&app) + .bounds(crate::allocation_input::CANVAS_ID); + let projected = projection::project_view(app.project(), diagram) + .nodes + .into_iter() + .find(|item| item.id == node) + .unwrap(); + let point = Point::new( + bounds.x + projected.position.x + 40., + bounds.y + projected.position.y + 35., + ); + mounted.click(&mut app, point).await; + assert_eq!((app.selected, app.selected_node), (object, Some(node))); + assert!(rename_emitted(&f2(&mut mounted, &mut app).await)); + let rename = app.architecture.as_ref().expect("canvas F2 opens rename"); + assert!(rename.inline()); + assert!(matches!( + &rename.saved.intent, + architecture_authoring::Intent::Rename { element, name } + if *element == object && name == "Original team" + )); + assert_eq!(app.document.bundle(), &before.bundle); + assert!( + !app.document.can_undo(), + "opening rename is not a model edit" + ); + + let task = app.update(Message::Escape); + mounted.perform(&mut app, task).await; + assert!(app.architecture.is_none()); + mounted.click(&mut app, point).await; + let field = mounted + .geometry(&app) + .text_inputs + .into_iter() + .filter(|bounds| bounds.x > app.window_size.width - 320.) + .min_by(|a, b| a.y.total_cmp(&b.y)) + .expect("Inspector name field is mounted"); + if programmatic { + mounted.operate(&app, &mut FocusField(field)); + } else { + let messages = mounted + .batch( + &mut app, + &[key(Named::Tab), key(Named::F2)], + mouse::Cursor::Unavailable, + ) + .await; + assert!( + !rename_emitted(&messages), + "Tab must retire F2 admission before its queued focus task" + ); + } + let geometry = mounted.geometry(&app); + assert!( + geometry + .focused + .iter() + .any(|(_, bounds, focused)| { *bounds == field && *focused }) + ); + assert!(geometry.focused.iter().any(|(id, _, focused)| { + id.as_ref() == Some(&iced::advanced::widget::Id::from("diagram-keyboard-owner")) + && !*focused + })); + assert!(!app.draft_dirty(), "the Inspector is still clean"); + assert!(!rename_emitted(&f2(&mut mounted, &mut app).await)); + assert!(app.architecture.is_none()); + assert_eq!( + app.draft.as_ref().unwrap().inputs["/name"].raw, + "Original team" + ); + assert_eq!(app.project(), &before.decoded.project); + assert_eq!(app.document.bundle(), &before.bundle); + assert!(!app.document.can_undo()); + } + } +} diff --git a/crates/oa-desktop/src/app/diagram_removal.rs b/crates/oa-desktop/src/app/diagram_removal.rs index 66272dc8..42af4699 100644 --- a/crates/oa-desktop/src/app/diagram_removal.rs +++ b/crates/oa-desktop/src/app/diagram_removal.rs @@ -73,6 +73,13 @@ impl App { } else { view.edges.iter().any(|edge| edge.id == intent.object) || view.ports.iter().any(|port| port.element == intent.object) + || (self.page == Page::Behavior + && crate::behavior::state_canvas::visible_row_relation( + self.project(), + intent.context.diagram, + &view, + intent.object, + )) }; if !visible { return; diff --git a/crates/oa-desktop/src/app/diagram_removal_tests.rs b/crates/oa-desktop/src/app/diagram_removal_tests.rs index be422fe2..784af1ec 100644 --- a/crates/oa-desktop/src/app/diagram_removal_tests.rs +++ b/crates/oa-desktop/src/app/diagram_removal_tests.rs @@ -4,6 +4,164 @@ use crate::behavior::canvas_workspace::tests::{Mounted, key}; use iced::advanced::widget::{Operation, operation}; use iced::{Event as Input, Point, Rectangle, Size, Vector, keyboard, mouse}; +#[tokio::test] +async fn mounted_state_action_and_internal_rows_delete_relations_and_preserve_reused_work() { + use crate::behavior::state_canvas::visible_row_relation; + for index in 0..4 { + let mut p = Project::empty(); + let machine = p + .add_element(Kind::StateMachine, Layer::Sa, "Machine", None) + .unwrap(); + let region = p + .add_element(Kind::Region, Layer::Sa, "Main", Some(machine)) + .unwrap(); + let state = p + .add_element(Kind::State, Layer::Sa, "Ready", Some(region)) + .unwrap(); + let other = p + .add_element(Kind::State, Layer::Sa, "Standby", Some(region)) + .unwrap(); + let work = p + .add_element(Kind::Function, Layer::Sa, "Monitor", None) + .unwrap(); + let mut rows = vec![]; + for (phase, name) in [ + (StateActionKind::Entry, "Start monitoring"), + (StateActionKind::Do, "Continue monitoring"), + (StateActionKind::Exit, "Stop monitoring"), + ] { + let id = p.relate(RelationKind::StateAction, name, state, work); + if let RelationBody::StateAction { phase: value, .. } = + &mut p.relations.get_mut(&id).unwrap().body + { + *value = phase; + } + rows.push(id); + } + let retained = p.relate(RelationKind::StateAction, "Other use", other, work); + let internal = p.relate(RelationKind::StateTransition, "Refresh", state, state); + if let RelationBody::StateTransition { + transition_kind, .. + } = &mut p.relations.get_mut(&internal).unwrap().body + { + *transition_kind = TransitionKind::Internal; + } + rows.push(internal); + let target = rows[index]; + let plan = crate::behavior::state_edit::create_view(&p, machine).unwrap(); + let diagram = plan.diagram; + let p = execute_batch(&p, &[plan.command]).unwrap(); + let mut app = App::with_project(p); + app.layer = Layer::Sa; + app.page = Page::Behavior; + app.behavior.active_owner = Some(machine); + app.behavior.diagram.diagram = Some(diagram); + app.window_size = Size::new(2000., 1400.); + let _ = app.update(Message::Select(state)); + let mut mounted = Mounted::new(&mut app).await; + app.behavior.diagram.zoom = 1.; + app.behavior.diagram.pan = Vector::new(0., 40.); + app.behavior.diagram.fit = false; + mounted.settle(&mut app).await; + let view = projection::project_view(app.project(), diagram); + assert!(visible_row_relation(app.project(), diagram, &view, target)); + assert!(!visible_row_relation(app.project(), diagram, &view, work)); + let node = view.nodes.iter().find(|n| n.element == state).unwrap(); + let mut collapsed = app.project().clone(); + collapsed + .diagrams + .get_mut(&diagram) + .unwrap() + .nodes + .get_mut(&node.id) + .unwrap() + .collapsed = true; + assert!(!visible_row_relation( + &collapsed, + diagram, + &projection::project_view(&collapsed, diagram), + target + )); + let mut ancestor_collapsed = app.project().clone(); + let parent = view.nodes.iter().find(|n| n.element == region).unwrap(); + ancestor_collapsed + .diagrams + .get_mut(&diagram) + .unwrap() + .nodes + .get_mut(&parent.id) + .unwrap() + .collapsed = true; + assert!(!visible_row_relation( + &ancestor_collapsed, + diagram, + &projection::project_view(&ancestor_collapsed, diagram), + target + )); + let unrelated = app + .project() + .diagrams + .values() + .find(|d| d.projection.state_machine().is_none()) + .map(|d| d.id); + if let Some(unrelated) = unrelated { + assert!(!visible_row_relation( + app.project(), + unrelated, + &view, + target + )); + } + let bounds = mounted + .geometry(&app) + .focused + .into_iter() + .find(|(id, _, _)| { + id.as_ref() == Some(&iced::advanced::widget::Id::from("diagram-keyboard-owner")) + }) + .unwrap() + .1; + let row_y = node.position.y + 64. + index as f32 * 28. + 12.; + let at = Point::new( + bounds.x + node.position.x + 45., + bounds.y + row_y + app.behavior.diagram.pan.y, + ); + mounted.click(&mut app, at).await; + assert_eq!(app.selected, target, "actual rendered row identity"); + assert!(app.behavior.diagram.placement.is_none()); + let before = app.document.snapshot(); + delete(&mut mounted, &mut app, false).await; + assert_eq!( + app.delete_target.as_ref().unwrap().snapshot.request.object, + target + ); + assert_eq!(app.document.bundle(), &before.bundle); + let task = app.update(Message::CancelDelete); + mounted.perform(&mut app, task).await; + mounted.click(&mut app, at).await; + delete(&mut mounted, &mut app, true).await; + let token = app.delete_target.as_ref().unwrap().snapshot.token; + let task = app.update(Message::ConfirmDelete(token)); + mounted.perform(&mut app, task).await; + assert!(!app.project().contains(target)); + assert!( + app.project().contains(work) + && app.project().contains(state) + && app.project().contains(other) + ); + assert!(app.project().contains(retained)); + for id in rows.iter().filter(|id| **id != target) { + assert!(app.project().contains(*id)); + } + consistent(&app); + let task = app.update(Message::Undo); + mounted.perform(&mut app, task).await; + assert_eq!(app.document.bundle(), &before.bundle); + assert_eq!(app.project(), &before.decoded.project); + assert!(!app.document.can_undo()); + } +} + struct Fixture { app: App, object: Id, diff --git a/crates/oa-desktop/src/behavior/state_canvas.rs b/crates/oa-desktop/src/behavior/state_canvas.rs index a51d7ddd..883a736a 100644 --- a/crates/oa-desktop/src/behavior/state_canvas.rs +++ b/crates/oa-desktop/src/behavior/state_canvas.rs @@ -402,6 +402,48 @@ fn state_action_rows<'a>( .collect() } +/// Action and internal-transition rows are selectable relations, not edges or +/// placements. Match the exact rows rendered in this dedicated machine view. +pub(crate) fn visible_row_relation( + project: &Project, + diagram: Id, + view: &ProjectedView, + id: Id, +) -> bool { + let Some(machine) = project + .diagrams + .get(&diagram) + .and_then(|d| d.projection.state_machine()) + else { + return false; + }; + let Some(relation) = project.relations.get(&id) else { + return false; + }; + let source = match relation.body { + RelationBody::StateAction { source, .. } => source, + RelationBody::StateTransition { + source, + target, + transition_kind: TransitionKind::Internal, + .. + } if projection::state_machine_contains(project, machine, target) => source, + _ => return false, + }; + if !projection::state_machine_contains(project, machine, source) { + return false; + } + view.nodes + .iter() + .filter(|node| node.element == source) + .any(|node| { + state_action_rows(project, node) + .into_iter() + .chain(internal_rows(project, node)) + .any(|(row, _)| row.id == id) + }) +} + fn state_action_label(project: &Project, relation: &Relation) -> String { let RelationBody::StateAction { target, phase, .. } = relation.body else { return String::new(); @@ -798,7 +840,10 @@ impl canvas::Program for StateCanvas<'_> { .iter() .find(|node| node.element == self.selected || node.id == self.selected); let object = placement.map_or(self.selected, |node| node.element); - if placement.is_none() && !scene.view.edges.iter().any(|edge| edge.id == object) { + if placement.is_none() + && !scene.view.edges.iter().any(|edge| edge.id == object) + && !visible_row_relation(self.project, self.diagram, &scene.view, object) + { return None; } return Some( diff --git a/crates/oa-desktop/src/behavior/state_menu.rs b/crates/oa-desktop/src/behavior/state_menu.rs index 49a0fcaf..dbe748fd 100644 --- a/crates/oa-desktop/src/behavior/state_menu.rs +++ b/crates/oa-desktop/src/behavior/state_menu.rs @@ -194,7 +194,13 @@ pub fn popup(app: &App, opened: &Opened, clean: bool) -> Option> { || projection::project_view(project, context.diagram) .edges .iter() - .any(|edge| edge.id == id)) + .any(|edge| edge.id == id) + || super::state_canvas::visible_row_relation( + project, + context.diagram, + &projection::project_view(project, context.diagram), + id, + )) }) { if request.placement.is_some() { "Shift+Delete"