diff --git a/crates/engine/src/cmd/library.rs b/crates/engine/src/cmd/library.rs index 3a5575ea81..39cfc30655 100644 --- a/crates/engine/src/cmd/library.rs +++ b/crates/engine/src/cmd/library.rs @@ -1334,17 +1334,38 @@ pub fn specs() -> Vec { Ok(result) } ), - cmd!("album.removePhotos", "Remove from Album", [], None, "{id: albumId, ids?}", has_selection, |s, p| { - let id = album_param(p, "id", "album.removePhotos")?; - let targets = ids_param(p).unwrap_or_else(|| s.targets(&Value::Null)); - let al = s.catalog.album(id).ok_or_else(|| bad("album.removePhotos", "no such album"))?; - if al.is_smart() { - return Err(bad("album.removePhotos", "smart albums update automatically: change their rules")); + cmd!( + "album.removePhotos", + "Remove from Album", + ["Photo"], + None, + "{id?: albumId (default: the album on screen), ids?} — takes the photos out of the album only: they stay in the library and in their folders (issue #1805; photo.delete removes them from the library) → {removed, album: name}", + has_selection, + |s, p| { + let id = match p.get("id") { + Some(_) => album_param(p, "id", "album.removePhotos")?, + None => match s.source { + LibrarySource::Album(a) => a, + _ => return Err(bad("album.removePhotos", "no album: pass `id` or show an album")), + }, + }; + let targets = ids_param(p).unwrap_or_else(|| s.targets(&Value::Null)); + let al = s.catalog.album(id).ok_or_else(|| bad("album.removePhotos", "no such album"))?; + if al.is_smart() { + return Err(bad("album.removePhotos", "smart albums update automatically: change their rules")); + } + if al.folder { + return Err(bad("album.removePhotos", "album folders don't hold photos")); + } + let name = al.name.clone(); + let photos: Vec = al.photos.iter().copied().filter(|x| !targets.contains(x)).collect(); + let removed = al.photos.len().saturating_sub(photos.len()); + if removed > 0 { + s.commit("Remove from Album", Op::SetAlbumPhotos { id, photos })?; + } + Ok(json!({"removed": removed, "album": name})) } - let photos: Vec = al.photos.iter().copied().filter(|x| !targets.contains(x)).collect(); - s.commit("Remove from Album", Op::SetAlbumPhotos { id, photos })?; - ok() - }), + ), cmd!("album.setCover", "Set as Album Cover", [], None, "{id: albumId, photo?: photoId}", has_active, |s, p| { let id = album_param(p, "id", "album.setCover")?; let photo = p.get("photo").and_then(Value::as_u64).map(PhotoId).or(s.active()); diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 7e05fcffd1..8b45eeecc1 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -996,6 +996,8 @@ mod tests; #[cfg(test)] mod tests_album_order; #[cfg(test)] +mod tests_album_remove; +#[cfg(test)] mod tests_albums_local; #[cfg(test)] mod tests_color; diff --git a/crates/engine/src/tests_album_remove.rs b/crates/engine/src/tests_album_remove.rs new file mode 100644 index 0000000000..041eb3812e --- /dev/null +++ b/crates/engine/src/tests_album_remove.rs @@ -0,0 +1,105 @@ +//! Removing photos from an album changes the album only (issue #1805): the photos stay in the +//! library, in their library folder and in the folder view of their folder on disk. Deleting a +//! photo (`photo.delete`) still removes it from all of them. + +use std::path::{Path, PathBuf}; + +use lightcraft_catalog::PhotoId; +use serde_json::json; + +use crate::{Selection, Session}; + +fn temp_dir(tag: &str) -> PathBuf { + let d = std::env::temp_dir().join(format!("lc-album-remove-{tag}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&d); + std::fs::create_dir_all(&d).unwrap(); + d +} + +fn png(p: &Path, seed: u8) { + let img = lightcraft_raster::Rgba8::from_fn(16, 12, |x, y| [(x * 9) as u8, (y * 11) as u8, seed, 255]); + let b = + crate::export::encode_image(&img, &crate::export::ExportOptions { format: crate::export::ExportFormat::Png, ..Default::default() }).unwrap(); + std::fs::write(p, b).unwrap(); +} + +/// A folder of three files imported into a new album "Trip": (session, folder, album id, photos). +fn imported(tag: &str) -> (Session, String, u64, Vec) { + let dir = temp_dir(tag); + for (i, n) in ["a.png", "b.png", "c.png"].iter().enumerate() { + png(&dir.join(n), i as u8 * 40); + } + let folder = dir.to_string_lossy().to_string(); + let mut s = Session::new().with_fs(); + let r = s.execute("library.import", &json!({"paths": [folder], "mode": "add", "albumName": "Trip"})).unwrap(); + let album = r["album"].as_u64().expect("the import made the album"); + let mut ids: Vec = r["imported"].as_array().unwrap().iter().filter_map(|v| v.as_u64()).map(PhotoId).collect(); + ids.sort(); + assert_eq!(ids.len(), 3); + (s, folder, album, ids) +} + +/// How many photos the library folder view, the folder (disk) view and the album show. +fn counts(s: &mut Session, folder: &str, album: u64) -> (u64, u64, u64) { + let lib = s.execute("library.source", &json!({"kind": "libraryFolder", "path": folder})).unwrap()["count"].as_u64().unwrap(); + let disk = s.execute("library.browse", &json!({"path": folder})).unwrap()["photos"].as_u64().unwrap(); + let disk_visible = s.visible_cloned().len() as u64; + assert_eq!(disk, disk_visible, "browse reports what it shows"); + let al = s.execute("library.source", &json!({"kind": "album", "id": album})).unwrap()["count"].as_u64().unwrap(); + (lib, disk, al) +} + +/// The reporter's steps: a folder added to an album, a photo removed in the album, then the folder +/// view: it still shows every photo of the folder. +#[test] +fn removing_from_an_album_keeps_the_photo_in_its_folder() { + let (mut s, folder, album, ids) = imported("keep"); + assert_eq!(counts(&mut s, &folder, album), (3, 3, 3)); + + // in the album (its source on screen), remove one photo; no `id`: the album on screen + s.execute("library.source", &json!({"kind": "album", "id": album})).unwrap(); + s.selection = Selection::single(ids[1]); + let r = s.execute("album.removePhotos", &json!({})).unwrap(); + assert_eq!(r["removed"], 1, "{r}"); + assert_eq!(r["album"], "Trip", "{r}"); + assert!(!s.catalog.photo(ids[1]).unwrap().deleted, "still in the library"); + assert_eq!(counts(&mut s, &folder, album), (3, 3, 2), "(library folder, folder on disk, album)"); + + // removing what isn't in the album changes nothing (and makes no undo step) + let undo = s.undo.len(); + s.execute("library.source", &json!({"kind": "album", "id": album})).unwrap(); + let r = s.execute("album.removePhotos", &json!({"ids": [ids[1].0]})).unwrap(); + assert_eq!(r["removed"], 0, "{r}"); + assert_eq!(s.undo.len(), undo); + let _ = std::fs::remove_dir_all(&folder); +} + +/// Delete (to Recently Deleted) is the way out of the library: the photo leaves its folder views +/// and the album with it. +#[test] +fn deleting_a_photo_still_removes_it_everywhere() { + let (mut s, folder, album, ids) = imported("delete"); + s.execute("library.source", &json!({"kind": "album", "id": album})).unwrap(); + s.selection = Selection::single(ids[0]); + s.execute("photo.delete", &json!({})).unwrap(); + assert!(s.catalog.photo(ids[0]).unwrap().deleted); + assert_eq!(counts(&mut s, &folder, album), (2, 2, 2), "(library folder, folder on disk, album)"); + let n = s.execute("library.source", &json!({"kind": "recentlyDeleted"})).unwrap()["count"].as_u64().unwrap(); + assert_eq!(n, 1); + let _ = std::fs::remove_dir_all(&folder); +} + +/// Without an album on screen and without `id` there is nothing to remove from; smart albums +/// follow their rules. +#[test] +fn remove_from_album_refuses_what_it_cannot_do() { + let (mut s, folder, _album, ids) = imported("refuse"); + s.execute("library.source", &json!({"kind": "all"})).unwrap(); + s.selection = Selection::single(ids[0]); + assert!(s.execute("album.removePhotos", &json!({})).is_err(), "no album on screen"); + let smart = s.execute("album.createSmart", &json!({"name": "All", "rules": {}})).unwrap()["id"].as_u64().unwrap(); + assert!(s.execute("album.removePhotos", &json!({"id": smart})).is_err(), "smart album"); + assert!(s.execute("album.removePhotos", &json!({"id": 9999})).is_err(), "no such album"); + assert!(s.execute("album.removePhotos", &json!({"id": "x"})).is_err(), "bad id"); + let _ = std::fs::remove_dir_all(&folder); +} diff --git a/crates/ui-egui/locales/de.json b/crates/ui-egui/locales/de.json index 25c411ce0e..d524ed5b64 100644 --- a/crates/ui-egui/locales/de.json +++ b/crates/ui-egui/locales/de.json @@ -2118,5 +2118,6 @@ "Adding to Apple Photos": "Wird zu Apple Fotos hinzugefügt", "No graphics processor found: LightCraft renders on the CPU, which is much slower. Details in Help → System Info.": "Kein Grafikprozessor gefunden: LightCraft rendert auf der CPU, was viel langsamer ist. Details unter Hilfe → Systeminformationen.", "On NixOS, start it from `nix develop`.": "Unter NixOS starten Sie es aus `nix develop`.", - "Files with the same content as another file in this import: only the first is imported.": "Dateien mit demselben Inhalt wie eine andere Datei dieses Imports: Nur die erste wird importiert." + "Files with the same content as another file in this import: only the first is imported.": "Dateien mit demselben Inhalt wie eine andere Datei dieses Imports: Nur die erste wird importiert.", + "Smart albums update automatically: change their rules, or use Photo ▸ Delete Photo": "Intelligente Alben aktualisieren sich selbst: Ändern Sie ihre Regeln oder verwenden Sie Foto ▸ Foto löschen" } diff --git a/crates/ui-egui/locales/es.json b/crates/ui-egui/locales/es.json index ba3e1e3316..aa0fb71051 100644 --- a/crates/ui-egui/locales/es.json +++ b/crates/ui-egui/locales/es.json @@ -1995,5 +1995,6 @@ "Adding to Apple Photos": "Añadiendo a Fotos de Apple", "No graphics processor found: LightCraft renders on the CPU, which is much slower. Details in Help → System Info.": "No se encontró ningún procesador gráfico: LightCraft renderiza en la CPU, lo que es mucho más lento. Detalles en Ayuda → Información del sistema.", "On NixOS, start it from `nix develop`.": "En NixOS, inícielo desde `nix develop`.", - "Files with the same content as another file in this import: only the first is imported.": "Archivos con el mismo contenido que otro archivo de esta importación: solo se importa el primero." + "Files with the same content as another file in this import: only the first is imported.": "Archivos con el mismo contenido que otro archivo de esta importación: solo se importa el primero.", + "Smart albums update automatically: change their rules, or use Photo ▸ Delete Photo": "Los álbumes inteligentes se actualizan solos: cambie sus reglas o use Foto ▸ Eliminar foto" } diff --git a/crates/ui-egui/locales/fr.json b/crates/ui-egui/locales/fr.json index b0becb154e..ae763e28cb 100644 --- a/crates/ui-egui/locales/fr.json +++ b/crates/ui-egui/locales/fr.json @@ -1957,5 +1957,6 @@ "Adding to Apple Photos": "Ajout à Apple Photos", "No graphics processor found: LightCraft renders on the CPU, which is much slower. Details in Help → System Info.": "Aucun processeur graphique trouvé : LightCraft effectue le rendu sur le processeur, ce qui est beaucoup plus lent. Détails dans Aide → Informations système.", "On NixOS, start it from `nix develop`.": "Sous NixOS, lancez-le depuis `nix develop`.", - "Files with the same content as another file in this import: only the first is imported.": "Fichiers au contenu identique à un autre fichier de cette importation : seul le premier est importé." + "Files with the same content as another file in this import: only the first is imported.": "Fichiers au contenu identique à un autre fichier de cette importation : seul le premier est importé.", + "Smart albums update automatically: change their rules, or use Photo ▸ Delete Photo": "Les albums intelligents se mettent à jour d'eux-mêmes : modifiez leurs règles ou utilisez Photo ▸ Supprimer la photo" } diff --git a/crates/ui-egui/locales/ja.json b/crates/ui-egui/locales/ja.json index 7a282e676e..ad7755842e 100644 --- a/crates/ui-egui/locales/ja.json +++ b/crates/ui-egui/locales/ja.json @@ -1961,5 +1961,6 @@ "Adding to Apple Photos": "Appleの写真に追加中", "No graphics processor found: LightCraft renders on the CPU, which is much slower. Details in Help → System Info.": "グラフィックプロセッサが見つかりません。LightCraftはCPUで描画するため、かなり遅くなります。詳細は「ヘルプ → システム情報」を参照してください。", "On NixOS, start it from `nix develop`.": "NixOSでは `nix develop` から起動してください。", - "Files with the same content as another file in this import: only the first is imported.": "この読み込み内の別のファイルと内容が同じファイルです。最初のファイルだけが読み込まれます。" + "Files with the same content as another file in this import: only the first is imported.": "この読み込み内の別のファイルと内容が同じファイルです。最初のファイルだけが読み込まれます。", + "Smart albums update automatically: change their rules, or use Photo ▸ Delete Photo": "スマートアルバムは自動で更新されます。ルールを変更するか、写真 ▸ 写真を削除を使ってください" } diff --git a/crates/ui-egui/locales/pt-br.json b/crates/ui-egui/locales/pt-br.json index f8e4753e5c..07d851014a 100644 --- a/crates/ui-egui/locales/pt-br.json +++ b/crates/ui-egui/locales/pt-br.json @@ -1998,5 +1998,6 @@ "Recognition models": "Modelos de reconhecimento", "No graphics processor found: LightCraft renders on the CPU, which is much slower. Details in Help → System Info.": "Nenhum processador gráfico encontrado: o LightCraft renderiza na CPU, o que é muito mais lento. Detalhes em Ajuda → Informações do sistema.", "On NixOS, start it from `nix develop`.": "No NixOS, inicie-o a partir de `nix develop`.", - "Files with the same content as another file in this import: only the first is imported.": "Arquivos com o mesmo conteúdo de outro arquivo desta importação: apenas o primeiro é importado." + "Files with the same content as another file in this import: only the first is imported.": "Arquivos com o mesmo conteúdo de outro arquivo desta importação: apenas o primeiro é importado.", + "Smart albums update automatically: change their rules, or use Photo ▸ Delete Photo": "Álbuns inteligentes se atualizam sozinhos: altere as regras ou use Foto ▸ Excluir foto" } diff --git a/crates/ui-egui/locales/ru.json b/crates/ui-egui/locales/ru.json index c840218a03..fa830f2120 100644 --- a/crates/ui-egui/locales/ru.json +++ b/crates/ui-egui/locales/ru.json @@ -2151,5 +2151,6 @@ "Adding to Apple Photos": "Добавление в Apple Фото", "No graphics processor found: LightCraft renders on the CPU, which is much slower. Details in Help → System Info.": "Графический процессор не найден: LightCraft выполняет рендеринг на CPU, а это намного медленнее. Подробности: Справка → Сведения о системе.", "On NixOS, start it from `nix develop`.": "В NixOS запускайте его из `nix develop`.", - "Files with the same content as another file in this import: only the first is imported.": "Файлы с тем же содержимым, что и другой файл этого импорта: импортируется только первый." + "Files with the same content as another file in this import: only the first is imported.": "Файлы с тем же содержимым, что и другой файл этого импорта: импортируется только первый.", + "Smart albums update automatically: change their rules, or use Photo ▸ Delete Photo": "Смарт-альбомы обновляются сами: измените их правила или выберите Фото ▸ Удалить фото" } diff --git a/crates/ui-egui/locales/uk.json b/crates/ui-egui/locales/uk.json index 85a8380980..5429ac1f82 100644 --- a/crates/ui-egui/locales/uk.json +++ b/crates/ui-egui/locales/uk.json @@ -2151,5 +2151,6 @@ "Adding to Apple Photos": "Додавання до Apple Фото", "No graphics processor found: LightCraft renders on the CPU, which is much slower. Details in Help → System Info.": "Графічний процесор не знайдено: LightCraft виконує рендеринг на ЦП, а це набагато повільніше. Докладніше: Довідка → Інформація про систему.", "On NixOS, start it from `nix develop`.": "У NixOS запускайте його з `nix develop`.", - "Files with the same content as another file in this import: only the first is imported.": "Файли з тим самим вмістом, що й інший файл цього імпорту: імпортується лише перший." + "Files with the same content as another file in this import: only the first is imported.": "Файли з тим самим вмістом, що й інший файл цього імпорту: імпортується лише перший.", + "Smart albums update automatically: change their rules, or use Photo ▸ Delete Photo": "Розумні альбоми оновлюються самі: змініть їхні правила або виберіть Фото ▸ Видалити фото" } diff --git a/crates/ui-egui/locales/zh-hans.json b/crates/ui-egui/locales/zh-hans.json index ad379f1ef3..a5eb130494 100644 --- a/crates/ui-egui/locales/zh-hans.json +++ b/crates/ui-egui/locales/zh-hans.json @@ -1997,5 +1997,6 @@ "Adding to Apple Photos": "正在添加到 Apple 照片", "No graphics processor found: LightCraft renders on the CPU, which is much slower. Details in Help → System Info.": "未找到图形处理器:LightCraft 改用 CPU 渲染,速度会慢很多。详情见“帮助 → 系统信息”。", "On NixOS, start it from `nix develop`.": "在 NixOS 上,请从 `nix develop` 启动。", - "Files with the same content as another file in this import: only the first is imported.": "与本次导入中另一个文件内容相同的文件:只导入第一个。" + "Files with the same content as another file in this import: only the first is imported.": "与本次导入中另一个文件内容相同的文件:只导入第一个。", + "Smart albums update automatically: change their rules, or use Photo ▸ Delete Photo": "智能相册会自动更新:请更改其规则,或使用“照片 ▸ 删除照片”" } diff --git a/crates/ui-egui/locales/zh-hant.json b/crates/ui-egui/locales/zh-hant.json index 647d153d0a..a7cc6c5674 100644 --- a/crates/ui-egui/locales/zh-hant.json +++ b/crates/ui-egui/locales/zh-hant.json @@ -1959,5 +1959,6 @@ "Adding to Apple Photos": "正在加入 Apple 照片", "No graphics processor found: LightCraft renders on the CPU, which is much slower. Details in Help → System Info.": "找不到圖形處理器:LightCraft 改用 CPU 算繪,速度會慢很多。詳情請見「說明 → 系統資訊」。", "On NixOS, start it from `nix develop`.": "在 NixOS 上,請從 `nix develop` 啟動。", - "Files with the same content as another file in this import: only the first is imported.": "與本次匯入中另一個檔案內容相同的檔案:只匯入第一個。" + "Files with the same content as another file in this import: only the first is imported.": "與本次匯入中另一個檔案內容相同的檔案:只匯入第一個。", + "Smart albums update automatically: change their rules, or use Photo ▸ Delete Photo": "智慧型相簿會自動更新:請變更其規則,或使用「照片 ▸ 刪除照片」" } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 37fb86df31..edebce995d 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -45,6 +45,8 @@ mod tests_activity; #[cfg(test)] mod tests_album_picker; #[cfg(test)] +mod tests_album_remove; +#[cfg(test)] mod tests_appearance; #[cfg(test)] mod tests_before_after; diff --git a/crates/ui-egui/src/menubar.rs b/crates/ui-egui/src/menubar.rs index fe6905cadb..61d8ff39f7 100644 --- a/crates/ui-egui/src/menubar.rs +++ b/crates/ui-egui/src/menubar.rs @@ -207,6 +207,7 @@ const LAYOUT: &[(&str, &[&str])] = &[ "dialog.captureTime", "photo.tagFromTracklog", "---", + "album.removePhotos", "photo.delete", "photo.restore", "photo.deletePermanently", @@ -286,12 +287,22 @@ fn host_supports(app: &LightcraftApp, id: &str) -> bool { "file.restoreLibrary" => app.services.restore_library.is_some(), "photo.restore" | "photo.deletePermanently" => selection_deleted(app), "photo.delete" => !selection_deleted(app), + // only in an album that holds its photos (not a smart one) + "album.removePhotos" => in_normal_album(app) && !selection_deleted(app), // only where the trash is on screen "library.emptyRecentlyDeleted" => app.session.source == lightcraft_engine::LibrarySource::RecentlyDeleted, _ => true, } } +/// The library shows an album that holds its photos (not a smart album, not a folder of albums). +pub(crate) fn in_normal_album(app: &LightcraftApp) -> bool { + match app.session.source { + lightcraft_engine::LibrarySource::Album(a) => app.session.catalog.album(a).is_some_and(|a| !a.folder && !a.is_smart()), + _ => false, + } +} + fn item(id: &str, params: Value, label: impl Into, shortcut: Option<&str>, enabled: bool, checked: Option) -> MenuNode { MenuNode::Item { id: id.into(), params, label: label.into(), shortcut: shortcut.map(str::to_string), enabled, checked } } @@ -670,6 +681,13 @@ fn expanded(app: &LightcraftApp, name: &str) -> Option> { fn node(app: &LightcraftApp, e: &MenuEntry) -> MenuNode { // Shift+P picks and advances in Library; don't advertise it on Presets in those views. let shortcut = if e.id == "panel.presets" && crate::shortcuts::library_grid(app) { None } else { e.shortcut.as_deref() }; + // in an album the Delete key removes from the album (issue #1805): the menu says so + let delete_key = crate::shortcuts::shortcut_of(&app.ui.settings.keymap, "photo.delete"); + let shortcut = match e.id.as_str() { + "album.removePhotos" => delete_key, + "photo.delete" if matches!(app.session.source, lightcraft_engine::LibrarySource::Album(_)) => None, + _ => shortcut, + }; item(&e.id, Value::Null, live_label(app, &e.id, &e.label), shortcut, e.enabled, checked(app, &e.id)) } diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index ad31c57bf8..722bd7f55e 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -1973,6 +1973,32 @@ pub fn menu_entries(app: &LightcraftApp) -> Vec { v } +/// The Delete key in an album: a normal album loses the selected photos (`album.removePhotos`), +/// which stay in the library and their folder; a smart album keeps them (its rules decide) and +/// says so. True when the key was handled here, false outside albums (then it deletes the photos +/// from the library). Photo ▸ Delete Photo still deletes from the library everywhere. +pub fn delete_in_album(app: &mut LightcraftApp, ctx: &egui::Context) -> bool { + let lightcraft_engine::LibrarySource::Album(aid) = app.session.source else { return false }; + let Some(album) = app.session.catalog.album(aid) else { return false }; + if album.is_smart() { + app.toast(ctx, crate::i18n::tr("Smart albums update automatically: change their rules, or use Photo ▸ Delete Photo")); + return true; + } + if app.session.targets(&json!({})).is_empty() { + return true; + } + match app.run("album.removePhotos", json!({"id": aid.0})) { + Ok(r) => { + let n = r["removed"].as_u64().unwrap_or(0); + let name = r["album"].as_str().unwrap_or_default().to_string(); + let what = crate::i18n::tr_format!("{n} photo{}", if n == 1 { "" } else { "s" }, n = n); + app.toast(ctx, crate::i18n::tr_format!("Removed {what} from {name}", what = what, name = name)); + } + Err(e) => app.toast(ctx, e), + } + true +} + /// With Settings → General → "Confirm before deleting" on, open the confirmation dialog instead /// of deleting; true when it did (the dialog's OK runs `photo.delete`). pub fn confirm_delete(app: &mut LightcraftApp) -> bool { diff --git a/crates/ui-egui/src/shortcuts.rs b/crates/ui-egui/src/shortcuts.rs index b10387c767..276840496d 100644 --- a/crates/ui-egui/src/shortcuts.rs +++ b/crates/ui-egui/src/shortcuts.rs @@ -484,6 +484,11 @@ pub fn handle(app: &mut LightcraftApp, ctx: &egui::Context) { R::RedEye => continue, _ => {} } + // in an album, Delete takes the photos out of the album only, as in Lightroom: they + // stay in the library and their folder (issue #1805) + if crate::menus::delete_in_album(app, ctx) { + continue; + } if crate::menus::confirm_delete(app) { continue; } diff --git a/crates/ui-egui/src/tests_album_remove.rs b/crates/ui-egui/src/tests_album_remove.rs new file mode 100644 index 0000000000..9c6f9d81fb --- /dev/null +++ b/crates/ui-egui/src/tests_album_remove.rs @@ -0,0 +1,134 @@ +//! In an album, the Delete key takes the selected photos out of the album only (issue #1805): they +//! stay in the library and in their folder, as in Lightroom. Photo ▸ Delete Photo still moves them +//! to Recently Deleted, from everywhere. + +use std::time::Duration; + +use lightcraft_catalog::PhotoId; +use serde_json::json; + +use crate::headless::Headless; +use crate::menubar::MenuNode; +use crate::{LightcraftApp, Services}; + +const T: Duration = Duration::from_secs(20); +const SETTLE: Duration = Duration::from_secs(120); + +fn demo() -> Headless { + let services = Services { png: None, ..Default::default() }; + let app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), services); + let mut h = Headless::new(app, [1400.0, 900.0], 1.0); + let r = h.request("ui.set", json!({"view": "photoGrid"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + h +} + +fn exec(h: &mut Headless, command: &str, params: serde_json::Value) -> serde_json::Value { + let r = h.request("engine.execute", json!({"command": command, "params": params}), T); + assert_eq!(r["ok"], true, "{command}: {r}"); + h.step(); + h.step(); + r["result"].clone() +} + +/// The Photo menu's items: (id, shortcut). +fn photo_menu(h: &Headless) -> Vec<(String, Option)> { + let bar = crate::menubar::menu_bar(&h.app); + let photo = &bar.iter().find(|(t, _)| t == "Photo").expect("Photo menu").1; + photo + .iter() + .filter_map(|n| match n { + MenuNode::Item { id, shortcut, .. } => Some((id.clone(), shortcut.clone())), + _ => None, + }) + .collect() +} + +fn garden(h: &Headless) -> u64 { + h.app.session.catalog.albums().find(|a| a.name == "Garden").map(|a| a.id.0).expect("the demo's Garden album") +} + +fn in_album(h: &Headless, album: u64, photo: PhotoId) -> bool { + h.app.session.catalog.album(lightcraft_catalog::AlbumId(album)).is_some_and(|a| a.photos.contains(&photo)) +} + +/// Given an album, when a photo is selected in it and Delete is pressed, then the photo leaves the +/// album and nothing else: it is not deleted, nothing asks, and All Photos still shows it. +#[test] +fn the_delete_key_in_an_album_removes_from_the_album_only() { + let mut h = demo(); + assert!(h.app.ui.settings.confirm_delete, "the delete confirmation is on by default"); + let album = garden(&h); + exec(&mut h, "library.source", json!({"kind": "album", "id": album})); + let photo = h.app.session.visible_cloned()[0]; + exec(&mut h, "library.select", json!({"ids": [photo.0]})); + + let r = h.request("ui.key", json!({"key": "delete"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + h.step(); + assert_eq!(h.app.ui.dialog, None, "removing from an album asks nothing (it is undoable and keeps the photo)"); + assert!(!in_album(&h, album, photo), "the photo left the album"); + assert!(!h.app.session.catalog.photo(photo).unwrap().deleted, "the photo is not deleted from the library"); + exec(&mut h, "library.source", json!({"kind": "all"})); + assert!(h.app.session.visible_cloned().contains(&photo), "All Photos still shows it"); + + // one undo brings it back into the album + exec(&mut h, "edit.undo", json!({})); + assert!(in_album(&h, album, photo)); +} + +/// In an album the Photo menu offers Remove from Album with the Delete key, and Delete Photo (no +/// key) still deletes from the library, after asking; outside albums there is no Remove from Album. +#[test] +fn the_photo_menu_in_an_album_has_remove_from_album_on_the_delete_key() { + let mut h = demo(); + let photo = h.app.session.visible_cloned()[0]; + exec(&mut h, "library.select", json!({"ids": [photo.0]})); + let items = photo_menu(&h); + assert!(!items.iter().any(|(id, _)| id == "album.removePhotos"), "not outside albums: {items:?}"); + assert!(items.iter().any(|(id, sc)| id == "photo.delete" && sc.as_deref() == Some("Delete")), "{items:?}"); + + let album = garden(&h); + exec(&mut h, "library.source", json!({"kind": "album", "id": album})); + let photo = h.app.session.visible_cloned()[0]; + exec(&mut h, "library.select", json!({"ids": [photo.0]})); + let items = photo_menu(&h); + assert!(items.iter().any(|(id, sc)| id == "album.removePhotos" && sc.as_deref() == Some("Delete")), "{items:?}"); + assert!(items.iter().any(|(id, sc)| id == "photo.delete" && sc.is_none()), "{items:?}"); + + // Photo ▸ Remove from Album (no params: the album on screen) + crate::menubar::run_item(&mut h.app, "album.removePhotos", json!({})).unwrap(); + assert!(!in_album(&h, album, photo)); + assert!(!h.app.session.catalog.photo(photo).unwrap().deleted); + + // Photo ▸ Delete Photo still deletes from the library: it asks, then OK deletes + let other = h.app.session.visible_cloned()[0]; + exec(&mut h, "library.select", json!({"ids": [other.0]})); + crate::menubar::run_item(&mut h.app, "photo.delete", json!({})).unwrap(); + assert_eq!(h.app.ui.dialog, Some(crate::state::Dialog::ConfirmDelete { count: 1 })); + let r = h.request("ui.dialog.confirm", json!({}), T); + assert_eq!(r["ok"], true, "{r}"); + assert!(h.app.session.catalog.photo(other).unwrap().deleted, "Delete Photo removes it from the library"); + exec(&mut h, "library.source", json!({"kind": "all"})); + assert!(!h.app.session.visible_cloned().contains(&other)); +} + +/// A smart album's photos follow its rules: Delete there neither removes nor deletes anything. +#[test] +fn the_delete_key_in_a_smart_album_deletes_nothing() { + let mut h = demo(); + let r = exec(&mut h, "album.createSmart", json!({"name": "Rated", "rules": {"rating": 0, "ratingOp": "atLeast"}})); + let smart = r["id"].as_u64().expect("smart album id"); + exec(&mut h, "library.source", json!({"kind": "album", "id": smart})); + let photo = *h.app.session.visible_cloned().first().expect("the smart album shows photos"); + exec(&mut h, "library.select", json!({"ids": [photo.0]})); + let r = h.request("ui.key", json!({"key": "delete"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + assert_eq!(h.app.ui.dialog, None); + assert!(!h.app.session.catalog.photo(photo).unwrap().deleted); + assert!(h.app.session.visible_cloned().contains(&photo)); + assert!(!photo_menu(&h).iter().any(|(id, _)| id == "album.removePhotos")); +} diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 7bb187811c..58f42a8905 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -108,7 +108,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-LIB-STACK | Stacks | P1 | ✅ | `cmd:stack.group`, `cmd:stack.ungroup`, `cmd:stack.toggle`, `cmd:stack.setTop`, `cmd:stack.remove`, `cmd:stack.auto`, `crates/catalog/src/stacks.rs` | grid/filmstrip count badges, expand/collapse, auto-stack by capture time; no visual-similarity auto-stack | | LR-LIB-VERSIONS | Versions | P1 | ✅ | `cmd:version.create` | see section M | | LR-LIB-DELETE | Delete / Recently Deleted | P0 | ✅ | `cmd:photo.delete`, `cmd:photo.restore`, `cmd:photo.deletePermanently` | Delete and Delete Permanently retain surviving selected photos; if none remain, select the next visible survivor (previous at the end) in the current source/filter/sort; permanent deletion removes catalog records, not files on disk; Restore and Delete Permanently in the Photo menu and the photo context menus for photos in Recently Deleted (before, the UI had no way to restore); importing a file that is in Recently Deleted again restores the photo with its edits, rating and albums (issues #298, #706: before, the import skipped it as "already in the library" while the grid showed it deleted) — the review dialog checks it, badges it "Will be restored", counts it as "in Recently Deleted" rather than "already in the library", and its choice Restore them / Import as new / Leave them starts on Restore; with Leave them such a file starts unchecked and Check All and the file-type toggles leave it out, but a click still checks it and it is restored (issue #1285: before, the click did nothing and the "In Recently Deleted" badge was drawn over the checkbox, which now sits above it); dropped files restore too ("Restored 1 photo from Recently Deleted"); a deleted photo matched by content whose file is gone is relinked to the imported file; Photo ▸ Empty Recently Deleted (`cmd:library.emptyRecentlyDeleted`, one undo step, shown while the trash is the source); import takes `onDeleted` restore (default) / skip / fresh for files in the trash (`cmd:library.import`; the report lists `restored`, and with skip flags the duplicate `existingDeleted`); browsing a folder in Local (`cmd:library.browse`), Auto Import and a Lightroom catalog import never restore a deleted photo; no confirmation dialog, no auto-purge after N days (needs a deleted-at date in the catalog); Delete asks first ("Move this photo to Recently Deleted?") from the key, the Photo menu and the photo context menu, on by default and switched off in Settings ▸ General (#1239: before, it was off by default and the context menu never asked); no auto-purge after N days (needs a deleted-at date in the catalog) | -| LR-LIB-REMOVE-ALBUM | Remove from album | P0 | ✅ | `cmd:album.removePhotos` | | +| LR-LIB-REMOVE-ALBUM | Remove from album | P0 | ✅ | `cmd:album.removePhotos`, `crates/engine/src/tests_album_remove.rs`, `crates/ui-egui/src/tests_album_remove.rs` | changes the album only: the photos stay in the library, their library folder and their folder on disk (issue #1805); `id` defaults to the album on screen; returns `{removed, album}`. In a normal album ⌫ removes from the album (as in Lightroom; no confirmation, one undo step), in a smart album it changes nothing and says to edit its rules | | LR-LIB-DUPLICATE | Duplicate a photo | P2 | ✅ | `cmd:photo.duplicate` | Photo ▸ Duplicate: a real `-copy` file with the same settings, metadata and albums (virtual copies share the file) | | LR-LIB-RENAME | Batch rename | P1 | ✅ | `cmd:photo.rename`, `cmd:photo.renamePreview`, `cmd:photo.renameTokens`, `cmd:dialog.rename`, `crates/engine/src/rename.rs` (`TOKENS`) | template tokens {name} {num} {seq:N} {date:%Y%m%d} {folder} {camera} {lens} {iso} {rating} {title} {creator} {ext} (shared with export naming and import rename); a Tags picker beside every template field (Rename Photos, import Rename, export File name) lists each tag with meaning + live example, the date directives and the rules, and inserts the clicked tag at the text cursor; unknown tags are flagged; preview; renames files on disk with their XMP sidecars (a stem sidecar another file still shares, e.g. raw + JPEG, is copied rather than taken), never overwriting (-1, -2… suffixes; a change of letter case only is checked by file identity, so on case-sensitive volumes `img_1.jpg` next to `IMG_1.JPG` is a collision), rolls back on failure (a file that can't be moved back is reported old → new and the library follows it as an undoable partial rename; the same for undo/redo); undo/redo move the files; virtual copies follow | | LR-LIB-CAPTURETIME | Edit capture time | P1 | ✅ | `cmd:photo.setCaptureTime`, `cmd:dialog.captureTime`, `crates/catalog/src/dates.rs` | set (the other selected photos shift by the same amount, or `each`), shift by days/hours/minutes, time-zone shift; one undo step, journaled; Info panel button; no “revert to original capture time”. Impossible calendar dates cannot anchor relative shifts; rejection preserves metadata, selection, Undo and Redo. | @@ -548,7 +548,7 @@ visible menu bar yet: the menu model is only exposed through the control channel | MENU-VIEW-SIDEBARSECTIONS | Sidebar sections | P2 | ✅ | `cmd:view.sidebarSection`, `cmd:view.sidebarReset` | View ▸ Sidebar Sections ▸ a checkable item per left-sidebar section, then Reset Sidebar Sections (see LR-BEHAV-SIDEBAR-HIDE) | | MENU-VIEW-HDR | HDR display options | P2 | ⬜ | | | | MENU-PHOTO-ADDTOALBUM | Add to album | P0 | ✅ | `cmd:album.addPhotos` | off for a Local-only selection, with an Add to My Photos First hint item (issue #684) | -| MENU-PHOTO-REMOVEFROMALBUM | Remove from album | P0 | ✅ | `cmd:album.removePhotos` | | +| MENU-PHOTO-REMOVEFROMALBUM | Remove from album | P0 | ✅ | `cmd:album.removePhotos` | Photo ▸ Remove from Album, shown in a normal album only, carries the ⌫ shortcut there (Delete Photo then has none) (issue #1805) | | MENU-PHOTO-RATE | Rate submenu | P0 | ✅ | `cmd:photo.rate` | | | MENU-PHOTO-FLAG | Flag submenu | P0 | ✅ | `cmd:photo.flag` | | | MENU-PHOTO-LABEL | Colour label submenu | P1 | ✅ | `cmd:photo.label`, `cmd:dialog.labelNames` | shows custom names; Edit Label Names… | @@ -634,7 +634,7 @@ From `06-shortcuts.md` part 1. Evidence is our binding; conflicts are explained | KEY-QUIT | Quit — ⌘Q | P0 | ✅ | `cmd:app.quit` | ⌘Q (Ctrl+Q off macOS) | | KEY-CREATEVERSION | Create version — ⇧M | P1 | ✅ | `cmd:version.create` | secondary binding (primary ⌘⇧S) | | KEY-CLOSEWINDOW | Close window — ⌘W | P1 | ✅ | `apps/lightcraft/src/native_menu.rs` | native File ▸ Close Window | -| KEY-DELETE | Delete photo — ⌫ | P0 | ✅ | `cmd:photo.delete` | | +| KEY-DELETE | Delete photo — ⌫ | P0 | ✅ | `cmd:photo.delete`, `cmd:album.removePhotos` | in a normal album ⌫ removes from the album only (`album.removePhotos`, issue #1805); in a smart album it does nothing; elsewhere it deletes (to Recently Deleted) | | KEY-ADDPHOTOS | Import photos — ⇧⌘I | P0 | ✅ | `cmd:file.addPhotos` | | | KEY-VERSIONS | Versions panel — ⇧V | P1 | ✅ | `cmd:panel.versions` | | | KEY-SECTIONS | Expand/collapse edit sections — ⌘1…⌘6 | P1 | ✅ | `cmd:section.light`, `cmd:section.color`, `cmd:section.effects`, `cmd:section.detail`, `cmd:section.optics` | ⌘1–⌘5 (Light, Color, Effects, Detail, Optics) as observed; our Edit panel has no Lens Blur / Geometry section for ⌘6 / ⌘7; Zoom 100% moved to ⌥⌘0 | diff --git a/docs/whats-new.md b/docs/whats-new.md index 1bf44c3f4e..56af86c48f 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -1,5 +1,14 @@ # What's new in LightCraft +## 2026-10-15 + +### Delete in an album removes from the album only +- In an album, the Delete key now takes the selected photos out of the album, as in Lightroom. It used to move them + to Recently Deleted, so they also left their folder's view (issue #1805). They stay in the library and in their + folder; Edit ▸ Undo puts them back in the album. Photo ▸ Remove from Album does the same and now shows the key. + To delete photos from the library from an album, use Photo ▸ Delete Photo or right-click ▸ Delete Photo. In a smart + album, Delete changes nothing: its rules decide what it shows. + ## 2026-10-14 ### Hard-edged Heal and Clone spots