Fix inline architecture naming and captured Escape cancellation

crates/oa-desktop/src/app.rs

@@ -1019,6 +1019,7 @@ impl App {
             iced::time::every(std::time::Duration::from_secs(10)).map(|_| Message::CheckpointTick),
             window::close_requests().map(|id| Message::Request(Pending::Close(id))),
             window::resize_events().map(|(_, size)| Message::WindowResized(size)),
+            architecture_authoring::subscription(self),
             iced::event::listen_with(|event, status, _| {
                 let iced::Event::Keyboard(keyboard::Event::KeyPressed {
                     key,
@@ -1076,9 +1077,7 @@ impl App {
         // navigation and close retain raw input until explicit completion/cancel.
         if self.architecture.is_some() {
             if matches!(message, Message::Escape) {
-                self.architecture = None;
-                self.canvas_epoch = self.canvas_epoch.wrapping_add(1);
-                return Task::none();
+                return self.architecture_event(architecture_authoring::Event::Cancel);
             }
             let placing = self.architecture.as_ref().is_some_and(|s| s.saved.placing);
             if !matches!(

crates/oa-desktop/src/app/architecture_authoring.rs

@@ -3,6 +3,8 @@ use super::*;
 use iced::widget::{button, column, pick_list, row, scrollable, text, text_input};
 use serde::{Deserialize, Serialize};
 mod palette;
+#[cfg(test)]
+mod tests;
 pub use palette::view as palette;
 
 #[derive(Debug, Clone)]
@@ -21,9 +23,35 @@ pub enum Event {
     Filter(Filter),
     ShowPlaced(bool),
     Page(usize),
-    Edit,
+    CapturedEscape,
     Cancel,
 }
+
+pub fn subscription(app: &App) -> Subscription<Message> {
+    if app.architecture.as_ref().is_some_and(State::inline) {
+        iced::event::listen_with(captured_escape)
+    } else {
+        Subscription::none()
+    }
+}
+
+fn captured_escape(
+    event: iced::Event,
+    status: iced::event::Status,
+    _: iced::window::Id,
+) -> Option<Message> {
+    // A focused Iced text input captures Escape while dropping its focus. Only
+    // inline architecture naming opts into also cancelling that same keypress.
+    (status == iced::event::Status::Captured
+        && matches!(
+            event,
+            iced::Event::Keyboard(iced::keyboard::Event::KeyPressed {
+                key: iced::keyboard::Key::Named(iced::keyboard::key::Named::Escape),
+                ..
+            })
+        ))
+    .then_some(Message::Architecture(Event::CapturedEscape))
+}
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
 #[serde(deny_unknown_fields)]
 pub enum Filter {
@@ -336,14 +364,20 @@ impl App {
                 }
                 return iced::widget::operation::focus("architecture-name");
             }
+            Event::CapturedEscape => {
+                if self.architecture.as_ref().is_some_and(State::inline) {
+                    return self.architecture_event(Event::Cancel);
+                }
+            }
             Event::Cancel => {
                 self.architecture = None;
                 self.canvas_epoch = self.canvas_epoch.wrapping_add(1);
+                self.status = "Cancelled · Select / move".into();
             }
             Event::Name(value) => {
                 if value.len() <= 16384
                     && let Some(s) = &mut self.architecture
-                    && let (Intent::Create { name, .. } | Intent::Rename { name, .. }) =
+                    && let Intent::Create { name, .. } | Intent::Rename { name, .. } =
                         &mut s.saved.intent
                 {
                     *name = value;
@@ -375,13 +409,6 @@ impl App {
                     s.saved.page = page.min(1_000_000);
                 }
             }
-            Event::Edit => {
-                self.canvas_epoch = self.canvas_epoch.wrapping_add(1);
-                if let Some(s) = &mut self.architecture {
-                    s.saved.placing = false;
-                }
-                return iced::widget::operation::focus("architecture-name");
-            }
             Event::Pick(id) => {
                 if !candidates(self)
                     .iter()
@@ -422,7 +449,10 @@ impl App {
                 }
             }
             Event::Click(context, point) => {
-                if self.route_context() != Some(context) {
+                if self.route_context() != Some(context)
+                    || !point.x.is_finite()
+                    || !point.y.is_finite()
+                {
                     return Task::none();
                 }
                 if self.architecture.as_ref().is_some_and(|s| s.saved.placing) {
@@ -460,6 +490,10 @@ impl App {
             self.status =
                 "The diagram changed. The name is retained; cancel and restart this placement."
                     .into();
+            if let Some(s) = &mut self.architecture {
+                s.saved.placing = false;
+                s.error = Some(self.status.clone());
+            }
             return;
         }
         let diagram = s.saved.diagram;
@@ -553,9 +587,10 @@ pub fn inline_editor(app: &App) -> Option<Element<'_, Message>> {
             row![
                 iced::widget::Space::new().width(x),
                 iced::widget::opaque(
-                    iced::widget::container(name_editor(app))
+                    iced::widget::container(scrollable(name_editor(app)))
                         .padding(10)
                         .width(width)
+                        .max_height((size.height - y - 8.).max(0.))
                         .style(crate::style::card)
                 )
             ]

crates/oa-desktop/src/app/architecture_authoring/palette.rs

@@ -165,7 +165,7 @@ fn tool<'a>(
     .into()
 }
 pub fn view(app: &App) -> Element<'_, Message> {
-    let mut content = column![tool(
+    let mut content = iced::widget::column![tool(
         app,
         Icon::Collapse(app.architecture_palette_expanded),
         if app.architecture_palette_expanded {

crates/oa-desktop/src/app/architecture_authoring/tests.rs

@@ -0,0 +1,378 @@
+use super::*;
+
+fn setup(layer: Layer) -> App {
+    let mut app = App::with_project(Project::empty());
+    app.layer = layer;
+    app.active_diagram = None;
+    let _ = app.update(Message::AddDiagram);
+    app.zoom = 0.75;
+    app.pan = iced::Vector::new(37., -19.);
+    app
+}
+fn send(app: &mut App, event: Event) {
+    let _ = app.update(Message::Architecture(event));
+}
+fn create(app: &mut App, kind: Kind, name: &str) -> Id {
+    send(app, Event::Start(kind, Some(Position { x: 120., y: 160. })));
+    send(app, Event::Name(name.into()));
+    send(app, Event::Submit);
+    assert!(app.architecture.is_none(), "{}", app.status);
+    app.selected
+}
+
+#[test]
+fn architecture_allocated_work_is_root_owned_and_one_exact_history_change() {
+    for (layer, participant, work) in [
+        (Layer::Oa, Kind::Entity, Kind::Activity),
+        (Layer::Sa, Kind::System, Kind::Function),
+    ] {
+        let mut app = setup(layer);
+        let performer = create(&mut app, participant, "Control team");
+        let before = app.document.bundle().clone();
+        let camera = (app.zoom, app.pan);
+        let point = Position { x: 151., y: 221. };
+        send(&mut app, Event::Allocate(work, performer, Some(point)));
+        send(&mut app, Event::Name("  Report 雨  ".into()));
+        assert_eq!(app.document.bundle(), &before);
+        send(&mut app, Event::Submit);
+        assert!(app.architecture.is_none(), "{}", app.status);
+        let created = app.selected;
+        let element = &app.project().elements[&created];
+        assert_eq!(element.kind(), work);
+        assert_eq!(element.name, "  Report 雨  ");
+        assert_eq!(element.parent, None);
+        assert!(app.project().relations.values().any(|r| {
+            r.kind() == RelationKind::Allocation && r.source() == performer && r.target() == created
+        }));
+        let d = &app.project().diagrams[&app.active_diagram.unwrap()];
+        assert_eq!(d.nodes[&app.selected_node.unwrap()].position, point);
+        assert_eq!((app.zoom, app.pan), camera);
+        let accepted = app.document.bundle().clone();
+        let _ = app.update(Message::Undo);
+        assert_eq!(app.document.bundle(), &before);
+        let _ = app.update(Message::Redo);
+        assert_eq!(app.document.bundle(), &accepted);
+    }
+}
+
+#[test]
+fn architecture_f2_routes_selected_node_to_inline_rename_and_exact_undo() {
+    use iced::widget::canvas::{Event as CanvasEvent, Program};
+    use iced::{keyboard, mouse};
+    let mut app = setup(Layer::Oa);
+    let element = create(&mut app, Kind::Entity, "Original team");
+    let before = app.document.bundle().clone();
+    let diagrams = app.project().diagrams.clone();
+    let camera = (app.zoom, app.pan);
+    let bounds = iced::Rectangle::new(iced::Point::ORIGIN, iced::Size::new(600., 380.));
+    let canvas = crate::diagram::DiagramCanvas {
+        project: app.project(),
+        view_index: None,
+        diagram: app.active_diagram.unwrap(),
+        selected: element,
+        zoom: app.zoom,
+        pan: app.pan,
+        context: app.route_context().unwrap(),
+        selected_bend: None,
+        editable: true,
+        placing: false,
+        connection: None,
+    };
+    let mut state = crate::diagram::State::default();
+    let key = CanvasEvent::Keyboard(keyboard::Event::KeyPressed {
+        key: keyboard::Key::Named(keyboard::key::Named::F2),
+        modified_key: keyboard::Key::Named(keyboard::key::Named::F2),
+        physical_key: keyboard::key::Physical::Code(keyboard::key::Code::F2),
+        location: keyboard::Location::Standard,
+        modifiers: keyboard::Modifiers::empty(),
+        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.)),
+    );
+    let action = canvas
+        .update(&mut state, &key, bounds, mouse::Cursor::Unavailable)
+        .unwrap();
+    let message = action.into_inner().0.unwrap();
+    let _ = app.update(message);
+    assert!(app.architecture.as_ref().unwrap().inline());
+    assert!(matches!(&app.architecture.as_ref().unwrap().saved.intent,
+        Intent::Rename { element: id, name } if *id == element && name == "Original team"));
+    send(&mut app, Event::Name("  Renamed team 雨  ".into()));
+    assert_eq!(app.document.bundle(), &before);
+    send(&mut app, Event::Submit);
+    assert!(app.architecture.is_none(), "{}", app.status);
+    assert_eq!(app.project().elements[&element].name, "  Renamed team 雨  ");
+    assert_eq!(app.project().diagrams, diagrams);
+    assert_eq!((app.zoom, app.pan), camera);
+    let accepted = app.document.bundle().clone();
+    let _ = app.update(Message::Undo);
+    assert_eq!(app.document.bundle(), &before);
+    let _ = app.update(Message::Redo);
+    assert_eq!(app.document.bundle(), &accepted);
+}
+
+#[test]
+fn architecture_blank_names_and_escape_never_accept_placeholder_or_rename() {
+    let mut app = setup(Layer::Oa);
+    let element = create(&mut app, Kind::Actor, "Dispatcher");
+    let before = app.document.bundle().clone();
+    let context = app.route_context().unwrap();
+    for event in [
+        Event::Start(Kind::Activity, Some(Position { x: 220., y: 330. })),
+        Event::Rename(context, element, Position { x: 150., y: 200. }),
+    ] {
+        // Every cancel changes the gesture context; use the current context for F2.
+        let event = match event {
+            Event::Rename(_, element, point) => {
+                Event::Rename(app.route_context().unwrap(), element, point)
+            }
+            event => event,
+        };
+        send(&mut app, event);
+        send(&mut app, Event::Name(" \t ".into()));
+        send(&mut app, Event::Submit);
+        assert!(app.architecture.as_ref().unwrap().inline());
+        assert!(app.architecture.as_ref().unwrap().error.is_some());
+        assert_eq!(app.document.bundle(), &before);
+        let _ = app.update(Message::Escape);
+        assert!(app.architecture.is_none());
+        assert_eq!(app.document.bundle(), &before);
+    }
+}
+
+#[test]
+fn architecture_invalid_click_cannot_poison_inline_or_checkpoint_geometry() {
+    let mut app = setup(Layer::Oa);
+    send(&mut app, Event::Start(Kind::Entity, None));
+    let before = app.document.bundle().clone();
+    let context = app.route_context().unwrap();
+    for point in [
+        Position { x: f32::NAN, y: 0. },
+        Position {
+            x: 0.,
+            y: f32::INFINITY,
+        },
+    ] {
+        send(&mut app, Event::Click(context, point));
+        let draft = app.architecture.as_ref().unwrap();
+        assert!(draft.unnamed_tool());
+        assert!(validate_saved(app.project(), &draft.saved).is_ok());
+        assert_eq!(app.document.bundle(), &before);
+    }
+    send(&mut app, Event::Click(context, Position { x: 25., y: 30. }));
+    assert!(app.architecture.as_ref().unwrap().inline());
+}
+
+#[test]
+fn architecture_stale_submit_keeps_create_and_rename_visible_and_exact() {
+    for rename in [false, true] {
+        for stale_document in [false, true] {
+            let mut app = setup(Layer::Oa);
+            let element = create(&mut app, Kind::Entity, "Accepted team");
+            let point = Position {
+                x: 151.5,
+                y: 227.25,
+            };
+            let event = if rename {
+                Event::Rename(app.route_context().unwrap(), element, point)
+            } else {
+                Event::Start(Kind::Activity, Some(point))
+            };
+            send(&mut app, event);
+            send(
+                &mut app,
+                Event::Name("  Retained engineering name 雨  ".into()),
+            );
+            // Change only the binding. The accepted model/source must not be
+            // rewritten or a rejected draft silently rebound to the new context.
+            if stale_document {
+                app.architecture.as_mut().unwrap().document = Id::new_v4();
+            } else {
+                app.architecture.as_mut().unwrap().revision = app.document.revision() + 1;
+            }
+            let draft = app.architecture.as_ref().unwrap();
+            let saved = serde_json::to_value(&draft.saved).unwrap();
+            let binding = (draft.document, draft.revision);
+            let model = app.project().clone();
+            let source = app.document.bundle().clone();
+            let revision = app.document.revision();
+            let camera = (app.zoom, app.pan);
+
+            for _ in 0..2 {
+                send(&mut app, Event::Submit);
+                let draft = app.architecture.as_ref().unwrap();
+                assert!(
+                    draft.inline(),
+                    "rename={rename}, stale_document={stale_document}"
+                );
+                assert!(inline_editor(&app).is_some());
+                assert_eq!(draft.error.as_deref(), Some(app.status.as_str()));
+                assert!(app.status.contains("cancel and restart"));
+                assert_eq!(serde_json::to_value(&draft.saved).unwrap(), saved);
+                assert_eq!((draft.document, draft.revision), binding);
+                assert_eq!(app.project(), &model);
+                assert_eq!(app.document.bundle(), &source);
+                assert_eq!(app.document.revision(), revision);
+                assert_eq!((app.zoom, app.pan), camera);
+            }
+            let _ = app.update(Message::Escape);
+            assert!(app.architecture.is_none());
+            assert_eq!(app.project(), &model);
+            assert_eq!(app.document.bundle(), &source);
+            let fresh = create(&mut app, Kind::Actor, "Fresh context");
+            assert_eq!(app.project().elements[&fresh].name, "Fresh context");
+            let _ = app.update(Message::Undo);
+            assert_eq!(app.project(), &model);
+            assert_eq!(app.document.bundle(), &source);
+        }
+    }
+}
+
+fn key_event(named: iced::keyboard::key::Named) -> iced::Event {
+    use iced::keyboard;
+    iced::Event::Keyboard(keyboard::Event::KeyPressed {
+        key: keyboard::Key::Named(named),
+        modified_key: keyboard::Key::Named(named),
+        physical_key: keyboard::key::Physical::Code(keyboard::key::Code::Escape),
+        location: keyboard::Location::Standard,
+        modifiers: keyboard::Modifiers::empty(),
+        text: None,
+        repeat: false,
+    })
+}
+
+fn focused_text_input_escape(name: &str) -> Message {
+    use iced::advanced::{Layout, Shell, layout, widget};
+    let mut field: iced::Element<'_, (), iced::Theme, ()> = text_input("Engineering name", name)
+        .id("architecture-name")
+        .on_input(|_| ())
+        .into();
+    let mut tree = widget::Tree::new(field.as_widget());
+    let size = iced::Size::new(300., 40.);
+    let node =
+        field
+            .as_widget_mut()
+            .layout(&mut tree, &(), &layout::Limits::new(iced::Size::ZERO, size));
+    tree.state
+        .downcast_mut::<iced::widget::text_input::State<()>>()
+        .focus();
+    let escape = key_event(iced::keyboard::key::Named::Escape);
+    let mut messages = Vec::new();
+    let mut shell = Shell::new(&mut messages);
+    field.as_widget_mut().update(
+        &mut tree,
+        &escape,
+        Layout::new(&node),
+        iced::mouse::Cursor::Unavailable,
+        &(),
+        &mut iced::advanced::clipboard::Null,
+        &mut shell,
+        &iced::Rectangle::new(iced::Point::ORIGIN, size),
+    );
+    assert!(
+        shell.is_event_captured(),
+        "Iced consumes focused-input Escape"
+    );
+    assert!(
+        !tree
+            .state
+            .downcast_ref::<iced::widget::text_input::State<()>>()
+            .is_focused()
+    );
+    captured_escape(
+        escape,
+        iced::event::Status::Captured,
+        iced::window::Id::unique(),
+    )
+    .unwrap()
+}
+
+#[test]
+fn architecture_first_focused_input_escape_cancels_create_and_rename() {
+    for rename in [false, true] {
+        let mut app = setup(Layer::Oa);
+        let element = create(&mut app, Kind::Actor, "Accepted dispatcher");
+        let model = app.project().clone();
+        let source = app.document.bundle().clone();
+        let revision = app.document.revision();
+        let camera = (app.zoom, app.pan);
+        let point = Position { x: 439., y: 328.2 };
+        let event = if rename {
+            Event::Rename(app.route_context().unwrap(), element, point)
+        } else {
+            Event::Start(Kind::Actor, Some(point))
+        };
+        send(&mut app, event);
+        let name = "  Unaccepted long engineering name 雨  ";
+        send(&mut app, Event::Name(name.into()));
+        assert!(app.architecture.as_ref().unwrap().inline());
+        let message = focused_text_input_escape(name);
+        let _ = app.update(message);
+        assert!(app.architecture.is_none());
+        assert!(inline_editor(&app).is_none());
+        assert_eq!(app.status, "Cancelled · Select / move");
+        assert_eq!(app.project(), &model);
+        assert_eq!(app.document.bundle(), &source);
+        assert_eq!(app.document.revision(), revision);
+        assert_eq!((app.zoom, app.pan), camera);
+    }
+}
+
+#[test]
+fn architecture_captured_escape_does_not_cancel_other_controls_or_placement() {
+    let mut app = setup(Layer::Oa);
+    for event in [
+        None,
+        Some(Event::Start(Kind::Entity, None)),
+        Some(Event::Existing(None)),
+    ] {
+        send(&mut app, Event::Cancel);
+        if let Some(event) = event {
+            send(&mut app, event);
+        }
+        let saved = app
+            .architecture
+            .as_ref()
+            .map(|s| serde_json::to_value(&s.saved).unwrap());
+        let before = app.document.bundle().clone();
+        let status = app.status.clone();
+        let epoch = app.canvas_epoch;
+        let message = focused_text_input_escape("Other field or existing-element search");
+        let _ = app.update(message);
+        assert_eq!(
+            app.architecture
+                .as_ref()
+                .map(|s| serde_json::to_value(&s.saved).unwrap()),
+            saved
+        );
+        assert_eq!(app.document.bundle(), &before);
+        assert_eq!(app.status, status);
+        assert_eq!(app.canvas_epoch, epoch);
+    }
+    assert!(
+        captured_escape(
+            key_event(iced::keyboard::key::Named::Escape),
+            iced::event::Status::Ignored,
+            iced::window::Id::unique(),
+        )
+        .is_none()
+    );
+    assert!(
+        captured_escape(
+            key_event(iced::keyboard::key::Named::Enter),
+            iced::event::Status::Captured,
+            iced::window::Id::unique(),
+        )
+        .is_none()
+    );
+}

crates/oa-desktop/src/diagram.rs

@@ -621,8 +621,9 @@ impl canvas::Program<Message> for DiagramCanvas<'_> {
                     Key::Named(Named::F2)
                         if self.editable && self.connection.is_none() && modifiers.is_empty() =>
                     {
-                        let view = self.scene(state).view;
-                        let node = view
+                        let scene = self.scene(state);
+                        let node = scene
+                            .view
                             .nodes
                             .iter()
                             .find(|n| n.element == self.selected || n.id == self.selected)?;

crates/oa-desktop/src/guide.rs

@@ -187,8 +187,8 @@ fn article(topic: Topic) -> Article {
                             &[
                                 "Choose New in the top bar. Use Project properties to name the model and describe its purpose. Apply changes commits the property draft.",
                                 "On Architecture, choose Create architecture view. Architecture · allocated work shows work inside its performer.",
-                                "Choose Entity in the diagram's Create toolbar. Enter Rescue coordinator, choose Choose position, then click the diagram. Use Actor for an external participant such as a Vessel in distress. You can also right-click a blank area, choose Create Entity here, enter its name and choose Create here.",
-                                "Choose Activity in the same toolbar, name it Assess incident and place it. Select Rescue coordinator and use + Relationship with Allocation to assign Assess incident to it. The allocated-work view then shows the activity inside its performer.",
+                                "Expand the diagram's side palette to see tool names. Choose Entity, click its position in the diagram, enter Rescue coordinator and press Enter. Escape cancels the draft. Use Actor for an external participant such as a Vessel in distress. Right-clicking a blank area also offers Create Entity here.",
+                                "Right-click Rescue coordinator and choose Add activity. Enter Assess incident and press Enter to create the activity and its performer allocation together. Ordinary Activity creation in the palette leaves work unallocated; use + Relationship with Allocation to assign existing work. The allocated-work view shows work inside its performer.",
                                 "Use Add existing to place an element that is already in the model. The initial choices match this view; no search is required. Include already shown offers Locate for elements already placed here.",
                                 "Add a second activity for another participant. Select an activity, choose + Relationship, select Interaction and choose the compatible target. Name the exchange after what is conveyed.",
                                 "Visit Model checks to review unfinished work. Save all writes the accepted model and applied drafts to your chosen file.",
@@ -262,8 +262,8 @@ fn article(topic: Topic) -> Article {
                             "Create and allocate work",
                             &[
                                 "Choose the Operational or System perspective. Create participants using Entity/Actor in OA or System/System actor in SA.",
-                                "In the diagram's Create toolbar, choose Activity in OA or Function in SA, enter a name and click to place it. For semantic decomposition, use Model explorer, select existing work and use + In selected to create smaller work units.",
-                                "To assign work to a participant, use + Relationship with Allocation from the participant to the leaf work. Model explorer also offers + Allocated when a participant and the appropriate work kind are selected.",
+                                "In the diagram's side palette, choose Activity in OA or Function in SA, click its position, enter a name and press Enter. Escape cancels without creating an element. Select a card and press F2 to rename it in place. For semantic decomposition, use Model explorer, select existing work and use + In selected to create smaller work units.",
+                                "Right-click a participant and choose Add activity or Add function to create new work and its allocation together. To assign existing leaf work, use + Relationship with Allocation from the participant to the work. Model explorer also offers + Allocated when a participant and the appropriate work kind are selected.",
                                 "Choose Architecture · allocated work to review responsibilities. Choose Functional dataflow to review the activity/function hierarchy and its exchanges independently.",
                             ],
                         ),