mirror of
https://github.com/storytold/lightcraft.git
synced 2026-11-01 14:07:34 -06:00
LRC-LIB-FOLDERS: Move Folder To… asks for the destination without freezing the window (#3140)
Move Folder To… in the Folders and Local menus opened the folder dialog synchronously on the UI thread, so the window stopped drawing until it closed, and on Linux X11 the app could hang. Both menus now go through the shared picker, like Import and Move to Folder…: the dialog runs off the UI thread where the host can do that and folder.move runs when it closes. The "Moved; N photo(s) relinked" message is shown either way, now translated in both menus.
This commit is contained in:
@@ -79,15 +79,7 @@ pub(super) fn folder_tree(
|
||||
key: "name".into(),
|
||||
});
|
||||
}
|
||||
if app.services.pick_folder.is_some() && ui.button(crate::i18n::tr("Move Folder To…")).clicked() {
|
||||
let into = app.services.pick_folder.as_mut().and_then(|f| f());
|
||||
if let Some(into) = into {
|
||||
match app.run("folder.move", json!({"path": path, "into": into})) {
|
||||
Ok(r) => app.toast(ui.ctx(), crate::i18n::tr_format!("Moved; {} photo(s) relinked", r["relinked"])),
|
||||
Err(e) => app.toast(ui.ctx(), e),
|
||||
}
|
||||
}
|
||||
}
|
||||
move_folder_item(app, ui, path);
|
||||
if app.services.reveal.is_some()
|
||||
&& ui.button(crate::i18n::tr(crate::menus::reveal_label())).clicked()
|
||||
&& let Some(f) = app.services.reveal.as_mut()
|
||||
@@ -560,6 +552,33 @@ fn library_node_for(tree: &[FolderNode], path: &str) -> Option<(FolderNode, Stri
|
||||
Some((node, name))
|
||||
}
|
||||
|
||||
/// Move Folder To… of a folder row (widget `folderMoveTo`): asks for the destination through the
|
||||
/// shared folder dialog, off the UI thread where the host has one (issue #3140: a dialog on the UI
|
||||
/// thread froze the window); `folder.move` then runs when it closes ([`crate::pick::poll`]).
|
||||
fn move_folder_item(app: &mut LightcraftApp, ui: &mut egui::Ui, path: &str) {
|
||||
if app.services.pick_folder.is_none() && app.services.picker.is_none() {
|
||||
return;
|
||||
}
|
||||
let r = ui.button(crate::i18n::tr("Move Folder To…"));
|
||||
register(ui, "folderMoveTo", r.rect);
|
||||
if !r.clicked() {
|
||||
return;
|
||||
}
|
||||
let req = crate::pick::PickRequest::folder(crate::i18n::tr("Move Folder To…"));
|
||||
let params = json!({"path": path});
|
||||
let picked = crate::pick::ask(app, "folder.move", ¶ms, "into", req, |s| s.pick_folder.as_mut().and_then(|f| f()).map(|x| vec![x]));
|
||||
if let crate::pick::Picked::Now(v) = picked
|
||||
&& let Some(into) = v.into_iter().next()
|
||||
{
|
||||
let said = app.run("folder.move", json!({"path": path, "into": into})).map(|r| crate::pick::answered_message("folder.move", &r));
|
||||
match said {
|
||||
Ok(Some(text)) | Err(text) => app.toast(ui.ctx(), text),
|
||||
Ok(None) => {}
|
||||
}
|
||||
}
|
||||
ui.close();
|
||||
}
|
||||
|
||||
/// Synchronize Folder… of a Folders row (widget `folderSynchronize`): opens its dialog, which
|
||||
/// scans the folder in the background.
|
||||
fn synchronize_item(app: &mut LightcraftApp, ui: &mut egui::Ui, n: &FolderNode, name: &str) {
|
||||
@@ -609,16 +628,7 @@ fn folder_menu_for_library(app: &mut LightcraftApp, resp: &egui::Response, n: &F
|
||||
});
|
||||
ui.close();
|
||||
}
|
||||
if app.services.pick_folder.is_some() && ui.button(crate::i18n::tr("Move Folder To…")).clicked() {
|
||||
let into = app.services.pick_folder.as_mut().and_then(|f| f());
|
||||
if let Some(into) = into {
|
||||
match app.run("folder.move", json!({"path": path, "into": into})) {
|
||||
Ok(r) => app.toast(ui.ctx(), format!("Moved; {} photo(s) relinked", r["relinked"])),
|
||||
Err(e) => app.toast(ui.ctx(), e),
|
||||
}
|
||||
}
|
||||
ui.close();
|
||||
}
|
||||
move_folder_item(app, ui, path);
|
||||
if ui.button(crate::i18n::tr("New Folder on Disk…")).clicked() {
|
||||
app.ui.dialog = Some(crate::state::Dialog::TextPrompt {
|
||||
title: crate::i18n::tr_format!("New Folder in “{name}”", name = label),
|
||||
|
||||
@@ -152,12 +152,26 @@ pub fn poll(app: &mut LightcraftApp, ctx: &egui::Context) {
|
||||
app.pending_picks = waiting;
|
||||
for (p, paths) in answered {
|
||||
let params = with_answer(&p.params, p.key, p.kind, &paths);
|
||||
if let Err(e) = app.run(&p.command, params) {
|
||||
app.toast_error(ctx, e);
|
||||
match app.run(&p.command, params) {
|
||||
Ok(r) => {
|
||||
if let Some(text) = answered_message(&p.command, &r) {
|
||||
app.toast(ctx, text);
|
||||
}
|
||||
}
|
||||
Err(e) => app.toast_error(ctx, e),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// What `command`, run with a dialog's answer, says when it worked (`None`: its result shows
|
||||
/// for itself). The same words whether the dialog answered at once or later.
|
||||
pub fn answered_message(command: &str, result: &Value) -> Option<String> {
|
||||
match command {
|
||||
"folder.move" => Some(crate::i18n::tr_format!("Moved; {} photo(s) relinked", result["relinked"])),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
@@ -51,6 +51,7 @@ mod masking_layout;
|
||||
mod menubar;
|
||||
mod merge_preview;
|
||||
mod model_setup;
|
||||
mod move_folder;
|
||||
mod offline;
|
||||
mod open_lightroom;
|
||||
pub(super) mod panels;
|
||||
|
||||
@@ -0,0 +1,138 @@
|
||||
//! Move Folder To… in a folder's menu (issue #3140): the folder dialog runs off the UI thread
|
||||
//! where the host can show it there, as the other file dialogs do (#191).
|
||||
|
||||
use std::sync::atomic::{AtomicUsize, Ordering};
|
||||
use std::sync::mpsc::{Sender, channel};
|
||||
use std::sync::{Arc, Mutex};
|
||||
use std::time::Duration;
|
||||
|
||||
use serde_json::json;
|
||||
|
||||
use crate::headless::Headless;
|
||||
use crate::pick::{PickKind, PickRequest};
|
||||
use crate::{LightcraftApp, Services};
|
||||
|
||||
const T: Duration = Duration::from_secs(20);
|
||||
const SETTLE: Duration = Duration::from_secs(120);
|
||||
|
||||
/// A scratch folder that goes away with the test, however it ends.
|
||||
struct Scratch(std::path::PathBuf);
|
||||
|
||||
impl Scratch {
|
||||
fn new(tag: &str) -> Scratch {
|
||||
let dir = std::env::temp_dir().join(format!("lc-ui-move-folder-{tag}-{}", std::process::id()));
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
std::fs::create_dir_all(&dir).unwrap();
|
||||
Scratch(dir)
|
||||
}
|
||||
/// The path as the folder tree writes it (forward slashes; widget ids carry it).
|
||||
fn path(&self, rel: &str) -> String {
|
||||
self.0.join(rel).to_string_lossy().replace('\\', "/")
|
||||
}
|
||||
}
|
||||
|
||||
impl Drop for Scratch {
|
||||
fn drop(&mut self) {
|
||||
let _ = std::fs::remove_dir_all(&self.0);
|
||||
}
|
||||
}
|
||||
|
||||
fn write_png(path: &str, seed: u8) {
|
||||
let img = lightcraft_raster::Rgba8::from_fn(24, 16, |x, y| [(x * 9) as u8, (y * 13) as u8, seed, 255]);
|
||||
let bytes = lightcraft_codecs::encode_png(&lightcraft_codecs::EncodeImage::rgba8(&img), &lightcraft_codecs::EncodeMeta::default()).unwrap();
|
||||
std::fs::create_dir_all(std::path::Path::new(path).parent().unwrap()).unwrap();
|
||||
std::fs::write(path, bytes).unwrap();
|
||||
}
|
||||
|
||||
type Asked = Arc<Mutex<Vec<(PickRequest, Sender<Vec<String>>)>>>;
|
||||
|
||||
/// The app with `trip/a.png` imported, a host that shows dialogs elsewhere (each request is
|
||||
/// recorded with the sender that answers it), and a synchronous folder dialog that only counts
|
||||
/// how often it was opened on the UI thread.
|
||||
fn app_with_trip(dir: &Scratch) -> (Headless, Asked, Arc<AtomicUsize>) {
|
||||
write_png(&dir.path("trip/a.png"), 1);
|
||||
let mut session = lightcraft_engine::Session::new().with_fs();
|
||||
session.execute("library.import", &json!({"paths": [dir.path("trip")]})).unwrap();
|
||||
let mut app = LightcraftApp::new(session, Services { png: None, ..Default::default() });
|
||||
let asked: Asked = Default::default();
|
||||
let a = asked.clone();
|
||||
app.services.picker = Some(Box::new(move |req| {
|
||||
let (tx, rx) = channel();
|
||||
a.lock().unwrap().push((req, tx));
|
||||
Ok(rx)
|
||||
}));
|
||||
let blocking = Arc::new(AtomicUsize::new(0));
|
||||
let b = blocking.clone();
|
||||
app.services.pick_folder = Some(Box::new(move || {
|
||||
b.fetch_add(1, Ordering::Relaxed);
|
||||
None
|
||||
}));
|
||||
let mut h = Headless::new(app, [1400.0, 900.0], 1.0);
|
||||
let r = h.request("ui.set", json!({"view": "photoGrid", "leftPanel": true}), T);
|
||||
assert_eq!(r["ok"], true, "{r}");
|
||||
h.settle(SETTLE);
|
||||
(h, asked, blocking)
|
||||
}
|
||||
|
||||
/// Right-click the row `row` in the sidebar, scrolled into view first.
|
||||
fn open_row_menu(h: &mut Headless, row: &str) {
|
||||
assert!(h.step_until(T, |h| h.app.widgets.iter().any(|(w, _)| w == row)), "no row {row}");
|
||||
let rect = |h: &Headless, id: &str| h.app.widgets.iter().find(|(w, _)| w == id).map(|(_, r)| *r);
|
||||
let (Some(before), Some(panel)) = (rect(h, row), rect(h, "panel:left_panel")) else { panic!("no row {row} or left panel") };
|
||||
if before.intersect(panel).height() < before.height() - 0.5 {
|
||||
crate::tests_panels::scroll_sidebar(h, panel.center().y - before.center().y);
|
||||
}
|
||||
let c = rect(h, row).expect("the row").center();
|
||||
let r = h.request("ui.click", json!({"x": c.x, "y": c.y, "button": "right"}), T);
|
||||
assert_eq!(r["ok"], true, "{r}");
|
||||
h.step();
|
||||
h.step();
|
||||
}
|
||||
|
||||
/// Move Folder To… in the Folders and Local sections asks for the destination off the UI thread,
|
||||
/// never with the blocking dialog, then moves the folder with the answer and says so.
|
||||
#[test]
|
||||
fn move_folder_to_asks_off_the_ui_thread_and_moves_with_the_answer() {
|
||||
let dir = Scratch::new("folders");
|
||||
std::fs::create_dir_all(dir.path("dest")).unwrap();
|
||||
let (mut h, asked, blocking) = app_with_trip(&dir);
|
||||
open_row_menu(&mut h, &format!("source:libfolder:{}", dir.path("trip")));
|
||||
let r = h.request("ui.clickWidget", json!({"id": "folderMoveTo"}), T);
|
||||
assert_eq!(r["ok"], true, "{r}");
|
||||
h.step();
|
||||
assert_eq!(blocking.load(Ordering::Relaxed), 0, "the UI thread never waits on a dialog");
|
||||
{
|
||||
let asked = asked.lock().unwrap();
|
||||
assert_eq!(asked.len(), 1, "one dialog");
|
||||
assert_eq!(asked[0].0.kind, PickKind::Folder);
|
||||
}
|
||||
assert!(std::path::Path::new(&dir.path("trip/a.png")).exists(), "nothing moves before the answer");
|
||||
// the user chooses the destination: the folder moves there and its photo follows
|
||||
asked.lock().unwrap()[0].1.send(vec![dir.path("dest")]).unwrap();
|
||||
h.settle(SETTLE);
|
||||
assert!(h.app.pending_picks.is_empty());
|
||||
assert!(std::path::Path::new(&dir.path("dest/trip/a.png")).exists(), "moved on disk");
|
||||
assert!(!std::path::Path::new(&dir.path("trip")).exists());
|
||||
let moved = h.app.session.catalog.photos().filter(|p| p.in_library()).all(|p| match &p.source {
|
||||
lightcraft_catalog::Source::File { path } => path.replace('\\', "/").starts_with(&dir.path("dest/trip")),
|
||||
lightcraft_catalog::Source::Demo { .. } => false,
|
||||
});
|
||||
assert!(moved, "the photo is relinked");
|
||||
let toast = h.app.ui.toast.as_ref().map(|t| t.0.clone()).unwrap_or_default();
|
||||
assert!(toast.contains("1 photo(s) relinked"), "{toast:?}");
|
||||
|
||||
// the same from Local (paths written the way the OS does): back to where it was
|
||||
let moved_trip = dir.0.join("dest").join("trip").to_string_lossy().to_string();
|
||||
let r = h.request("engine.execute", json!({"command": "local.addRoot", "params": {"path": moved_trip}}), T);
|
||||
assert_eq!(r["ok"], true, "{r}");
|
||||
h.step();
|
||||
open_row_menu(&mut h, &format!("source:local:{moved_trip}"));
|
||||
let r = h.request("ui.clickWidget", json!({"id": "folderMoveTo"}), T);
|
||||
assert_eq!(r["ok"], true, "{r}");
|
||||
h.step();
|
||||
assert_eq!(blocking.load(Ordering::Relaxed), 0);
|
||||
assert_eq!(asked.lock().unwrap().len(), 2, "its own dialog");
|
||||
asked.lock().unwrap()[1].1.send(vec![dir.0.to_string_lossy().to_string()]).unwrap();
|
||||
h.settle(SETTLE);
|
||||
assert!(std::path::Path::new(&dir.path("trip/a.png")).exists(), "moved back");
|
||||
}
|
||||
File diff suppressed because one or more lines are too long
@@ -1,5 +1,13 @@
|
||||
# What's new in LightCraft
|
||||
|
||||
## 2026-10-29
|
||||
|
||||
### Move Folder To… no longer freezes the window
|
||||
- Move Folder To…, in the menu of a folder in Folders or Local, now opens its folder dialog the way Import and Move to
|
||||
Folder… do: the window keeps drawing and answering while you choose, and the folder moves when the dialog closes,
|
||||
with the same "Moved; N photo(s) relinked" message. Before, the dialog held up the whole window until it closed;
|
||||
on Linux with X11 the app could stop responding altogether.
|
||||
|
||||
## 2026-10-24
|
||||
|
||||
### Relative-date filters and smart albums follow the clock
|
||||
|
||||
Reference in New Issue
Block a user