From 64a54ca408d53425686d74270f0193caf1435f0a Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Wed, 14 Oct 2026 11:11:00 +0200 Subject: [PATCH] M7: double-clicking a page in the Pages list no longer closes the app (fixes #264) --- crates/ui-egui/src/layers.rs | 15 +++++-- crates/ui-egui/src/layers_behaviour_tests.rs | 43 +++++++++++++++++++- 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/crates/ui-egui/src/layers.rs b/crates/ui-egui/src/layers.rs index 956709f4..927e3ae4 100644 --- a/crates/ui-egui/src/layers.rs +++ b/crates/ui-egui/src/layers.rs @@ -241,9 +241,9 @@ fn pages(app: &mut FigApp, ui: &mut Ui) { } if renaming { let tr = Rect::from_min_max(pos2(r.min.x + 8.0, r.min.y + 4.0), pos2(r.max.x - 4.0, r.max.y - 4.0)); - if let Some(new) = - crate::widgets::text_field_inline(ui, Id::new(("pagename", id)), &name, tr, if id == cur { semibold(11.0) } else { regular(11.0) }) - { + let start = ui.data_mut(|d| d.remove_temp::(page_rename_start_id())).unwrap_or(false); + let font = if id == cur { semibold(11.0) } else { regular(11.0) }; + if let (Some(new), _) = crate::widgets::rename_field(ui, Id::new(("pagename", id)), &name, tr, font, start) { rename_done = Some((id, new)); } } else if !divider { @@ -255,7 +255,9 @@ fn pages(app: &mut FigApp, ui: &mut Ui) { } if resp.double_clicked() { app.ui.rename_page = Some((id, name.clone())); - ui.memory_mut(|m| m.request_focus(Id::new(("pagename", id)).with("te"))); + // The field takes the focus next frame, once it's drawn: focusing it now would put a + // node that isn't there yet in the screen reader tree, which AccessKit aborts on. + ui.data_mut(|d| d.insert_temp(page_rename_start_id(), true)); } crate::right::context_popup(&resp).show(|ui| { if ui.button(tr("Rename page")).clicked() { @@ -295,6 +297,11 @@ fn pages(app: &mut FigApp, ui: &mut Ui) { crate::widgets::separator(ui); } +/// A page rename just asked for by a double click: its field takes the focus when drawn (temp data). +fn page_rename_start_id() -> Id { + Id::new("page_rename_start") +} + /// The layer row being dragged in the panel (temp data). fn layer_drag_id() -> Id { Id::new("layer_drag") diff --git a/crates/ui-egui/src/layers_behaviour_tests.rs b/crates/ui-egui/src/layers_behaviour_tests.rs index e1fdecc4..155f0d6d 100644 --- a/crates/ui-egui/src/layers_behaviour_tests.rs +++ b/crates/ui-egui/src/layers_behaviour_tests.rs @@ -4,7 +4,7 @@ //! list (dragging pages, dividers). Rows the documentation is silent on say so and pin today's //! behaviour. What the gestures do to the document is in the engine's `panels_behaviour_tests`. -use egui::{Event, Modifiers, Pos2}; +use egui::{Event, Id, Modifiers, Pos2}; use figcraft_doc::NodeId; use figcraft_engine::Session; use serde_json::json; @@ -226,3 +226,44 @@ fn pages_drag_to_reorder_and_dividers_open_nothing() { assert_eq!(u.app.session.page().unwrap(), p1, "clicking a divider opens nothing"); assert!(u.app.ui.last_error.is_none(), "{:?}", u.app.ui.last_error); } + +/// A double click at `p`, with the screen reader tree checked on every frame: the platform adapter +/// (UI Automation on Windows) aborts the app when the focused node isn't in the tree. +fn double_click_checking_focus(u: &mut Ui, p: Pos2) { + for _ in 0..30 { + u.frame(vec![Event::PointerMoved(p)]); + } + let presses = [true, false, true, false].into_iter().map(Some); + for (k, pressed) in presses.chain(std::iter::repeat_n(None, 30)).enumerate() { + let events = + pressed.map(|pressed| Event::PointerButton { pos: p, button: egui::PointerButton::Primary, pressed, modifiers: Modifiers::NONE }); + let out = u.frame_output(events.into_iter().collect()); + let tree = out.platform_output.accesskit_update.expect("an AccessKit update"); + assert!(tree.nodes.iter().any(|(i, _)| *i == tree.focus), "frame {k}: the focused node {:?} isn't in the tree", tree.focus); + } +} + +#[test] +fn double_clicking_a_page_renames_it_with_the_focus_in_the_screen_reader_tree() { + let mut s = Session::new(); + let p1 = s.page().unwrap(); + let p2 = id(s.execute("page.add", &json!({})).unwrap()); + let mut u = Ui::new(s); + let (r1, r2) = (page_row(&mut u, p1, p2), page_row(&mut u, p2, p1)); + u.ctx.enable_accesskit(); + // Another page: the first click opens it, the second renames it. + double_click_checking_focus(&mut u, r2); + assert_eq!(u.app.session.page().unwrap(), p2); + assert_eq!(u.app.ui.rename_page.as_ref().map(|r| r.0), Some(p2), "a double click renames the page"); + assert!(u.ctx.memory(|m| m.has_focus(Id::new(("pagename", p2)).with("te"))), "with the keyboard in its name"); + // The page that's open. + u.app.ui.rename_page = None; + u.ctx.memory_mut(|m| m.surrender_focus(Id::new(("pagename", p2)).with("te"))); + u.app.session.execute("page.select", &json!({"id": p1})).unwrap(); + for _ in 0..30 { + u.frame(vec![]); + } + double_click_checking_focus(&mut u, r1); + assert_eq!(u.app.ui.rename_page.as_ref().map(|r| r.0), Some(p1)); + assert!(u.app.ui.last_error.is_none(), "{:?}", u.app.ui.last_error); +}