#253: a dialog's result goes to the file and window it was opened for; dialogs are parented to the main window

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,5 +1,6 @@
 //! 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};
@@ -198,30 +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]) -> DialogFuture {
-        let title = title.to_owned();
-        let exts: Vec<String> = exts.iter().map(|ext| (*ext).to_owned()).collect();
-        Box::pin(async move {
-            rfd::AsyncFileDialog::new().set_title(title).add_filter("Supported files", &exts).pick_file().await.map(|file| file.path().to_path_buf())
-        })
+        picked(dialog(title).add_filter("Supported files", exts).pick_file())
     }
     fn pick_save(&mut self, title: &str, name: &str, exts: &[&str]) -> DialogFuture {
-        let (title, name) = (title.to_owned(), name.to_owned());
-        let exts: Vec<String> = exts.iter().map(|ext| (*ext).to_owned()).collect();
-        Box::pin(async move {
-            rfd::AsyncFileDialog::new()
-                .set_title(title)
-                .set_file_name(name)
-                .add_filter("File", &exts)
-                .save_file()
-                .await
-                .map(|file| file.path().to_path_buf())
-        })
+        picked(dialog(title).set_file_name(name).add_filter("File", exts).save_file())
     }
     fn pick_folder(&mut self, title: &str) -> DialogFuture {
-        let title = title.to_owned();
-        Box::pin(async move { rfd::AsyncFileDialog::new().set_title(title).pick_folder().await.map(|file| file.path().to_path_buf()) })
+        picked(dialog(title).pick_folder())
     }
     fn clipboard_set_text(&mut self, s: &str) {
         if let Some(c) = self.clipboard.as_mut() {

crates/ui-egui/src/async_dialog_tests.rs

@@ -32,3 +32,61 @@ fn open_dialog_finishes_after_the_async_picker_returns() {
     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/lib.rs

@@ -231,14 +231,28 @@ 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();
+        self.0.request_repaint_of(egui::ViewportId::ROOT);
     }
 
     fn wake_by_ref(self: &Arc<Self>) {
-        self.0.request_repaint();
+        self.0.request_repaint_of(egui::ViewportId::ROOT);
     }
 }
 
@@ -554,7 +568,7 @@ pub struct FigApp {
     /// 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<(DialogFuture, DialogDone)>,
+    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>,
 }
@@ -867,12 +881,18 @@ impl FigApp {
     }
 
     pub fn open_dialog(&mut self) {
-        self.pick_open(i18n::tr("Open"), &["figcraft", "fig", "json", "svg", "png", "jpg", "jpeg", "webp", "gif", "tif", "tiff"], |app, p| {
-            let ps = p.to_string_lossy().to_string();
-            if app.run("file.open", json!({"path": ps})).is_some() {
-                app.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) {
@@ -888,37 +908,71 @@ impl FigApp {
         });
     }
 
+    /// 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), done);
+        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), done);
+        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), done);
+        self.start_dialog(|services| services.pick_folder(title), true, done);
     }
 
-    fn start_dialog(&mut self, start: impl FnOnce(&mut dyn Services) -> DialogFuture, done: impl FnOnce(&mut FigApp, PathBuf) + 'static) {
+    /// 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;
         }
-        self.pending_dialog = Some((start(self.services.as_mut()), Box::new(done)));
+        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((future, _)) = &mut self.pending_dialog else { return };
-        let waker = Waker::from(Arc::new(RepaintWaker(ctx.clone())));
-        let mut task = TaskContext::from_waker(&waker);
-        let Poll::Ready(path) = future.as_mut().poll(&mut task) else { return };
-        if let Some((_, done)) = self.pending_dialog.take()
-            && let Some(path) = path
+        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))
         {
-            done(self, path);
+            self.session.active = i;
         }
     }
 
@@ -1020,7 +1074,6 @@ impl FigApp {
         self.ui_context = Some(ctx.clone());
         self.frame_stats.record(&ctx);
         let first = !self.fonts_installed;
-        self.poll_dialog(&ctx);
         self.logic(&ctx);
         self.autosave_tick(&ctx);
         if first {
@@ -1062,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);