From 5c5370fc9bfabf810dce64a65d8e74f2428b1bce Mon Sep 17 00:00:00 2001 From: Jason Baldwin <33934381+bitshiftza@users.noreply.github.com> Date: Thu, 15 Oct 2026 00:54:17 +0200 Subject: [PATCH] LR-LIB-REMOVE-ALBUM: removing a photo from an album keeps it in its folder (fixes #1805) In an album the Delete key ran photo.delete, which moved the photo to Recently Deleted, so it also left its folder views. It now removes the selection from the album only (album.removePhotos), as in Lightroom; in a smart album it changes nothing. Photo > Remove from Album shows in normal albums with the Delete key; Photo > Delete Photo still deletes from the library. album.removePhotos defaults to the album on screen and returns {removed, album}. --- crates/engine/src/cmd/library.rs | 41 +++++-- crates/engine/src/lib.rs | 2 + crates/engine/src/tests_album_remove.rs | 105 ++++++++++++++++++ crates/ui-egui/locales/de.json | 3 +- crates/ui-egui/locales/es.json | 3 +- crates/ui-egui/locales/fr.json | 3 +- crates/ui-egui/locales/ja.json | 3 +- crates/ui-egui/locales/pt-br.json | 3 +- crates/ui-egui/locales/ru.json | 3 +- crates/ui-egui/locales/uk.json | 3 +- crates/ui-egui/locales/zh-hans.json | 3 +- crates/ui-egui/locales/zh-hant.json | 3 +- crates/ui-egui/src/lib.rs | 2 + crates/ui-egui/src/menubar.rs | 18 +++ crates/ui-egui/src/menus.rs | 26 +++++ crates/ui-egui/src/shortcuts.rs | 5 + crates/ui-egui/src/tests_album_remove.rs | 134 +++++++++++++++++++++++ docs/parity-checklist.md | 6 +- docs/whats-new.md | 9 ++ 19 files changed, 353 insertions(+), 22 deletions(-) create mode 100644 crates/engine/src/tests_album_remove.rs create mode 100644 crates/ui-egui/src/tests_album_remove.rs 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