diff --git a/crates/oa-desktop/src/app.rs b/crates/oa-desktop/src/app.rs index 01d78bc8..2e29c001 100644 --- a/crates/oa-desktop/src/app.rs +++ b/crates/oa-desktop/src/app.rs @@ -3,6 +3,9 @@ pub(crate) mod architecture_authoring; #[cfg(test)] mod architecture_authoring_tests; mod checkpoints; +pub(crate) mod diagram_removal; +#[cfg(test)] +mod diagram_removal_tests; #[cfg(test)] mod diagram_selection_tests; mod hosted; @@ -393,7 +396,9 @@ pub enum Message { RelationKind(RelationKind), Command(Command), Delete, - ConfirmDelete, + ConfirmDelete(u64), + DiagramRemove(diagram_removal::Intent), + DiagramTraverse(bool), CancelDelete, Undo, Redo, @@ -492,6 +497,7 @@ pub struct App { pub busy: bool, pub pending: Option, pub delete_pending: bool, + pub delete_target: Option, pub path: Option, /// A disk transaction requiring an explicit action, independent of lint. pub pending_recovery: Option, @@ -602,6 +608,7 @@ impl App { busy: false, pending: None, delete_pending: false, + delete_target: None, path: None, pending_recovery: None, disk_project: None, @@ -1366,6 +1373,8 @@ impl App { } if self.delete_pending { self.delete_pending = false; + self.delete_target = None; + self.canvas_epoch = self.canvas_epoch.wrapping_add(1); return Task::none(); } if self.pending.take().is_some() { @@ -1400,7 +1409,7 @@ impl App { Message::FilePath(_) | Message::SubmitFile | Message::CancelFile )) || (self.delete_pending - && !matches!(message, Message::ConfirmDelete | Message::CancelDelete)) + && !matches!(message, Message::ConfirmDelete(_) | Message::CancelDelete)) || (self.pending.is_some() && self.file_prompt.is_none() && !matches!( @@ -2778,19 +2787,25 @@ impl App { } } } + Message::DiagramRemove(intent) => self.remove_from_diagram(intent), + Message::DiagramTraverse(back) => { + return if back { + iced::widget::operation::focus_previous() + } else { + iced::widget::operation::focus_next() + }; + } Message::Delete => { if self.selected != self.project().id && self.apply() { - self.delete_pending = true; + self.begin_model_delete(None); } } - Message::ConfirmDelete => { - if !self.delete_pending || self.behavior.has_pending() { - return Task::none(); - } + Message::ConfirmDelete(token) => self.confirm_model_delete(token), + Message::CancelDelete => { self.delete_pending = false; - self.execute(Command::Delete(self.selected)); + self.delete_target = None; + self.canvas_epoch = self.canvas_epoch.wrapping_add(1); } - Message::CancelDelete => self.delete_pending = false, Message::Undo | Message::Redo => { let redo = matches!(message, Message::Redo); if self.apply() { diff --git a/crates/oa-desktop/src/app/behavior_lifecycle_tests.rs b/crates/oa-desktop/src/app/behavior_lifecycle_tests.rs index 3219c766..563a5521 100644 --- a/crates/oa-desktop/src/app/behavior_lifecycle_tests.rs +++ b/crates/oa-desktop/src/app/behavior_lifecycle_tests.rs @@ -291,7 +291,7 @@ fn pending_behavior_blocks_navigation_property_edits_and_model_history() { Message::BrowseGroup(crate::explorer::Group::Work), Message::Field("/name".into(), "Overwrite properties".into()), Message::Command(Command::Rename(fixture.source, "Overwrite model".into())), - Message::ConfirmDelete, + Message::ConfirmDelete(0), Message::Undo, Message::Redo, Message::ShortcutUndo(false), diff --git a/crates/oa-desktop/src/app/diagram_removal.rs b/crates/oa-desktop/src/app/diagram_removal.rs new file mode 100644 index 00000000..63ac9772 --- /dev/null +++ b/crates/oa-desktop/src/app/diagram_removal.rs @@ -0,0 +1,152 @@ +//! Captured diagram removal and model-confirmation lifetimes. +use super::*; +use crate::{model_menu, route_edit::Context}; + +#[derive(Debug, Clone)] +pub struct Intent { + pub context: Context, + pub object: Id, + pub placement: Option, + pub model: bool, +} + +pub struct Confirmation { + pub snapshot: model_menu::Opened, + pub selection: Id, + pub placement: Option, +} + +pub fn enabled(app: &App) -> bool { + crate::behavior::canvas_workspace::keyboard_enabled(app) + && !app.draft_dirty() + && !app.source_dirty() + && !app.behavior.has_pending() + && app + .architecture + .as_ref() + .is_none_or(|state| !state.inline()) +} + +impl App { + pub(super) fn remove_from_diagram(&mut self, intent: Intent) { + if !enabled(self) || self.connection.is_some() || self.selected_bend.is_some() { + return; + } + let current = match self.page { + Page::Diagram if self.architecture.as_ref().is_none_or(|s| !s.saved.placing) => { + self.route_context() + } + Page::Behavior + if self.behavior.diagram.tool == crate::behavior::state_edit::Tool::Select => + { + self.state_context() + } + _ => None, + }; + if current != Some(intent.context) || self.selected != intent.object { + return; + } + let view = projection::project_view(self.project(), intent.context.diagram); + let placement = if self.page == Page::Behavior { + self.behavior.diagram.placement + } else { + self.selected_node + }; + if intent.placement != placement || intent.object == intent.context.diagram { + return; + } + let visible = if let Some(id) = intent.placement { + self.project().diagrams[&intent.context.diagram] + .nodes + .get(&id) + .is_some_and(|node| node.element == intent.object) + && view + .nodes + .iter() + .any(|node| node.id == id && node.element == intent.object) + } else { + view.edges.iter().any(|edge| edge.id == intent.object) + || view.ports.iter().any(|port| port.element == intent.object) + }; + if !visible { + return; + } + if !intent.model + && let Some(placement) = intent.placement + { + if self.execute(Command::Delete(placement)) { + self.selected_node = None; + self.behavior.diagram.placement = None; + self.selected_bend = None; + self.status = "Hidden from this diagram · Model element preserved · Undo restores the placement".into(); + } + } else { + self.begin_model_delete(Some(intent.context)); + } + } + + pub(super) fn begin_model_delete(&mut self, canvas: Option) { + if self.selected == self.project().id || self.behavior.has_pending() { + return; + } + let mut request = model_menu::Request::object(self.selected, iced::Point::ORIGIN); + request.canvas = canvas; + request.placement = if self.page == Page::Behavior { + self.behavior.diagram.placement + } else { + self.selected_node + }; + // Opening a modal retires any pending canvas intent before capturing + // the confirmation; every new dialog receives its own replay token. + self.canvas_epoch = self.canvas_epoch.wrapping_add(1); + if canvas.is_some() { + request.canvas = if self.page == Page::Behavior { + self.state_context() + } else { + self.route_context() + }; + } + self.menu_sequence = self.menu_sequence.wrapping_add(1); + self.delete_target = + model_menu::Opened::new(self, request, self.menu_sequence).map(|snapshot| { + Confirmation { + snapshot, + selection: self.selected, + placement: if self.page == Page::Behavior { + self.behavior.diagram.placement + } else { + self.selected_node + }, + } + }); + self.delete_pending = self.delete_target.is_some(); + } + + pub(super) fn confirm_model_delete(&mut self, token: u64) { + let Some(target) = self.delete_target.as_ref() else { + return; + }; + if target.snapshot.token != token { + return; + } + let valid = self.delete_pending + && target.snapshot.valid(self) + && target.selection == self.selected + && target.placement + == if self.page == Page::Behavior { + self.behavior.diagram.placement + } else { + self.selected_node + } + && !self.behavior.has_pending() + && !self.source_dirty() + && !self.draft_dirty(); + let id = target.snapshot.request.object; + self.delete_pending = false; + self.delete_target = None; + self.canvas_epoch = self.canvas_epoch.wrapping_add(1); + if valid { + self.execute(Command::Delete(id)); + } + } +} diff --git a/crates/oa-desktop/src/app/diagram_removal_tests.rs b/crates/oa-desktop/src/app/diagram_removal_tests.rs new file mode 100644 index 00000000..58ef9280 --- /dev/null +++ b/crates/oa-desktop/src/app/diagram_removal_tests.rs @@ -0,0 +1,681 @@ +//! Real App tree regression tests; no private canvas focus or gesture seeding. +use super::*; +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}; + +struct Fixture { + app: App, + object: Id, + node: Id, + other: Id, + diagram: Id, + copy: Id, +} +fn fixture(layer: Layer) -> Fixture { + let mut p = Project::empty(); + let kind = if layer == Layer::Oa { + Kind::Entity + } else { + Kind::System + }; + let object = p.add_element(kind, layer, "Target", None).unwrap(); + let other = p + .add_element( + if layer == Layer::Oa { + Kind::Actor + } else { + Kind::SystemActor + }, + layer, + "Other", + None, + ) + .unwrap(); + let diagram = p.ensure_diagram(layer); + p.diagrams.get_mut(&diagram).unwrap().projection = ProjectionKind::Overview; + let node = p.place(diagram, object, Position { x: 80., y: 80. }); + p.place(diagram, other, Position { x: 500., y: 80. }); + let mut second = p.diagrams[&diagram].clone(); + second.id = Id::new_v4(); + second.name = "Other diagram".into(); + second.alias = "other_diagram".into(); + second.nodes.clear(); + second.routes.clear(); + let copy = second.id; + p.diagrams.insert(copy, second); + p.place(copy, object, Position { x: 70., y: 70. }); + let mut app = App::with_project(p); + app.page = Page::Diagram; + app.layer = layer; + app.active_diagram = Some(diagram); + app.window_size = Size::new(2000., 1400.); + Fixture { + app, + object, + node, + other, + diagram, + copy, + } +} +async fn mount(layer: Layer) -> (Fixture, Mounted) { + let mut f = fixture(layer); + let mut mounted = Mounted::new(&mut f.app).await; + f.app.zoom = 1.; + f.app.pan = Vector::ZERO; + f.app.fit_to_view = false; + mounted.settle(&mut f.app).await; + select(&mut mounted, &mut f).await; + (f, mounted) +} +async fn select(mounted: &mut Mounted, f: &mut Fixture) { + let bounds = mounted + .geometry(&f.app) + .bounds(crate::allocation_input::CANVAS_ID); + let node = projection::project_view(f.app.project(), f.diagram) + .nodes + .into_iter() + .find(|node| node.id == f.node) + .unwrap(); + let point = Point::new( + bounds.x + (node.position.x + 40.) * f.app.zoom + f.app.pan.x, + bounds.y + (node.position.y + 35.) * f.app.zoom + f.app.pan.y, + ); + mounted.click(&mut f.app, point).await; + assert_eq!(f.app.selected, f.object); + assert_eq!(f.app.selected_node, Some(f.node)); +} +fn modified(named: keyboard::key::Named, modifiers: keyboard::Modifiers) -> Input { + let mut input = key(named); + if let Input::Keyboard(keyboard::Event::KeyPressed { + modifiers: value, .. + }) = &mut input + { + *value = modifiers; + } + input +} +async fn delete(mounted: &mut Mounted, app: &mut App, shift: bool) -> Vec { + mounted + .batch( + app, + &[modified( + keyboard::key::Named::Delete, + if shift { + keyboard::Modifiers::SHIFT + } else { + keyboard::Modifiers::empty() + }, + )], + mouse::Cursor::Unavailable, + ) + .await +} +fn consistent(app: &App) { + assert_eq!( + source_project::compile_bundle(app.document.bundle(), LIMITS) + .unwrap() + .decoded + .project, + *app.project() + ); + assert_eq!( + source_workspace::reachable(&app.source_workspace.bundle()).unwrap(), + *app.document.bundle() + ); +} + +#[tokio::test] +async fn mounted_oa_sa_delete_hides_exact_placement_and_round_trips_source_and_model() { + for layer in [Layer::Oa, Layer::Sa] { + let (mut f, mut mounted) = mount(layer).await; + let before = f.app.document.snapshot(); + let messages = delete(&mut mounted, &mut f.app, false).await; + assert!( + messages + .iter() + .any(|message| matches!(message, Message::DiagramRemove(_))) + ); + assert!(f.app.project().contains(f.object)); + assert!( + !f.app.project().diagrams[&f.diagram] + .nodes + .contains_key(&f.node) + ); + assert!( + f.app.project().diagrams[&f.copy] + .nodes + .values() + .any(|node| node.element == f.object) + ); + assert!(!f.app.delete_pending); + consistent(&f.app); + let after = f.app.document.snapshot(); + let task = f.app.update(Message::Undo); + mounted.perform(&mut f.app, task).await; + assert_eq!(f.app.project(), &before.decoded.project); + assert_eq!(f.app.document.bundle(), &before.bundle); + assert!(!f.app.document.can_undo(), "one removal, one undo unit"); + let task = f.app.update(Message::Redo); + mounted.perform(&mut f.app, task).await; + assert_eq!(f.app.project(), &after.decoded.project); + assert_eq!(f.app.document.bundle(), &after.bundle); + consistent(&f.app); + } +} + +#[tokio::test] +async fn mounted_shift_delete_captures_target_cancel_escape_and_confirmation_replay() { + let (mut f, mut mounted) = mount(Layer::Oa).await; + let before = f.app.document.snapshot(); + delete(&mut mounted, &mut f.app, true).await; + let target = f.app.delete_target.as_ref().unwrap(); + assert_eq!(target.snapshot.request.object, f.object); + let stale_token = target.snapshot.token; + let task = f.app.update(Message::CancelDelete); + mounted.perform(&mut f.app, task).await; + let task = f.app.update(Message::ConfirmDelete(stale_token)); + mounted.perform(&mut f.app, task).await; + assert_eq!(f.app.document.bundle(), &before.bundle); + select(&mut mounted, &mut f).await; + delete(&mut mounted, &mut f.app, true).await; + let token = f.app.delete_target.as_ref().unwrap().snapshot.token; + let task = f.app.update(Message::ConfirmDelete(stale_token)); + mounted.perform(&mut f.app, task).await; + assert!( + f.app.delete_pending, + "old dialog must not accept a new confirmation" + ); + let task = f.app.update(Message::ConfirmDelete(token)); + mounted.perform(&mut f.app, task).await; + assert!(!f.app.project().contains(f.object)); + assert!(f.app.project().contains(f.other)); + consistent(&f.app); + let task = f.app.update(Message::Undo); + mounted.perform(&mut f.app, task).await; + assert_eq!(f.app.document.bundle(), &before.bundle); + select(&mut mounted, &mut f).await; + delete(&mut mounted, &mut f.app, true).await; + let task = f.app.update(Message::Escape); + mounted.perform(&mut f.app, task).await; + assert!(!f.app.delete_pending); + assert!(f.app.delete_target.is_none()); + assert_eq!(f.app.document.bundle(), &before.bundle); +} + +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(); + } + } +} +fn name_field(mounted: &mut Mounted, app: &App) -> Rectangle { + 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)) + .unwrap() +} + +#[tokio::test] +async fn mounted_clean_text_programmatic_focus_and_tab_remove_canvas_keyboard_ownership() { + for programmatic in [true, false] { + let (mut f, mut mounted) = mount(Layer::Oa).await; + let before = f.app.document.snapshot(); + let field = name_field(&mut mounted, &f.app); + if programmatic { + mounted.operate(&f.app, &mut FocusField(field)); + } else { + let messages = mounted + .batch( + &mut f.app, + &[ + key(keyboard::key::Named::Tab), + key(keyboard::key::Named::Delete), + ], + mouse::Cursor::Unavailable, + ) + .await; + assert!( + !messages + .iter() + .any(|message| matches!(message, Message::DiagramRemove(_))), + "Tab retires canvas admission before its queued focus operation runs" + ); + } + let geometry = mounted.geometry(&f.app); + assert!( + geometry + .focused + .iter() + .any(|(_, bounds, focused)| *bounds == field && *focused), + "clean Inspector receives real focus" + ); + assert!(!f.app.draft_dirty()); + assert!( + delete(&mut mounted, &mut f.app, true) + .await + .iter() + .all(|message| !matches!(message, Message::DiagramRemove(_))) + ); + assert!(!f.app.delete_pending); + mounted + .batch( + &mut f.app, + &[key(keyboard::key::Named::Home)], + mouse::Cursor::Unavailable, + ) + .await; + let messages = delete(&mut mounted, &mut f.app, false).await; + assert!( + messages + .iter() + .any(|message| matches!(message, Message::Field(path, _) if path == "/name")) + ); + assert_eq!(f.app.draft.as_ref().unwrap().inputs["/name"].raw, "arget"); + assert_eq!(f.app.document.bundle(), &before.bundle); + assert!(f.app.project().contains(f.node)); + assert!(!f.app.delete_pending); + } +} + +#[tokio::test] +async fn mounted_removal_intents_reject_stale_selection_revision_owner_and_document() { + for boundary in 0..5 { + let (mut f, mut mounted) = mount(Layer::Oa).await; + let messages = mounted.input( + &f.app, + &[key(keyboard::key::Named::Delete)], + mouse::Cursor::Unavailable, + ); + let intent = messages + .into_iter() + .find(|message| matches!(message, Message::DiagramRemove(_))) + .unwrap(); + match boundary { + 0 => { + let _ = f.app.update(Message::Select(f.other)); + } + 1 => { + f.app.canvas_epoch = f.app.canvas_epoch.wrapping_add(1); + } + 2 => { + f.app.active_diagram = Some(f.copy); + } + 3 => { + f.app.canvas_document = Id::new_v4(); + } + _ => { + let _ = f + .app + .update(Message::Command(Command::Rename(f.other, "Renamed".into()))); + } + } + let before = f.app.document.snapshot(); + let task = f.app.update(intent); + mounted.perform(&mut f.app, task).await; + assert_eq!(f.app.document.bundle(), &before.bundle); + assert!(f.app.project().contains(f.node)); + assert!(!f.app.delete_pending); + } +} + +#[tokio::test] +async fn mounted_window_modal_dirty_and_modifier_boundaries_retire_delete_focus() { + for boundary in 0..5 { + let (mut f, mut mounted) = mount(Layer::Oa).await; + let before = f.app.document.snapshot(); + match boundary { + 0 => { + mounted + .batch( + &mut f.app, + &[Input::Window(iced::window::Event::Unfocused)], + mouse::Cursor::Unavailable, + ) + .await; + } + 1 => { + let _ = f.app.update(Message::Menu(crate::model_menu::Event::Open( + crate::model_menu::Request::object(f.object, Point::ORIGIN), + ))); + mounted.settle(&mut f.app).await; + let _ = f.app.update(Message::Menu(crate::model_menu::Event::Close)); + } + 2 => { + let _ = f.app.update(Message::Field("/name".into(), "Dirty".into())); + } + 3 => { + let _ = f.app.update(Message::Delete); + } + _ => { + f.app.canvas_epoch = f.app.canvas_epoch.wrapping_add(1); + } + } + delete(&mut mounted, &mut f.app, false).await; + assert_eq!(f.app.document.bundle(), &before.bundle); + assert!(f.app.project().contains(f.node)); + } + let (mut f, mut mounted) = mount(Layer::Oa).await; + let before = f.app.document.snapshot(); + mounted + .batch( + &mut f.app, + &[modified( + keyboard::key::Named::Delete, + keyboard::Modifiers::CTRL, + )], + mouse::Cursor::Unavailable, + ) + .await; + assert_eq!(f.app.document.bundle(), &before.bundle); + assert!(!f.app.delete_pending); + // Refusal is temporary: a fresh live canvas click owns Delete again. + select(&mut mounted, &mut f).await; + delete(&mut mounted, &mut f.app, false).await; + assert!(!f.app.project().contains(f.node)); +} + +#[tokio::test] +async fn mounted_bend_delete_has_priority_and_semantic_edge_delete_is_confirmed() { + let mut p = Project::empty(); + let a = p.add(Kind::Activity, "Send", 0., 0.); + let b = p.add(Kind::Activity, "Receive", 650., 0.); + let edge = p.relate(RelationKind::Interaction, "Report", a, b); + let diagram = p.ensure_diagram(Layer::Oa); + let p = execute_batch( + &p, + &[Command::SetEdgeRoute { + diagram, + route: EdgeRoute { + edge, + waypoints: vec![Position { x: 400., y: -100. }], + label: None, + }, + }], + ) + .unwrap(); + let mut app = App::with_project(p); + app.page = Page::Diagram; + app.active_diagram = Some(diagram); + app.window_size = Size::new(2000., 1400.); + let _ = app.update(Message::Select(edge)); + let mut mounted = Mounted::new(&mut app).await; + app.zoom = 1.; + app.pan = Vector::new(100., 250.); + app.fit_to_view = false; + mounted.settle(&mut app).await; + let bounds = mounted + .geometry(&app) + .bounds(crate::allocation_input::CANVAS_ID); + let pin = Point::new(bounds.x + 500., bounds.y + 150.); + mounted.click(&mut app, pin).await; + assert_eq!( + app.selected_bend, + Some((edge, 0)), + "real pointer selects route pin" + ); + let before = app.document.snapshot(); + let messages = delete(&mut mounted, &mut app, false).await; + assert!(messages.iter().any(|message| matches!( + message, + Message::Route(crate::route_edit::Event::RemoveBend) + ))); + assert!( + !messages + .iter() + .any(|message| matches!(message, Message::DiagramRemove(_))) + ); + assert!(app.project().contains(edge) && app.project().contains(a) && app.project().contains(b)); + assert!( + crate::route_edit::stored_route(&app.project().diagrams[&diagram], edge) + .waypoints + .is_empty() + ); + assert!(!app.delete_pending); + consistent(&app); + let task = app.update(Message::Undo); + mounted.perform(&mut app, task).await; + assert_eq!(app.document.bundle(), &before.bundle); + // Select a real edge segment away from its handle, then Delete opens the + // same semantic confirmation instead of inventing an edge hide operation. + let view = projection::project_view(app.project(), diagram); + let routed = routing::route_diagram(app.project(), &app.project().diagrams[&diagram], &view); + let route = &routed.edges[&edge]; + let segment = route + .points + .windows(2) + .max_by(|a, b| { + let len = |s: &[Position]| (s[1].x - s[0].x).abs() + (s[1].y - s[0].y).abs(); + len(a).total_cmp(&len(b)) + }) + .unwrap(); + let at = Point::new( + bounds.x + (segment[0].x + segment[1].x) / 2. + app.pan.x, + bounds.y + (segment[0].y + segment[1].y) / 2. + app.pan.y, + ); + mounted.click(&mut app, at).await; + assert_eq!(app.selected, edge); + assert!(app.selected_bend.is_none()); + delete(&mut mounted, &mut app, false).await; + assert_eq!( + app.delete_target.as_ref().unwrap().snapshot.request.object, + edge + ); + assert_eq!(app.document.bundle(), &before.bundle); +} + +#[tokio::test] +async fn mounted_state_delete_uses_state_placement_and_preserves_architecture_placement() { + 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 architecture = p.ensure_diagram(Layer::Sa); + let architecture_node = p.place(architecture, state, Position { x: 50., y: 50. }); + let plan = crate::behavior::state_edit::create_view(&p, machine).unwrap(); + let diagram = plan.diagram; + let p = execute_batch(&p, &[plan.command]).unwrap(); + let node = p.diagrams[&diagram] + .nodes + .values() + .find(|node| node.element == state) + .unwrap() + .id; + let mut app = App::with_project(p); + app.layer = Layer::Sa; + app.page = Page::Behavior; + app.active_diagram = Some(architecture); + 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; + 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 projected = projection::project_view(app.project(), diagram) + .nodes + .into_iter() + .find(|n| n.id == node) + .unwrap(); + let at = Point::new( + bounds.x + + (projected.position.x + 35.) * app.behavior.diagram.zoom + + app.behavior.diagram.pan.x, + bounds.y + + (projected.position.y + 35.) * app.behavior.diagram.zoom + + app.behavior.diagram.pan.y, + ); + mounted.click(&mut app, at).await; + assert_eq!(app.selected, state); + assert_eq!(app.behavior.diagram.placement, Some(node)); + let before = app.document.snapshot(); + let messages = delete(&mut mounted, &mut app, false).await; + assert!(messages.iter().any(|message| matches!( + message, + Message::StateDiagram(crate::behavior::state_edit::Event::Remove(_)) + ))); + assert!(!app.project().contains(node)); + assert!(app.project().contains(architecture_node) && app.project().contains(state)); + consistent(&app); + let task = app.update(Message::Undo); + mounted.perform(&mut app, task).await; + assert_eq!(app.document.bundle(), &before.bundle); +} + +#[tokio::test] +async fn captured_confirmation_rejects_selection_document_revision_and_placement_changes() { + for boundary in 0..5 { + let (mut f, mut mounted) = mount(Layer::Oa).await; + delete(&mut mounted, &mut f.app, true).await; + let token = f.app.delete_target.as_ref().unwrap().snapshot.token; + // Model asynchronous adoption / retained stale UI using a changed App + // state while preserving the old confirmation payload deliberately. + match boundary { + 0 => { + f.app.selected = f.other; + } + 1 => { + f.app.canvas_document = Id::new_v4(); + } + 2 => { + f.app.canvas_epoch = f.app.canvas_epoch.wrapping_add(1); + } + 3 => { + f.app.selected_node = None; + } + _ => { + assert!( + f.app + .document + .execute(Command::Rename(f.other, "New revision".into())) + .is_ok() + ); + } + } + let before = f.app.document.snapshot(); + let task = f.app.update(Message::ConfirmDelete(token)); + mounted.perform(&mut f.app, task).await; + assert_eq!(f.app.document.bundle(), &before.bundle); + assert!(f.app.project().contains(f.object)); + assert!(!f.app.delete_pending); + } +} + +#[tokio::test] +async fn mounted_programmatic_canvas_entry_needs_no_mouse_focus_boolean() { + let mut f = fixture(Layer::Oa); + let _ = f.app.update(Message::Select(f.object)); + let mut mounted = Mounted::new(&mut f.app).await; + let task = + iced::widget::operation::focus(iced::advanced::widget::Id::from("diagram-keyboard-owner")); + mounted.perform(&mut f.app, task).await; + let messages = delete(&mut mounted, &mut f.app, false).await; + assert!( + messages + .iter() + .any(|message| matches!(message, Message::DiagramRemove(_))) + ); + assert!(!f.app.project().contains(f.node)); + assert!(f.app.project().contains(f.object)); +} + +#[tokio::test] +async fn mounted_allocation_work_click_then_delete_without_redraw_removes_placement_once() { + for layer in [Layer::Oa, Layer::Sa] { + let mut p = Project::empty(); + let performer = p + .add_element( + if layer == Layer::Oa { + Kind::Entity + } else { + Kind::System + }, + layer, + "Performer", + None, + ) + .unwrap(); + let work = p + .add_element( + if layer == Layer::Oa { + Kind::Activity + } else { + Kind::Function + }, + layer, + "Work", + None, + ) + .unwrap(); + let diagram = p.ensure_diagram(layer); + p.diagrams.get_mut(&diagram).unwrap().projection = ProjectionKind::Allocation; + p.place(diagram, performer, Position { x: 100., y: 100. }); + let node = p.place(diagram, work, Position { x: 100., y: 500. }); + let mut app = App::with_project(p); + app.page = Page::Diagram; + app.layer = layer; + app.active_diagram = Some(diagram); + app.window_size = Size::new(2000., 1400.); + 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; + let bounds = mounted + .geometry(&app) + .bounds(crate::allocation_input::CANVAS_ID); + let point = Point::new(bounds.x + 140., bounds.y + 535.); + let messages = mounted.click(&mut app, point).await; + assert!(messages.iter().any(|message| matches!( + message, + Message::WorkDrag(crate::app::allocation_drag::Event::Press(_)) + ))); + assert_eq!(app.selected, work); + assert_eq!(app.selected_node, Some(node)); + let before = app.document.snapshot(); + // Deliberately do not settle/redraw between completed click and key. + let messages = delete(&mut mounted, &mut app, false).await; + assert!( + messages + .iter() + .any(|message| matches!(message, Message::DiagramRemove(_))), + "retired allocation press cannot consume first Delete" + ); + assert!(!app.project().contains(node)); + assert!(app.project().contains(work) && app.project().contains(performer)); + consistent(&app); + let task = app.update(Message::Undo); + mounted.perform(&mut app, task).await; + assert_eq!(app.document.bundle(), &before.bundle); + assert!(!app.document.can_undo()); + } +} diff --git a/crates/oa-desktop/src/app/state_diagram.rs b/crates/oa-desktop/src/app/state_diagram.rs index bf6cd7bc..090de012 100644 --- a/crates/oa-desktop/src/app/state_diagram.rs +++ b/crates/oa-desktop/src/app/state_diagram.rs @@ -114,6 +114,7 @@ impl App { } } match event { + Event::Remove(intent) => self.remove_from_diagram(intent), Event::Viewport(_, size) => { if size.width.is_finite() && size.height.is_finite() diff --git a/crates/oa-desktop/src/behavior/state_canvas.rs b/crates/oa-desktop/src/behavior/state_canvas.rs index 145aad92..a51d7ddd 100644 --- a/crates/oa-desktop/src/behavior/state_canvas.rs +++ b/crates/oa-desktop/src/behavior/state_canvas.rs @@ -38,7 +38,6 @@ pub struct State { context: Option, tool: Option, pointer: Option, - focused: bool, press: Option, source: Option, preview: Option<(Id, Position)>, @@ -771,7 +770,6 @@ impl canvas::Program for StateCanvas<'_> { } canvas::Event::Window(iced::window::Event::Unfocused) => { state.reset(); - state.focused = false; return None; } _ => cursor, @@ -779,14 +777,40 @@ impl canvas::Program for StateCanvas<'_> { let window = cursor.position(); let local = window.map(|p| Point::new(p.x - bounds.x, p.y - bounds.y)); let inside = local.is_some_and(|p| Self::content(bounds).contains(p)); - if let canvas::Event::Keyboard(keyboard::Event::KeyPressed { key, modifiers, .. }) = event - && state.focused - { + if let canvas::Event::Keyboard(keyboard::Event::KeyPressed { key, modifiers, .. }) = event { use keyboard::{Key, key::Named}; if *key == Key::Named(Named::Escape) { state.reset(); return Some(Action::publish(Event::Choose(Tool::Select)).and_capture()); } + if *key == Key::Named(Named::Delete) + && self.editable + && self.tool == Tool::Select + && state.press.is_none() + && state.pan_from.is_none() + && state.source.is_none() + && (modifiers.is_empty() || *modifiers == keyboard::Modifiers::SHIFT) + { + let scene = self.scene(state)?; + let placement = scene + .view + .nodes + .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) { + return None; + } + return Some( + Action::publish(Event::Remove(crate::app::diagram_removal::Intent { + context: self.context, + object, + placement: placement.map(|node| node.id), + model: modifiers.shift(), + })) + .and_capture(), + ); + } if modifiers.is_empty() { let pan = match key.as_ref() { Key::Named(Named::ArrowLeft) => Some(Vector::new(60., 0.)), @@ -804,7 +828,6 @@ impl canvas::Program for StateCanvas<'_> { let scene = self.scene(state)?; match event { canvas::Event::Mouse(mouse::Event::ButtonPressed(button)) => { - state.focused = inside; if !inside { return None; } diff --git a/crates/oa-desktop/src/behavior/state_edit.rs b/crates/oa-desktop/src/behavior/state_edit.rs index 9aae33cb..ee73c801 100644 --- a/crates/oa-desktop/src/behavior/state_edit.rs +++ b/crates/oa-desktop/src/behavior/state_edit.rs @@ -24,6 +24,7 @@ pub enum Tool { #[derive(Debug, Clone)] pub enum Event { + Remove(crate::app::diagram_removal::Intent), Choose(Tool), CreateView(Id), Select(Context, Id), @@ -70,6 +71,7 @@ pub enum Event { impl Event { pub fn context(&self) -> Option { match self { + Self::Remove(intent) => Some(intent.context), Self::Choose(_) | Self::CreateView(_) => None, Self::Select(c, _) | Self::Pan(c, _) diff --git a/crates/oa-desktop/src/behavior/state_menu.rs b/crates/oa-desktop/src/behavior/state_menu.rs index a50a5e8d..49a0fcaf 100644 --- a/crates/oa-desktop/src/behavior/state_menu.rs +++ b/crates/oa-desktop/src/behavior/state_menu.rs @@ -139,11 +139,14 @@ pub fn popup(app: &App, opened: &Opened, clean: bool) -> Option> { true, )); } - entries.push(item( - "Hide from this diagram", - Message::Command(Command::Delete(node.id)), - true, - )); + entries.push( + item( + "Hide from this diagram", + Message::Command(Command::Delete(node.id)), + true, + ) + .shortcut("Delete"), + ); } } if let Some(relation) = project.relations.get(&id) @@ -184,7 +187,24 @@ pub fn popup(app: &App, opened: &Opened, clean: bool) -> Option> { } if id != project.id { entries.push(Entry::separator()); - entries.push(item("Delete from model…", Message::Delete, true)); + entries.push(item("Delete from model…", Message::Delete, true).shortcut( + if context.is_some_and(|context| { + context.diagram != id + && (request.placement.is_some() + || projection::project_view(project, context.diagram) + .edges + .iter() + .any(|edge| edge.id == id)) + }) { + if request.placement.is_some() { + "Shift+Delete" + } else { + "Delete / Shift+Delete" + } + } else { + "" + }, + )); } } Some(Menu { diff --git a/crates/oa-desktop/src/behavior/state_view.rs b/crates/oa-desktop/src/behavior/state_view.rs index 73527ddc..9be45821 100644 --- a/crates/oa-desktop/src/behavior/state_view.rs +++ b/crates/oa-desktop/src/behavior/state_view.rs @@ -173,7 +173,12 @@ pub fn view<'a>(app: &'a App, machine: Id, mut out: Column<'a, Message>) -> Elem .width(Fill) .height(Fill) .into(); - out = out.push(canvas.map(event)); + out = out.push(crate::diagram::focus::wrap( + canvas.map(event), + context, + crate::app::diagram_removal::enabled(app), + Message::DiagramTraverse, + )); let base: Element<'_, Message> = container(out.spacing(8)) .padding(14) .width(Fill) diff --git a/crates/oa-desktop/src/diagram.rs b/crates/oa-desktop/src/diagram.rs index 09d18d06..8008704f 100644 --- a/crates/oa-desktop/src/diagram.rs +++ b/crates/oa-desktop/src/diagram.rs @@ -34,6 +34,8 @@ mod view_options_tests; #[path = "diagram/port_labels.rs"] mod port_labels; pub use port_labels::PortLabels; +#[path = "diagram/focus.rs"] +pub(crate) mod focus; #[path = "diagram/work_drag.rs"] mod work_drag; pub use work_drag::WorkFeedback; @@ -58,7 +60,6 @@ pub struct State { work: Option>, work_cache: RefCell>>, pointer: Option, - keyboard_focus: bool, drag: Option<(Id, Point, Position)>, preview: Option, pan_start: Option<(mouse::Button, Point)>, @@ -558,6 +559,21 @@ impl canvas::Program for DiagramCanvas<'_> { bounds: Rectangle, cursor: mouse::Cursor, ) -> Option> { + // A completed work click retires App's allocation epoch before the + // retained canvas sees a redraw. Its already-idle press must not eat + // the first Delete. A live or not-yet-dispatched press stays guarded. + if matches!(event, canvas::Event::Keyboard(iced::keyboard::Event::KeyPressed { key, .. }) + if *key == iced::keyboard::Key::Named(iced::keyboard::key::Named::Delete)) + && self.allocation.is_some_and(|capability| { + capability.clean + && state + .work + .as_ref() + .is_some_and(|press| press.stamp.epoch != capability.state.epoch) + }) + { + state.work = None; + } if let Some(action) = self.work_lifetime(state, event, bounds) { return Some(action); } @@ -628,9 +644,6 @@ impl canvas::Program for DiagramCanvas<'_> { let local = cursor .position() .map(|p| Point::new(p.x - bounds.x, p.y - bounds.y)); - if matches!(event, canvas::Event::Mouse(mouse::Event::ButtonPressed(_))) { - state.keyboard_focus = cursor.is_over(bounds); - } match event { canvas::Event::Mouse(mouse::Event::ButtonPressed(mouse::Button::Right)) if cursor.is_over(bounds) => @@ -650,7 +663,8 @@ impl canvas::Program for DiagramCanvas<'_> { } canvas::Event::Keyboard(iced::keyboard::Event::KeyPressed { key, modifiers, .. - }) if state.keyboard_focus => { + }) => { + // The mounted focus::Owner is the sole keyboard admission gate. use iced::keyboard::{Key, key::Named}; match key.as_ref() { Key::Named(Named::F2) @@ -705,10 +719,37 @@ impl canvas::Program for DiagramCanvas<'_> { Key::Named(Named::Delete) if self.editable && self.connection.is_none() - && modifiers.is_empty() - && self.selected_bend.is_some() => + && !self.placing + && state.drag.is_none() + && state.route_drag.is_none() + && state.port_drag.is_none() + && state.pan_start.is_none() + && state.work.is_none() + && (modifiers.is_empty() + || *modifiers == iced::keyboard::Modifiers::SHIFT) => { - Some(Action::publish(Message::Route(RouteEvent::RemoveBend)).and_capture()) + // An invalid/stale bend never falls through to whole-item removal. + if self.selected_bend.is_some() { + return modifiers.is_empty().then(|| { + Action::publish(Message::Route(RouteEvent::RemoveBend)) + .and_capture() + }); + } + let request = self.selection_context_request(state, bounds); + if request.object == self.diagram { + return None; + } + Some( + Action::publish(Message::DiagramRemove( + crate::app::diagram_removal::Intent { + context: self.context, + object: request.object, + placement: request.placement, + model: modifiers.shift(), + }, + )) + .and_capture(), + ) } _ => None, } diff --git a/crates/oa-desktop/src/diagram/focus.rs b/crates/oa-desktop/src/diagram/focus.rs new file mode 100644 index 00000000..bbf5d5b5 --- /dev/null +++ b/crates/oa-desktop/src/diagram/focus.rs @@ -0,0 +1,244 @@ +//! Iced focus ownership shared by architecture and persisted state canvases. +//! Focus operations (including clean Inspector focus) unfocus this widget. +use crate::route_edit::Context; +use iced::advanced::widget::{Operation, Tree, operation}; +use iced::advanced::{Clipboard, Layout, Shell, Widget, layout, overlay, renderer}; +use iced::{Element, Event as InputEvent, Length, Point, Rectangle, Size, Theme, Vector, mouse}; +#[derive(Default)] +struct Input { + context: Option, + focused: bool, + traversing: bool, + pointer: Option, +} +impl operation::Focusable for Input { + fn is_focused(&self) -> bool { + self.focused + } + fn focus(&mut self) { + self.focused = true; + self.traversing = false; + } + fn unfocus(&mut self) { + self.focused = false; + self.traversing = false; + } +} +struct Owner<'a, Message, R: renderer::Renderer> { + context: Context, + enabled: bool, + traverse: Box Message + 'a>, + content: Element<'a, Message, Theme, R>, +} +pub fn wrap<'a, Message: 'a, R: renderer::Renderer + 'a>( + content: impl Into>, + context: Context, + enabled: bool, + traverse: impl Fn(bool) -> Message + 'a, +) -> Element<'a, Message, Theme, R> { + Element::new(Owner { + context, + enabled, + traverse: Box::new(traverse), + content: content.into(), + }) +} +impl Widget for Owner<'_, Message, R> { + fn tag(&self) -> iced::advanced::widget::tree::Tag { + iced::advanced::widget::tree::Tag::of::() + } + fn state(&self) -> iced::advanced::widget::tree::State { + iced::advanced::widget::tree::State::new(Input { + context: Some(self.context), + ..Default::default() + }) + } + fn children(&self) -> Vec { + vec![Tree::new(self.content.as_widget())] + } + + fn diff(&self, tree: &mut Tree) { + let state = tree.state.downcast_mut::(); + if state.context != Some(self.context) || !self.enabled { + state.focused = false; + state.traversing = false; + state.pointer = None; + } + state.context = Some(self.context); + tree.diff_children(std::slice::from_ref(&self.content)); + } + + fn size(&self) -> Size { + self.content.as_widget().size() + } + + fn layout(&mut self, tree: &mut Tree, renderer: &R, limits: &layout::Limits) -> layout::Node { + self.content + .as_widget_mut() + .layout(&mut tree.children[0], renderer, limits) + } + + fn operate( + &mut self, + tree: &mut Tree, + layout: Layout<'_>, + renderer: &R, + operation: &mut dyn Operation, + ) { + if self.enabled { + operation.focusable( + Some(&iced::advanced::widget::Id::from("diagram-keyboard-owner")), + layout.bounds(), + tree.state.downcast_mut::(), + ); + } + self.content + .as_widget_mut() + .operate(&mut tree.children[0], layout, renderer, operation); + } + + fn update( + &mut self, + tree: &mut Tree, + event: &InputEvent, + layout: Layout<'_>, + cursor: mouse::Cursor, + renderer: &R, + clipboard: &mut dyn Clipboard, + shell: &mut Shell<'_, Message>, + viewport: &Rectangle, + ) { + let state = tree.state.downcast_mut::(); + if let InputEvent::Mouse(mouse::Event::CursorMoved { position }) = event { + state.pointer = Some(*position); + } + if matches!( + event, + InputEvent::Window(iced::window::Event::Unfocused | iced::window::Event::Resized(_)) + ) || !self.enabled + { + state.focused = false; + state.traversing = false; + state.pointer = None; + } + if matches!(event, InputEvent::Mouse(mouse::Event::ButtonPressed(_))) { + state.traversing = false; + let pointer = state.pointer.or_else(|| cursor.position()); + state.focused = self.enabled + && !shell.is_event_captured() + && cursor.position().is_some() + && pointer.is_some_and(|p| { + layout + .bounds() + .intersection(viewport) + .is_some_and(|b| b.contains(p)) + }); + } + if let InputEvent::Keyboard(iced::keyboard::Event::KeyPressed { + key: iced::keyboard::Key::Named(iced::keyboard::key::Named::Tab), + modifiers, + .. + }) = event + && state.focused + && !state.traversing + && self.enabled + && !shell.is_event_captured() + && (*modifiers == iced::keyboard::Modifiers::SHIFT || modifiers.is_empty()) + { + // The focus task runs after the native batch. Retain focus for its + // traversal count, but reject later keys in this same batch now. + state.traversing = true; + shell.publish((self.traverse)(modifiers.shift())); + shell.capture_event(); + return; + } + if matches!(event, InputEvent::Keyboard(_)) + && (!state.focused || state.traversing || !self.enabled) + { + return; + } + if shell.is_event_captured() { + return; + } + self.content.as_widget_mut().update( + &mut tree.children[0], + event, + layout, + cursor, + renderer, + clipboard, + shell, + viewport, + ); + } + + fn draw( + &self, + tree: &Tree, + renderer: &mut R, + theme: &Theme, + style: &renderer::Style, + layout: Layout<'_>, + cursor: mouse::Cursor, + viewport: &Rectangle, + ) { + self.content.as_widget().draw( + &tree.children[0], + renderer, + theme, + style, + layout, + cursor, + viewport, + ); + let state = tree.state.downcast_ref::(); + if state.focused && !state.traversing && self.enabled { + renderer.fill_quad( + renderer::Quad { + bounds: layout.bounds(), + border: iced::Border { + color: crate::style::TEAL, + width: 1., + ..Default::default() + }, + ..Default::default() + }, + iced::Color::TRANSPARENT, + ); + } + } + + fn mouse_interaction( + &self, + tree: &Tree, + layout: Layout<'_>, + cursor: mouse::Cursor, + viewport: &Rectangle, + renderer: &R, + ) -> mouse::Interaction { + self.content.as_widget().mouse_interaction( + &tree.children[0], + layout, + cursor, + viewport, + renderer, + ) + } + + fn overlay<'b>( + &'b mut self, + tree: &'b mut Tree, + layout: Layout<'b>, + renderer: &R, + viewport: &Rectangle, + translation: Vector, + ) -> Option> { + self.content.as_widget_mut().overlay( + &mut tree.children[0], + layout, + renderer, + viewport, + translation, + ) + } +} diff --git a/crates/oa-desktop/src/model_menu.rs b/crates/oa-desktop/src/model_menu.rs index 17608344..2a10d12f 100644 --- a/crates/oa-desktop/src/model_menu.rs +++ b/crates/oa-desktop/src/model_menu.rs @@ -415,7 +415,9 @@ pub fn popup(app: &App) -> Option> { true, )); entries.push(item("Placement properties", Message::Select(node.id), true)); - entries.push(item("Hide from this diagram", Message::HideSelected, true)); + entries.push( + item("Hide from this diagram", Message::HideSelected, true).shortcut("Delete"), + ); } else if e.kind() != Kind::Port && let Some(d) = app.active_diagram.and_then(|d| p.diagrams.get(&d)) { @@ -465,7 +467,25 @@ pub fn popup(app: &App) -> Option> { true, )); if id != p.id { - entries.push(item("Delete from model…", Message::Delete, true)); + entries.push( + item("Delete from model…", Message::Delete, true).shortcut( + if request.canvas.is_some_and(|context| context.diagram != id) + && (request.placement.is_some() + || p.relations.contains_key(&id) + || p.elements + .get(&id) + .is_some_and(|element| element.kind() == Kind::Port)) + { + if request.placement.is_some() { + "Shift+Delete" + } else { + "Delete / Shift+Delete" + } + } else { + "" + }, + ), + ); } } Some(Menu { diff --git a/crates/oa-desktop/src/views.rs b/crates/oa-desktop/src/views.rs index f4651b77..e78b4b4c 100644 --- a/crates/oa-desktop/src/views.rs +++ b/crates/oa-desktop/src/views.rs @@ -343,8 +343,8 @@ pub fn view(app: &App) -> View<'_, Message> { .into(), ); } - if app.delete_pending { - return modal(root,column![text("Delete from model?").size(21),text(format!("{} and its owned elements will be removed. Incident relationships, occurrences and view placements are updated together. Counterparts in the other layer remain.",p.named(app.selected))).size(13),label("Undo restores the complete change. Use Hide for diagram-only removal."),row![action("Cancel",Message::CancelDelete),button("Delete from model").style(button::danger).padding(10).on_press(Message::ConfirmDelete)].spacing(10)].spacing(16).into()); + if let Some(target) = app.delete_target.as_ref().filter(|_| app.delete_pending) { + return modal(root,column![text("Delete from model?").size(21),text(format!("{} and its owned elements will be removed. Incident relationships, occurrences and view placements are updated together. Counterparts in the other layer remain.",p.named(target.snapshot.request.object))).size(13),label("Undo restores the complete change. Use Hide for diagram-only removal."),row![action("Cancel",Message::CancelDelete),button("Delete from model").style(button::danger).padding(10).on_press(Message::ConfirmDelete(target.snapshot.token))].spacing(10)].spacing(16).into()); } if app.pending.is_some() { return modal(root,column![text("Keep your changes?").size(21),text("Save the model and pending edits before continuing. Invalid drafts stay available for correction.").size(13),row![action("Cancel",Message::Cancel),action("Discard changes",Message::Discard),button("Save & continue").padding(10).on_press(Message::SaveAndContinue)].spacing(8)].spacing(16).into()); @@ -459,7 +459,12 @@ fn diagram(app: &App) -> View<'_, Message> { connection: app.connection.as_ref(), }; let mut scene = widget::stack![ - widget::canvas(program).width(Fill).height(Fill), + crate::diagram::focus::wrap( + widget::canvas(program).width(Fill).height(Fill), + program.context, + crate::app::diagram_removal::enabled(app), + Message::DiagramTraverse + ), widget::canvas(crate::diagram::PortLabels(program)) .width(Fill) .height(Fill), diff --git a/harness/diagram-delete-candidate-20260912.md b/harness/diagram-delete-candidate-20260912.md new file mode 100644 index 00000000..23a8a9bb --- /dev/null +++ b/harness/diagram-delete-candidate-20260912.md @@ -0,0 +1,63 @@ +# Diagram Delete candidate handoff — 2026-09-12 + +Frozen, uncommitted implementation on `codex/diagram-delete-956atf-20260912`, isolated worktree `C:\Users\User\Documents\Git\ARCADIA-Studio\.local\diagram-delete-20260912`, HEAD/base `5a70181`. Parent owns issue 15 comment 551, review, Windows verification and integration. No Gitea, Git index/commit, compiler/cargo, native application, WSL or VPN operations performed by this worker. Files are ready for parent review/copy; do not interpret this as verified or complete issue 15. + +## Candidate behavior + +- Architecture OA/SA and persisted state diagrams: focused canvas Delete hides the selected explicit placement through ordinary `execute(Command::Delete(placement))`; semantic object remains. Shift+Delete opens the existing model-removal dialog. Visible architecture edges/ports and state transitions without explicit placements use confirmed model removal for Delete too. +- Captured `diagram_removal::Intent` carries diagram document/revision/epoch, object and explicit placement. App independently checks clean draft/source/behavior, modal/menu/reference state, selected object/placement, current diagram/page/tool and visible projected membership. Keyboard path never calls `apply()`. +- `diagram/focus.rs` supplies actual Iced Focusable ownership around the architecture/state canvas. It draws the existing behavior viewport style (1 px teal border) while keyboard-focused, without changing layout. It registers for standard focus operations; programmatic focus of any other field unfocuses it. It rejects captured/unfocused/disabled keys and resets at context/window boundaries. The old mouse-only DiagramCanvas.keyboard_focus and StateCanvas.focused gates were removed: the wrapper is the single keyboard authority. Standalone Program tests still exercise already-admitted events. +- Canvas Tab uses ordinary Iced next/previous focus operations. A transient traversal flag rejects subsequent keys in the same native event batch before that task runs, while retaining focus for the operation's traversal count. +- Delete requires selection mode and no active pointer/route/port/pan/work gesture. Existing bend Delete takes priority and a selected invalid bend cannot fall through to object removal. Unsupported modifiers are rejected. Diagram background is never a shortcut target. +- A completed allocation work click retires its epoch before retained canvas state necessarily sees redraw. `diagram.rs` clears only an already-retired local work press before Delete; same-epoch/live work presses remain guarded. This avoids consuming the first Delete merely to send stale WorkDrag::Cancel. `diagram/work_drag.rs` and app/allocation_drag.rs are untouched. +- Model confirmation is captured through existing model_menu::Opened plus selected identity/placement and a unique token. Rendered text uses the captured object. ConfirmDelete now requires the token. Cancel/Escape clear the snapshot and advance the canvas epoch. Old tokens cannot confirm a later dialog; changed document/revision/epoch/page/layer/diagram/object selection or placement refuses the command. Existing pointer/model menu deletion still invokes apply as before, then creates the captured confirmation. +- Existing architecture/state menus gain scope-appropriate Delete/Shift+Delete hints. Diagram background and unsupported state-action rows do not claim a removal shortcut. + +## Changed files + +New: +- `crates/oa-desktop/src/app/diagram_removal.rs` — intent validation, confirmation lifetime, shared keyboard admission predicate. +- `crates/oa-desktop/src/diagram/focus.rs` — actual Iced Focusable wrapper. +- `crates/oa-desktop/src/app/diagram_removal_tests.rs` — 10 mounted App-tree test functions. + +Modified: +- `crates/oa-desktop/src/app.rs` — module/fields/messages, tokenized modal handling and focused delegation hunks. +- `crates/oa-desktop/src/app/behavior_lifecycle_tests.rs` — existing ConfirmDelete constructor updated to token signature. +- `crates/oa-desktop/src/app/state_diagram.rs` — delegates the captured removal event. +- `crates/oa-desktop/src/behavior/state_canvas.rs` — admitted Delete events; removes obsolete private focus boolean. +- `crates/oa-desktop/src/behavior/state_edit.rs` — captured removal event/context. +- `crates/oa-desktop/src/behavior/state_menu.rs` — shortcut hints. +- `crates/oa-desktop/src/behavior/state_view.rs` — focused wrapper. +- `crates/oa-desktop/src/diagram.rs` — admitted Delete, bend/gesture priority, retired work-press cleanup; removes obsolete private focus boolean. +- `crates/oa-desktop/src/model_menu.rs` — shortcut hints. +- `crates/oa-desktop/src/views.rs` — captured confirmation text/token and architecture canvas wrapper only. Allocation toolbar region untouched. + +## Authored verification, not executed + +Reuses production `behavior::canvas_workspace::tests::Mounted`, real App::view/UserInterface/renderer, retained widget cache, ordered input batches and task/widget-operation draining. Tests never seed canvas private gesture or focus booleans. + +1. OA/SA real pointer selection → Delete exact placement; other diagram placement and semantic object retained; source/model agreement and complete one-unit Undo/Redo. +2. Shift+Delete captured target; Cancel, Escape, old-token replay versus a new dialog; intended object removal and Undo source round trip. +3. CLEAN Inspector focus by actual widget operation and canvas Tab; Tab+Delete in one native batch cannot remove model; Shift+Delete cannot reach canvas; Home/Delete edits Name while project/source remain accepted and unchanged. +4. Captured emitted removal intent replay after selection, epoch, owner diagram, document or revision change. +5. Window Unfocused, menu open/close, dirty draft, modal, epoch and unsupported modifier refusal, followed by explicit reacquisition. +6. Real route-pin selection → Delete only bend; source round trip; actual edge segment selection → confirmed semantic deletion. +7. Real state-canvas pointer selection → deletes state placement while architecture placement and state semantic object survive; Undo/source round trip. +8. Captured confirmation refuses changed selection/document/epoch/placement/revision. +9. Programmatic canvas focus without preceding mouse entry → Delete works through the single focus owner. +10. OA/SA Allocation work body click → first Delete without intervening redraw hides exact placement once; semantic work/performer preserved; Undo restores source. + +Static checks performed: scoped rustfmt --edition 2024 --config skip_children=true on the 13 owned files; `git diff --check` passes. Formatter exit 0 is only a syntax-formatting check. No compilation or tests were run. Parent should run new `diagram_removal_tests` plus existing state/lifecycle, behavior lifecycle, model-menu, architecture and allocation full-tree tests on the combined integrated candidate. + +## Explicit remaining gaps / review risks + +- Process and scenario occurrence/link/fragment removal remains unimplemented; do not route their selections through architecture/state context or delete referenced reusable actors/functions. Scenario's modeled Delete participant event is untouched. +- Explorer keyboard removal remains unimplemented; existing Actions/pointer menu remains available. +- State action labels (state-action relation identities represented outside projected edges) remain pointer-menu deletion only. No shortcut hint is claimed for them. +- Mounted tests have not executed. Inspector Tab order, route-segment geometry, state selection hit geometry and the allocation no-redraw case are particularly important concrete checks. No screenshot/native UI acceptance has been done. +- Added tests exercise generated modal state and dispatch actual modal action messages through App; they do not locate/click the rendered confirmation buttons or read rendered glyph text. Parent native QA should confirm target wording and Cancel/Confirm accessibility. +- Tool/gesture admission guards are authored, but dedicated new mounted drag/connection/creation/reconnect and stale-invalid-bend cases are not yet authored. Existing relevant regression tests should be retained and broadened if they expose missing coverage. +- Intent validation compares current selection plus document/context lifetime. Switching selection away and back without any context/revision change is not recorded by a distinct selection-generation counter. Immediate changed-selection replay is covered. Parent should judge whether a stronger input-generation token is required for issue acceptance. +- `delete_pending` boolean remains for compatibility with existing guards/tests; production opening always also creates delete_target. Legacy tests that manually set only the boolean do not create a rendered target dialog, though App modal admission still blocks underlying action messages. + +All candidate source files are frozen after this handoff. Further edits require parent follow-up. \ No newline at end of file