Preserve diagram focus across edits, history and passive saves

This commit is contained in:
2026-09-12 06:40:43 -07:00
parent f7d84faf30
commit 24bb24839b
6 changed files with 354 additions and 11 deletions
+5 -1
View File
@@ -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()
@@ -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);
}
}
@@ -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
+2 -1
View File
@@ -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))
+42 -6
View File
@@ -8,6 +8,8 @@ use iced::{Element, Event as InputEvent, Length, Point, Rectangle, Size, Theme,
struct Input {
context: Option<Context>,
focused: bool,
can_focus: bool,
suspended: bool,
traversing: bool,
pointer: Option<Point>,
}
@@ -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<dyn Fn(bool) -> 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<Element<'a, Message, Theme, R>>,
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<Message, R: renderer::Renderer> Widget<Message, Theme, R> 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<Message, R: renderer::Renderer> Widget<Message, Theme, R> for Owner<'_, Mes
fn diff(&self, tree: &mut Tree) {
let state = tree.state.downcast_mut::<Input>();
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<Length> {
@@ -85,6 +113,8 @@ impl<Message, R: renderer::Renderer> Widget<Message, Theme, R> 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<Message, R: renderer::Renderer> Widget<Message, Theme, R> 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<Message, R: renderer::Renderer> Widget<Message, Theme, R> 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<Message, R: renderer::Renderer> Widget<Message, Theme, R> for Owner<'_, Mes
viewport,
);
let state = tree.state.downcast_ref::<Input>();
if state.focused && !state.traversing && self.enabled {
if state.focused && !state.traversing && self.enabled && !self.suspended {
renderer.fill_quad(
renderer::Quad {
bounds: layout.bounds(),
+2 -1
View File
@@ -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))