diff --git a/crates/oa-desktop/src/app/diagram_removal.rs b/crates/oa-desktop/src/app/diagram_removal.rs index 39df546e..02b4b456 100644 --- a/crates/oa-desktop/src/app/diagram_removal.rs +++ b/crates/oa-desktop/src/app/diagram_removal.rs @@ -18,7 +18,11 @@ pub struct Confirmation { } pub fn enabled(app: &App) -> bool { - crate::behavior::canvas_workspace::keyboard_enabled(app) + !app.busy && focus_enabled(app) +} + +pub(crate) fn focus_enabled(app: &App) -> bool { + crate::behavior::canvas_workspace::keyboard_owner_enabled(app) && !app.draft_dirty() && !app.source_dirty() && !app.behavior.has_pending() diff --git a/crates/oa-desktop/src/app/diagram_removal_tests.rs b/crates/oa-desktop/src/app/diagram_removal_tests.rs index f47e892f..2fc7b279 100644 --- a/crates/oa-desktop/src/app/diagram_removal_tests.rs +++ b/crates/oa-desktop/src/app/diagram_removal_tests.rs @@ -284,6 +284,35 @@ fn consistent(app: &App) { ); } +async fn save_with_busy_view(mounted: &mut Mounted, app: &mut App, path: &std::path::Path) { + let before = app.document.bundle().clone(); + let task = app.save_to(path.to_owned()); + assert!(app.busy); + // Render and exercise the real busy tree before completing the real save. + let messages = mounted + .batch( + app, + &[ + key(keyboard::key::Named::F2), + modified(keyboard::key::Named::Delete, keyboard::Modifiers::SHIFT), + ], + mouse::Cursor::Unavailable, + ) + .await; + assert!(messages.iter().all(|message| !matches!( + message, + Message::DiagramRemove(_) + | Message::StateDiagram(crate::behavior::state_edit::Event::Remove(_)) + | Message::Architecture(crate::app::architecture_authoring::Event::Rename(..)) + ))); + assert_eq!(app.document.bundle(), &before); + mounted.perform(app, task).await; + assert!(!app.busy); + assert!(path.is_file()); + assert!(!app.document.dirty(), "{}", app.status); + assert_eq!(app.document.bundle(), &before); +} + #[tokio::test] async fn mounted_oa_sa_delete_hides_exact_placement_and_round_trips_source_and_model() { for layer in [Layer::Oa, Layer::Sa] { @@ -320,6 +349,21 @@ async fn mounted_oa_sa_delete_hides_exact_placement_and_round_trips_source_and_m assert_eq!(f.app.project(), &after.decoded.project); assert_eq!(f.app.document.bundle(), &after.bundle); consistent(&f.app); + // History changes the revision, while this same canvas still owns the + // keyboard. Repeated hide and then model deletion need no extra click. + let task = f.app.update(Message::Undo); + mounted.perform(&mut f.app, task).await; + delete(&mut mounted, &mut f.app, false).await; + assert_eq!(f.app.document.bundle(), &after.bundle); + let task = f.app.update(Message::Undo); + mounted.perform(&mut f.app, task).await; + assert_eq!(f.app.document.bundle(), &before.bundle); + delete(&mut mounted, &mut f.app, true).await; + assert_eq!( + f.app.delete_target.as_ref().unwrap().snapshot.request.object, + f.object + ); + assert_eq!(f.app.document.bundle(), &before.bundle); } } @@ -533,6 +577,231 @@ async fn mounted_clean_text_programmatic_focus_and_tab_remove_canvas_keyboard_ow } } +#[tokio::test] +async fn mounted_revision_and_history_preserve_the_actual_canvas_or_inspector_focus_owner() { + for inspector in [false, true] { + let (mut f, mut mounted) = mount(Layer::Oa).await; + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join(&f.app.document.bundle().entry); + let field = name_field(&mut mounted, &f.app); + if inspector { + mounted.click(&mut f.app, field.center()).await; + } + let before = f.app.document.snapshot(); + let task = f.app.update(Message::Command(Command::Rename( + f.other, + "Changed other element".into(), + ))); + mounted.perform(&mut f.app, task).await; + for message in [Message::Undo, Message::Redo, Message::Undo] { + let task = f.app.update(message); + mounted.perform(&mut f.app, task).await; + save_with_busy_view(&mut mounted, &mut f.app, &path).await; + let geometry = mounted.geometry(&f.app); + assert_eq!( + geometry.focused.iter().any(|(id, _, focused)| { + *focused + && id.as_ref() + == Some(&iced::advanced::widget::Id::from("diagram-keyboard-owner")) + }), + !inspector, + "history cannot grant the canvas focus held by an Inspector field" + ); + assert!(geometry.focused.iter().any(|(id, bounds, focused)| { + *focused + && if inspector { + *bounds == field + } else { + id.as_ref() + == Some(&iced::advanced::widget::Id::from("diagram-keyboard-owner")) + } + })); + } + assert_eq!(f.app.document.bundle(), &before.bundle); + let messages = mounted + .batch( + &mut f.app, + &[key(keyboard::key::Named::F2)], + mouse::Cursor::Unavailable, + ) + .await; + assert_eq!( + messages.iter().any(|message| matches!( + message, + Message::Architecture(crate::app::architecture_authoring::Event::Rename(..)) + )), + !inspector, + "F2 follows the actual focus owner after edits and history" + ); + // Both the existing Inspector and the newly opened inline rename own + // their text. A second F2 or Delete must not reach the diagram. + let messages = mounted + .batch( + &mut f.app, + &[ + key(keyboard::key::Named::F2), + key(keyboard::key::Named::Delete), + ], + mouse::Cursor::Unavailable, + ) + .await; + assert!(messages.iter().all(|message| !matches!( + message, + Message::DiagramRemove(_) + | Message::Architecture(crate::app::architecture_authoring::Event::Rename(..)) + ))); + assert!(!f.app.delete_pending); + assert_eq!(f.app.document.bundle(), &before.bundle); + } +} + +#[tokio::test] +async fn mounted_passive_save_never_grants_canvas_focus_or_restores_window_lost_focus() { + let mut f = fixture(Layer::Oa); + let _ = f.app.update(Message::Select(f.object)); + let mut mounted = Mounted::new(&mut f.app).await; + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join(&f.app.document.bundle().entry); + let task = f.app.save_to(path.clone()); + assert!(f.app.busy); + let focus = + iced::widget::operation::focus(iced::advanced::widget::Id::from("diagram-keyboard-owner")); + mounted.perform(&mut f.app, focus).await; + let canvas = mounted + .geometry(&f.app) + .bounds(crate::allocation_input::CANVAS_ID); + mounted.click(&mut f.app, canvas.center()).await; + mounted.perform(&mut f.app, task).await; + assert!(!f.app.busy); + assert!(delete(&mut mounted, &mut f.app, false).await.is_empty()); + assert!(f.app.project().contains(f.node)); + select(&mut mounted, &mut f).await; + let task = f.app.save_to(path); + assert!(f.app.busy); + mounted + .batch( + &mut f.app, + &[Input::Window(iced::window::Event::Unfocused)], + mouse::Cursor::Unavailable, + ) + .await; + mounted.perform(&mut f.app, task).await; + assert!(!f.app.busy); + assert!(delete(&mut mounted, &mut f.app, false).await.is_empty()); + assert!(f.app.project().contains(f.node)); +} + +#[tokio::test] +async fn mounted_inspector_can_take_focus_during_passive_save() { + for programmatic in [false, true] { + let (mut f, mut mounted) = mount(Layer::Oa).await; + let field = name_field(&mut mounted, &f.app); + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join(&f.app.document.bundle().entry); + let task = f.app.save_to(path); + assert!(f.app.busy); + if programmatic { + mounted.operate(&f.app, &mut FocusField(field)); + } else { + mounted.click(&mut f.app, field.center()).await; + } + mounted.perform(&mut f.app, task).await; + let geometry = mounted.geometry(&f.app); + assert!(geometry.focused.iter().any(|(_, bounds, focused)| { + *bounds == field && *focused + })); + assert!(!geometry.focused.iter().any(|(id, _, focused)| { + *focused + && id.as_ref() + == Some(&iced::advanced::widget::Id::from("diagram-keyboard-owner")) + })); + let before = f.app.document.bundle().clone(); + let messages = mounted + .batch( + &mut f.app, + &[ + key(keyboard::key::Named::F2), + key(keyboard::key::Named::Delete), + ], + mouse::Cursor::Unavailable, + ) + .await; + assert!(messages.iter().all(|message| !matches!( + message, + Message::DiagramRemove(_) + | Message::Architecture(crate::app::architecture_authoring::Event::Rename(..)) + ))); + assert!(!f.app.delete_pending); + assert_eq!(f.app.document.bundle(), &before); + } +} + +#[tokio::test] +async fn mounted_passive_save_retires_held_canvas_drag_but_allows_a_fresh_drag() { + let (mut f, mut mounted) = mount(Layer::Oa).await; + let bounds = mounted + .geometry(&f.app) + .bounds(crate::allocation_input::CANVAS_ID); + let start = bounds.position() + Vector::new(120., 115.); + let moved = start + Vector::new(35., 20.); + let before = f.app.document.snapshot(); + mounted + .batch( + &mut f.app, + &[ + Input::Mouse(mouse::Event::CursorMoved { position: start }), + Input::Mouse(mouse::Event::ButtonPressed(mouse::Button::Left)), + Input::Mouse(mouse::Event::CursorMoved { position: moved }), + ], + mouse::Cursor::Available(moved), + ) + .await; + assert_eq!(f.app.document.bundle(), &before.bundle); + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join(&f.app.document.bundle().entry); + let task = f.app.save_to(path); + assert!(f.app.busy); + mounted + .batch( + &mut f.app, + &[Input::Mouse(mouse::Event::ButtonReleased(mouse::Button::Left))], + mouse::Cursor::Available(moved), + ) + .await; + mounted.perform(&mut f.app, task).await; + let end = moved + Vector::new(50., 30.); + mounted + .batch( + &mut f.app, + &[ + Input::Mouse(mouse::Event::CursorMoved { position: end }), + Input::Mouse(mouse::Event::ButtonReleased(mouse::Button::Left)), + ], + mouse::Cursor::Available(end), + ) + .await; + assert_eq!(f.app.document.bundle(), &before.bundle); + assert!(!f.app.document.can_undo()); + mounted + .batch( + &mut f.app, + &[ + Input::Mouse(mouse::Event::CursorMoved { position: start }), + Input::Mouse(mouse::Event::ButtonPressed(mouse::Button::Left)), + Input::Mouse(mouse::Event::CursorMoved { position: moved }), + Input::Mouse(mouse::Event::ButtonReleased(mouse::Button::Left)), + ], + mouse::Cursor::Available(moved), + ) + .await; + assert_ne!(f.app.document.bundle(), &before.bundle); + assert!(f.app.document.can_undo()); + 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); +} + #[tokio::test] async fn mounted_removal_intents_reject_stale_selection_revision_owner_and_document() { for boundary in 0..5 { @@ -785,9 +1054,18 @@ async fn mounted_state_delete_uses_state_placement_and_preserves_architecture_pl assert!(!app.project().contains(node)); assert!(app.project().contains(architecture_node) && app.project().contains(state)); consistent(&app); + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join(&app.document.bundle().entry); + save_with_busy_view(&mut mounted, &mut app, &path).await; let task = app.update(Message::Undo); mounted.perform(&mut app, task).await; assert_eq!(app.document.bundle(), &before.bundle); + delete(&mut mounted, &mut app, true).await; + assert_eq!( + app.delete_target.as_ref().unwrap().snapshot.request.object, + state + ); + assert_eq!(app.document.bundle(), &before.bundle); } #[tokio::test] @@ -912,9 +1190,28 @@ async fn mounted_allocation_work_click_then_delete_without_redraw_removes_placem assert!(!app.project().contains(node)); assert!(app.project().contains(work) && app.project().contains(performer)); consistent(&app); + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join(&app.document.bundle().entry); + save_with_busy_view(&mut mounted, &mut app, &path).await; let task = app.update(Message::Undo); mounted.perform(&mut app, task).await; assert_eq!(app.document.bundle(), &before.bundle); assert!(!app.document.can_undo()); + save_with_busy_view(&mut mounted, &mut app, &path).await; + let task = app.update(Message::Redo); + mounted.perform(&mut app, task).await; + assert!(!app.project().contains(node)); + save_with_busy_view(&mut mounted, &mut app, &path).await; + let task = app.update(Message::Undo); + mounted.perform(&mut app, task).await; + assert_eq!(app.document.bundle(), &before.bundle); + // Native regression: work click -> hide -> Undo/Redo/Undo -> Shift+Delete. + // No pointer event or explicit focus operation occurs after the click. + delete(&mut mounted, &mut app, true).await; + assert_eq!( + app.delete_target.as_ref().unwrap().snapshot.request.object, + work + ); + assert_eq!(app.document.bundle(), &before.bundle); } } diff --git a/crates/oa-desktop/src/behavior/canvas_workspace.rs b/crates/oa-desktop/src/behavior/canvas_workspace.rs index a30d5070..b95ab5bf 100644 --- a/crates/oa-desktop/src/behavior/canvas_workspace.rs +++ b/crates/oa-desktop/src/behavior/canvas_workspace.rs @@ -10,8 +10,12 @@ pub use control::Control; pub use viewport::Viewport; pub fn keyboard_enabled(app: &crate::app::App) -> bool { - !app.busy - && !app.hosted.visible + !app.busy && keyboard_owner_enabled(app) +} + +/// Passive I/O suspends commands without changing the logical input owner. +pub(crate) fn keyboard_owner_enabled(app: &crate::app::App) -> bool { + !app.hosted.visible && app.workflow.is_none() && app.file_prompt.is_none() && !app.delete_pending diff --git a/crates/oa-desktop/src/behavior/state_view.rs b/crates/oa-desktop/src/behavior/state_view.rs index 9be45821..eb46407d 100644 --- a/crates/oa-desktop/src/behavior/state_view.rs +++ b/crates/oa-desktop/src/behavior/state_view.rs @@ -176,7 +176,8 @@ pub fn view<'a>(app: &'a App, machine: Id, mut out: Column<'a, Message>) -> Elem out = out.push(crate::diagram::focus::wrap( canvas.map(event), context, - crate::app::diagram_removal::enabled(app), + crate::app::diagram_removal::focus_enabled(app), + app.busy, Message::DiagramTraverse, )); let base: Element<'_, Message> = container(out.spacing(8)) diff --git a/crates/oa-desktop/src/diagram/focus.rs b/crates/oa-desktop/src/diagram/focus.rs index bbf5d5b5..9c27ab07 100644 --- a/crates/oa-desktop/src/diagram/focus.rs +++ b/crates/oa-desktop/src/diagram/focus.rs @@ -8,6 +8,8 @@ use iced::{Element, Event as InputEvent, Length, Point, Rectangle, Size, Theme, struct Input { context: Option, focused: bool, + can_focus: bool, + suspended: bool, traversing: bool, pointer: Option, } @@ -16,8 +18,10 @@ impl operation::Focusable for Input { self.focused } fn focus(&mut self) { - self.focused = true; - self.traversing = false; + if self.can_focus { + self.focused = true; + self.traversing = false; + } } fn unfocus(&mut self) { self.focused = false; @@ -27,6 +31,7 @@ impl operation::Focusable for Input { struct Owner<'a, Message, R: renderer::Renderer> { context: Context, enabled: bool, + suspended: bool, traverse: Box Message + 'a>, content: Element<'a, Message, Theme, R>, } @@ -34,11 +39,13 @@ pub fn wrap<'a, Message: 'a, R: renderer::Renderer + 'a>( content: impl Into>, context: Context, enabled: bool, + suspended: bool, traverse: impl Fn(bool) -> Message + 'a, ) -> Element<'a, Message, Theme, R> { Element::new(Owner { context, enabled, + suspended, traverse: Box::new(traverse), content: content.into(), }) @@ -50,6 +57,8 @@ impl Widget for Owner<'_, Mes fn state(&self) -> iced::advanced::widget::tree::State { iced::advanced::widget::tree::State::new(Input { context: Some(self.context), + can_focus: self.enabled && !self.suspended, + suspended: self.suspended, ..Default::default() }) } @@ -59,13 +68,32 @@ impl Widget for Owner<'_, Mes fn diff(&self, tree: &mut Tree) { let state = tree.state.downcast_mut::(); - if state.context != Some(self.context) || !self.enabled { + let suspend_started = self.suspended && !state.suspended; + state.suspended = self.suspended; + state.can_focus = self.enabled && !self.suspended; + // A new model revision retires pointer intent, not keyboard ownership + // of the same canvas. Ordinary edits and Undo/Redo keep that focus. + if state.context != Some(self.context) { + state.pointer = None; + } + if state.context.is_none_or(|previous| { + (previous.document, previous.diagram, previous.epoch) + != (self.context.document, self.context.diagram, self.context.epoch) + }) || !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)); + if suspend_started { + // A release may arrive while I/O owns commands. Retire the wrapped + // canvas's transient gestures without retiring this focus owner. + state.pointer = None; + tree.children[0] = Tree::new(self.content.as_widget()); + } else { + tree.diff_children(std::slice::from_ref(&self.content)); + } } fn size(&self) -> Size { @@ -85,6 +113,8 @@ impl Widget for Owner<'_, Mes renderer: &R, operation: &mut dyn Operation, ) { + // Suspended focus can still be revoked by another field's operation. + // Input::focus denies acquiring it until the passive task completes. if self.enabled { operation.focusable( Some(&iced::advanced::widget::Id::from("diagram-keyboard-owner")), @@ -124,7 +154,8 @@ impl Widget for Owner<'_, Mes if matches!(event, InputEvent::Mouse(mouse::Event::ButtonPressed(_))) { state.traversing = false; let pointer = state.pointer.or_else(|| cursor.position()); - state.focused = self.enabled + state.focused = (state.focused || !self.suspended) + && self.enabled && !shell.is_event_captured() && cursor.position().is_some() && pointer.is_some_and(|p| { @@ -134,6 +165,11 @@ impl Widget for Owner<'_, Mes .is_some_and(|b| b.contains(p)) }); } + // Save/export I/O denies commands and acquisition, while real focus + // operations and outside clicks can still revoke the retained owner. + if self.suspended && matches!(event, InputEvent::Keyboard(_) | InputEvent::Mouse(_)) { + return; + } if let InputEvent::Keyboard(iced::keyboard::Event::KeyPressed { key: iced::keyboard::Key::Named(iced::keyboard::key::Named::Tab), modifiers, @@ -192,7 +228,7 @@ impl Widget for Owner<'_, Mes viewport, ); let state = tree.state.downcast_ref::(); - if state.focused && !state.traversing && self.enabled { + if state.focused && !state.traversing && self.enabled && !self.suspended { renderer.fill_quad( renderer::Quad { bounds: layout.bounds(), diff --git a/crates/oa-desktop/src/views.rs b/crates/oa-desktop/src/views.rs index 22b63a83..0e0af3a1 100644 --- a/crates/oa-desktop/src/views.rs +++ b/crates/oa-desktop/src/views.rs @@ -486,7 +486,8 @@ fn diagram(app: &App) -> View<'_, Message> { crate::diagram::focus::wrap( widget::canvas(program).width(Fill).height(Fill), program.context, - crate::app::diagram_removal::enabled(app), + crate::app::diagram_removal::focus_enabled(app), + app.busy, Message::DiagramTraverse ), widget::canvas(crate::diagram::PortLabels(program))