Merge PR #3563

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() {

crates/ui-egui/src/tests/compare.rs

@@ -359,3 +359,60 @@ 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<u64> = 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);
+}
+
+/// 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 sel = |h: &Headless| h.app.session.selection.ids.iter().map(|p| p.0).collect::<Vec<u64>>();
+    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");
+}
+
+/// ⌘/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<u64> = 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)));
+}

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:<pane>`) or “This photo can't be opened” (`compare:renderFailed:<pane>`) 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) |