Merge #271: M7: double-clicking a page in the Pages list no longer closes the app (@marcoarnulfo)

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::<bool>(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")

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);
+}