mirror of
https://github.com/storytold/lightcraft.git
synced 2026-10-24 10:09:01 -06:00
LR-IMP-LOCAL: browsing a Local folder keeps or picks the active photo (#1447)
library.browse (engine and the UI's background read) switched the source to the folder without touching the selection, so the previous view's photo stayed active: Photo Grid marked nothing, and Detail, the histogram and the info showed a photo the folder doesn't hold. Browsing now follows library.source's rule through a shared Session::keep_active_in_view: the active photo stays when the folder shows it, else the folder's first photo becomes active (also as the background read's batches arrive).
This commit is contained in:
@@ -45,6 +45,7 @@ fn browse(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
stamp_browsed(s, &dir_s, &files);
|
||||
s.browse = Some(Browse { path: dir_s.clone(), subfolders });
|
||||
s.source = LibrarySource::Folder;
|
||||
s.keep_active_in_view();
|
||||
let shown = s.visible().len();
|
||||
Ok(json!({"path": dir_s, "subfolders": subfolders, "photos": shown, "new": report.imported.len(), "failed": report.failed.len()}))
|
||||
}
|
||||
|
||||
@@ -362,11 +362,8 @@ pub fn specs() -> Vec<CommandSpec> {
|
||||
}
|
||||
other => return Err(bad("library.source", format!("unknown source `{other}`"))),
|
||||
};
|
||||
let vis = s.visible_cloned();
|
||||
if s.selection.active.is_none_or(|a| !vis.contains(&a)) {
|
||||
s.selection = vis.first().map(|f| Selection::single(*f)).unwrap_or_default();
|
||||
}
|
||||
Ok(json!({"count": vis.len()}))
|
||||
s.keep_active_in_view();
|
||||
Ok(json!({"count": s.visible().len()}))
|
||||
}
|
||||
),
|
||||
cmd!(
|
||||
|
||||
@@ -955,6 +955,21 @@ impl Session {
|
||||
self.visible().to_vec()
|
||||
}
|
||||
|
||||
/// After the source changes, the active photo follows the rule every source shares: it stays
|
||||
/// when the view shows it, else the view's first photo becomes the active (and only selected)
|
||||
/// one, or nothing when the view is empty. A view never keeps an active photo it doesn't show
|
||||
/// (issue #1447: a Local folder kept a library photo active, so Detail and the info showed a
|
||||
/// photo the grid didn't).
|
||||
pub fn keep_active_in_view(&mut self) {
|
||||
let active = self.selection.active;
|
||||
let vis = self.visible();
|
||||
if active.is_some_and(|a| vis.contains(&a)) {
|
||||
return;
|
||||
}
|
||||
let first = vis.first().copied();
|
||||
self.selection = first.map(Selection::single).unwrap_or_default();
|
||||
}
|
||||
|
||||
/// The visible photos without copying them, with their generation: equal generations mean
|
||||
/// the very same list (same catalog revision, source, filter, sort), so a frontend can cache
|
||||
/// whatever it derives from the list under that number.
|
||||
|
||||
@@ -917,6 +917,26 @@ fn missing_photos_skip_local_browse_records() {
|
||||
assert_eq!(s.visible_cloned().len(), 4, "the view lists what the count counts");
|
||||
}
|
||||
|
||||
/// `library.browse` follows `library.source`'s rule for the active photo (issue #1447): a library
|
||||
/// photo the folder doesn't show gives way to the folder's first photo; one it shows stays.
|
||||
#[test]
|
||||
fn browsing_a_folder_keeps_or_picks_the_active_photo() {
|
||||
let dir = temp_dir("browse-active");
|
||||
write_png(&dir.join("a.png"), 1);
|
||||
write_png(&dir.join("b.png"), 2);
|
||||
let mut s = Session::with_demo().with_fs();
|
||||
let library = s.visible_cloned()[0];
|
||||
s.execute("library.select", &json!({"ids": [library.0]})).unwrap();
|
||||
s.execute("library.browse", &json!({"path": dir.to_string_lossy()})).unwrap();
|
||||
let vis = s.visible_cloned();
|
||||
assert_eq!(vis.len(), 2);
|
||||
assert_eq!(s.selection.active, Some(vis[0]), "the folder's first photo, not {library:?}");
|
||||
s.execute("library.select", &json!({"ids": [vis[1].0]})).unwrap();
|
||||
s.execute("library.browse", &json!({"path": dir.to_string_lossy()})).unwrap();
|
||||
assert_eq!(s.selection.active, Some(vis[1]), "a photo the folder shows stays active");
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
/// Import a file that is in Recently Deleted (issues #298, #706): by default the photo comes
|
||||
/// back with its edits; `onDeleted: skip` leaves it there but the report says it is in the trash;
|
||||
/// `onDeleted: fresh` replaces the trashed record with a new one.
|
||||
|
||||
@@ -484,6 +484,7 @@ pub fn browse(app: &mut LightcraftApp, path: &str, subfolders: Option<bool>) ->
|
||||
if running && app.session.browse.as_ref().is_some_and(|b| b.path == dir_s && b.subfolders == subfolders) {
|
||||
// already reading this folder: clicking it again must not restart the progress
|
||||
app.session.source = lightcraft_engine::LibrarySource::Folder;
|
||||
app.session.keep_active_in_view();
|
||||
return Ok(json!({"path": dir_s, "subfolders": subfolders, "scanning": true}));
|
||||
}
|
||||
if let Some(t) = app.scan.take() {
|
||||
@@ -492,6 +493,8 @@ pub fn browse(app: &mut LightcraftApp, path: &str, subfolders: Option<bool>) ->
|
||||
app.import = None;
|
||||
app.session.browse = Some(lightcraft_engine::Browse { path: dir_s.clone(), subfolders });
|
||||
app.session.source = lightcraft_engine::LibrarySource::Folder;
|
||||
// the photos the library already knows from this folder show at once: keep or pick the active one
|
||||
app.session.keep_active_in_view();
|
||||
let (input, _) = ScanInput::new(&mut app.session, std::slice::from_ref(&dir_s));
|
||||
let progress = std::sync::Arc::new(ScanProgress::default());
|
||||
let (tx, rx) = std::sync::mpsc::channel();
|
||||
@@ -703,6 +706,10 @@ pub fn tick(app: &mut LightcraftApp, ctx: &egui::Context) {
|
||||
if let Some(sel) = &task.keep_selection {
|
||||
app.session.selection = sel.clone();
|
||||
}
|
||||
// a Local folder still on screen: its first photo becomes active once there is one (#1447)
|
||||
if task.browse && app.session.source == lightcraft_engine::LibrarySource::Folder {
|
||||
app.session.keep_active_in_view();
|
||||
}
|
||||
}
|
||||
task.left = Some((app.session.selection.clone(), app.session.source));
|
||||
if let Some(run) = &task.run {
|
||||
|
||||
@@ -49,6 +49,8 @@ mod tests_appearance;
|
||||
#[cfg(test)]
|
||||
mod tests_before_after;
|
||||
#[cfg(test)]
|
||||
mod tests_browse_active;
|
||||
#[cfg(test)]
|
||||
mod tests_compare;
|
||||
#[cfg(test)]
|
||||
mod tests_copy_paste_dialogs;
|
||||
|
||||
@@ -0,0 +1,95 @@
|
||||
//! Showing a Local folder picks the active photo the way every other source does (issue #1447):
|
||||
//! the current one when the folder shows it, else the folder's first photo, so Photo Grid marks
|
||||
//! it, the info and histogram describe it, and Detail opens on it instead of a photo from
|
||||
//! another source.
|
||||
|
||||
use std::time::Duration;
|
||||
|
||||
use lightcraft_catalog::PhotoId;
|
||||
use serde_json::{Value, json};
|
||||
|
||||
use crate::LightcraftApp;
|
||||
use crate::headless::Headless;
|
||||
use crate::state::ViewMode;
|
||||
|
||||
const T: Duration = Duration::from_secs(10);
|
||||
const SETTLE: Duration = Duration::from_secs(120);
|
||||
|
||||
fn demo() -> Headless {
|
||||
let services = crate::Services { png: None, ..Default::default() };
|
||||
let mut app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), services);
|
||||
app.ui.view = ViewMode::PhotoGrid;
|
||||
Headless::new(app, [1300.0, 900.0], 1.0)
|
||||
}
|
||||
|
||||
fn exec(h: &mut Headless, command: &str, params: Value) -> Value {
|
||||
let r = h.request("engine.execute", json!({"command": command, "params": params}), T);
|
||||
assert_eq!(r["ok"], true, "{command} {params}: {r}");
|
||||
h.settle(SETTLE);
|
||||
r
|
||||
}
|
||||
|
||||
/// A folder of `n` small PNGs (different bytes each) in the temporary folder.
|
||||
fn folder(name: &str, n: u8) -> std::path::PathBuf {
|
||||
let dir = std::env::temp_dir().join(format!("lc-ui-{name}-{}", std::process::id()));
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
std::fs::create_dir_all(&dir).unwrap();
|
||||
let o = lightcraft_engine::export::ExportOptions { format: lightcraft_engine::export::ExportFormat::Png, ..Default::default() };
|
||||
for i in 0..n {
|
||||
let img = lightcraft_raster::Rgba8::from_fn(24, 16, |x, y| [(x * 9) as u8, (y * 12) as u8, 40 * i, 255]);
|
||||
std::fs::write(dir.join(format!("p{i}.png")), lightcraft_engine::export::encode_image(&img, &o).unwrap()).unwrap();
|
||||
}
|
||||
dir
|
||||
}
|
||||
|
||||
fn active(h: &Headless) -> Option<PhotoId> {
|
||||
h.app.session.selection.active
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn browsing_a_local_folder_makes_its_first_photo_active() {
|
||||
let mut h = demo();
|
||||
h.settle(SETTLE);
|
||||
let dir = folder("browse-active", 3);
|
||||
// a library photo is active: the folder doesn't show it
|
||||
let before = h.app.session.visible_cloned()[0];
|
||||
exec(&mut h, "library.select", json!({"ids": [before.0]}));
|
||||
exec(&mut h, "library.browse", json!({"path": dir.to_string_lossy()}));
|
||||
let vis = h.app.session.visible_cloned();
|
||||
assert_eq!(vis.len(), 3);
|
||||
assert_eq!(active(&h), Some(vis[0]), "the folder's first photo is active, not a photo the grid doesn't show");
|
||||
assert_eq!(h.app.session.selection.ids, vec![vis[0]], "and selected");
|
||||
// the grid marks it, and Detail opens on it
|
||||
assert!(h.app.widgets.iter().any(|(w, _)| *w == format!("thumb:{}", vis[0].0)));
|
||||
h.request("ui.set", json!({"view": "detail"}), T);
|
||||
h.settle(SETTLE);
|
||||
assert_eq!(active(&h), Some(vis[0]));
|
||||
|
||||
// the folder shows the current photo again: it stays active
|
||||
h.request("ui.set", json!({"view": "photoGrid"}), T);
|
||||
exec(&mut h, "library.select", json!({"ids": [vis[2].0]}));
|
||||
exec(&mut h, "library.source", json!({"kind": "all"}));
|
||||
assert_ne!(active(&h), Some(vis[2]), "All Photos doesn't list Local photos");
|
||||
exec(&mut h, "library.select", json!({"ids": [vis[2].0]}));
|
||||
exec(&mut h, "library.browse", json!({"path": dir.to_string_lossy()}));
|
||||
assert_eq!(active(&h), Some(vis[2]), "a photo the folder shows stays active");
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
/// An album behaves the same (it already did: kept here as the rule's reference).
|
||||
#[test]
|
||||
fn choosing_an_album_keeps_or_picks_the_active_photo() {
|
||||
let mut h = demo();
|
||||
h.settle(SETTLE);
|
||||
let album = h.app.session.catalog.albums().find(|a| !a.folder && !a.is_smart() && a.photos.len() > 1).map(|a| (a.id, a.photos.clone()));
|
||||
let (album, photos) = album.expect("the demo library has an album");
|
||||
let outside = h.app.session.visible_cloned().into_iter().find(|p| !photos.contains(p)).expect("a photo outside the album");
|
||||
exec(&mut h, "library.select", json!({"ids": [outside.0]}));
|
||||
exec(&mut h, "library.source", json!({"kind": "album", "id": album.0}));
|
||||
let vis = h.app.session.visible_cloned();
|
||||
assert_eq!(active(&h), vis.first().copied());
|
||||
exec(&mut h, "library.select", json!({"ids": [vis[1].0]}));
|
||||
exec(&mut h, "library.source", json!({"kind": "all"}));
|
||||
exec(&mut h, "library.source", json!({"kind": "album", "id": album.0}));
|
||||
assert_eq!(active(&h), Some(vis[1]));
|
||||
}
|
||||
File diff suppressed because one or more lines are too long
@@ -1,5 +1,12 @@
|
||||
# What's new in LightCraft
|
||||
|
||||
## 2026-10-15
|
||||
|
||||
### Local folders select a photo
|
||||
- Showing a Local folder now makes its first photo the active one, as albums and the other sources do (issue
|
||||
#1447), or keeps the photo you had when the folder holds it. Before, the photo from the previous view stayed active:
|
||||
Photo Grid marked nothing, and Detail, the histogram and the photo info showed a photo that isn't in the folder.
|
||||
|
||||
## 2026-10-14
|
||||
|
||||
### Hard-edged Heal and Clone spots
|
||||
|
||||
Reference in New Issue
Block a user