diff --git a/crates/oa-desktop/src/app.rs b/crates/oa-desktop/src/app.rs index 596d4b43..28ff8eec 100644 --- a/crates/oa-desktop/src/app.rs +++ b/crates/oa-desktop/src/app.rs @@ -14,6 +14,7 @@ mod diagram_reveal; #[cfg(test)] mod diagram_selection_tests; mod diagram_usages; +pub(crate) mod escape; mod hosted; mod reference_authoring; #[cfg(test)] @@ -1164,7 +1165,7 @@ impl App { crate::model_menu::popup(self).or_else(|| self.text_workspace_menu()), ) .into(); - crate::source_navigation::interaction::observe( + let content = crate::source_navigation::interaction::observe( crate::behavior::scenario::focus::root( behavior::canvas_workspace::focus_root( crate::hosted::view::overlay( @@ -1176,7 +1177,8 @@ impl App { self.scenario_focus_owner(), ), self.source_symbols.raw_interaction.clone(), - ) + ); + escape::root(self, content) } pub fn subscription(&self) -> Subscription { Subscription::batch([ @@ -1218,7 +1220,6 @@ 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, _, _| { matches!( event, @@ -1226,20 +1227,6 @@ impl App { ) .then_some(Message::WorkDrag(allocation_drag::Event::KeyboardBoundary)) }), - // Use one captured-Escape bridge: a focused Find input consumes - // Escape, while modal priority remains in the ordinary handler. - behavior::creation::subscription( - self.text_workspace.menu().is_none() - && (self.behavior.creation.has_pending() - || self.diagram_usages.is_some() - || self.behavior.source_active() - || (self.page == Page::Text - && (self.text_workspace.drawer().is_some() - || self.source_navigation.open - || self.source_rename.open()) - && self.architecture.is_none())), - ) - .map(|()| Message::Escape), iced::event::listen_with(|event, status, _| { if matches!(event, iced::Event::Window(window::Event::Unfocused)) { return Some(Message::WorkDrag(allocation_drag::Event::Unfocused)); diff --git a/crates/oa-desktop/src/app/architecture_authoring.rs b/crates/oa-desktop/src/app/architecture_authoring.rs index 0e01bee8..243c243c 100644 --- a/crates/oa-desktop/src/app/architecture_authoring.rs +++ b/crates/oa-desktop/src/app/architecture_authoring.rs @@ -27,30 +27,11 @@ pub enum Event { Cancel, } -pub fn subscription(app: &App) -> Subscription { - 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 { - // 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)) +pub(super) fn captured_escape_message(app: &App) -> Option { + app.architecture + .as_ref() + .is_some_and(State::inline) + .then_some(Message::Architecture(Event::CapturedEscape)) } #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(deny_unknown_fields)] diff --git a/crates/oa-desktop/src/app/architecture_authoring/tests.rs b/crates/oa-desktop/src/app/architecture_authoring/tests.rs index 681e1882..3ab9d565 100644 --- a/crates/oa-desktop/src/app/architecture_authoring/tests.rs +++ b/crates/oa-desktop/src/app/architecture_authoring/tests.rs @@ -233,65 +233,6 @@ fn architecture_stale_submit_keeps_create_and_rename_visible_and_exact() { } } -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::>() - .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::>() - .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] { @@ -311,8 +252,9 @@ fn architecture_first_focused_input_escape_cancels_create_and_rename() { 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); + let mut messages = crate::app::escape::tests::focused_input_messages(&app); + assert_eq!(messages.len(), 1); + let _ = app.update(messages.remove(0)); assert!(app.architecture.is_none()); assert!(inline_editor(&app).is_none()); assert_eq!(app.status, "Cancelled · Select / move"); @@ -342,8 +284,8 @@ fn architecture_captured_escape_does_not_cancel_other_controls_or_placement() { 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); + let messages = crate::app::escape::tests::focused_input_messages(&app); + assert!(messages.is_empty()); assert_eq!( app.architecture .as_ref() @@ -355,19 +297,7 @@ fn architecture_captured_escape_does_not_cancel_other_controls_or_placement() { 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() + crate::app::escape::tests::focused_input_key(&app, iced::keyboard::key::Named::Enter,) + .is_empty() ); } diff --git a/crates/oa-desktop/src/app/escape.rs b/crates/oa-desktop/src/app/escape.rs new file mode 100644 index 00000000..8683f30a --- /dev/null +++ b/crates/oa-desktop/src/app/escape.rs @@ -0,0 +1,198 @@ +//! Keep overlay Escape ahead of modal cancellation in the ordinary widget tree. +//! +//! Iced skips root.update when an overlay captures an event. Keeping this +//! wrapper present even when inactive preserves the child's widget state. A +//! current-overlay preflight also handles pickers opened mid-batch. Passive +//! overlays may see an ignored Escape again: the raw-input observer only makes +//! pending focus requests stale; it does not publish modal cancellation. +use super::{App, Message, Page, architecture_authoring}; +use iced::advanced::{Clipboard, Layout, Shell, Widget, layout, overlay, renderer, widget}; +use iced::{Element, Event, Length, Rectangle, Size, Theme, Vector, keyboard, mouse}; + +pub(crate) fn root<'a, R: renderer::Renderer + 'a>( + app: &App, + content: Element<'a, Message, Theme, R>, +) -> Element<'a, Message, Theme, R> { + let modal = app.text_workspace.menu().is_none() + && (app.behavior.creation.has_pending() + || app.diagram_usages.is_some() + || app.behavior.source_active() + || (app.page == Page::Text + && (app.text_workspace.drawer().is_some() + || app.source_navigation.open + || app.source_rename.open()) + && app.architecture.is_none())); + Element::new(Bridge { + content, + messages: [ + architecture_authoring::captured_escape_message(app), + modal.then_some(Message::Escape), + ], + }) +} + +struct Bridge<'a, R> { + content: Element<'a, Message, Theme, R>, + messages: [Option; 2], +} + +impl Widget for Bridge<'_, R> { + fn children(&self) -> Vec { + vec![widget::Tree::new(self.content.as_widget())] + } + + fn diff(&self, tree: &mut widget::Tree) { + tree.diff_children(std::slice::from_ref(&self.content)); + } + + fn size(&self) -> Size { + self.content.as_widget().size() + } + + fn size_hint(&self) -> Size { + self.content.as_widget().size_hint() + } + + fn layout( + &mut self, + tree: &mut widget::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 widget::Tree, + layout: Layout<'_>, + renderer: &R, + operation: &mut dyn widget::Operation, + ) { + self.content + .as_widget_mut() + .operate(&mut tree.children[0], layout, renderer, operation); + } + + fn update( + &mut self, + tree: &mut widget::Tree, + event: &Event, + layout: Layout<'_>, + cursor: mouse::Cursor, + renderer: &R, + clipboard: &mut dyn Clipboard, + shell: &mut Shell<'_, Message>, + viewport: &Rectangle, + ) { + let escape = matches!( + event, + Event::Keyboard(keyboard::Event::KeyPressed { + key: keyboard::Key::Named(keyboard::key::Named::Escape), + .. + }) + ); + // Iced collects overlays before processing a batch of base events. A + // preceding click in that batch can therefore open a picker whose + // Escape would otherwise reach focused fields underneath it. Offer only + // Escape to the current overlay before updating the ordinary tree. + if escape { + if let Some(element) = self.content.as_widget_mut().overlay( + &mut tree.children[0], + layout, + renderer, + viewport, + Vector::ZERO, + ) { + let mut overlay = overlay::Nested::new(element); + let node = overlay.layout(renderer, viewport.size()); + overlay.update( + event, + Layout::new(&node), + cursor, + renderer, + clipboard, + shell, + ); + } + if shell.is_event_captured() { + return; + } + } + self.content.as_widget_mut().update( + &mut tree.children[0], + event, + layout, + cursor, + renderer, + clipboard, + shell, + viewport, + ); + if shell.is_event_captured() && escape { + for message in self.messages.iter().flatten() { + shell.publish(message.clone()); + } + } + } + + fn draw( + &self, + tree: &widget::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, + ); + } + + fn mouse_interaction( + &self, + tree: &widget::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 widget::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, + ) + } +} + +#[cfg(test)] +pub(crate) mod tests; diff --git a/crates/oa-desktop/src/app/escape/tests.rs b/crates/oa-desktop/src/app/escape/tests.rs new file mode 100644 index 00000000..705d1619 --- /dev/null +++ b/crates/oa-desktop/src/app/escape/tests.rs @@ -0,0 +1,618 @@ +use super::*; +use crate::behavior::canvas_workspace::tests::key; +use iced::advanced::{subscription, widget::operation}; +use iced::futures::{StreamExt, stream, task::noop_waker_ref}; +use iced::keyboard::key::Named; +use iced::widget::{column, container, pick_list, text_input}; +use iced_runtime::user_interface::{Cache, UserInterface}; +use oa_core::*; +use std::task::{Context, Poll}; + +struct FocusInputs; +impl widget::Operation for FocusInputs { + fn traverse(&mut self, visit: &mut dyn FnMut(&mut dyn widget::Operation)) { + visit(self); + } + fn focusable( + &mut self, + _: Option<&widget::Id>, + _: Rectangle, + state: &mut dyn operation::Focusable, + ) { + state.focus(); + } +} + +pub(crate) fn subscriptions( + app: &App, + event: Event, + status: iced::event::Status, + messages: &mut Vec, +) { + let interaction = subscription::Event::Interaction { + window: iced::window::Id::unique(), + event, + status, + }; + let mut boundaries = 0; + for recipe in subscription::into_recipes(app.subscription()) { + let mut output = recipe.stream(Box::pin(stream::iter([interaction.clone()]))); + let mut cx = Context::from_waker(noop_waker_ref()); + while let Poll::Ready(Some(message)) = output.poll_next_unpin(&mut cx) { + if matches!( + message, + Message::WorkDrag(crate::app::allocation_drag::Event::KeyboardBoundary) + ) { + boundaries += 1; + } else { + messages.push(message); + } + } + } + assert_eq!( + boundaries, 1, + "one passive allocation keyboard notification" + ); +} + +/// A real focused text input, production root bridge, UserInterface dispatch, +/// and actual subscriptions. Existing app-policy tests use this instead of +/// pretending every captured event came from a text control. +pub(crate) fn focused_input_messages(app: &App) -> Vec { + focused_input_key(app, Named::Escape) +} + +pub(crate) fn focused_input_key(app: &App, named: Named) -> Vec { + input_messages(app, named, true) +} + +pub(crate) fn input_messages(app: &App, named: Named, focused: bool) -> Vec { + let runtime = tokio::runtime::Builder::new_current_thread() + .enable_time() + .build() + .unwrap(); + let _entered = runtime.enter(); + let input: Element<'_, Message, Theme, ()> = text_input("Retained input", "Uncommitted 雨") + .on_input(|_| Message::Escape) + .into(); + let mut renderer = (); + let mut ui = UserInterface::build( + root(app, input), + Size::new(400., 100.), + Cache::new(), + &mut renderer, + ); + if focused { + ui.operate(&renderer, &mut FocusInputs); + } + let event = key(named); + let mut messages = vec![]; + let (_, statuses) = ui.update( + std::slice::from_ref(&event), + mouse::Cursor::Unavailable, + &mut renderer, + &mut iced::advanced::clipboard::Null, + &mut messages, + ); + if named == Named::Escape { + assert_eq!( + statuses, + [if focused { + iced::event::Status::Captured + } else { + iced::event::Status::Ignored + }] + ); + } + subscriptions(app, event, statuses[0], &mut messages); + messages +} + +#[derive(Clone, Debug, PartialEq)] +enum PickMessage { + Selected(&'static str), + Opened, + Closed, +} + +fn picker( + selected: Option<&'static str>, + notify: bool, +) -> Element<'static, PickMessage, Theme, ()> { + let list = pick_list(["Alpha", "Beta"], selected, PickMessage::Selected) + .width(200) + .padding(10) + .text_size(16) + .on_open(PickMessage::Opened); + if notify { + list.on_close(PickMessage::Closed).into() + } else { + list.into() + } +} + +fn update( + ui: &mut UserInterface<'_, M, Theme, ()>, + events: &[Event], + cursor: mouse::Cursor, +) -> (Vec, Vec) { + let mut messages = vec![]; + let (_, statuses) = ui.update( + events, + cursor, + &mut (), + &mut iced::advanced::clipboard::Null, + &mut messages, + ); + (messages, statuses) +} + +fn press_at(point: iced::Point) -> (Vec, mouse::Cursor) { + ( + vec![ + Event::Mouse(mouse::Event::CursorMoved { position: point }), + Event::Mouse(mouse::Event::ButtonPressed(mouse::Button::Left)), + Event::Mouse(mouse::Event::ButtonReleased(mouse::Button::Left)), + ], + mouse::Cursor::Available(point), + ) +} + +#[test] +fn picker_overlay_escape_closes_once_reopens_selects_and_retains_outside_click() { + for selected in [None, Some("Alpha")] { + for notify in [false, true] { + let mut ui = UserInterface::build( + picker(selected, notify), + Size::new(400., 300.), + Cache::new(), + &mut (), + ); + let (click, cursor) = press_at(iced::Point::new(50., 15.)); + assert_eq!(update(&mut ui, &click, cursor).0, [PickMessage::Opened]); + let cache = ui.into_cache(); + let mut ui = UserInterface::build( + picker(selected, notify), + Size::new(400., 300.), + cache, + &mut (), + ); + let (messages, statuses) = update( + &mut ui, + &[key(Named::Escape), key(Named::Escape)], + mouse::Cursor::Unavailable, + ); + assert_eq!( + messages, + if notify { + vec![PickMessage::Closed] + } else { + vec![] + } + ); + assert_eq!( + statuses, + [iced::event::Status::Captured, iced::event::Status::Ignored] + ); + let cache = ui.into_cache(); + let mut ui = UserInterface::build( + picker(selected, notify), + Size::new(400., 300.), + cache, + &mut (), + ); + assert_eq!(update(&mut ui, &click, cursor).0, [PickMessage::Opened]); + let cache = ui.into_cache(); + let mut ui = UserInterface::build( + picker(selected, notify), + Size::new(400., 300.), + cache, + &mut (), + ); + let (outside, cursor) = press_at(iced::Point::new(350., 250.)); + assert_eq!( + update(&mut ui, &outside, cursor).0, + if notify { + vec![PickMessage::Closed] + } else { + vec![] + } + ); + let cache = ui.into_cache(); + let mut ui = UserInterface::build( + picker(selected, notify), + Size::new(400., 300.), + cache, + &mut (), + ); + let (click, cursor) = press_at(iced::Point::new(50., 15.)); + assert_eq!(update(&mut ui, &click, cursor).0, [PickMessage::Opened]); + let cache = ui.into_cache(); + let mut ui = UserInterface::build( + picker(selected, notify), + Size::new(400., 300.), + cache, + &mut (), + ); + // Default 16px text uses 1.3 line-height: button/option height 40.8. + let (option, cursor) = press_at(iced::Point::new(50., 100.)); + assert_eq!( + update(&mut ui, &option, cursor).0, + [PickMessage::Selected("Beta")] + ); + let cache = ui.into_cache(); + let mut ui = UserInterface::build( + picker(Some("Beta"), notify), + Size::new(400., 300.), + cache, + &mut (), + ); + assert_eq!( + update(&mut ui, &[key(Named::Escape)], mouse::Cursor::Unavailable), + (vec![], vec![iced::event::Status::Ignored]) + ); + } + } +} + +#[test] +fn initially_closed_picker_handles_open_then_escape_in_one_runtime_batch() { + for notify in [false, true] { + let mut ui = UserInterface::build( + picker(Some("Alpha"), notify), + Size::new(400., 300.), + Cache::new(), + &mut (), + ); + let (mut events, cursor) = press_at(iced::Point::new(50., 15.)); + events.push(key(Named::Escape)); + let (messages, statuses) = update(&mut ui, &events, cursor); + assert_eq!( + messages, + if notify { + vec![PickMessage::Opened, PickMessage::Closed] + } else { + vec![PickMessage::Opened] + } + ); + assert_eq!(statuses.last(), Some(&iced::event::Status::Captured)); + assert_eq!( + update(&mut ui, &[key(Named::Escape)], mouse::Cursor::Unavailable), + (vec![], vec![iced::event::Status::Ignored]) + ); + } +} + +#[test] +fn dismissed_picker_ignores_a_queued_click_in_its_old_option_area() { + let mut ui = UserInterface::build( + picker(Some("Alpha"), true), + Size::new(400., 300.), + Cache::new(), + &mut (), + ); + let (click, cursor) = press_at(iced::Point::new(50., 15.)); + assert_eq!(update(&mut ui, &click, cursor).0, [PickMessage::Opened]); + let cache = ui.into_cache(); + let mut ui = UserInterface::build( + picker(Some("Alpha"), true), + Size::new(400., 300.), + cache, + &mut (), + ); + let (click, cursor) = press_at(iced::Point::new(50., 100.)); + let mut batch = vec![key(Named::Escape)]; + batch.extend(click); + let (messages, _) = update(&mut ui, &batch, cursor); + assert_eq!( + messages, + [PickMessage::Closed], + "queued old-option click must not select Beta" + ); +} + +fn draft_with_picker(app: &App) -> Element<'_, Message, Theme, ()> { + // This composition deliberately keeps a focused input beneath the actual + // picker overlay. It tests event ownership, not a claimed production form. + root( + app, + crate::source_navigation::interaction::observe( + column![ + container( + text_input("Raw draft", "Keep exact bytes 雨").on_input(|_| Message::Escape) + ) + .height(40), + pick_list([Layer::Oa, Layer::Sa], Some(Layer::Oa), Message::Layer) + .width(200) + .padding(10), + ] + .into(), + app.source_symbols.raw_interaction.clone(), + ), + ) +} + +#[test] +fn picker_overlay_precedes_focused_input_and_creation_source_drafts() { + use crate::behavior::{Event as BehaviorEvent, creation, source_draft}; + let runtime = tokio::runtime::Builder::new_current_thread() + .enable_time() + .build() + .unwrap(); + let _entered = runtime.enter(); + for (source, same_batch) in [(false, false), (true, false), (false, true), (true, true)] { + let mut app = App::with_project(Project::empty()); + let send = |app: &mut App, event| { + let _ = app.update(Message::Behavior(BehaviorEvent::Creation(event))); + }; + send(&mut app, creation::Event::Open(Layer::Oa)); + send(&mut app, creation::Event::Choose(creation::Family::Machine)); + send( + &mut app, + creation::Event::Name("Retained machine 雨".into()), + ); + if source { + let recovery = app.behavior.creation.recovery_snapshot().unwrap().unwrap(); + let invocation = serde_json::from_value(recovery["invocation"].clone()).unwrap(); + send( + &mut app, + creation::Event::Source(source_draft::Event::Open(invocation)), + ); + let recovery = app.behavior.creation.recovery_snapshot().unwrap().unwrap(); + let token = + serde_json::from_value(recovery["source"]["editing"]["token"].clone()).unwrap(); + send( + &mut app, + creation::Event::Source(source_draft::Event::Edit( + token, + source_draft::Edit::Name("Uncommitted owner 雨".into()), + )), + ); + assert_eq!( + app.behavior.creation.recovery_snapshot().unwrap().unwrap()["source"]["editing"]["name"], + "Uncommitted owner 雨" + ); + } + let draft = app.behavior.recovery_snapshot().unwrap(); + let project = app.project().clone(); + let bundle = app.document.bundle().clone(); + let revision = app.document.revision(); + let selection = (app.selected, app.selected_node, app.active_diagram); + let camera = (app.pan, app.zoom); + let raw_epoch = app + .source_symbols + .raw_interaction + .load(std::sync::atomic::Ordering::Acquire); + let mut ui = UserInterface::build( + draft_with_picker(&app), + Size::new(500., 400.), + Cache::new(), + &mut (), + ); + let event = key(Named::Escape); + let (mut click, cursor) = press_at(iced::Point::new(50., 55.)); + let (mut messages, statuses) = if same_batch { + ui.operate(&(), &mut FocusInputs); + click.push(event.clone()); + update(&mut ui, &click, cursor) + } else { + assert!(update(&mut ui, &click, cursor).0.is_empty()); + let cache = ui.into_cache(); + ui = UserInterface::build( + draft_with_picker(&app), + Size::new(500., 400.), + cache, + &mut (), + ); + ui.operate(&(), &mut FocusInputs); + update( + &mut ui, + std::slice::from_ref(&event), + mouse::Cursor::Unavailable, + ) + }; + assert_eq!(statuses.last(), Some(&iced::event::Status::Captured)); + subscriptions(&app, event, *statuses.last().unwrap(), &mut messages); + assert!( + messages.is_empty(), + "picker must not forward global modal cancellation" + ); + let cache = ui.into_cache(); + assert_eq!(app.behavior.recovery_snapshot().unwrap(), draft); + assert!(!crate::source_navigation::interaction::is_current( + &app.source_symbols.raw_interaction, + raw_epoch, + )); + let mut ui = UserInterface::build( + draft_with_picker(&app), + Size::new(500., 400.), + cache, + &mut (), + ); + let event = key(Named::Escape); + let (mut messages, statuses) = update( + &mut ui, + std::slice::from_ref(&event), + mouse::Cursor::Unavailable, + ); + subscriptions(&app, event, statuses[0], &mut messages); + assert_eq!(messages.len(), 1); + assert!(matches!(messages[0], Message::Escape)); + drop(ui); + let _ = app.update(messages.remove(0)); + if source { + assert!(app.behavior.creation.has_pending()); + assert!(!app.behavior.creation.source_open()); + assert_eq!( + app.behavior.creation.recovery_snapshot().unwrap().unwrap()["name"], + "Retained machine 雨" + ); + } else { + assert!(!app.behavior.creation.has_pending()); + } + assert_eq!(app.project(), &project); + assert_eq!(app.document.bundle(), &bundle); + assert_eq!(app.document.revision(), revision); + assert_eq!( + (app.selected, app.selected_node, app.active_diagram), + selection + ); + assert_eq!((app.pan, app.zoom), camera); + assert!(!app.document.can_undo() && !app.document.can_redo()); + } +} + +#[test] +fn ignored_tooltip_escape_preserves_modal_priority_and_rejects_stale_focus() { + use crate::source_navigation::interaction; + use iced::widget::{text, tooltip}; + use std::sync::atomic::Ordering; + + let runtime = tokio::runtime::Builder::new_current_thread() + .enable_time() + .build() + .unwrap(); + let _entered = runtime.enter(); + let mut app = App::with_project(Project::empty()); + app.page = Page::Text; + app.source_navigation.open = true; + let project = app.project().clone(); + let source = app.document.bundle().clone(); + let context = ( + app.selected, + app.selected_node, + app.active_diagram, + app.pan, + app.zoom, + ); + let content = || -> Element<'_, Message, Theme, ()> { + let field = text_input("Find", "Retained query 雨").on_input(|_| Message::Escape); + let tip = tooltip(field, text("Help"), tooltip::Position::Bottom) + .delay(std::time::Duration::ZERO); + root( + &app, + interaction::observe(tip.into(), app.source_symbols.raw_interaction.clone()), + ) + }; + let mut ui = UserInterface::build(content(), Size::new(400., 180.), Cache::new(), &mut ()); + let point = iced::Point::new(10., 10.); + let cursor = mouse::Cursor::Available(point); + assert!( + update( + &mut ui, + &[Event::Mouse(mouse::Event::CursorMoved { position: point })], + cursor + ) + .0 + .is_empty() + ); + let cache = ui.into_cache(); + let mut ui = UserInterface::build(content(), Size::new(400., 180.), cache, &mut ()); + ui.operate(&(), &mut FocusInputs); + let epoch = app.source_symbols.raw_interaction.load(Ordering::Acquire); + let event = key(Named::Escape); + let (mut messages, statuses) = update(&mut ui, std::slice::from_ref(&event), cursor); + assert_eq!(statuses, [iced::event::Status::Captured]); + subscriptions(&app, event, statuses[0], &mut messages); + assert_eq!(messages.len(), 1); + assert!(matches!(messages[0], Message::Escape)); + assert!(!interaction::is_current( + &app.source_symbols.raw_interaction, + epoch + )); + drop(ui); + let _ = app.update(messages.remove(0)); + assert!(!app.source_navigation.open); + assert_eq!(app.project(), &project); + assert_eq!(app.document.bundle(), &source); + assert_eq!( + ( + app.selected, + app.selected_node, + app.active_diagram, + app.pan, + app.zoom + ), + context + ); + assert!(!app.document.can_undo() && !app.document.can_redo()); +} + +#[test] +fn toggling_bridge_policy_retains_focused_child_state() { + let mut app = App::with_project(Project::empty()); + let content = || -> Element<'static, Message, Theme, ()> { + text_input("Raw field", "Still focused") + .on_input(|_| Message::Escape) + .into() + }; + let mut ui = UserInterface::build( + root(&app, content()), + Size::new(400., 100.), + Cache::new(), + &mut (), + ); + ui.operate(&(), &mut FocusInputs); + let cache = ui.into_cache(); + app.page = Page::Text; + app.source_navigation.open = true; + let mut ui = UserInterface::build(root(&app, content()), Size::new(400., 100.), cache, &mut ()); + let (messages, status) = update(&mut ui, &[key(Named::Escape)], mouse::Cursor::Unavailable); + assert_eq!(status, [iced::event::Status::Captured]); + assert_eq!(messages.len(), 1); + assert!(matches!(messages[0], Message::Escape)); +} + +#[tokio::test] +async fn mounted_creation_name_escape_keeps_pending_modal_priority() { + use crate::behavior::canvas_workspace::tests::Mounted; + use crate::behavior::{Event as BehaviorEvent, creation}; + for pending in [false, true] { + let mut app = App::with_project(Project::empty()); + for event in [ + creation::Event::Open(Layer::Oa), + creation::Event::Choose(creation::Family::Sequence), + creation::Event::Name("Exact unfinished process 雨".into()), + ] { + let _ = app.update(Message::Behavior(BehaviorEvent::Creation(event))); + } + app.page = Page::Behavior; + let mut mounted = Mounted::new(&mut app).await; + mounted + .perform( + &mut app, + iced::widget::operation::focus(creation::NAME_INPUT), + ) + .await; + if pending { + app.pending = Some(super::super::Pending::Walkthrough); + } + let before = app.document.snapshot(); + let draft = app.behavior.recovery_snapshot().unwrap(); + let event = key(Named::Escape); + let (mut messages, statuses) = mounted.input_with_status( + &app, + std::slice::from_ref(&event), + mouse::Cursor::Unavailable, + ); + subscriptions(&app, event, statuses[0], &mut messages); + assert_eq!( + messages + .iter() + .filter(|message| matches!(message, Message::Escape)) + .count(), + 1 + ); + for message in messages { + let _ = app.update(message); + } + if pending { + assert!(app.pending.is_none()); + assert_eq!(app.behavior.recovery_snapshot().unwrap(), draft); + } else { + assert!(!app.behavior.creation.has_pending()); + } + assert!(std::sync::Arc::ptr_eq(&before, &app.document.snapshot())); + assert!(!app.document.can_undo() && !app.document.can_redo()); + } +} diff --git a/crates/oa-desktop/src/app/source_rename/tests.rs b/crates/oa-desktop/src/app/source_rename/tests.rs index afc3fca7..49280a6d 100644 --- a/crates/oa-desktop/src/app/source_rename/tests.rs +++ b/crates/oa-desktop/src/app/source_rename/tests.rs @@ -754,76 +754,7 @@ fn invalid_open_retains_accepted_document_but_refuses_alias_rename() { } fn captured_escape(app: &App) -> Vec { - use iced::advanced::{Layout, Shell, layout, subscription, widget}; - use iced::futures::{StreamExt, stream, task::noop_waker_ref}; - use iced::keyboard::{self, key::Named}; - use std::task::{Context as PollContext, Poll}; - let mut input: iced::Element<'_, (), iced::Theme, ()> = - iced::widget::text_input("Source alias", "retained_alias") - .on_input(|_| ()) - .into(); - let mut tree = widget::Tree::new(input.as_widget()); - let size = iced::Size::new(300., 40.); - let node = - input - .as_widget_mut() - .layout(&mut tree, &(), &layout::Limits::new(iced::Size::ZERO, size)); - tree.state - .downcast_mut::>() - .focus(); - let event = iced::Event::Keyboard(keyboard::Event::KeyPressed { - key: keyboard::Key::Named(Named::Escape), - modified_key: keyboard::Key::Named(Named::Escape), - physical_key: keyboard::key::Physical::Code(keyboard::key::Code::Escape), - location: keyboard::Location::Standard, - modifiers: keyboard::Modifiers::empty(), - text: None, - repeat: false, - }); - let mut widget_messages = Vec::new(); - let mut shell = Shell::new(&mut widget_messages); - input.as_widget_mut().update( - &mut tree, - &event, - 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()); - let event = subscription::Event::Interaction { - window: iced::window::Id::unique(), - event, - status: iced::event::Status::Captured, - }; - let runtime = tokio::runtime::Builder::new_current_thread() - .enable_time() - .build() - .unwrap(); - let _entered = runtime.enter(); - let mut messages = Vec::new(); - let mut keyboard_boundaries = 0; - for recipe in subscription::into_recipes(app.subscription()) { - let mut output = recipe.stream(Box::pin(stream::iter([event.clone()]))); - let mut context = PollContext::from_waker(noop_waker_ref()); - while let Poll::Ready(Some(message)) = output.poll_next_unpin(&mut context) { - if matches!( - &message, - Message::WorkDrag(crate::app::allocation_drag::Event::KeyboardBoundary) - ) { - keyboard_boundaries += 1; - } else { - messages.push(message); - } - } - } - assert_eq!( - keyboard_boundaries, 1, - "each key has exactly one passive allocation notification" - ); - messages + crate::app::escape::tests::focused_input_messages(app) } #[test] diff --git a/crates/oa-desktop/src/app/source_symbols/tests.rs b/crates/oa-desktop/src/app/source_symbols/tests.rs index 3bab7195..866e465f 100644 --- a/crates/oa-desktop/src/app/source_symbols/tests.rs +++ b/crates/oa-desktop/src/app/source_symbols/tests.rs @@ -864,99 +864,11 @@ fn captured_input_subscription_messages(app: &App) -> Vec { } fn input_subscription_messages(app: &App, focused: bool) -> Vec { - use iced::advanced::{Layout, Shell, layout, subscription, widget}; - use iced::futures::{StreamExt, stream, task::noop_waker_ref}; - use iced::keyboard::{self, key::Named}; - use std::task::{Context, Poll}; - - let mut input: iced::Element<'_, (), iced::Theme, ()> = - iced::widget::text_input("Find / modal input", "Retain exact text 雨") - .on_input(|_| ()) - .into(); - let mut tree = widget::Tree::new(input.as_widget()); - let size = iced::Size::new(300., 40.); - let node = - input - .as_widget_mut() - .layout(&mut tree, &(), &layout::Limits::new(iced::Size::ZERO, size)); - if focused { - tree.state - .downcast_mut::>() - .focus(); - } - let event = iced::Event::Keyboard(keyboard::Event::KeyPressed { - key: keyboard::Key::Named(Named::Escape), - modified_key: keyboard::Key::Named(Named::Escape), - physical_key: keyboard::key::Physical::Code(keyboard::key::Code::Escape), - location: keyboard::Location::Standard, - modifiers: keyboard::Modifiers::empty(), - text: None, - repeat: false, - }); - let mut input_messages = Vec::new(); - let mut shell = Shell::new(&mut input_messages); - input.as_widget_mut().update( - &mut tree, - &event, - Layout::new(&node), - iced::mouse::Cursor::Unavailable, - &(), - &mut iced::advanced::clipboard::Null, - &mut shell, - &iced::Rectangle::new(iced::Point::ORIGIN, size), - ); - let captured = shell.is_event_captured(); - assert_eq!(captured, focused); - assert!(input_messages.is_empty()); - assert!( - !tree - .state - .downcast_ref::>() - .is_focused() - ); - let event = subscription::Event::Interaction { - window: iced::window::Id::unique(), - event, - status: if captured { - iced::event::Status::Captured - } else { - iced::event::Status::Ignored - }, - }; - let runtime = tokio::runtime::Builder::new_current_thread() - .enable_time() - .build() - .unwrap(); - let _entered = runtime.enter(); - let mut messages = Vec::new(); - let mut keyboard_boundaries = 0; - for recipe in subscription::into_recipes(app.subscription()) { - let mut output = recipe.stream(Box::pin(stream::iter([event.clone()]))); - let mut context = Context::from_waker(noop_waker_ref()); - while let Poll::Ready(Some(message)) = output.poll_next_unpin(&mut context) { - assert!( - !matches!(message, Message::CheckpointTick), - "timer must remain pending" - ); - if matches!( - &message, - Message::WorkDrag(crate::app::allocation_drag::Event::KeyboardBoundary) - ) { - keyboard_boundaries += 1; - } else { - messages.push(message); - } - } - } - assert_eq!( - keyboard_boundaries, 1, - "each key has exactly one passive allocation notification" - ); - messages + crate::app::escape::tests::input_messages(app, iced::keyboard::key::Named::Escape, focused) } #[test] -fn captured_find_and_modal_escape_use_one_actual_subscription_and_preserve_pending_work() { +fn captured_find_and_modal_escape_use_one_actual_delivery_and_preserve_pending_work() { for pending in [false, true] { for modal in 0..4 { let mut app = fixture(); @@ -1053,7 +965,7 @@ fn captured_find_and_modal_escape_use_one_actual_subscription_and_preserve_pendi } #[test] -fn captured_subscription_does_not_duplicate_existing_behavior_or_architecture_routes() { +fn captured_delivery_does_not_duplicate_existing_behavior_or_architecture_routes() { let mut app = fixture(); app.text_workspace.show_drawer(Some(DrawerTab::References)); let project = app.project().clone(); diff --git a/crates/oa-desktop/src/app/state_diagram_tests.rs b/crates/oa-desktop/src/app/state_diagram_tests.rs index ba4f9aae..c25de676 100644 --- a/crates/oa-desktop/src/app/state_diagram_tests.rs +++ b/crates/oa-desktop/src/app/state_diagram_tests.rs @@ -4,6 +4,8 @@ use super::*; mod attachment_escape; #[path = "state_bend_tests.rs"] mod bends; +#[path = "state_picker_escape_tests.rs"] +mod picker_escape; use crate::behavior::{ self, state_edit::{self, Context, Endpoint, Event, Tool}, diff --git a/crates/oa-desktop/src/app/state_picker_escape_tests.rs b/crates/oa-desktop/src/app/state_picker_escape_tests.rs new file mode 100644 index 00000000..63cbfcaf --- /dev/null +++ b/crates/oa-desktop/src/app/state_picker_escape_tests.rs @@ -0,0 +1,105 @@ +//! The actual State workspace picker, App root and subscriptions. +use super::*; +use crate::behavior::canvas_workspace::tests::{Mounted, key}; +use iced::{keyboard::key::Named, mouse}; + +async fn press_escape(mounted: &mut Mounted, app: &mut App) -> Vec { + let input = key(Named::Escape); + let (mut messages, statuses) = mounted.input_with_status( + app, + std::slice::from_ref(&input), + mouse::Cursor::Unavailable, + ); + crate::app::escape::tests::subscriptions(app, input, statuses[0], &mut messages); + for message in messages.iter().cloned() { + let task = app.update(message); + mounted.perform(app, task).await; + } + mounted.settle(app).await; + messages +} + +#[tokio::test] +async fn mounted_state_missing_picker_escape_preserves_exact_document_then_reopens_and_places() { + let mut f = fixture(); + let diagram = create_view(&mut f).diagram; + let mut project = f.app.project().clone(); + project + .diagrams + .get_mut(&diagram) + .unwrap() + .nodes + .retain(|_, node| node.element != f.b); + let mut app = App::with_project(project); + app.window_size = iced::Size::new(1800., 1100.); + send(&mut app, Message::Select(f.a)); + send(&mut app, Message::Page(Page::Behavior)); + let choice = Choice { + id: diagram, + label: app.project().diagrams[&diagram].name.clone(), + }; + send(&mut app, Message::BehaviorDiagram(choice)); + let mut mounted = Mounted::new(&mut app).await; + assert_eq!(state_edit::unplaced(app.project(), diagram), vec![f.b]); + let focus_id = iced::advanced::widget::Id::from("diagram-keyboard-owner"); + let (_, canvas, _) = mounted + .geometry(&app) + .focused + .into_iter() + .find(|(id, _, _)| id.as_ref() == Some(&focus_id)) + .unwrap(); + // The existing picker is immediately above the measured canvas, separated + // by 8px. Its 12px text and 8px padding give a 31.6px row. + let picker = iced::Point::new(canvas.center_x(), canvas.y - 24.); + let option = iced::Point::new(picker.x, canvas.y + 8.); + let before = app.document.snapshot(); + let original = app.project().clone(); + let source = app.document.bundle().clone(); + let selection = ( + app.selected, + app.selected_node, + app.behavior.diagram.placement, + ); + let camera = ( + app.behavior.diagram.zoom, + app.behavior.diagram.pan, + app.behavior.diagram.fit, + ); + let revision = app.document.revision(); + let history = (app.document.can_undo(), app.document.can_redo()); + mounted.click(&mut app, picker).await; + assert!( + press_escape(&mut mounted, &mut app).await.is_empty(), + "open picker owns Escape, not the State canvas or App" + ); + assert!(std::sync::Arc::ptr_eq(&before, &app.document.snapshot())); + assert_eq!(app.document.bundle(), &source); + assert_eq!( + ( + app.selected, + app.selected_node, + app.behavior.diagram.placement + ), + selection + ); + assert_eq!( + ( + app.behavior.diagram.zoom, + app.behavior.diagram.pan, + app.behavior.diagram.fit + ), + camera + ); + assert_eq!(app.document.revision(), revision); + assert_eq!((app.document.can_undo(), app.document.can_redo()), history); + // If Escape left the menu open this click would close it, and the following + // option click would fail to place. This checks actual overlay dismissal. + mounted.click(&mut app, picker).await; + let messages = mounted.click(&mut app, option).await; + assert!(messages.iter().any(|message| matches!(message, Message::StateDiagram(Event::Place { element, .. }) if *element == f.b))); + assert!(state_edit::unplaced(app.project(), diagram).is_empty()); + assert_eq!(app.project().elements, original.elements); + assert_eq!(app.project().relations, original.relations); + let _ = app.update(Message::Undo); + assert_eq!(app.document.bundle(), &source); +} diff --git a/crates/oa-desktop/src/behavior/creation.rs b/crates/oa-desktop/src/behavior/creation.rs index 6940c7b9..60af0d2c 100644 --- a/crates/oa-desktop/src/behavior/creation.rs +++ b/crates/oa-desktop/src/behavior/creation.rs @@ -10,32 +10,6 @@ use serde::{Deserialize, Serialize}; const PAGE_SIZE: usize = 6; pub const NAME_INPUT: &str = "behavior-creation-name"; -// A focused text input consumes Escape. Preserve the application's modal -// priority by emitting its ordinary Escape message from this narrow bridge. -pub fn subscription(active: bool) -> iced::Subscription<()> { - if active { - iced::event::listen_with(captured_escape) - } else { - iced::Subscription::none() - } -} - -fn captured_escape( - event: iced::Event, - status: iced::event::Status, - _: iced::window::Id, -) -> Option<()> { - (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(()) -} - #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] pub enum Family { Sequence, diff --git a/crates/oa-desktop/src/behavior/creation/app_tests.rs b/crates/oa-desktop/src/behavior/creation/app_tests.rs index 06db4c0c..b7652456 100644 --- a/crates/oa-desktop/src/behavior/creation/app_tests.rs +++ b/crates/oa-desktop/src/behavior/creation/app_tests.rs @@ -18,59 +18,11 @@ fn begin(app: &mut App, layer: Layer, family: Family, name: &str) { send(app, Event::Name(name.into())); } -fn focused_escape() -> Message { - use iced::advanced::{Layout, Shell, layout, widget}; - use iced::keyboard::{self, key::Named}; - let mut input: iced::Element<'_, (), iced::Theme, ()> = text_input("Name", "Unfinished name") - .id(NAME_INPUT) - .on_input(|_| ()) - .into(); - let mut tree = widget::Tree::new(input.as_widget()); - let size = iced::Size::new(300., 40.); - let node = - input - .as_widget_mut() - .layout(&mut tree, &(), &layout::Limits::new(iced::Size::ZERO, size)); - tree.state - .downcast_mut::>() - .focus(); - let event = iced::Event::Keyboard(keyboard::Event::KeyPressed { - key: keyboard::Key::Named(Named::Escape), - modified_key: keyboard::Key::Named(Named::Escape), - physical_key: keyboard::key::Physical::Code(keyboard::key::Code::Escape), - location: keyboard::Location::Standard, - modifiers: keyboard::Modifiers::empty(), - text: None, - repeat: false, - }); - let mut messages = Vec::new(); - let mut shell = Shell::new(&mut messages); - input.as_widget_mut().update( - &mut tree, - &event, - 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()); - assert_eq!( - captured_escape( - event.clone(), - iced::event::Status::Ignored, - iced::window::Id::unique() - ), - None - ); - captured_escape( - event, - iced::event::Status::Captured, - iced::window::Id::unique(), - ) - .unwrap(); - Message::Escape +fn focused_escape(app: &App) -> Message { + let mut messages = crate::app::escape::tests::focused_input_messages(app); + assert_eq!(messages.len(), 1); + assert!(matches!(messages[0], Message::Escape)); + messages.remove(0) } #[test] @@ -94,12 +46,12 @@ fn app_first_captured_escape_dismisses_file_or_pending_overlay_before_creation() } else { app.pending = Some(Pending::Walkthrough); } - let _ = app.update(focused_escape()); + let _ = app.update(focused_escape(&app)); assert!(app.file_prompt.is_none()); assert!(app.pending.is_none()); assert_eq!(app.behavior.recovery_snapshot().unwrap(), raw); assert!(Arc::ptr_eq(&accepted, &app.document.snapshot())); - let _ = app.update(focused_escape()); + let _ = app.update(focused_escape(&app)); assert!(!app.behavior.has_pending()); assert!(Arc::ptr_eq(&accepted, &app.document.snapshot())); assert!(!app.document.can_undo()); diff --git a/crates/vendor/iced_widget/src/overlay/menu.rs b/crates/vendor/iced_widget/src/overlay/menu.rs index 7eb67c0c..6dce55fa 100644 --- a/crates/vendor/iced_widget/src/overlay/menu.rs +++ b/crates/vendor/iced_widget/src/overlay/menu.rs @@ -1,6 +1,7 @@ //! Build and show dropdown menus. use crate::core::alignment; use crate::core::border::{self, Border}; +use crate::core::keyboard; use crate::core::layout::{self, Layout}; use crate::core::mouse; use crate::core::overlay; @@ -15,6 +16,7 @@ use crate::core::{ }; use crate::core::{Element, Shell, Widget}; use crate::scrollable::{self, Scrollable}; +use std::cell::Cell; /// A list of selectable options. pub struct Menu< @@ -34,6 +36,7 @@ pub struct Menu< hovered_option: &'a mut Option, on_selected: Box Message + 'a>, on_option_hovered: Option<&'a dyn Fn(T) -> Message>, + escape: Option<(&'a Cell, Option)>, width: f32, padding: Padding, text_size: Option, @@ -68,6 +71,7 @@ where hovered_option, on_selected: Box::new(on_selected), on_option_hovered, + escape: None, width: 0.0, padding: Padding::ZERO, text_size: None, @@ -78,6 +82,17 @@ where } } + // PickList owns this flag. Other Menu consumers retain their existing + // keyboard behavior unless they explicitly opt into this private bridge. + pub(crate) fn close_on_escape( + mut self, + open: &'a Cell, + on_close: Option, + ) -> Self { + self.escape = Some((open, on_close)); + self + } + /// Sets the width of the [`Menu`]. pub fn width(mut self, width: f32) -> Self { self.width = width; @@ -170,6 +185,7 @@ where viewport: Rectangle, tree: &'a mut Tree, list: Scrollable<'a, Message, Theme, Renderer>, + escape: Option<(&'a Cell, Option)>, width: f32, target_height: f32, class: &'a ::Class<'b>, @@ -198,6 +214,7 @@ where hovered_option, on_selected, on_option_hovered, + escape, width, padding, font, @@ -228,6 +245,7 @@ where viewport, tree: &mut state.tree, list, + escape, width, target_height, class, @@ -278,6 +296,31 @@ where clipboard: &mut dyn Clipboard, shell: &mut Shell<'_, Message>, ) { + // UserInterface may retain this overlay for the rest of an event batch. + // A dismissed owner must not select an option on a queued later click. + if self.escape.as_ref().is_some_and(|(open, _)| !open.get()) { + return; + } + // Consume dismissal in the overlay so Iced never sends the same key + // into focused controls or canvases beneath this picker. The shared + // owner flag also prevents duplicate close notifications in one batch. + if matches!( + event, + Event::Keyboard(keyboard::Event::KeyPressed { + key: keyboard::Key::Named(keyboard::key::Named::Escape), + .. + }) + ) && let Some((open, on_close)) = &mut self.escape + && open.replace(false) + { + if let Some(message) = on_close.take() { + shell.publish(message); + } + shell.capture_event(); + shell.request_redraw(); + return; + } + let bounds = layout.bounds(); self.list.update( @@ -292,6 +335,9 @@ where cursor: mouse::Cursor, renderer: &Renderer, ) -> mouse::Interaction { + if self.escape.as_ref().is_some_and(|(open, _)| !open.get()) { + return mouse::Interaction::None; + } self.list.mouse_interaction( self.tree, layout, @@ -309,6 +355,9 @@ where layout: Layout<'_>, cursor: mouse::Cursor, ) { + if self.escape.as_ref().is_some_and(|(open, _)| !open.get()) { + return; + } let bounds = layout.bounds(); let style = Catalog::style(theme, self.class); diff --git a/crates/vendor/iced_widget/src/pick_list.rs b/crates/vendor/iced_widget/src/pick_list.rs index ba39143f..adf2a4ce 100644 --- a/crates/vendor/iced_widget/src/pick_list.rs +++ b/crates/vendor/iced_widget/src/pick_list.rs @@ -78,6 +78,7 @@ use crate::core::{ use crate::overlay::menu::{self, Menu}; use std::borrow::Borrow; +use std::cell::Cell; use std::f32; /// A widget for selecting a single value from a list of options. @@ -448,12 +449,24 @@ where let state = tree.state.downcast_mut::>(); match event { + Event::Keyboard(keyboard::Event::KeyPressed { + key: keyboard::Key::Named(keyboard::key::Named::Escape), + .. + }) if state.is_open.replace(false) => { + // The picker may have opened earlier in this base-event batch, + // after UserInterface collected its overlays. + if let Some(on_close) = &self.on_close { + shell.publish(on_close.clone()); + } + shell.capture_event(); + shell.request_redraw(); + } Event::Mouse(mouse::Event::ButtonPressed(mouse::Button::Left)) | Event::Touch(touch::Event::FingerPressed { .. }) => { - if state.is_open { + if state.is_open.get() { // Event wasn't processed by overlay, so cursor was clicked either outside its // bounds or on the drop-down, either way we close the overlay. - state.is_open = false; + state.is_open.set(false); if let Some(on_close) = &self.on_close { shell.publish(on_close.clone()); @@ -463,7 +476,7 @@ where } else if cursor.is_over(layout.bounds()) { let selected = self.selected.as_ref().map(Borrow::borrow); - state.is_open = true; + state.is_open.set(true); state.hovered_option = self .options .borrow() @@ -482,7 +495,7 @@ where }) => { if state.keyboard_modifiers.command() && cursor.is_over(layout.bounds()) - && !state.is_open + && !state.is_open.get() { fn find_next<'a, T: PartialEq>( selected: &'a T, @@ -528,7 +541,7 @@ where let status = { let is_hovered = cursor.is_over(layout.bounds()); - if state.is_open { + if state.is_open.get() { Status::Opened { is_hovered } } else if is_hovered { Status::Hovered @@ -612,7 +625,7 @@ where shaping, }) => Some((*font, *code_point, *size, *line_height, *shaping)), Handle::Dynamic { open, closed } => { - if state.is_open { + if state.is_open.get() { Some(( open.font, open.code_point, @@ -703,7 +716,7 @@ where let state = tree.state.downcast_mut::>(); let font = self.font.unwrap_or_else(|| renderer.default_font()); - if state.is_open { + if state.is_open.get() { let bounds = layout.bounds(); let on_select = &self.on_select; @@ -713,13 +726,14 @@ where self.options.borrow(), &mut state.hovered_option, |option| { - state.is_open = false; + state.is_open.set(false); (on_select)(option) }, None, &self.menu_class, ) + .close_on_escape(&state.is_open, self.on_close.clone()) .width(bounds.width) .padding(self.padding) .font(font) @@ -763,7 +777,7 @@ where struct State { menu: menu::State, keyboard_modifiers: keyboard::Modifiers, - is_open: bool, + is_open: Cell, hovered_option: Option, options: Vec>, placeholder: paragraph::Plain

