diff --git a/crates/engine/src/cmd/library.rs b/crates/engine/src/cmd/library.rs --- a/crates/engine/src/cmd/library.rs +++ b/crates/engine/src/cmd/library.rs @@ -36,3 +36,7 @@ mod enable_filters_tests; #[cfg(test)] #[path = "library/tests_source_album_filter.rs"] mod source_album_filter_tests; + +#[cfg(test)] +#[path = "library/tests_select.rs"] +mod tests_select; diff --git a/crates/engine/src/cmd/library/selection.rs b/crates/engine/src/cmd/library/selection.rs --- a/crates/engine/src/cmd/library/selection.rs +++ b/crates/engine/src/cmd/library/selection.rs @@ -238,7 +238,12 @@ pub(super) fn select_by(s: &mut Session, p: &Value) -> Result { } let active = s.selection.active.filter(|a| ids.contains(a)).or(ids.first().copied()); let n = ids.len(); - s.selection = Selection { ids, active }; + let selection = Selection { ids, active }; + if selection != s.selection { + // chosen by flag, rating or label: no longer "every photo of the view" + s.all_selected = None; + } + s.selection = selection; Ok(json!({"selected": n})) } diff --git a/crates/engine/src/cmd/library/specs.rs b/crates/engine/src/cmd/library/specs.rs --- a/crates/engine/src/cmd/library/specs.rs +++ b/crates/engine/src/cmd/library/specs.rs @@ -329,7 +329,7 @@ pub fn specs() -> Vec { "Select Photos", [], None, - "{ids: [id], active?: id, mode?: replace|add|toggle|range} — every id must be a photo of the library", + "{ids: [id], active?: id, mode?: replace|add|toggle|range|click} — every id must be a photo of the library. As in Lightroom: toggle (⌘/Ctrl-click) and range (Shift-click, from the active photo) keep the active photo; click (a plain click) on a photo of a selection of several makes it the active one and keeps the selection, elsewhere it selects that photo alone", always, |s, p| { let ids = crate::cmd::photo_ids(p, "library.select")?.unwrap_or_default(); @@ -342,6 +342,7 @@ pub fn specs() -> Vec { return Err(bad("library.select", format!("no such photo {}", id.0))); } s.end_interaction()?; + let before = s.selection.clone(); match mode { "replace" => { s.selection = Selection { ids: ids.clone(), active: active.or(ids.first().copied()) }; @@ -355,6 +356,11 @@ pub fn specs() -> Vec { } } "toggle" => ids.into_iter().for_each(|id| s.selection.toggle(id)), + "click" => { + if let Some(id) = ids.first() { + s.selection.click(*id); + } + } "range" => { let vis = s.visible_cloned(); if let Some(id) = ids.first() { @@ -363,16 +369,30 @@ pub fn specs() -> Vec { } other => return Err(bad("library.select", format!("unknown mode `{other}`"))), } + // a selection changed by hand is no longer "every photo of the view" (a click that only makes another + // selected photo the active one keeps it) + if s.selection.ids != before.ids { + s.all_selected = None; + } s.active_mask = None; s.active_spot = None; Ok(json!({"selected": s.selection.ids.len()})) } ), - cmd!("library.selectAll", "Select All", ["Edit"], Some("Cmd+A"), "{}", always, |s, _| { - let vis = s.visible_cloned(); - s.selection = Selection { active: s.selection.active.filter(|a| vis.contains(a)).or(vis.first().copied()), ids: vis }; - Ok(json!({"selected": s.selection.ids.len()})) - }), + cmd!( + "library.selectAll", + "Select All", + ["Edit"], + Some("Cmd+A"), + "{} — every photo of the view; photos that join the view later (an import still running) are selected too, until the selection or the view changes", + always, + |s, _| { + let vis = s.visible_cloned(); + s.selection = Selection { active: s.selection.active.filter(|a| vis.contains(a)).or(vis.first().copied()), ids: vis }; + s.all_selected = s.visible_view.clone(); + Ok(json!({"selected": s.selection.ids.len()})) + } + ), cmd!(query "library.devices", "Cameras and Cards", [], None, "{} → [{name, path (its DCIM folder), root}] — mounted volumes with a DCIM folder", always, |_, _| { Ok(serde_json::to_value(crate::devices::devices_now()).unwrap_or_default()) }), @@ -422,6 +442,7 @@ pub fn specs() -> Vec { cmd!("library.selectNone", "Deselect All", ["Edit"], Some("Cmd+Shift+A"), "{}", always, |s, _| { let a = s.selection.active; s.selection = Selection { ids: a.into_iter().collect(), active: a }; + s.all_selected = None; ok() }), cmd!("library.next", "Next Photo", [], Some("Right"), "{}", always, |s, _| step(s, 1)), diff --git a/crates/engine/src/cmd/library/tests_select.rs b/crates/engine/src/cmd/library/tests_select.rs new file mode 100644 --- /dev/null +++ b/crates/engine/src/cmd/library/tests_select.rs @@ -0,0 +1,166 @@ +//! Selecting as in Lightroom: Shift / ⌘-click keep the active photo, a click inside the selection +//! keeps it, and Select All keeps up with a view that grows. +use lightcraft_catalog::{Op, Photo, PhotoId, Source}; +use serde_json::json; + +use crate::{Result, Session}; + +fn add(s: &mut Session, id: u64) -> Result<()> { + let photo = Photo::new( + PhotoId(id), + Source::File { path: format!("/lightcraft-select/{id:03}.jpg") }, + &format!("{id:03}.jpg"), + "JPEG", + 40, + 30, + "2026-10-22", + ); + s.catalog.apply(Op::AddPhoto { photo: Box::new(photo) })?; + Ok(()) +} + +fn session(n: u64) -> Result { + let mut s = Session::new(); + for id in 1..=n { + add(&mut s, id)?; + } + s.execute("library.sort", &json!({"key": "fileName", "ascending": true}))?; + Ok(s) +} + +#[test] +fn shift_and_ctrl_clicks_keep_the_active_photo_and_a_click_inside_keeps_the_selection() -> Result<()> { + let mut s = session(10)?; + s.execute("library.select", &json!({"ids": [3], "mode": "click"}))?; + s.execute("library.select", &json!({"ids": [7], "mode": "range"}))?; + assert_eq!(s.selection.active, Some(PhotoId(3)), "Shift-click: the loupe stays on 3"); + assert_eq!(s.selection.ids.len(), 5); + s.execute("library.select", &json!({"ids": [9], "mode": "toggle"}))?; + assert_eq!(s.selection.active, Some(PhotoId(3)), "⌘-click: the loupe stays on 3"); + assert_eq!(s.selection.ids.len(), 6); + s.execute("library.select", &json!({"ids": [5], "mode": "click"}))?; + assert_eq!((s.selection.active, s.selection.ids.len()), (Some(PhotoId(5)), 6), "a click inside: 5 active, the set stays"); + s.execute("library.select", &json!({"ids": [10], "mode": "click"}))?; + assert_eq!((s.selection.active, s.selection.ids.len()), (Some(PhotoId(10)), 1), "a click outside replaces it"); + Ok(()) +} + +#[test] +fn select_all_takes_in_the_photos_that_join_the_view() -> Result<()> { + let mut s = session(5)?; + s.execute("library.selectAll", &json!({}))?; + assert_eq!(s.selection.ids.len(), 5); + // an import still running adds photos to the view + for id in 6..=9 { + add(&mut s, id)?; + } + assert_eq!(s.visible().len(), 9); + assert_eq!(s.selection.ids.len(), 9, "every photo of the view stays selected"); + // a selection changed by hand is the user's: new photos don't join it + s.execute("library.select", &json!({"ids": [2], "mode": "toggle"}))?; + add(&mut s, 10)?; + assert_eq!(s.visible().len(), 10); + assert_eq!(s.selection.ids.len(), 8); + // nor do they in another view + s.execute("library.selectAll", &json!({}))?; + s.execute("library.filter", &json!({"text": "00"}))?; + add(&mut s, 11)?; + let _ = s.visible(); + assert_eq!(s.selection.ids.len(), 10); + Ok(()) +} + +#[test] +fn select_all_ends_with_another_view_and_does_not_come_back_with_its_view() -> Result<()> { + let mut s = session(2)?; + s.execute("library.selectAll", &json!({}))?; + // narrowed by hand to one photo, then a view that shows only that photo + s.execute("library.select", &json!({"ids": [1]}))?; + s.execute("library.filter", &json!({"text": "001"}))?; + assert_eq!(s.visible().len(), 1); + assert_eq!(s.selection.ids, vec![PhotoId(1)]); + // back to the first view: photo 2 stays deselected (the old Select All is over) + s.execute("library.filter", &json!({"text": ""}))?; + assert_eq!(s.visible().len(), 2); + assert_eq!(s.selection.ids, vec![PhotoId(1)]); + Ok(()) +} + +#[test] +fn a_click_that_changes_nothing_keeps_select_all_and_select_by_ends_it() -> Result<()> { + let mut s = session(3)?; + s.execute("library.selectAll", &json!({}))?; + let active = s.selection.active.map(|a| a.0); + // a plain click on the active photo of the selection changes nothing: still "every photo of the view" + s.execute("library.select", &json!({"ids": [active], "mode": "click"}))?; + add(&mut s, 4)?; + assert_eq!(s.visible().len(), 4); + assert_eq!(s.selection.ids.len(), 4, "the new photo joins Select All"); + // narrowed by flag, then every photo picked again by flag: chosen by hand, new photos don't join + s.execute("library.selectBy", &json!({"flag": "pick"}))?; + s.execute("library.selectBy", &json!({"flag": "none"}))?; + assert_eq!(s.selection.ids.len(), 4); + add(&mut s, 5)?; + assert_eq!(s.visible().len(), 5); + assert_eq!(s.selection.ids.len(), 4, "Select All ended with the selection by flag"); + Ok(()) +} + +#[test] +fn a_click_that_only_moves_the_active_photo_keeps_select_all() -> Result<()> { + let mut s = session(3)?; + s.execute("library.selectAll", &json!({}))?; + let other = s.selection.ids.iter().find(|id| Some(**id) != s.selection.active).map(|id| id.0); + // a plain click on another photo of the selection (grid or filmstrip): it becomes active, the set stays + s.execute("library.select", &json!({"ids": [other], "mode": "click"}))?; + assert_eq!(s.selection.active.map(|a| a.0), other); + add(&mut s, 4)?; + assert_eq!(s.visible().len(), 4); + assert_eq!(s.selection.ids.len(), 4, "the new photo joins Select All"); + Ok(()) +} + +/// Missing Photos fills in as the background checks find files gone: a photo found gone after Select All joins the +/// selection (the checks are not another view). +#[test] +fn select_all_in_missing_photos_takes_in_a_photo_the_checks_find_gone() -> Result<()> { + use std::sync::Arc; + use std::time::{Duration, Instant}; + let mut s = session(2)?; + // every file is gone (none of these paths exist); answered by a worker, as in the app + s.media.availability.set_probe(Arc::new(|_: &str| false)); + s.media.availability.run_in_background(Arc::new(|| {})); + s.source = crate::LibrarySource::Missing; + let settle = |s: &mut Session, n: usize| { + let until = Instant::now() + Duration::from_secs(10); + while s.visible().len() < n && Instant::now() < until { + std::thread::sleep(Duration::from_millis(5)); + } + s.visible().len() + }; + assert_eq!(settle(&mut s, 2), 2); + s.execute("library.selectAll", &json!({}))?; + add(&mut s, 3)?; + assert_eq!(settle(&mut s, 3), 3, "the check finds photo 3 gone"); + assert_eq!(s.selection.ids.len(), 3, "photo 3 joins Select All"); + Ok(()) +} + +/// An arrow key after Select All moves the selection on: Select All is over, and a photo that leaves the view and +/// comes back is not selected again (CodeRabbit on #3030). +#[test] +fn select_all_ends_when_an_arrow_key_moves_the_selection() -> Result<()> { + let mut s = session(2)?; + s.execute("library.filter", &json!({"flag": "none"}))?; + s.execute("library.selectAll", &json!({}))?; + s.execute("library.select", &json!({"ids": [1], "active": 1, "mode": "click"}))?; + s.execute("library.next", &json!({}))?; + assert_eq!(s.selection.ids, vec![PhotoId(2)]); + // photo 1 leaves the view (rejected), then comes back + s.execute("photo.flag", &json!({"flag": "reject", "ids": [1]}))?; + assert_eq!(s.visible().len(), 1); + s.execute("photo.flag", &json!({"flag": "none", "ids": [1]}))?; + assert_eq!(s.visible().len(), 2); + assert_eq!(s.selection.ids, vec![PhotoId(2)], "photo 1 is not selected again"); + Ok(()) +} diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -240,6 +240,10 @@ pub struct Session { /// Shared, so frontends can hold it across frames without copying ([`Session::visible_shared`]). visible: Arc<[PhotoId]>, visible_key: Option<(u64, String)>, + /// The view [`Session::visible`] last worked out, told apart as a held view is: by source, filter, sort and + /// smart-album rules, not by the background checks of Missing Photos that fill it in. What Select All + /// (`Session::all_selected`) is tied to. + pub(crate) visible_view: Option, /// The minute used for a cached view whose rules depend on the session clock. visible_clock_minute: Option, /// A view holds the photos it shows until it is refreshed (`library.holdView`): an edit that makes a photo stop @@ -250,6 +254,11 @@ pub struct Session { /// place in the order ([`Session::visible`]). It is also what a held view holds ([`Session::hold_view`]). /// Another source, filter or sort is another view, and so is a smart album whose rules were changed. base: Option<(String, u64, Vec)>, + /// Select All (`library.selectAll`) was the last change to the selection, in the view named here: while the + /// selection is still exactly what the view shows, photos that join the view (an import or the finishing + /// queue still adding them) join the selection too (⌘A during an import had selected 312 of 569). Any other + /// change to the selection, and any other view, ends it. + pub(crate) all_selected: Option, /// How many times the view followed an edit from the list it had, and how many times it was worked out from /// every photo (`ui.inspect` → `perf`): what tells a slow library from a slow edit. pub view_followed: u64, @@ -456,9 +465,11 @@ impl Session { selection: Selection::default(), visible: Vec::new().into(), visible_key: None, + visible_view: None, visible_clock_minute: None, hold_view: false, base: None, + all_selected: None, view_followed: 0, view_worked_out: 0, #[cfg(test)] @@ -1142,7 +1153,7 @@ impl Session { }; // what the view shows now is what the next edit is followed from, and what it holds until it is // refreshed (taken before stacks are arranged, so a photo folded into a stack is held with it) - self.base = (!whole || holds).then(|| (view, self.catalog.revision, visible.clone())); + self.base = (!whole || holds).then(|| (view.clone(), self.catalog.revision, visible.clone())); if visible.is_empty() && self.source == LibrarySource::LibraryFolder && !self.folder_holds_photos() { // the shown folder lost its last photo (deleted, moved, removed): everything, not // an empty grid under the name of a folder that is gone from the sidebar @@ -1189,9 +1200,23 @@ impl Session { if self.source != LibrarySource::RecentlyDeleted { visible = self.catalog.arrange_stacks(&visible); } + // another view ends Select All: coming back to its view later must not select what was deselected meanwhile + // (the view as a held view tells it apart: a photo the Missing Photos checks add keeps Select All) + // and so does a selection that is no longer every photo the view showed (an arrow key moved it on) + if self.all_selected.as_deref().is_some_and(|k| k != view) || self.selection.ids[..] != self.visible[..] { + self.all_selected = None; + } + if self.all_selected.as_deref() == Some(view.as_str()) && self.selection.ids[..] == self.visible[..] && visible[..] != self.visible[..] { + // still "every photo of this view": the photos that came in are selected too + self.selection.ids = visible.clone(); + if self.selection.active.is_none_or(|a| !visible.contains(&a)) { + self.selection.active = visible.first().copied(); + } + } self.visible = visible.into(); self.visible_gen = VISIBLE_GEN.fetch_add(1, std::sync::atomic::Ordering::Relaxed); self.visible_key = Some(key); + self.visible_view = Some(view); self.visible_clock_minute = clocked.then(|| clock_minute(&now).to_owned()); } &self.visible diff --git a/crates/engine/src/view.rs b/crates/engine/src/view.rs --- a/crates/engine/src/view.rs +++ b/crates/engine/src/view.rs @@ -105,18 +105,25 @@ impl Selection { pub fn contains(&self, id: PhotoId) -> bool { self.ids.contains(&id) } + /// ⌘/Ctrl-click, as in Lightroom: `id` joins or leaves the selection and the active photo (the + /// one the loupe shows) stays. Taking the active photo itself out makes the nearest selected + /// photo before it (else after it) the active one; a photo added to an empty selection is it. pub fn toggle(&mut self, id: PhotoId) { if let Some(i) = self.ids.iter().position(|x| *x == id) { self.ids.remove(i); if self.active == Some(id) { - self.active = self.ids.last().copied(); + self.active = self.ids.get(i.saturating_sub(1)).or(self.ids.first()).copied(); } } else { self.ids.push(id); - self.active = Some(id); + if self.active.is_none_or(|a| !self.ids.contains(&a)) { + self.active = Some(id); + } } } - /// Shift-click: select the range between the active photo and `id` in `order`. + /// Shift-click, as in Lightroom: the photos between the active one and `id` in `order` join the + /// selection, and the active photo stays (the loupe keeps showing it). With no active photo in + /// `order`, `id` alone is selected. pub fn extend_to(&mut self, id: PhotoId, order: &[PhotoId]) { let anchor = self.active.unwrap_or(id); let (Some(a), Some(b)) = (order.iter().position(|x| *x == anchor), order.iter().position(|x| *x == id)) else { @@ -124,12 +131,23 @@ impl Selection { return; }; let (lo, hi) = (a.min(b), a.max(b)); - for x in &order[lo..=hi] { + for x in order.get(lo..=hi).unwrap_or_default() { if !self.ids.contains(x) { self.ids.push(*x); } } - self.active = Some(id); + if self.active.is_none() { + self.active = Some(id); + } + } + /// A plain click, as in Lightroom: on a photo of a selection of several it becomes the active + /// photo and the selection stays; anywhere else the photo alone is selected. + pub fn click(&mut self, id: PhotoId) { + if self.ids.len() > 1 && self.contains(id) { + self.active = Some(id); + } else { + *self = Selection::single(id); + } } } @@ -253,4 +271,48 @@ mod selection_state_tests { assert_eq!(s.state_of(ids[1]), SelectionState::Selected); assert_eq!(s.state_of(PhotoId(9)), SelectionState::NotSelected); } + + fn ids(n: u64) -> Vec { + (1..=n).map(PhotoId).collect() + } + + /// Client note 19a: Shift-click selects the range and leaves the active photo where it was. + #[test] + fn shift_click_extends_the_range_and_keeps_the_active_photo() { + let order = ids(10); + let mut s = Selection::single(order[2]); + s.extend_to(order[6], &order); + assert_eq!(s.active, Some(order[2]), "the loupe stays on the photo it showed"); + assert_eq!(s.ids.len(), 5); + assert!(order[2..=6].iter().all(|id| s.contains(*id))); + } + + /// Client note 19b: ⌘/Ctrl-click adds and removes photos without changing the active one. + #[test] + fn ctrl_click_adds_and_removes_without_moving_the_active_photo() { + let order = ids(5); + let mut s = Selection::single(order[0]); + s.toggle(order[3]); + assert_eq!(s.active, Some(order[0])); + assert!(s.contains(order[3])); + s.toggle(order[3]); + assert_eq!((s.active, s.ids.len()), (Some(order[0]), 1)); + // the active photo itself taken out: another selected photo takes its place + s.toggle(order[2]); + s.toggle(order[0]); + assert_eq!(s.active, Some(order[2])); + } + + /// Client note 19c: a click inside the selection makes that photo active and keeps the + /// selection; a click outside it replaces the selection. + #[test] + fn a_click_inside_the_selection_keeps_it() { + let order = ids(8); + let mut s = Selection::single(order[1]); + s.extend_to(order[5], &order); + s.click(order[3]); + assert_eq!((s.active, s.ids.len()), (Some(order[3]), 5), "the whole set stays"); + s.click(order[7]); + assert_eq!(s, Selection::single(order[7]), "a photo outside it replaces the selection"); + } } diff --git a/crates/ui-egui/src/panels/detail/filmstrip.rs b/crates/ui-egui/src/panels/detail/filmstrip.rs --- a/crates/ui-egui/src/panels/detail/filmstrip.rs +++ b/crates/ui-egui/src/panels/detail/filmstrip.rs @@ -116,6 +116,8 @@ pub(crate) fn filmstrip(app: &mut LightcraftApp, ui: &mut egui::Ui, r: Rect) { p.text(pos2(cr.right() - 8.0, cr.top() + 10.0), Align2::RIGHT_CENTER, &ph.format, t.semibold(8.5), t.text_dim); } let img_area = Rect::from_min_max(cr.min + vec2(10.0, 22.0), cr.max - vec2(10.0, 8.0)); + // the picture: a click outside it (on the frame) selects this photo alone (note 1) + let mut picture = img_area; crate::panels::grid::request_thumb(app, *id, (256.0 * ppp.min(2.0) / 2.0) as usize * 2, 8); let thumb = app.renderer.thumb(*id); if thumb.is_none() { @@ -128,6 +130,7 @@ pub(crate) fn filmstrip(app: &mut LightcraftApp, ui: &mut egui::Ui, r: Rect) { let [tw, th] = tex.size; let s = (img_area.width() / tw as f32).min(img_area.height() / th as f32); let fr = Rect::from_center_size(img_area.center(), vec2(tw as f32 * s, th as f32 * s)); + picture = fr; p.image(tex.tex.id(), fr, Rect::from_min_max(pos2(0.0, 0.0), pos2(1.0, 1.0)), Color32::WHITE); if sel { p.rect_stroke(fr, 0.0, Stroke::new(1.5, Color32::WHITE), StrokeKind::Outside); @@ -163,6 +166,7 @@ pub(crate) fn filmstrip(app: &mut LightcraftApp, ui: &mut egui::Ui, r: Rect) { } } } + register(ui, format!("filmpic:{}", id.0), picture); if resp.clicked() { let m = ui.input(|i| i.modifiers); let compare = app.ui.view == crate::state::ViewMode::Compare; @@ -173,14 +177,9 @@ pub(crate) fn filmstrip(app: &mut LightcraftApp, ui: &mut egui::Ui, r: Rect) { // ⌘/Ctrl- and Shift-clicks change the selection; the pair on screen stays (#187) crate::panels::compare::select_with_modifier(app, *id, !m.command) } else { - let mode = if m.command { - "toggle" - } else if m.shift { - "range" - } else { - "replace" - }; - app.run("library.select", json!({"ids": [id.0], "mode": mode})) + // on the picture: the active photo changes, the set stays (note 19c); on the frame: this photo alone + let on_picture = resp.interact_pointer_pos().is_none_or(|at| picture.contains(at)); + app.run("library.select", json!({"ids": [id.0], "mode": crate::panels::grid::click_mode(m, on_picture)})) }; if compare && let Err(e) = r { app.toast(ui.ctx(), e); diff --git a/crates/ui-egui/src/panels/grid.rs b/crates/ui-egui/src/panels/grid.rs --- a/crates/ui-egui/src/panels/grid.rs +++ b/crates/ui-egui/src/panels/grid.rs @@ -763,6 +763,21 @@ pub(crate) fn skips_the_real_render(embedded_is_final: bool, from_embedded: bool /// Rendered thumbnails replacing embedded previews: after everything on screen. pub const BACKGROUND_THUMB_PRIORITY: u32 = 3; +/// The `library.select` mode of a click on a photo cell, as in Lightroom Classic: Shift = range, ⌘/Ctrl = toggle; a +/// plain click on the picture of a selected photo makes it the active one and keeps the selection (note 19c), a plain +/// click on the frame around the picture selects that photo alone (test notes 2026-10-24 note 1). +pub(crate) fn click_mode(m: egui::Modifiers, on_picture: bool) -> &'static str { + if m.shift { + "range" + } else if m.command { + "toggle" + } else if on_picture { + "click" + } else { + "replace" + } +} + fn cell(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId, r: Rect, square: bool, onscreen: bool, ppp: f32) { let t = Tokens::get(ui.ctx()); let Some(photo) = app.session.catalog.photo(id).cloned() else { return }; @@ -794,12 +809,15 @@ fn cell(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId, r: Rect, square r }; // thumbnail + // the picture itself: a click on it keeps a selection, a click on the frame around it selects this photo alone (note 1) + let picture: Rect; let size = thumb_px(img_rect.width().max(img_rect.height()), ppp); request_thumb(app, id, size, if onscreen { 10 } else { 5 }); if let Some(tex) = app.renderer.thumb(id) { let [tw, th] = tex.size; // Preserve the decoded preview's ratio when it differs from the catalog dimensions. let fit = super::detail::fit_texture_rect(img_rect, [tw, th]); + picture = fit; p.image(tex.tex.id(), fit, Rect::from_min_max(pos2(0.0, 0.0), pos2(1.0, 1.0)), Color32::WHITE); if active { p.rect_stroke(fit.expand(if square { 2.0 } else { 0.0 }), 0.0, Stroke::new(2.0, Color32::WHITE), StrokeKind::Outside); @@ -808,6 +826,7 @@ fn cell(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId, r: Rect, square } } else { let ph = img_rect.shrink(if square { 20.0 } else { 0.0 }); + picture = ph; p.rect_filled(ph, 0.0, t.skeleton); // not drawn yet (or unreadable): the selection is still visible if active { @@ -928,19 +947,14 @@ fn cell(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId, r: Rect, square } return; } + register(ui, format!("thumbpic:{}", id.0), picture); if resp.clicked() { let m = ui.input(|i| i.modifiers); - let mode = if m.shift { - "range" - } else if m.command { - "toggle" - } else { - "replace" - }; - let _ = app.run("library.select", json!({"ids": [id.0], "mode": mode})); + let on_picture = resp.interact_pointer_pos().is_none_or(|at| picture.contains(at)); + let _ = app.run("library.select", json!({"ids": [id.0], "mode": click_mode(m, on_picture)})); } if resp.double_clicked() { - let _ = app.run("library.select", json!({"ids": [id.0]})); + let _ = app.run("library.select", json!({"ids": [id.0], "mode": "click"})); let _ = app.run("view.detail", json!({})); } // drag the selection (this photo joins it, or replaces it when it wasn't selected) diff --git a/crates/ui-egui/src/tests.rs b/crates/ui-egui/src/tests.rs --- a/crates/ui-egui/src/tests.rs +++ b/crates/ui-egui/src/tests.rs @@ -27,6 +27,7 @@ mod error_kinds; mod filmstrip; mod focus_point; mod folder_drop; +mod frame_click; mod grading; mod grid; mod guided_guides; diff --git a/crates/ui-egui/src/tests/frame_click.rs b/crates/ui-egui/src/tests/frame_click.rs new file mode 100644 --- /dev/null +++ b/crates/ui-egui/src/tests/frame_click.rs @@ -0,0 +1,79 @@ +//! A plain click on the frame around a selected thumbnail (outside the picture) selects that photo alone; a click on +//! the picture keeps the selection and makes it the active photo, as in Lightroom Classic (test notes 2026-10-24 +//! note 1). Grid and filmstrip. + +use std::time::Duration; + +use serde_json::json; + +use crate::headless::Headless; +use crate::state::ViewMode; +use crate::{LightcraftApp, Services}; + +const T: Duration = Duration::from_secs(20); +const SETTLE: Duration = Duration::from_secs(120); + +fn widget(h: &Headless, id: &str) -> egui::Rect { + h.app.widgets.iter().find(|(w, _)| w == id).map(|(_, r)| *r).unwrap_or_else(|| panic!("{id} drawn")) +} + +fn click_at(h: &mut Headless, at: egui::Pos2) { + h.events.push(egui::Event::PointerMoved(at)); + h.events.push(egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed: true, modifiers: egui::Modifiers::NONE }); + h.step(); + h.events.push(egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed: false, modifiers: egui::Modifiers::NONE }); + h.step(); + // the next click is apart from this one (no double-click) + for _ in 0..40 { + h.step(); + } + h.settle(SETTLE); +} + +fn select_all(h: &mut Headless) { + let r = h.request("engine.execute", json!({"command": "library.selectAll", "params": {}}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); +} + +/// Clicks the picture and then the frame of the second photo's cell (`cell` / `pic` widget prefixes). +fn picture_keeps_frame_replaces(h: &mut Headless, cell: &str, pic: &str) { + let ids = h.app.session.visible_cloned(); + assert!(ids.len() >= 3, "a few photos"); + let second = ids[1]; + select_all(h); + let n = h.app.session.selection.ids.len(); + assert_eq!(n, ids.len()); + let picture = widget(h, &format!("{pic}{}", second.0)); + click_at(h, picture.center()); + assert_eq!(h.app.session.selection.ids.len(), n, "a click on the picture keeps the selection"); + assert_eq!(h.app.session.selection.active, Some(second), "and makes that photo the active one"); + let frame = widget(h, &format!("{cell}{}", second.0)); + let picture = widget(h, &format!("{pic}{}", second.0)); + // the frame above the picture (where the file name sits) + let at = egui::pos2(frame.center().x, (frame.top() + picture.top()) / 2.0); + assert!(frame.contains(at) && !picture.contains(at), "{at:?} on the frame: {frame:?} / {picture:?}"); + click_at(h, at); + assert_eq!(h.app.session.selection.ids, vec![second], "a click on the frame selects that photo alone"); + assert_eq!(h.app.session.selection.active, Some(second)); +} + +#[test] +fn a_click_on_a_grid_cells_frame_selects_that_photo_alone() { + let app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), Services { png: None, ..Default::default() }); + let mut h = Headless::new(app, [1200.0, 800.0], 1.0); + h.app.ui.view = ViewMode::SquareGrid; + h.app.ui.thumb_size = 200.0; + h.settle(SETTLE); + picture_keeps_frame_replaces(&mut h, "thumb:", "thumbpic:"); +} + +#[test] +fn a_click_on_a_filmstrip_cells_frame_selects_that_photo_alone() { + let app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), Services { png: None, ..Default::default() }); + let mut h = Headless::new(app, [1200.0, 800.0], 1.0); + let r = h.request("ui.set", json!({"view": "detail"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + picture_keeps_frame_replaces(&mut h, "film:", "filmpic:"); +} diff --git a/crates/ui-egui/src/tests/panels.rs b/crates/ui-egui/src/tests/panels.rs --- a/crates/ui-egui/src/tests/panels.rs +++ b/crates/ui-egui/src/tests/panels.rs @@ -87,7 +87,7 @@ fn assert_inside_right_panel(h: &Headless, what: &str) { for (id, r) in &h.app.widgets { // widgets of the photo area (pins, filmstrip cells clipped at its edge) and of the tool // strip are outside on purpose - if ["panel:", "film:", "maskPin"].iter().any(|p| id.starts_with(p)) + if ["panel:", "film:", "filmpic:", "maskPin"].iter().any(|p| id.starts_with(p)) || r.bottom() <= panel.top() || r.right() <= panel.left() + 1.5 || r.left() >= panel.right() - 1.5 diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -1,6 +1,6 @@ # Lightroom parity checklist -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-25 · **Change:** minor (LR-BEHAV-TITLEBAR: a double-click follows the system setting, and a panel over the bar keeps the window still; KEYC-METADATA, LRC-LIB-KEYWORDS: ⌥0 / ⌥⇧0 go to the next / previous keyword set; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings; KEYC-METADATA: ⌘K / Ctrl+K Add Keywords; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-26 · **Change:** minor (LR-BEHAV-MULTISELECT, MENU-EDIT-SELECTALL, KEY-SELECTALL: Shift / ⌘-click keep the active photo, a click on the picture keeps the selection, a click on the frame selects that photo alone, Select All grows with the view; LR-BEHAV-TITLEBAR: a double-click follows the system setting, and a panel over the bar keeps the window still; KEYC-METADATA, LRC-LIB-KEYWORDS: ⌥0 / ⌥⇧0 go to the next / previous keyword set; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings; KEYC-METADATA: ⌘K / Ctrl+K Add Keywords; LRC-DEV-HISTOGRAM-DRAG: the histogram's five tonal zones — hover for the value, drag to adjust the Basic control the press locked, double-click to reset; LR-VIEW-HISTOGRAM note updated; MENU-BAR: on macOS a menu item that yields its key to a focused text field or to shortcut recording really lets it go; LR-MERGE-HDR / LR-MERGE-PANO: Lanczos-3 warps; LR-IMP-LOCAL: browsing from the sidebar records the folder; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LR-IMP-ADD-DIALOG: hierarchical import album destination reconciled with main, #2362; KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them; LR-EDIT-OPTICS-PROFILE: Fujifilm nine-knot lens tables read at the plain radius; LR-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict; LRC-LIB-LIBFOLDERS, LR-BEHAV-BG-TASKS: Synchronize Folder's scan shows its folder listing and stops it on ✕; LRC-LIB-LIBFOLDERS: Synchronize Folder says a folder is up to date; LR-LIB-STACK: filmstrip stack badge click; LRC-LIB-REFVIEW / CATALOG-SWITCH: Escape preserves edits; catalog-local pins; deleted and malformed references refused; LRC-LIB-COMPARE: the Info panel labels the active photo while culling, #2398; KEYC-RATING: `\` shows / hides the Filter Bar in the grids; LR-VIEW-PHOTOGRID: the grid reads only the photos an import batch or an edit touched, not the whole list; earlier: LRC-PRINT-PAGE: Print Page Options; LR-META-XMP: fewer false unimported-XMP warnings) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 The measured, row-by-row checklist: one row per Lightroom feature, menu item and shortcut, what we have and what is missing. It answers *does the feature exist?* (feature breadth). Whether it is good @@ -457,7 +457,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). |---|---|---|---|---|---| | LR-BEHAV-AUTOSAVE | Instant autosave | P0 | ✅ | `crates/catalog/src/journal.rs`, `crates/engine/src/library.rs` | every change fsynced before the command returns; a failed write fails the command ("saved in memory but not written to disk"), keeps the change queued and retries; top-bar warning until saved; a write that fails part-way is cut back to the last whole record so the retry never reads back as damage (logs from older builds with such a fragment load in full); quitting saves queued changes in the closing snapshot even when the log can't be appended to; versioned catalog format (`journal::VERSION`): older libraries are upgraded on open, a library from a newer LightCraft is refused untouched ("written by a newer version"); a damaged or hand-edited snapshot that still reads is made consistent on open (`Catalog::repair`: records filed under another id, stacks of missing, repeated or too few photos, folders inside themselves, id counters behind the ids in use) instead of panicking or hanging later | | LR-BEHAV-UNDO | Global undo | P0 | ✅ | `cmd:edit.undo`, `crates/engine/src/tests.rs` (`rating_flag_undo_redo`), `crates/engine/src/tests/filesystem_history.rs` | covers ratings, albums, deletes, edits; partial file/XMP moves keep unfinished repairs across retries and discarded history; merged renames move directly to their final catalog path | -| LR-BEHAV-MULTISELECT | Multi-selection | P0 | ✅ | `cmd:library.select` (`replace`/`add`/`toggle`/`range`), `cmd:library.selectAll` | | +| LR-BEHAV-MULTISELECT | Multi-selection | P0 | ✅ | `cmd:library.select` (`replace`/`add`/`toggle`/`range`/`click`), `cmd:library.selectAll` | as in Lightroom Classic: Shift-click selects the range from the active photo and ⌘/Ctrl-click adds or removes a photo, both without changing the active photo or the loupe; a plain click on the picture of a photo inside a selection of several makes it the active photo and keeps the selection, a click on the frame around the picture (outside the image) selects that photo alone (`grid::click_mode`), a click outside the selection replaces it (grid, filmstrip, double-click); Select All also selects photos that join the view later (an import still running) until the selection or the view changes | | LR-BEHAV-BATCH | Batch apply to selection | P0 | ✅ | `cmd:photo.rate`, `cmd:develop.paste`, `cmd:preset.apply`, `cmd:app.export` | | | LR-BEHAV-PREVIEW-HOVER | Hover previews | P1 | ✅ | `crates/ui-egui/src/panels/presets.rs`, `crates/ui-egui/src/panels/profiles.rs`, `crates/ui-egui/src/panels/right.rs` (`versions`), `crates/ui-egui/src/panels/detail.rs` | presets, profile menu, profile browser and versions preview in the loupe | | LR-BEHAV-PROGRESSIVE | Progressive rendering | P0 | ✅ | `crates/engine/src/media.rs`, `crates/preview/src/lib.rs` | | @@ -523,7 +523,7 @@ visible menu bar yet: the menu model is only exposed through the control channel | MENU-EDIT-COPYPASTE | Copy / paste (edit settings) | P0 | ✅ | `cmd:develop.copy`, `cmd:develop.paste` | | | MENU-EDIT-CHOOSECOPY | Choose Edit Settings to Copy… | P0 | ✅ | `cmd:dialog.copySettings` | Return copies, Esc cancels | | MENU-EDIT-PASTESELECTED | Paste Selected Settings | P0 | ✅ | `cmd:dialog.pasteSettings`, `cmd:develop.paste` (`groups`) | checklist prefilled with the copied groups; only copied groups can be pasted; Return pastes, Esc cancels | -| MENU-EDIT-SELECTALL | Select All | P0 | ✅ | `cmd:library.selectAll` | | +| MENU-EDIT-SELECTALL | Select All | P0 | ✅ | `cmd:library.selectAll` | every photo of the view; photos that join the view afterwards (an import or the finishing queue still adding them) are selected too while the selection is untouched (⌘A during an import had selected 312 of 569 photos); collapsed stacks give their top photo | | MENU-EDIT-SELECTNONE | Select None | P0 | ✅ | `cmd:library.selectNone` | | | MENU-EDIT-SELECTBY | Select by flag / rating | P1 | ✅ | `cmd:library.selectBy` | Edit → Select by: picks, rejects, unflagged, ★…★★★★★ and higher, unrated, colour labels; `add` extends the selection | | MENU-EDIT-FIND | Find… | P0 | ✅ | `cmd:view.focusSearch`, `crates/ui-egui/src/panels/topbar.rs` | ⌘F focuses the search field | @@ -614,7 +614,7 @@ From `06-shortcuts.md` part 1. Evidence is our binding; conflicts are explained | KEY-PHOTOSHOP | External editor — ⇧⌘E | P2 | ✅ | `cmd:photo.editInPhotoCraft`, `cmd:photo.editInExternal` | ⇧⌘E as observed (Edit in PhotoCraft, LightCraft's Photoshop); ⌥⌘E the other editor; the export dialog is ⇧E | | KEY-ROTATE | Rotate — ⌘[ / ⌘] | P0 | ✅ | `cmd:photo.rotateLeft`, `cmd:photo.rotateRight` | | | KEY-ZOOM | Zoom in / out — ⌘= / ⌘− | P0 | ✅ | `cmd:view.zoomIn`, `cmd:view.zoomOut`, `crates/ui-egui/src/tests/zoom_keys.rs` | the keys zoom the photo only: egui's own interface zoom on ⌘= / ⌘− / ⌘0 (Ctrl on Windows and Linux, where no native menu bar took the keys first) is off (issue #566) | -| KEY-SELECTALL | Select all — ⌘A | P0 | ✅ | `cmd:library.selectAll` | | +| KEY-SELECTALL | Select all — ⌘A | P0 | ✅ | `cmd:library.selectAll` | every photo of the view; photos that join the view afterwards (an import or the finishing queue still adding them) are selected too while the selection is untouched (⌘A during an import had selected 312 of 569 photos); collapsed stacks give their top photo | | KEY-SELECTNONE | Select none — ⌘D | P0 | ✅ | `cmd:library.selectNone` | secondary binding (primary ⌘⇧A) | | KEYC-SELECTONLYACTIVE | Select only active photo — ⌘⇧D / Ctrl+Shift+D | P1 | 🟡 | `cmd:library.selectOnlyActive`, `crates/ui-egui/src/shortcuts.rs` (`select_only_active_*` tests) | Edit → Select Only Active Photo; keeps the active photo, narrows the selection to it and leaves the edit Undo step alone; works in any view that has an active photo; rebindable in Help → Keyboard Shortcuts; text fields and dialogs keep their keys; covered by headless key tests, the macOS native-menu path is not verified | | KEY-PASTESELECTED | Paste selected — ⇧⌘V | P0 | ✅ | `cmd:dialog.pasteSettings` | | @@ -799,6 +799,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | Date | Change | Summary | |---|---|---| +| 2026-10-26 | minor | LR-BEHAV-MULTISELECT, MENU-EDIT-SELECTALL, KEY-SELECTALL: as in Lightroom Classic, Shift-click and ⌘/Ctrl-click keep the active photo and the loupe; a plain click on the picture of a selected photo makes it the active one and keeps the selection (`library.select` mode `click`), a click on the frame around the picture selects that photo alone; Select All also takes photos that join the view while an import runs; status unchanged (✅) | | 2026-10-25 | patch | LR-BEHAV-TITLEBAR: the top bar's title-bar gestures come from craft-window's `title_bar`: a double-click follows the system setting, and a press on a button, the search field or a panel floating over the bar never moves the window; status counts unchanged | | 2026-10-24 | minor | KEYC-METADATA, LRC-LIB-KEYWORDS: ⌥0 / ⌥⇧0 (`keyword.nextSet`, `keyword.previousSet`) go to the next / previous keyword set; status counts unchanged (KEYC-METADATA stays 🟡) | | 2026-10-24 | minor | KEYC-METADATA: ⌘K / Ctrl+K (`panel.addKeywords`, Add Keywords) shows the Keywords panel, opens Keywording and puts the cursor in the Add keyword box; status counts unchanged (KEYC-METADATA stays 🟡) |