From 3e3955bb3031791900c8da41c8ead73d0e1a4175 Mon Sep 17 00:00:00 2001 From: Fridtjof Michels Date: Sun, 1 Nov 2026 21:12:47 +0100 Subject: [PATCH 1/3] =?UTF-8?q?LR-VIEW-SURVEY:=20=E2=8C=98/Ctrl-click=20on?= =?UTF-8?q?=20a=20survey=20tile=20takes=20the=20photo=20out=20of=20the=20s?= =?UTF-8?q?urvey?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As in Lightroom Classic, a ⌘/Ctrl-click on a tile in Survey deselects that photo (library.select, mode toggle), as the hover × does; a plain click still only makes it active, and the last photo stays so the survey never empties. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/panels/compare.rs | 7 +++++- crates/ui-egui/src/tests/compare.rs | 34 ++++++++++++++++++++++++++++ docs/parity-checklist.md | 2 +- 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/panels/compare.rs b/crates/ui-egui/src/panels/compare.rs index dad49923f..b7034f3a6 100644 --- a/crates/ui-egui/src/panels/compare.rs +++ b/crates/ui-egui/src/panels/compare.rs @@ -569,7 +569,12 @@ pub fn show_survey(app: &mut LightcraftApp, ui: &mut egui::Ui) { if app.session.selection.active == Some(*id) { app.image_rect = Some(img); } - if resp.clicked() { + // ⌘/Ctrl-click removes the photo from the survey (deselects it), as the × does + if resp.clicked() && ui.input(|i| i.modifiers.command) { + if photos.len() > 1 { + let _ = app.run("library.select", json!({"ids": [id.0], "mode": "toggle"})); + } + } else if resp.clicked() { app.session.selection.active = Some(*id); } if resp.double_clicked() { diff --git a/crates/ui-egui/src/tests/compare.rs b/crates/ui-egui/src/tests/compare.rs index b0ccb0b14..e4d838974 100644 --- a/crates/ui-egui/src/tests/compare.rs +++ b/crates/ui-egui/src/tests/compare.rs @@ -359,3 +359,37 @@ fn survey_rings_the_active_tile_in_the_theme_colour() { .count(); assert_eq!(white, 0, "no white 2 pt frame"); } + +/// Survey with three photos selected and the second active; returns the three ids. +fn survey_of_three(h: &mut Headless) -> [u64; 3] { + let vis: Vec = h.app.session.visible_cloned().iter().map(|p| p.0).collect(); + let ids = [vis[0], vis[1], vis[2]]; + h.request("engine.execute", json!({"command": "library.select", "params": {"ids": ids, "active": ids[1]}}), T); + let r = h.request("engine.execute", json!({"command": "view.survey"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + ids +} + +/// Move the pointer to `pos` and press and release the primary button there, with `modifiers`. +fn click_with(h: &mut Headless, pos: egui::Pos2, modifiers: egui::Modifiers) { + h.app.synthetic.push(egui::Event::PointerMoved(pos)); + h.settle(SETTLE); + h.app.synthetic.push(egui::Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed: true, modifiers }); + h.app.synthetic.push(egui::Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed: false, modifiers }); + h.settle(SETTLE); +} + +/// ⌘/Ctrl-click on a survey tile takes it out of the survey, as in Lightroom Classic; a plain +/// click only makes it active. +#[test] +fn survey_command_click_takes_the_photo_out() { + let mut h = demo(); + let [a, b, c] = survey_of_three(&mut h); + let img = h.app.image_rect.expect("the active tile is laid out"); + click_with(&mut h, img.center(), egui::Modifiers::NONE); + assert_eq!(h.app.session.selection.ids.len(), 3, "a plain click keeps the photo"); + click_with(&mut h, img.center(), egui::Modifiers::COMMAND); + let sel: Vec = h.app.session.selection.ids.iter().map(|p| p.0).collect(); + assert_eq!(sel, vec![a, c], "the ⌘/Ctrl-clicked photo left the survey (was {b})"); +} diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 5e851e721..46aabad73 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -134,7 +134,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | 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; in side by side and top / bottom, clicks with the white-balance selector, colour samplers and targeted adjustment work in the Before view too (`crates/ui-egui/src/tests/before_after.rs`). Zoomed in, the side by side and stacked views each stay in their pane, with their labels under it. Click zoom animates both panes together, including cached window tiles; input follows the displayed image and render requests use the final size (#1821) | | LR-VIEW-COMPARE | Compare two photos | P1 | ✅ | `cmd:view.compare`, `cmd:compare.swap`, `cmd:compare.makeSelect`, `cmd:compare.set`, `crates/ui-egui/src/panels/compare.rs`, `crates/ui-egui/src/tests/compare.rs` | select / candidate, synced zoom + pan, arrows move the candidate; a filmstrip click puts the photo into the active side (a photo on the other side swaps), so the active photo is always one of the two shown; a ⌘/Ctrl- or Shift-click on the filmstrip changes the selection as in the grid while the two photos shown stay selected and the active photo is unchanged (#187); the active pane is framed in the active photo's two-tone selection ring (theme tokens, 3 pt), as in the grids; 1:1 to 8:1 sharp on both sides (zoom windows, see LR-VIEW-ZOOM); no zoom-link toggle; each pane says “Rendering…” (widget `compare:rendering:`) or “This photo can't be opened” (`compare:renderFailed:`) as the loupe does | -| LR-VIEW-SURVEY | Survey view [Classic] | P2 | ✅ | `cmd:view.survey`, `crates/ui-egui/src/panels/compare.rs` | selection tiled (≤ 48), keys, the bottom bar and the Info panel's stars and colour labels act on the active photo (ringed as in the grid), each tile's caption shows its colour label, hover × removes | +| LR-VIEW-SURVEY | Survey view [Classic] | P2 | ✅ | `cmd:view.survey`, `crates/ui-egui/src/panels/compare.rs` | selection tiled (≤ 48), keys, the bottom bar and the Info panel's stars and colour labels act on the active photo (ringed as in the grid), each tile's caption shows its colour label, hover × or ⌘/Ctrl-click removes a photo | | LR-VIEW-INFOOVERLAY | Info overlay on the photo | P1 | ✅ | `cmd:view.infoOverlay`, `cmd:view.viewOptions`, `crates/ui-egui/src/panels/detail.rs` (`info_overlay`) | off / Info 1 (by default file name, capture date, size) / Info 2 (file name, exposure, camera + lens), each three lines chosen in View Options; ⌘I cycles (I in full screen; elsewhere I stays the Info panel); the star rating and flag show under the lines when the photo has them, and in the full-screen preview on macOS the overlay starts below the window controls (issue #2460) | | LR-VIEW-SLIDESHOW | Slideshow | P2 | ✅ | `cmd:view.slideshow` | View ▸ Slideshow (⌥⌘↩): the photos in view full screen from the selected one, played with the library's playback settings (see LRC-SS-PLAYBACK: fades, random order, repeat, previews prepared ahead; `interval` / any setting overrides one run); Space pauses, ←/→ step (a cut), Esc ends; playback controls on pointer movement | | LR-VIEW-SECONDWINDOW | Second display window [Classic] | P2 | ✅ | `cmd:view.secondWindow`, `crates/ui-egui/src/panels/second.rs` | Window ▸ Second Window (⌘F11): the active photo fitted in its own native window with its own render (a floating panel where there are no native windows); loupe view only (no grid / compare / survey there) | From 3609fe846654f7b54b1c3263754fc36b1d175ca5 Mon Sep 17 00:00:00 2001 From: Fridtjof Michels Date: Sun, 1 Nov 2026 22:13:54 +0100 Subject: [PATCH 2/3] =?UTF-8?q?LR-VIEW-SURVEY:=20the=20survey=20=E2=8C=98/?= =?UTF-8?q?Ctrl-click=20test=20covers=20a=20non-active=20tile=20and=20the?= =?UTF-8?q?=20last=20photo?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up (#3563): a plain click on another tile makes it active, a ⌘/Ctrl-click on a selected tile that is not active removes only that one, and ⌘/Ctrl-click on the last photo leaves it in the survey and active. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/tests/compare.rs | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/crates/ui-egui/src/tests/compare.rs b/crates/ui-egui/src/tests/compare.rs index e4d838974..26b069f50 100644 --- a/crates/ui-egui/src/tests/compare.rs +++ b/crates/ui-egui/src/tests/compare.rs @@ -380,16 +380,27 @@ fn click_with(h: &mut Headless, pos: egui::Pos2, modifiers: egui::Modifiers) { h.settle(SETTLE); } -/// ⌘/Ctrl-click on a survey tile takes it out of the survey, as in Lightroom Classic; a plain -/// click only makes it active. +/// Click the centre of photo `id`'s survey tile with `modifiers`. +fn click_tile(h: &mut Headless, id: u64, modifiers: egui::Modifiers) { + let name = format!("cull:{id}"); + let pos = h.app.widgets.iter().find(|(w, _)| *w == name).map(|(_, r)| r.center()).expect("the tile is laid out"); + click_with(h, pos, modifiers); +} + +/// ⌘/Ctrl-click on a survey tile takes that photo out of the survey, as in Lightroom Classic, +/// whether or not it is the active one; a plain click only makes it active, and the last photo stays. #[test] fn survey_command_click_takes_the_photo_out() { let mut h = demo(); let [a, b, c] = survey_of_three(&mut h); - let img = h.app.image_rect.expect("the active tile is laid out"); - click_with(&mut h, img.center(), egui::Modifiers::NONE); - assert_eq!(h.app.session.selection.ids.len(), 3, "a plain click keeps the photo"); - click_with(&mut h, img.center(), egui::Modifiers::COMMAND); - let sel: Vec = h.app.session.selection.ids.iter().map(|p| p.0).collect(); - assert_eq!(sel, vec![a, c], "the ⌘/Ctrl-clicked photo left the survey (was {b})"); + let sel = |h: &Headless| h.app.session.selection.ids.iter().map(|p| p.0).collect::>(); + let active = |h: &Headless| h.app.session.selection.active.map(|p| p.0); + click_tile(&mut h, a, egui::Modifiers::NONE); + assert_eq!((sel(&h), active(&h)), (vec![a, b, c], Some(a)), "a plain click keeps the photo and makes it active"); + click_tile(&mut h, c, egui::Modifiers::COMMAND); + assert_eq!((sel(&h), active(&h)), (vec![a, b], Some(a)), "the ⌘/Ctrl-clicked photo (not the active one) left"); + click_tile(&mut h, b, egui::Modifiers::COMMAND); + assert_eq!(sel(&h), vec![a]); + click_tile(&mut h, a, egui::Modifiers::COMMAND); + assert_eq!((sel(&h), active(&h)), (vec![a], Some(a)), "the last photo stays in the survey"); } From da558f5b8fba9a46d97ae74846716b98e1a73825 Mon Sep 17 00:00:00 2001 From: Fridtjof Michels Date: Sun, 1 Nov 2026 22:34:35 +0100 Subject: [PATCH 3/3] =?UTF-8?q?LR-VIEW-SURVEY:=20test=20that=20=E2=8C=98/C?= =?UTF-8?q?trl-click=20on=20the=20active=20survey=20photo=20activates=20th?= =?UTF-8?q?e=20last=20one=20left?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up (#3563). Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/tests/compare.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/crates/ui-egui/src/tests/compare.rs b/crates/ui-egui/src/tests/compare.rs index 26b069f50..f570df33a 100644 --- a/crates/ui-egui/src/tests/compare.rs +++ b/crates/ui-egui/src/tests/compare.rs @@ -404,3 +404,15 @@ fn survey_command_click_takes_the_photo_out() { click_tile(&mut h, a, egui::Modifiers::COMMAND); assert_eq!((sel(&h), active(&h)), (vec![a], Some(a)), "the last photo stays in the survey"); } + +/// ⌘/Ctrl-click on the active survey photo while others remain: it leaves, and the last +/// remaining photo becomes active, so the keys still act on a photo on screen. +#[test] +fn survey_command_click_on_the_active_photo_activates_the_last_one() { + let mut h = demo(); + let [a, b, c] = survey_of_three(&mut h); + click_tile(&mut h, a, egui::Modifiers::NONE); + click_tile(&mut h, a, egui::Modifiers::COMMAND); + let sel: Vec = h.app.session.selection.ids.iter().map(|p| p.0).collect(); + assert_eq!((sel, h.app.session.selection.active.map(|p| p.0)), (vec![b, c], Some(c))); +}