, @@ -775,7 +789,7 @@ impl State

{ Self { menu: menu::State::default(), keyboard_modifiers: keyboard::Modifiers::default(), - is_open: bool::default(), + is_open: Cell::new(false), hovered_option: Option::default(), options: Vec::new(), placeholder: paragraph::Plain::default(), diff --git a/harness/picker-escape-20260923.md b/harness/picker-escape-20260923.md new file mode 100644 index 00000000..76f38441 --- /dev/null +++ b/harness/picker-escape-20260923.md @@ -0,0 +1,74 @@ +# Picker Escape ownership — issue 35 + +Branch-specific design above `301126f6098286bf440c1bd1b1ed685753984723`. +[Issue 35](http://217.177.34.108:3000/sorokin.oleg.edu/ARCADIA-Studio/issues/35) +owns live scope, progress and acceptance. This document records the selected +contract; it does not assert completed implementation, execution or integration. + +## Required interaction + +An open picker owns Escape: close its popup, request redraw and consume the key. +Publish its optional close notification at most once; do not select an option +or change the surrounding draft, model, source, history, selection or camera. +A later Escape follows the existing surrounding workflow's policy. Reopening, +selection and outside-click dismissal retain their existing behavior. Selection +does not gain a new close callback as an incidental change to this repair. + +Event batches matter. A popup already closed by Escape must not select a stale +option or emit another close notification when later queued events reach the +same overlay object. This does not introduce a new global keyboard-repeat policy. + +## Decision and evidence + +The shared picker and its menu overlay currently omit Escape handling. Adding +widget capture alone is insufficient: application subscriptions translate some +captured Escape events back into modal cancellation. Those bridges also preserve +legitimate cancellation from focused text inputs, so simply removing them loses +existing behavior. + +The selected design gives dismissal to the picker overlay and moves the existing +captured-key forwarding into a scoped wrapper around the ordinary application +widget tree. Keep the wrapper structurally present even when inactive, preserve +the existing activation predicates and message priorities, and delegate widget +state, operations, layout and overlays transparently. + +The locked Iced 0.14 runtime processes overlays first and skips ordinary root +updates for events captured by an overlay (`user_interface.rs`). Nested overlays +likewise give their deepest child first refusal (`overlay/nested.rs`). Thus the +Escape from an already mounted picker cannot reach ordinary-tree forwarding, +while focused text-input cancellation can retain its previous policy. The runtime +can reuse an overlay across an event batch; close-state checks must cover that +case. Conversely, opening a picker and pressing Escape in the same batch can +reach the ordinary tree before the runtime constructs its overlay. The wrapper +therefore offers the currently available nested overlay first refusal for Escape +before forwarding to ordinary children; the picker also handles base delivery. +An overlay that already ignored Escape may receive this narrow preflight again. +Audited reachable passive overlays perform no selection or draft action; the +source-navigation observer can advance its conservative focus-freshness epoch +again. Regression checks must retain stale-focus rejection and observable state. +Do not generalize this replay to other keyboard or pointer events. + +Alternatives rejected: a widget-only patch leaves outer-draft cancellation; +global open-picker bookkeeping duplicates widget state and requires coordinating +every open/select/close/removal path; per-text-input forwarding can work but +spreads the existing policy across many controls. Revisit the wrapper choice if +runtime delivery changes or actual mounted tests contradict these assumptions. + +## Acceptance + +Use mounted runtime delivery for open/dismiss/reopen/select, optional callbacks, +outside click and same-batch Escape/click sequences. Exercise the real application +subscriptions with returned event status, including focused text cancellation, +architecture editing, behavior drafts and Text workflows. Compare complete model +and source state where dismissal must conserve them; direct helper calls alone +do not establish event ownership. + +Native coverage should include the State hidden-element picker and the Add +existing chooser's filter dropdown with a retained query: first Escape dismisses +the picker, then a distinct Escape follows the chooser policy. Behavior creation, +behavior source forms and Text Find/rename currently have no internal dropdown; +picker coexistence there is composed mounted coverage, not a claim of a native +control that does not exist. Verify their ordinary focused-text Escape behavior. + +Linux remains user-paused. Shared integration and release acceptance remain with +the integration owner; this branch does not establish combined-release behavior.