mirror of
https://github.com/storytold/lightcraft.git
synced 2026-10-31 21:48:53 -06:00
Merge PR #3325: LRC-LIB-FOLDERS: Move Folder To… asks for the destination without freezing the window
crates/ui-egui/src/panels/left/folders.rs: main's built-in-place guard around Rename and Move kept; the Move item is the PR's move_folder_item inside it. docs/parity-checklist.md: the PR's edits to its rows applied to main's rows. docs/whats-new.md: the PR's entry added under the 2026-10-29 heading.
This commit is contained in:
@@ -3,7 +3,10 @@ use super::{LightcraftApp, Value, json};
|
||||
// ------------------------------------------------------------------------------- Open Library…
|
||||
|
||||
/// Switching libraries is refused while an import (or the folder scan before one) is running: its
|
||||
/// remaining batches would be committed into the new library (issue #1509).
|
||||
/// remaining batches would be committed into the new library (issue #1509). So is a switch while
|
||||
/// Move Folder To… waits for its destination: the move runs when the dialog closes, and in another
|
||||
/// library it would move the folder on disk with nothing to relink, leaving this library's photos
|
||||
/// missing (issue #3140).
|
||||
pub(crate) fn can_switch_library(app: &LightcraftApp) -> Result<(), String> {
|
||||
if crate::lightroom_import::is_running(app) {
|
||||
return Err("wait for Lightroom catalog import to finish before switching libraries".into());
|
||||
@@ -11,6 +14,9 @@ pub(crate) fn can_switch_library(app: &LightcraftApp) -> Result<(), String> {
|
||||
if app.import.is_some() || app.scan.is_some() {
|
||||
return Err("wait for the import to finish before switching libraries".into());
|
||||
}
|
||||
if app.pending_picks.iter().any(|p| p.command == "folder.move") {
|
||||
return Err("wait for Move Folder To… to finish before switching libraries".into());
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
@@ -82,15 +82,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()
|
||||
@@ -564,6 +556,52 @@ fn library_node_for(tree: &[FolderNode], path: &str) -> Option<(FolderNode, Stri
|
||||
Some((node, name))
|
||||
}
|
||||
|
||||
/// Move Folder To… can't start while Open Library… or New Library… waits for its folder: the
|
||||
/// dialogs answer in the order they were opened ([`crate::pick::poll`]), so the library could
|
||||
/// switch first and the move then run in the other library, moving the folder on disk with
|
||||
/// nothing to relink (issue #3140).
|
||||
fn can_move_folder(app: &LightcraftApp) -> Result<(), String> {
|
||||
for (command, label) in [("app.openLibrary", "Open Library…"), ("app.newLibrary", "New Library…")] {
|
||||
if app.pending_picks.iter().any(|p| p.command == command) {
|
||||
return Err(format!("wait for {label} to finish before moving a folder"));
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// 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`]).
|
||||
/// Disabled while a library dialog is up ([`can_move_folder`]).
|
||||
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.add_enabled(can_move_folder(app).is_ok(), egui::Button::new(crate::i18n::tr("Move Folder To…")));
|
||||
register(ui, "folderMoveTo", r.rect);
|
||||
if !r.clicked() {
|
||||
return;
|
||||
}
|
||||
if let Err(e) = can_move_folder(app) {
|
||||
app.toast(ui.ctx(), e);
|
||||
ui.close();
|
||||
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) {
|
||||
@@ -613,16 +651,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::*;
|
||||
|
||||
@@ -52,6 +52,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,211 @@
|
||||
//! 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>) {
|
||||
app_with_trip_in(dir, None)
|
||||
}
|
||||
|
||||
/// [`app_with_trip`], with the photo imported into the library at `library` (`None`: in memory).
|
||||
fn app_with_trip_in(dir: &Scratch, library: Option<&str>) -> (Headless, Asked, Arc<AtomicUsize>) {
|
||||
write_png(&dir.path("trip/a.png"), 1);
|
||||
let mut session = lightcraft_engine::Session::new().with_fs();
|
||||
if let Some(library) = library {
|
||||
session.open_library(library, false).unwrap();
|
||||
}
|
||||
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");
|
||||
}
|
||||
|
||||
/// Issue #3140: while Move Folder To… waits for its destination, the library can't be switched
|
||||
/// (the move would run in the other library, moving the folder with nothing to relink). The answer
|
||||
/// moves the folder in the library it was asked in; then switching works again.
|
||||
#[test]
|
||||
fn no_library_switch_while_move_folder_to_waits_for_its_destination() {
|
||||
let dir = Scratch::new("switch");
|
||||
std::fs::create_dir_all(dir.path("dest")).unwrap();
|
||||
let (first, other) = (dir.path("First"), dir.path("Other"));
|
||||
let (mut h, asked, _) = app_with_trip_in(&dir, Some(&first));
|
||||
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!(asked.lock().unwrap().len(), 1, "the dialog is up");
|
||||
let library = |h: &Headless| h.app.session.library.as_ref().map(|l| l.dir.to_string_lossy().replace('\\', "/"));
|
||||
// another library (from the control channel or an agent, say) is refused meanwhile
|
||||
let e = h.app.run("app.openLibrary", json!({"path": other})).expect_err("no switch while the dialog is up");
|
||||
assert!(e.contains("Move Folder To…"), "{e}");
|
||||
assert_eq!(library(&h), Some(first.clone()), "the first library stays open");
|
||||
// the answer moves the folder 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_eq!(library(&h), Some(first));
|
||||
let relinked = 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!(relinked, "the photo is relinked in the first library");
|
||||
// and now the switch goes through
|
||||
h.app.run("app.openLibrary", json!({"path": other})).expect("the switch works once the move is done");
|
||||
assert_eq!(library(&h), Some(other));
|
||||
}
|
||||
|
||||
/// Issue #3140: while Open Library… waits for its folder, Move Folder To… can't start (both
|
||||
/// dialogs answer in the order opened, so the library could switch before the move ran, which
|
||||
/// would then move the folder in the other library with nothing to relink): the item is disabled,
|
||||
/// no destination is asked for, and the folder stays. Once the library dialog is cancelled it works.
|
||||
#[test]
|
||||
fn move_folder_to_waits_for_open_library_to_finish() {
|
||||
let dir = Scratch::new("after-open");
|
||||
std::fs::create_dir_all(dir.path("dest")).unwrap();
|
||||
let (mut h, asked, blocking) = app_with_trip(&dir);
|
||||
assert_eq!(h.app.run("app.openLibrary", json!({})).ok(), Some(serde_json::Value::Null));
|
||||
assert_eq!(asked.lock().unwrap().len(), 1, "the Open Library dialog is up");
|
||||
open_row_menu(&mut h, &format!("source:libfolder:{}", dir.path("trip")));
|
||||
let _ = h.request("ui.clickWidget", json!({"id": "folderMoveTo"}), T);
|
||||
h.settle(SETTLE);
|
||||
assert_eq!(asked.lock().unwrap().len(), 1, "no destination is asked for");
|
||||
assert_eq!(blocking.load(Ordering::Relaxed), 0, "not on the UI thread either");
|
||||
assert!(!h.app.pending_picks.iter().any(|p| p.command == "folder.move"), "nothing waits to move");
|
||||
assert!(std::path::Path::new(&dir.path("trip/a.png")).exists(), "the folder stays");
|
||||
// the Open Library dialog is cancelled: Move Folder To… asks for its destination again
|
||||
asked.lock().unwrap()[0].1.send(vec![]).unwrap();
|
||||
h.settle(SETTLE);
|
||||
assert!(h.app.pending_picks.is_empty());
|
||||
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!(asked.lock().unwrap().len(), 2, "the destination is asked for");
|
||||
assert!(h.app.pending_picks.iter().any(|p| p.command == "folder.move"));
|
||||
}
|
||||
File diff suppressed because one or more lines are too long
@@ -2,6 +2,14 @@
|
||||
|
||||
## 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. While the dialog is open, another library can't be
|
||||
opened (the move belongs to the library it was started in), and while Open Library… or New Library… waits for
|
||||
its folder, Move Folder To… is unavailable.
|
||||
|
||||
### A library on a disconnected drive is reported at launch, not replaced
|
||||
- When the library LightCraft last opened is missing at launch, because its drive or network share isn't connected
|
||||
yet or it was moved, the window now says it couldn't open the library and offers Try Again, Choose Another
|
||||
|
||||
Reference in New Issue
Block a user