shifathosn
/
lightcraft
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Issue #187: the filmstrip highlights every selected photo, not only the active one 

The filmstrip painted selection chrome only for the active photo, so after
Select All or a ⌘/Shift-click multi-selection the other selected photos
looked unselected, although the selection itself was right.

Paint it as the grid does: every selected photo gets the selected cell
fill; the active one keeps its white frame and the others get the grid's
grey one.

Test: tests_scroll::filmstrip_shows_every_selected_photo samples a cell's
background in the headless render after Select All, after selecting one
photo, and after a toggle (what a ⌘-click runs); before the fix a selected
photo that was not the active one was painted like an unselected one.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
wstock 2026-10-08 11:55:57 +00:00
parent 629e39380e
commit 7177b9b9f3
3 changed files with 35 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
crates/ui-egui/src/panels/detail.rs
View File

@ -1527,8 +1527,10 @@ pub(crate) fn filmstrip(app: &mut LightcraftApp, ui: &mut egui::Ui, r: Rect) {
let resp = ui.interact(cr, egui::Id::new(("film", id.0)), Sense::click());
register(ui.ctx(), format!("film:{}", id.0), cr);
let sel = Some(*id) == active;
// as in the grid: every selected photo is filled, the active one framed in white (#187)
let selected = sel || app.session.selection.contains(*id);
let p = ui.painter();
if sel {
if selected {
p.rect_filled(cr, 0.0, t.cell_selected);
} else if resp.hovered() {
p.rect_filled(cr, 0.0, t.cell_selected.gamma_multiply(0.6));
@ -1549,6 +1551,8 @@ pub(crate) fn filmstrip(app: &mut LightcraftApp, ui: &mut egui::Ui, r: Rect) {
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);
} else if selected {
p.rect_stroke(fr, 0.0, Stroke::new(1.5, Color32::from_gray(170)), StrokeKind::Outside);
}
if app.ui.settings.film_badges
&& let Some(ph) = app.session.catalog.photo(*id)

30
crates/ui-egui/src/tests_scroll.rs
View File

@ -1,6 +1,6 @@
//! Headless tests of the photo grid's and the filmstrip's scrolling (issue #11): the user's
//! scroll position stays put until the active photo changes; the mouse wheel scrolls the
//! filmstrip sideways.
//! filmstrip sideways. The filmstrip also shows which photos are selected (issue #187).
use std::time::Duration;
@ -115,3 +115,31 @@ fn filmstrip_wheel_scrolls_and_keeps_its_position() {
let cell = widget(&h, &format!("film:{second}"));
assert!(cell.left() >= 0.0 && cell.right() <= 1200.0, "the new active photo is in view ({cell:?})");
}
#[test]
fn filmstrip_shows_every_selected_photo() {
let mut h = demo("detail");
let ids = h.app.session.visible_cloned();
let (first, second) = (ids[0].0, ids[1].0);
// a cell's background, left of its thumbnail
let fill = |h: &mut Headless, id: u64| {
let r = widget(h, &format!("film:{id}"));
let img = h.snapshot(SETTLE);
img.pixels[(r.center().y as usize) * img.width() + r.left() as usize + 4]
};
let active = fill(&mut h, first);
let idle = fill(&mut h, second);
assert_ne!(active, idle, "only the active photo is selected");
// Select All: the other photos are selected too, the first stays active
let r = h.request("engine.execute", json!({"command": "library.selectAll"}), T);
assert_eq!(r["ok"], true, "{r}");
assert_eq!(h.app.session.active().map(|p| p.0), Some(first));
assert_eq!(fill(&mut h, second), active, "a selected photo that is not the active one looks selected");
// back to one photo; then what a ⌘-click on a filmstrip cell runs (toggle) adds the second
let r = h.request("engine.execute", json!({"command": "library.select", "params": {"ids": [first]}}), T);
assert_eq!(r["ok"], true, "{r}");
assert_eq!(fill(&mut h, second), idle);
let r = h.request("engine.execute", json!({"command": "library.select", "params": {"ids": [second], "mode": "toggle"}}), T);
assert_eq!(r["ok"], true, "{r}");
assert_eq!(fill(&mut h, second), active);
}

2
docs/parity.md
View File

@ -171,7 +171,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins.
| LR-VIEW-DETAIL | Single-photo view | P0 | ✅ | `cmd:view.detail`, `crates/ui-egui/src/panels/detail.rs` | |
| LR-VIEW-EDIT | Edit view | P0 | ✅ | `cmd:panel.edit` | |
| LR-VIEW-FULLSCREEN | Full-screen preview | P1 | ✅ | `cmd:view.fullScreenPreview`, `cmd:view.enterFullScreen` | photo on black, arrows step, Esc exits; ⇧⌘F window full screen |
| LR-VIEW-FILMSTRIP | Filmstrip | P0 | ✅ | `cmd:view.filmstrip`, `crates/ui-egui/src/panels/detail.rs` | the photo context menu on right-click; the mouse wheel scrolls it sideways; it follows the active photo only when that changes (centred if off screen), so a scrolled strip stays put |
| LR-VIEW-FILMSTRIP | Filmstrip | P0 | ✅ | `cmd:view.filmstrip`, `crates/ui-egui/src/panels/detail.rs` | the photo context menu on right-click; the mouse wheel scrolls it sideways; it follows the active photo only when that changes (centred if off screen), so a scrolled strip stays put; every selected photo is highlighted, the active one framed in white, as in the grid |
| LR-VIEW-ZOOM | Zoom & pan | P0 | ✅ | `cmd:view.zoomFit`, `cmd:view.zoom100`, `cmd:view.zoomIn`, `cmd:view.zoomOut`, `cmd:view.zoomToggle`, `cmd:view.clickZoom` | steps 25–800 % (not 6–1600 %); Fill only in the bottom bar; a click on the photo (and Z / Space) eases to the click-zoom ratio (1:1 default, 2:1, 3:1, 4:1, 8:1 in the bottom bar) |
| LR-VIEW-NAVIGATOR | Navigator mini map | P1 | ✅ | `cmd:view.navigator`, `crates/ui-egui/src/panels/detail.rs` | shown while zoomed (bottom right); click/drag pans |
| LR-VIEW-BEFOREAFTER | Before / after | P0 | ✅ | `cmd:view.showOriginal`, `cmd:view.beforeAfter`, `cmd:view.beforeAfterSplit`, `cmd:view.beforeAfterTopBottom`, `cmd:view.beforeAfterSplitTopBottom`, `cmd:beforeAfter.setBefore`, `cmd:beforeAfter.copyAfterToBefore`, `cmd:beforeAfter.copyBeforeToAfter`, `cmd:beforeAfter.swap`, `cmd:beforeAfter.resetBefore` | all four layouts; before = the import state (defaults + import preset) or a chosen history step / version / the current settings; copy and swap (View → Before/After Settings, History row menu). The chosen before lasts for the session |