Guard reorder shortcuts and focus against current overlays and stale tasks

This commit is contained in:
2026-09-24 17:20:52 -07:00
parent 652ffb4d1b
commit 9bfa113aa5
4 changed files with 173 additions and 12 deletions
+30 -1
View File
@@ -1,4 +1,4 @@
//! Keep overlay Escape ahead of modal cancellation in the ordinary widget tree.
//! Keep overlay ownership ahead of cancellation and captured reorder Apply.
//!
//! Iced skips root.update when an overlay captures an event. Keeping this
//! wrapper present even when inactive preserves the child's widget state. A
@@ -103,6 +103,28 @@ impl<R: renderer::Renderer> Widget<Message, Theme, R> for Bridge<'_, R> {
Event::Keyboard(keyboard::Event::KeyPressed { key: keyboard::Key::Named(keyboard::key::Named::Enter), modifiers, .. })
if modifiers.control() && !modifiers.alt() && !modifiers.shift() && !modifiers.logo())
{
// A picker may have opened earlier in this same input batch, before
// App state can invalidate the captured reorder action.
if let Some(element) = self.content.as_widget_mut().overlay(
&mut tree.children[0],
layout,
renderer,
viewport,
Vector::ZERO,
) {
let mut current = overlay::Nested::new(element);
let node = current.layout(renderer, viewport.size());
current.update(
event,
Layout::new(&node),
cursor,
renderer,
clipboard,
shell,
);
shell.capture_event();
return;
}
if matches!(
event,
Event::Keyboard(keyboard::Event::KeyPressed { repeat: false, .. })
@@ -146,6 +168,13 @@ impl<R: renderer::Renderer> Widget<Message, Theme, R> for Bridge<'_, R> {
if shell.is_event_captured() {
return;
}
if self.reorder_active
&& let Some(cancel) = &self.messages[1]
{
shell.publish(cancel.clone());
shell.capture_event();
return;
}
}
self.content.as_widget_mut().update(
&mut tree.children[0],
@@ -163,9 +163,7 @@ impl App {
Ok(session) => {
self.scenario_reorder.session = Some(session);
self.status = "Choose a semantic destination, then apply reorder. Saved positions are preserved.".into();
return iced::widget::text_input::focus(scenario::reorder_view::query_id(
event.token,
));
return scenario::reorder_view::focus_query(event.token);
}
Err(reason) => self.status = reason,
}
@@ -798,14 +798,6 @@ async fn mounted_refused_preview_blocks_ctrl_enter_and_old_focus_task_cannot_foc
let new_focus = app.update(Message::ScenarioReorder(Box::new(start)));
let token = app.scenario_reorder.session.as_ref().unwrap().token();
let query_id = scenario::reorder_view::query_id(token);
mounted.perform(&mut app, old_focus).await;
assert!(
!mounted
.geometry(&app)
.focused
.iter()
.any(|(id, _, focused)| id.as_ref() == Some(&query_id) && *focused)
);
mounted.perform(&mut app, new_focus).await;
assert!(
mounted
@@ -814,6 +806,31 @@ async fn mounted_refused_preview_blocks_ctrl_enter_and_old_focus_task_cannot_foc
.iter()
.any(|(id, _, focused)| id.as_ref() == Some(&query_id) && *focused)
);
mounted.perform(&mut app, old_focus).await;
assert!(
mounted
.geometry(&app)
.focused
.iter()
.any(|(id, _, focused)| id.as_ref() == Some(&query_id) && *focused)
);
let mut typing = enter(false, false);
if let iced::Event::Keyboard(iced::keyboard::Event::KeyPressed {
key,
modified_key,
text,
..
}) = &mut typing
{
*key = iced::keyboard::Key::Character("x".into());
*modified_key = key.clone();
*text = Some("x".into());
}
let messages = mounted
.batch(&mut app, &[typing], mouse::Cursor::Unavailable)
.await;
assert!(messages.iter().any(|message| matches!(message, Message::ScenarioReorder(event) if matches!(&event.intent, Intent::Query(query) if query == "x"))));
assert_eq!(app.scenario_reorder.query, "x");
choose(
&mut app,
Slot {
@@ -849,3 +866,83 @@ async fn mounted_refused_preview_blocks_ctrl_enter_and_old_focus_task_cannot_foc
assert_eq!(app.document.bundle(), &baseline.bundle);
assert!(!app.document.can_undo());
}
#[tokio::test]
async fn mounted_same_batch_arrangement_picker_owns_ctrl_enter_before_app_observes_it() {
use crate::behavior::canvas_workspace::tests::{Mounted, key};
use iced::{keyboard::key::Named, mouse};
let (mut app, owner, _) = setup(true);
app.window_size = iced::Size::new(1440., 1000.);
let mut mounted = Mounted::new(&mut app).await;
let start = packet(&app, Intent::Begin);
let task = app.update(Message::ScenarioReorder(Box::new(start)));
mounted.perform(&mut app, task).await;
choose(
&mut app,
Slot {
parent: owner,
before: None,
},
);
mounted.settle(&mut app).await;
assert!(matches!(
app.scenario_reorder
.session
.as_ref()
.unwrap()
.current_preview()
.unwrap()
.outcome,
Outcome::Change
));
let token = app.scenario_reorder.session.as_ref().unwrap().token();
let epoch = app.canvas_epoch;
let baseline = app.document.snapshot();
let selected = app.multiselect_snapshot().unwrap();
let camera = app.behavior.scenario.camera_boundary();
let point = mounted
.geometry(&app)
.bounds("scenario-arrangement-picker")
.center();
// One UserInterface update: the picker becomes an overlay before App can
// receive any message announcing a changed keyboard owner.
let (messages, statuses) = mounted.input_with_status(
&app,
&[
iced::Event::Mouse(mouse::Event::CursorMoved { position: point }),
iced::Event::Mouse(mouse::Event::ButtonPressed(mouse::Button::Left)),
iced::Event::Mouse(mouse::Event::ButtonReleased(mouse::Button::Left)),
enter(true, false),
],
mouse::Cursor::Available(point),
);
assert_eq!(statuses.last(), Some(&iced::event::Status::Captured));
assert!(!messages.iter().any(|message| matches!(message, Message::ScenarioReorder(event) if matches!(event.intent, Intent::Finish(_) | Intent::Choose(_) | Intent::Cancel))), "picker must own the combo despite the cached valid proposal: {messages:?}");
for message in messages {
let task = app.update(message);
mounted.perform(&mut app, task).await;
}
assert_eq!(app.canvas_epoch, epoch);
assert_eq!(
app.scenario_reorder.session.as_ref().unwrap().token(),
token
);
assert_eq!(app.document.bundle(), &baseline.bundle);
assert_eq!(app.project(), &baseline.decoded.project);
assert_eq!(app.multiselect_snapshot().unwrap(), selected);
assert!(camera == app.behavior.scenario.camera_boundary());
assert!(!app.document.can_undo());
// Establish that the actual picker opened: first Escape closes only its
// overlay, while the following Escape reaches the reorder operation.
let messages = mounted
.batch(&mut app, &[key(Named::Escape)], mouse::Cursor::Unavailable)
.await;
assert!(!messages.iter().any(|message| matches!(message, Message::ScenarioReorder(event) if matches!(event.intent, Intent::Cancel))));
assert!(app.scenario_reorder.session.is_some());
let messages = mounted
.batch(&mut app, &[key(Named::Escape)], mouse::Cursor::Unavailable)
.await;
assert!(messages.iter().any(|message| matches!(message, Message::ScenarioReorder(event) if matches!(event.intent, Intent::Cancel))));
assert!(app.scenario_reorder.session.is_none());
assert_eq!(app.document.bundle(), &baseline.bundle);
}
@@ -10,6 +10,43 @@ pub(crate) fn query_id(token: Id) -> iced::widget::Id {
iced::widget::Id::from(format!("scenario-reorder-query:{token}"))
}
pub(crate) fn focus_query(token: Id) -> iced::Task<Message> {
use iced::advanced::widget::{Operation, operation};
struct Find {
target: iced::widget::Id,
present: bool,
}
impl Operation for Find {
fn traverse(&mut self, visit: &mut dyn FnMut(&mut dyn Operation)) {
visit(self);
}
fn focusable(
&mut self,
id: Option<&iced::widget::Id>,
_: iced::Rectangle,
_: &mut dyn operation::Focusable,
) {
self.present |= id == Some(&self.target);
}
fn finish(&mut self) -> operation::Outcome<()> {
if self.present {
operation::Outcome::Chain(Box::new(operation::focusable::focus(
self.target.clone(),
)))
} else {
operation::Outcome::None
}
}
}
// Generic focus would blur every current widget if the old token vanished.
// Discover the exact target without changing focus before chaining it.
iced::advanced::widget::operate(Find {
target: query_id(token),
present: false,
})
.discard()
}
pub(crate) fn apply_message(app: &App) -> Option<Message> {
if !app.scenario_reorder_enabled() {
return None;