Merge #253: Avoid macOS file dialog reentrancy crashes (@justinjohn0306)

# Conflicts:
#	contributors/people.toml

apps/figcraft/src/main.rs

@@ -90,9 +90,9 @@ impl eframe::App for App {
         }
     }
 
-    fn ui(&mut self, ui: &mut egui::Ui, _frame: &mut eframe::Frame) {
+    fn ui(&mut self, ui: &mut egui::Ui, frame: &mut eframe::Frame) {
         let ctx = ui.ctx().clone();
-        with_app(|fig| figcraft_ui_egui::windows::guard(fig, &ctx, |fig| fig.ui(ui)));
+        services::parented(frame, || with_app(|fig| figcraft_ui_egui::windows::guard(fig, &ctx, |fig| fig.ui(ui))));
     }
 
     fn on_exit(&mut self) {

apps/figcraft/src/services.rs

@@ -1,11 +1,12 @@
 //! Desktop services: native file dialogs, clipboard, URLs, persisted preferences.
 
+use std::cell::RefCell;
 use std::path::{Path, PathBuf};
 use std::sync::Arc;
 use std::sync::atomic::{AtomicBool, Ordering};
 
 use figcraft_engine::{Prefs, Session};
-use figcraft_ui_egui::{FigApp, Services};
+use figcraft_ui_egui::{DialogFuture, FigApp, Services};
 use serde_json::{Value, json};
 
 pub struct Desktop {
@@ -198,15 +199,39 @@ fn write_private(path: &Path, bytes: &[u8]) -> std::io::Result<()> {
     f.write_all(bytes)
 }
 
+thread_local! {
+    /// While the main window draws: a dialog parented to it, so a dialog opened from it stays in
+    /// front of it (a sheet on macOS). eframe gives other windows no handle: theirs have no parent.
+    static PARENT: RefCell<Option<rfd::AsyncFileDialog>> = const { RefCell::new(None) };
+}
+
+/// Run the main window's frame `f` with the dialogs it opens parented to `frame`'s window.
+pub fn parented<R>(frame: &eframe::Frame, f: impl FnOnce() -> R) -> R {
+    PARENT.with(|p| p.replace(Some(rfd::AsyncFileDialog::new().set_parent(frame))));
+    let r = f();
+    PARENT.with(|p| p.replace(None));
+    r
+}
+
+/// A dialog with `title`, parented to the main window while it draws.
+fn dialog(title: &str) -> rfd::AsyncFileDialog {
+    PARENT.with(|p| p.borrow().clone()).unwrap_or_default().set_title(title)
+}
+
+/// The dialog opens now; its future gives the path picked.
+fn picked(pick: impl Future<Output = Option<rfd::FileHandle>> + 'static) -> DialogFuture {
+    Box::pin(async move { pick.await.map(|f| f.path().to_path_buf()) })
+}
+
 impl Services for Desktop {
-    fn pick_open(&mut self, title: &str, exts: &[&str]) -> Option<PathBuf> {
-        rfd::FileDialog::new().set_title(title).add_filter("Supported files", exts).pick_file()
+    fn pick_open(&mut self, title: &str, exts: &[&str]) -> DialogFuture {
+        picked(dialog(title).add_filter("Supported files", exts).pick_file())
     }
-    fn pick_save(&mut self, title: &str, name: &str, exts: &[&str]) -> Option<PathBuf> {
-        rfd::FileDialog::new().set_title(title).set_file_name(name).add_filter("File", exts).save_file()
+    fn pick_save(&mut self, title: &str, name: &str, exts: &[&str]) -> DialogFuture {
+        picked(dialog(title).set_file_name(name).add_filter("File", exts).save_file())
     }
-    fn pick_folder(&mut self, title: &str) -> Option<PathBuf> {
-        rfd::FileDialog::new().set_title(title).pick_folder()
+    fn pick_folder(&mut self, title: &str) -> DialogFuture {
+        picked(dialog(title).pick_folder())
     }
     fn clipboard_set_text(&mut self, s: &str) {
         if let Some(c) = self.clipboard.as_mut() {

contributors/people.toml

@@ -34,3 +34,6 @@ sync_github_name = false
 [people.yyamandevrim]
 real_name = "Yaman Devrim"
 sync_github_name = true
+
+[people.justinjohn0306]
+real_name = "Justin John"

crates/ui-egui/src/assets_panel.rs

@@ -1100,11 +1100,12 @@ fn enabled_libraries(app: &mut FigApp, ui: &mut Ui, w: f32, doc: &Document) {
             trn("Swapped {n} assets; {missed} had no match and still use the old library", moved as usize, &[("missed", &missed)])
         });
     }
-    if add
-        && let Some(p) = app.services.pick_open(tr("Add library"), &["figcraft"])
-        && let Some(r) = app.run(ADD, json!({"path": p.to_string_lossy()}))
-    {
-        added(app, &r);
+    if add {
+        app.pick_open(tr("Add library"), &["figcraft"], |app, p| {
+            if let Some(r) = app.run(ADD, json!({"path": p.to_string_lossy()})) {
+                added(app, &r);
+            }
+        });
     }
 }
 

crates/ui-egui/src/async_dialog_tests.rs

@@ -0,0 +1,92 @@
+use std::path::PathBuf;
+
+use figcraft_engine::Session;
+
+use crate::{DialogFuture, FigApp, Services};
+
+struct OpenFile(PathBuf);
+
+impl Services for OpenFile {
+    fn pick_open(&mut self, _title: &str, _exts: &[&str]) -> DialogFuture {
+        Box::pin(std::future::ready(Some(self.0.clone())))
+    }
+}
+
+#[test]
+fn open_dialog_finishes_after_the_async_picker_returns() {
+    let dir = std::env::temp_dir().join(format!("figcraft-async-open-{}", std::process::id()));
+    let _ = std::fs::remove_dir_all(&dir);
+    std::fs::create_dir_all(&dir).unwrap();
+    let path = dir.join("opened.figcraft");
+    let doc = figcraft_doc::Document::new("Opened");
+    std::fs::write(&path, figcraft_format::save_bytes(&doc, None).unwrap()).unwrap();
+
+    let mut app = FigApp::new(Session::empty(), Box::new(OpenFile(path.clone())));
+    app.ui.home = true;
+    app.open_dialog();
+    assert!(app.session.docs.is_empty(), "opening waits for the picker result");
+
+    app.poll_dialog(&egui::Context::default());
+    assert_eq!(app.session.docs.len(), 1);
+    assert_eq!(app.session.docs[0].path.as_deref(), Some(path.to_string_lossy().as_ref()));
+    assert!(!app.ui.home);
+    let _ = std::fs::remove_dir_all(dir);
+}
+
+/// Every picker answers `self.0`.
+struct Picks(PathBuf);
+
+impl Services for Picks {
+    fn pick_open(&mut self, _title: &str, _exts: &[&str]) -> DialogFuture {
+        Box::pin(std::future::ready(Some(self.0.clone())))
+    }
+    fn pick_save(&mut self, _title: &str, _name: &str, _exts: &[&str]) -> DialogFuture {
+        Box::pin(std::future::ready(Some(self.0.clone())))
+    }
+}
+
+/// Two files, A (the first) in front, and where the picker saves.
+fn two_files(tag: &str) -> (FigApp, PathBuf, PathBuf) {
+    let dir = std::env::temp_dir().join(format!("figcraft-async-{tag}-{}", std::process::id()));
+    let _ = std::fs::remove_dir_all(&dir);
+    std::fs::create_dir_all(&dir).unwrap();
+    let path = dir.join("a.figcraft");
+    let mut app = FigApp::new(Session::empty(), Box::new(Picks(path.clone())));
+    for _ in 0..2 {
+        app.session.execute("file.new", &serde_json::json!({})).unwrap();
+    }
+    app.session.active = 0;
+    (app, dir, path)
+}
+
+#[test]
+fn save_as_saves_the_file_it_was_opened_for_after_another_tab_comes_to_the_front() {
+    let (mut app, dir, path) = two_files("pin");
+    app.save_as_dialog();
+    // The user switches tabs while the dialog is open.
+    app.session.active = 1;
+    // Another window draws first: the result waits for the window holding the file.
+    app.windows.current = 7;
+    app.poll_dialog(&egui::Context::default());
+    assert!(app.session.docs.iter().all(|d| d.path.is_none()));
+    app.windows.current = 0;
+    app.poll_dialog(&egui::Context::default());
+    let p = path.to_string_lossy();
+    assert_eq!(app.session.docs[0].path.as_deref(), Some(p.as_ref()), "A is saved where the user picked");
+    assert_eq!(app.session.docs[1].path, None, "B is untouched");
+    assert_eq!(app.session.active, 1, "the tab the user switched to stays in front");
+    assert!(path.exists());
+    let _ = std::fs::remove_dir_all(dir);
+}
+
+#[test]
+fn a_dialog_whose_file_closed_meanwhile_does_nothing() {
+    let (mut app, dir, path) = two_files("closed");
+    app.save_as_dialog();
+    app.session.execute("file.close", &serde_json::json!({"index": 0, "force": true})).unwrap();
+    app.poll_dialog(&egui::Context::default());
+    assert!(app.session.docs.iter().all(|d| d.path.is_none()), "B isn't saved in A's place");
+    assert!(!path.exists());
+    assert!(app.pending_dialog.is_none());
+    let _ = std::fs::remove_dir_all(dir);
+}

crates/ui-egui/src/colorpicker.rs

@@ -460,23 +460,21 @@ pub fn show(app: &mut FigApp, ctx: &egui::Context) {
                     ui.add_space(12.0);
                     if let Some(i) = widgets::segmented(ui, &kinds, Some(active), 30.0) {
                         let base = paint.preview_color();
+                        let mut image_pending = false;
                         paint.kind = match i {
                             0 => PaintKind::Solid { color: base.with_alpha(1.0) },
                             5 => {
-                                // Pick an image file.
-                                match app.services.pick_open(tr("Choose image"), &["png", "jpg", "jpeg", "webp", "gif", "tif", "tiff"]) {
-                                    Some(p) => match std::fs::read(&p) {
-                                        Ok(bytes) => {
-                                            let hash = app.session.doc_mut().ok().and_then(|d| figcraft_format_add(d, bytes));
-                                            match hash {
-                                                Some(h) => Paint::image(h, ImageScaleMode::Fill).kind,
-                                                None => paint.kind.clone(),
-                                            }
-                                        }
-                                        Err(_) => paint.kind.clone(),
-                                    },
-                                    None => paint.kind.clone(),
-                                }
+                                image_pending = true;
+                                let target = target.clone();
+                                app.pick_open(tr("Choose image"), &["png", "jpg", "jpeg", "webp", "gif", "tif", "tiff"], move |app, p| {
+                                    let Ok(bytes) = std::fs::read(&p) else { return };
+                                    let Some(hash) = app.session.doc_mut().ok().and_then(|d| figcraft_format_add(d, bytes)) else { return };
+                                    if let Some(mut paint) = current_paint(app, &target) {
+                                        paint.kind = Paint::image(hash, ImageScaleMode::Fill).kind;
+                                        write(app, &target, &paint, false);
+                                    }
+                                });
+                                paint.kind.clone()
                             }
                             6 => match pattern_sources(app, &target).first() {
                                 Some((id, _)) => Paint::pattern(id.0).kind,
@@ -495,7 +493,9 @@ pub fn show(app: &mut FigApp, ctx: &egui::Context) {
                                 }
                             }
                         };
-                        write(app, &target, &paint, false);
+                        if !image_pending {
+                            write(app, &target, &paint, false);
+                        }
                     }
                 });
             }

crates/ui-egui/src/dialogs.rs

@@ -464,15 +464,17 @@ fn color_profile_dialog(app: &mut FigApp, ctx: &Context) {
 
 /// Save the whole file as `.fig`; the toast says how much was left out.
 fn save_fig(app: &mut FigApp, name: &str) {
-    let Some(p) = app.services.pick_save(tr("Export"), &format!("{name}.fig"), &["fig"]) else { return };
-    let file = p.file_name().map(|f| f.to_string_lossy().to_string()).unwrap_or_default();
-    if let Some(r) = app.run("file.export", json!({"format": "fig", "path": p.to_string_lossy()})) {
-        let left_out = r["warnings"].as_array().map_or(0, Vec::len);
-        app.toast(match left_out {
-            0 => trf("Exported {name}", &[("name", &file)]),
-            n => trn("Exported {name}; {n} kinds of content were left out", n, &[("name", &file)]),
-        });
-    }
+    let filename = format!("{name}.fig");
+    app.pick_save(tr("Export"), &filename, &["fig"], |app, p| {
+        let file = p.file_name().map(|f| f.to_string_lossy().to_string()).unwrap_or_default();
+        if let Some(r) = app.run("file.export", json!({"format": "fig", "path": p.to_string_lossy()})) {
+            let left_out = r["warnings"].as_array().map_or(0, Vec::len);
+            app.toast(match left_out {
+                0 => trf("Exported {name}", &[("name", &file)]),
+                n => trn("Exported {name}; {n} kinds of content were left out", n, &[("name", &file)]),
+            });
+        }
+    });
 }
 
 /// What the Export dialog was asked to do.

crates/ui-egui/src/export_panel.rs

@@ -318,11 +318,13 @@ pub(crate) fn export_layers(app: &mut FigApp, ids: &[NodeId], assets: bool) {
         // The suggested name keeps only the file's own name (a layer's folders apply to folders).
         [one] => save_one(app, &[one.id], &one.setting, one.file.rsplit('/').next().unwrap_or(&one.file)),
         _ => {
-            let Some(dir) = app.services.pick_folder(tr("Export")) else { return };
-            if let Some(r) = app.run("file.exportLayers", json!({"ids": ids, "dir": dir.to_string_lossy(), "assets": assets})) {
-                let n = r["files"].as_array().map_or(0, Vec::len);
-                app.toast(trn("Exported {n} files", n, &[]));
-            }
+            let ids = ids.to_vec();
+            app.pick_folder(tr("Export"), move |app, dir| {
+                if let Some(r) = app.run("file.exportLayers", json!({"ids": ids, "dir": dir.to_string_lossy(), "assets": assets})) {
+                    let n = r["files"].as_array().map_or(0, Vec::len);
+                    app.toast(trn("Exported {n} files", n, &[]));
+                }
+            });
         }
     }
 }
@@ -330,14 +332,14 @@ pub(crate) fn export_layers(app: &mut FigApp, ids: &[NodeId], assets: bool) {
 /// Export `ids` together as one file with `setting`, through a save dialog suggesting `file`.
 pub(crate) fn save_one(app: &mut FigApp, ids: &[NodeId], setting: &ExportSetting, file: &str) {
     let ext = setting.format.ext();
-    let Some(p) = app.services.pick_save(tr("Export"), file, &[ext]) else { return };
-    let path = p.to_string_lossy().to_string();
     let mut params = setting_params(setting);
     params["ids"] = json!(ids);
-    params["path"] = json!(path);
-    if app.run("file.export", params).is_some() {
-        app.toast(trf("Exported {name}", &[("name", &p.file_name().map(|f| f.to_string_lossy().to_string()).unwrap_or_default())]));
-    }
+    app.pick_save(tr("Export"), file, &[ext], move |app, p| {
+        params["path"] = json!(p.to_string_lossy());
+        if app.run("file.export", params).is_some() {
+            app.toast(trf("Exported {name}", &[("name", &p.file_name().map(|f| f.to_string_lossy().to_string()).unwrap_or_default())]));
+        }
+    });
 }
 
 /// The file name a one-off export of `ids` suggests: the first layer's name (or `fallback`) with

crates/ui-egui/src/lib.rs

@@ -69,6 +69,8 @@ mod theme_startup_tests;
 #[cfg(test)]
 mod a11y_tests;
 #[cfg(test)]
+mod async_dialog_tests;
+#[cfg(test)]
 mod branch_ui_tests;
 #[cfg(test)]
 mod canvas_drag_tests;
@@ -168,7 +170,11 @@ mod windows_qa_tests;
 mod windows_tests;
 
 use std::collections::BTreeSet;
+use std::future::Future;
 use std::path::PathBuf;
+use std::pin::Pin;
+use std::sync::Arc;
+use std::task::{Context as TaskContext, Poll, Wake, Waker};
 
 use figcraft_doc::NodeId;
 use figcraft_engine::Session;
@@ -178,15 +184,15 @@ pub use control::{ControlReply, ControlRequest};
 
 /// Platform services the app crate provides (file dialogs, clipboard, URLs).
 pub trait Services {
-    fn pick_open(&mut self, _title: &str, _exts: &[&str]) -> Option<PathBuf> {
-        None
+    fn pick_open(&mut self, _title: &str, _exts: &[&str]) -> DialogFuture {
+        Box::pin(std::future::ready(None))
     }
-    fn pick_save(&mut self, _title: &str, _name: &str, _exts: &[&str]) -> Option<PathBuf> {
-        None
+    fn pick_save(&mut self, _title: &str, _name: &str, _exts: &[&str]) -> DialogFuture {
+        Box::pin(std::future::ready(None))
     }
     /// A folder to write several files into.
-    fn pick_folder(&mut self, _title: &str) -> Option<PathBuf> {
-        None
+    fn pick_folder(&mut self, _title: &str) -> DialogFuture {
+        Box::pin(std::future::ready(None))
     }
     fn clipboard_set_text(&mut self, _s: &str) {}
     fn clipboard_get_text(&mut self) -> Option<String> {
@@ -222,6 +228,34 @@ pub trait Services {
     }
 }
 
+pub type DialogFuture = Pin<Box<dyn Future<Output = Option<PathBuf>>>>;
+type DialogDone = Box<dyn FnOnce(&mut FigApp, PathBuf)>;
+
+/// A native dialog in flight. The app keeps working while it is open, so its result goes to the
+/// file and window it was opened for, not to whichever is in front when it closes.
+struct PendingDialog {
+    future: DialogFuture,
+    /// The path picked, kept until the window that holds `doc` draws.
+    path: Option<PathBuf>,
+    done: DialogDone,
+    /// The file it acts on (`None`: none, such as Open).
+    doc: Option<u64>,
+    /// The window it was opened from.
+    window: u64,
+}
+
+/// Wakes the main window, which hands the result on to the window it belongs to.
+struct RepaintWaker(egui::Context);
+impl Wake for RepaintWaker {
+    fn wake(self: Arc<Self>) {
+        self.0.request_repaint_of(egui::ViewportId::ROOT);
+    }
+
+    fn wake_by_ref(self: &Arc<Self>) {
+        self.0.request_repaint_of(egui::ViewportId::ROOT);
+    }
+}
+
 /// No-op services (tests, headless screenshots).
 pub struct NoServices;
 impl Services for NoServices {}
@@ -533,6 +567,10 @@ pub struct FigApp {
     window_requests: Vec<figcraft_engine::UiRequest>,
     /// Commands waiting for a server ([`FigApp::run_background`]) and what to do with their results.
     waiting: Vec<(String, Done)>,
+    /// Native dialogs complete asynchronously so their modal event loop never re-enters winit.
+    pending_dialog: Option<PendingDialog>,
+    /// The latest UI context schedules the first poll and wakes the app when a dialog completes.
+    ui_context: Option<egui::Context>,
 }
 
 /// What to do with the result of a command run in the background.
@@ -560,6 +598,8 @@ impl FigApp {
             windows: windows::Windows::default(),
             window_requests: Vec::new(),
             waiting: Vec::new(),
+            pending_dialog: None,
+            ui_context: None,
         }
     }
 
@@ -820,12 +860,12 @@ impl FigApp {
 
     pub fn save_as_dialog(&mut self) {
         let name = self.session.doc().map(|d| d.name.clone()).unwrap_or_else(|_| "Untitled".into());
-        if let Some(p) = self.services.pick_save(i18n::tr("Save as"), &format!("{name}.figcraft"), &["figcraft"]) {
+        self.pick_save(i18n::tr("Save as"), &format!("{name}.figcraft"), &["figcraft"], |app, p| {
             let ps = p.to_string_lossy().to_string();
-            if self.run("file.saveAs", json!({"path": ps})).is_some() {
-                self.toast(i18n::trf("Saved {name}", &[("name", &p.file_name().map(|f| f.to_string_lossy().to_string()).unwrap_or_default())]));
+            if app.run("file.saveAs", json!({"path": ps})).is_some() {
+                app.toast(i18n::trf("Saved {name}", &[("name", &p.file_name().map(|f| f.to_string_lossy().to_string()).unwrap_or_default())]));
             }
-        }
+        });
     }
 
     pub fn save(&mut self) {
@@ -841,14 +881,18 @@ impl FigApp {
     }
 
     pub fn open_dialog(&mut self) {
-        if let Some(p) =
-            self.services.pick_open(i18n::tr("Open"), &["figcraft", "fig", "json", "svg", "png", "jpg", "jpeg", "webp", "gif", "tif", "tiff"])
-        {
-            let ps = p.to_string_lossy().to_string();
-            if self.run("file.open", json!({"path": ps})).is_some() {
-                self.ui.home = false;
-            }
-        }
+        let exts = ["figcraft", "fig", "json", "svg", "png", "jpg", "jpeg", "webp", "gif", "tif", "tiff"];
+        // Opening needs no file in front: it goes ahead even if that file closed meanwhile.
+        self.start_dialog(
+            |services| services.pick_open(i18n::tr("Open"), &exts),
+            false,
+            |app, p| {
+                let ps = p.to_string_lossy().to_string();
+                if app.run("file.open", json!({"path": ps})).is_some() {
+                    app.ui.home = false;
+                }
+            },
+        );
     }
 
     pub fn import_dialog(&mut self, place: bool) {
@@ -857,9 +901,78 @@ impl FigApp {
         } else {
             &["svg", "png", "jpg", "jpeg", "webp", "gif", "tif", "tiff", "figcraft", "fig"]
         };
-        if let Some(p) = self.services.pick_open(if place { i18n::tr("Place image") } else { i18n::tr("Import") }, exts) {
+        let title = if place { i18n::tr("Place image") } else { i18n::tr("Import") };
+        self.pick_open(title, exts, move |app, p| {
             let ps = p.to_string_lossy().to_string();
-            self.run("file.import", json!({"path": ps}));
+            app.run("file.import", json!({"path": ps}));
+        });
+    }
+
+    /// Pick a file to open. `done` runs with the file in front now back in front (and not at all
+    /// if that file closed meanwhile).
+    pub fn pick_open(&mut self, title: &str, exts: &[&str], done: impl FnOnce(&mut FigApp, PathBuf) + 'static) {
+        self.start_dialog(|services| services.pick_open(title, exts), true, done);
+    }
+
+    /// Pick where to save; `done` runs as [`FigApp::pick_open`]'s does.
+    pub fn pick_save(&mut self, title: &str, name: &str, exts: &[&str], done: impl FnOnce(&mut FigApp, PathBuf) + 'static) {
+        self.start_dialog(|services| services.pick_save(title, name, exts), true, done);
+    }
+
+    /// Pick a folder; `done` runs as [`FigApp::pick_open`]'s does.
+    pub fn pick_folder(&mut self, title: &str, done: impl FnOnce(&mut FigApp, PathBuf) + 'static) {
+        self.start_dialog(|services| services.pick_folder(title), true, done);
+    }
+
+    /// Open a native dialog from the window drawing now, for the file in front (`pin`).
+    fn start_dialog(&mut self, start: impl FnOnce(&mut dyn Services) -> DialogFuture, pin: bool, done: impl FnOnce(&mut FigApp, PathBuf) + 'static) {
+        if self.pending_dialog.is_some() {
+            return;
+        }
+        let doc = if pin { self.session.ds().ok().filter(|d| self.shows(d.uid)).map(|d| d.uid) } else { None };
+        let future = start(self.services.as_mut());
+        self.pending_dialog = Some(PendingDialog { future, path: None, done: Box::new(done), doc, window: self.windows.current });
+        if let Some(ctx) = &self.ui_context {
+            ctx.request_repaint();
+        }
+    }
+
+    /// Once the dialog has a path, its continuation runs in the window that holds its file, with
+    /// that file in front; then the tab in front before comes back (unless the dialog opened a file).
+    fn poll_dialog(&mut self, ctx: &egui::Context) {
+        let Some(d) = &mut self.pending_dialog else { return };
+        if d.path.is_none() {
+            let waker = Waker::from(Arc::new(RepaintWaker(ctx.clone())));
+            let Poll::Ready(path) = d.future.as_mut().poll(&mut TaskContext::from_waker(&waker)) else { return };
+            d.path = path;
+        }
+        let (doc, origin, picked) = (d.doc, d.window, d.path.is_some());
+        let index = doc.map(|u| self.session.docs.iter().position(|ds| ds.uid == u));
+        // Cancelled, or its file closed meanwhile.
+        if !picked || index == Some(None) {
+            self.pending_dialog = None;
+            return;
+        }
+        let window = match doc {
+            Some(u) => self.windows.of(u),
+            None if self.windows.exists(origin) => origin,
+            None => 0,
+        };
+        if window != self.windows.current {
+            ctx.request_repaint_of(windows::viewport(window));
+            return;
+        }
+        let Some(PendingDialog { path: Some(path), done, .. }) = self.pending_dialog.take() else { return };
+        let before = self.session.ds().ok().map(|d| d.uid);
+        if let Some(Some(i)) = index {
+            self.session.active = i;
+        }
+        done(self, path);
+        if doc.is_some()
+            && self.session.ds().ok().map(|d| d.uid) == doc
+            && let Some(i) = before.and_then(|u| self.session.docs.iter().position(|d| d.uid == u))
+        {
+            self.session.active = i;
         }
     }
 
@@ -958,6 +1071,7 @@ impl FigApp {
         #[cfg(not(target_arch = "wasm32"))]
         let t0 = std::time::Instant::now();
         let ctx = ui.ctx().clone();
+        self.ui_context = Some(ctx.clone());
         self.frame_stats.record(&ctx);
         let first = !self.fonts_installed;
         self.logic(&ctx);
@@ -1001,6 +1115,8 @@ impl FigApp {
         // even if it lets go during the frame (Enter commits it), or as it began (egui drops the
         // focus on Escape before the frame, which cancels the field's edit and nothing else).
         let typing = std::mem::take(&mut self.field_focus) || a11y::field_focused(&ctx);
+        // A dialog that closed acts first, in the window it was opened from.
+        self.poll_dialog(&ctx);
         // Control requests run first so their results reflect this frame.
         control::drain(self, &ctx);
         windows::requests(self, &ctx);

crates/ui-egui/src/plugins_ui.rs

@@ -124,10 +124,11 @@ fn run(app: &mut FigApp, cmd: &str, params: Value) {
 
 /// Plugins ▸ Development ▸ Import plugin from manifest…: pick a `figcraft-plugin.json`.
 pub fn import_dialog(app: &mut FigApp) {
-    let Some(p) = app.services.pick_open(tr("Import plugin from manifest"), &["json"]) else { return };
-    if let Some(v) = app.run("plugin.import", json!({"path": p.to_string_lossy()})) {
-        app.toast(trf("Imported {name}", &[("name", &v["name"].as_str().unwrap_or(tr("the plugin")))]));
-    }
+    app.pick_open(tr("Import plugin from manifest"), &["json"], |app, p| {
+        if let Some(v) = app.run("plugin.import", json!({"path": p.to_string_lossy()})) {
+            app.toast(trf("Imported {name}", &[("name", &v["name"].as_str().unwrap_or(tr("the plugin")))]));
+        }
+    });
 }
 
 /// The installed plugins at the top of the Plugins menu: a plugin with one command is an item, one

crates/ui-egui/src/variables_panel.rs

@@ -207,24 +207,28 @@ fn header(app: &mut FigApp, ui: &mut Ui, open: &mut bool) {
 /// Export this file's collections through a save dialog.
 fn export_dialog(app: &mut FigApp, format: &str) {
     let name = app.session.doc().map(|d| format!("{}.variables.{format}", d.name)).unwrap_or_else(|_| format!("variables.{format}"));
-    let Some(p) = app.services.pick_save(tr("Export variables"), &name, &[format]) else { return };
-    if app.run("variable.export", json!({"format": format, "path": p.to_string_lossy()})).is_some() {
-        let file = p.file_name().map(|f| f.to_string_lossy().to_string()).unwrap_or_default();
-        app.toast(trf("Exported {file}", &[("file", &file)]));
-    }
+    let format = format.to_owned();
+    let ext = format.clone();
+    app.pick_save(tr("Export variables"), &name, &[&ext], move |app, p| {
+        if app.run("variable.export", json!({"format": format, "path": p.to_string_lossy()})).is_some() {
+            let file = p.file_name().map(|f| f.to_string_lossy().to_string()).unwrap_or_default();
+            app.toast(trf("Exported {file}", &[("file", &file)]));
+        }
+    });
 }
 
 /// Import a variables JSON picked in an open dialog.
 fn import_dialog(app: &mut FigApp) {
-    let Some(p) = app.services.pick_open(tr("Import variables"), &["json"]) else { return };
-    let Some(r) = app.run("variable.import", json!({"variables": p.to_string_lossy()})) else { return };
-    let (added, updated) = (r["variables"].as_u64().unwrap_or(0), r["updated"].as_u64().unwrap_or(0));
-    let skipped = r["warnings"].as_array().map_or(0, Vec::len);
-    let mut msg = trn("Imported {n} new variables, updated {updated}", added as usize, &[("updated", &updated)]);
-    if skipped > 0 {
-        msg = format!("{msg} {}", trn("({n} warnings: some entries were skipped)", skipped, &[]));
-    }
-    app.toast(msg);
+    app.pick_open(tr("Import variables"), &["json"], |app, p| {
+        let Some(r) = app.run("variable.import", json!({"variables": p.to_string_lossy()})) else { return };
+        let (added, updated) = (r["variables"].as_u64().unwrap_or(0), r["updated"].as_u64().unwrap_or(0));
+        let skipped = r["warnings"].as_array().map_or(0, Vec::len);
+        let mut msg = trn("Imported {n} new variables, updated {updated}", added as usize, &[("updated", &updated)]);
+        if skipped > 0 {
+            msg = format!("{msg} {}", trn("({n} warnings: some entries were skipped)", skipped, &[]));
+        }
+        app.toast(msg);
+    });
 }
 
 /// Extend library `lib`'s collection `c` and show the new collection.