Merge PR #1264

crates/engine/src/cmd/library.rs

@@ -219,6 +219,12 @@ fn step(s: &mut Session, d: isize) -> Result<Value> {
     Ok(json!({"active": vis[next].0}))
 }
 
+/// `library.up` / `library.down`: how many photos to move (at least 1, capped so hostile input
+/// can't overflow).
+fn row_step(p: &Value) -> isize {
+    p.get("rows").and_then(Value::as_u64).unwrap_or(1).clamp(1, 1 << 20) as isize
+}
+
 fn rotate(s: &mut Session, p: &Value, cw: bool) -> Result<Value> {
     let targets = s.targets(p);
     let mut ops = Vec::new();
@@ -641,6 +647,24 @@ pub fn specs() -> Vec<CommandSpec> {
         }),
         cmd!("library.next", "Next Photo", [], Some("Right"), "{}", always, |s, _| step(s, 1)),
         cmd!("library.previous", "Previous Photo", [], Some("Left"), "{}", always, |s, _| step(s, -1)),
+        cmd!(
+            "library.down",
+            "Photo Below",
+            [],
+            Some("Down"),
+            "{rows?: n (photos to move forward; default 1, the grid passes the distance to the cell below)}",
+            always,
+            |s, p| { step(s, row_step(p)) }
+        ),
+        cmd!(
+            "library.up",
+            "Photo Above",
+            [],
+            Some("Up"),
+            "{rows?: n (photos to move back; default 1, the grid passes the distance to the cell above)}",
+            always,
+            |s, p| { step(s, -row_step(p)) }
+        ),
         // ---- rating / flags / labels
         cmd!("photo.rate", "Set Rating", ["Photo", "Set Rating"], None, "{rating: 0..5, ids?, advance?: bool}", has_selection, |s, p| {
             let r = super::f64_req(p, "rating", "photo.rate")? as i64;

crates/engine/src/tests_spots.rs

@@ -4,6 +4,23 @@ use serde_json::json;
 
 use crate::Session;
 
+/// `library.up` / `library.down` (#1171): move by the given number of photos, clamped at the ends.
+#[test]
+fn up_and_down_move_by_rows_and_stop_at_the_ends() {
+    let mut s = Session::with_demo();
+    let vis = s.visible_cloned();
+    assert!(vis.len() >= 6);
+    s.selection = crate::Selection::single(vis[0]);
+    s.execute("library.down", &json!({"rows": 4})).unwrap();
+    assert_eq!(s.selection.active, Some(vis[4]));
+    s.execute("library.up", &json!({})).unwrap();
+    assert_eq!(s.selection.active, Some(vis[3]), "default is one photo (the filmstrip)");
+    s.execute("library.up", &json!({"rows": 99})).unwrap();
+    assert_eq!(s.selection.active, Some(vis[0]), "clamped at the first photo");
+    s.execute("library.down", &json!({"rows": u64::MAX})).unwrap();
+    assert_eq!(s.selection.active, vis.last().copied(), "hostile counts clamp, never overflow");
+}
+
 fn spots(s: &Session) -> Vec<lightcraft_develop::Spot> {
     s.develop_of(s.active().unwrap()).unwrap().spots.clone()
 }

crates/ui-egui/src/headless.rs

@@ -750,6 +750,51 @@ mod tests {
         assert!(egui::Popup::is_any_open(&h.view.ctx), "its context menu");
     }
 
+    /// #1171: Up / Down in the grid go to the cell above / below; in the filmstrip, one photo.
+    #[test]
+    fn up_and_down_move_through_the_grid_rows() {
+        let mut h = demo([1200.0, 760.0]);
+        let t = Duration::from_secs(10);
+        h.request("ui.set", json!({"view": "grid"}), t);
+        h.settle(SETTLE);
+        let first = h.app.session.visible_cloned()[0];
+        h.request("engine.execute", json!({"command": "library.select", "params": {"ids": [first.0]}}), t);
+        h.settle(SETTLE);
+        let rect = |h: &Headless| {
+            let a = h.app.session.selection.active.unwrap();
+            h.app.widgets.iter().find(|(w, _)| *w == format!("thumb:{}", a.0)).map(|(_, r)| *r).expect("grid cell")
+        };
+        let top = rect(&h);
+        h.request("ui.key", json!({"key": "down"}), t);
+        h.settle(SETTLE);
+        let below = rect(&h);
+        let pos = {
+            let a = h.app.session.selection.active;
+            h.app.session.visible_cloned().iter().position(|x| Some(*x) == a)
+        }
+        .unwrap();
+        assert!(below.top() > top.bottom(), "the next row, at {pos}: {top:?} -> {below:?}");
+        assert!(below.left() < top.right() && below.right() > top.left(), "overlapping horizontally");
+        h.request("ui.key", json!({"key": "up"}), t);
+        h.settle(SETTLE);
+        assert_eq!(h.app.session.selection.active, Some(first), "back up");
+        h.request("ui.key", json!({"key": "up"}), t);
+        h.settle(SETTLE);
+        assert_eq!(h.app.session.selection.active, Some(first), "the top row stays put");
+        // the filmstrip (one row): Down is the next photo
+        h.request("ui.set", json!({"view": "detail"}), t);
+        h.settle(SETTLE);
+        h.request("ui.key", json!({"key": "down"}), t);
+        h.settle(SETTLE);
+        assert_eq!(
+            {
+                let a = h.app.session.selection.active;
+                h.app.session.visible_cloned().iter().position(|x| Some(*x) == a)
+            },
+            Some(1)
+        );
+    }
+
     /// The search field is a shared text field: right-click ▸ Paste puts the clipboard's text in
     /// and filters by it.
     #[test]

crates/ui-egui/src/panels/grid.rs

@@ -319,6 +319,36 @@ pub struct GridLayout {
     pub height: f32,
 }
 
+/// The cell straight above (`down` false) or below cell `i`: in the adjacent row, the one whose
+/// centre is nearest horizontally (justified rows don't line up in columns). None in the first /
+/// last row or for an index outside the layout.
+pub fn vertical_neighbor(lay: &GridLayout, i: usize, down: bool) -> Option<usize> {
+    let row = lay.rows.iter().position(|r| (r.start..r.end).contains(&i))?;
+    let next = if down { row.checked_add(1)? } else { row.checked_sub(1)? };
+    let target = lay.rows.get(next)?;
+    let x = lay.cells.get(i)?.center().x;
+    (target.start..target.end).filter_map(|k| Some((k, (lay.cells.get(k)?.center().x - x).abs()))).min_by(|a, b| a.1.total_cmp(&b.1)).map(|(k, _)| k)
+}
+
+/// Up / Down in the grids: how many photos `library.up` / `library.down` move to reach the cell
+/// above / below the active one in the grid as last laid out; None when the grid isn't laid out
+/// for the current photos (the filmstrip then steps one photo).
+pub fn row_distance(app: &mut LightcraftApp, down: bool) -> Option<u64> {
+    if !matches!(app.ui.view, ViewMode::PhotoGrid | ViewMode::SquareGrid) {
+        return None;
+    }
+    let active = app.session.selection.active?;
+    let (generation, ids) = app.session.visible_shared();
+    let i = ids.iter().position(|x| *x == active)?;
+    let (key, lay) = app.caches.grid.layout.as_ref()?;
+    if key.0.0 != generation {
+        return None;
+    }
+    // at the top / bottom row there's no cell to go to: stay put rather than step sideways
+    let j = vertical_neighbor(lay, i, down).unwrap_or(i);
+    Some(j.abs_diff(i) as u64)
+}
+
 /// One row of cells: `cells[start..end]`, between `top` and `bottom`.
 #[derive(Clone, Copy, Debug, PartialEq)]
 pub struct GridRow {

crates/ui-egui/src/shortcuts.rs

@@ -532,6 +532,19 @@ pub fn handle(app: &mut LightcraftApp, ctx: &egui::Context) {
                     continue;
                 }
             }
+            // Up / Down in the grids go to the cell above / below (the filmstrip steps one photo)
+            if matches!(f.as_str(), "library.up" | "library.down") {
+                match crate::panels::grid::row_distance(app, f == "library.down") {
+                    Some(0) => {}
+                    Some(rows) => {
+                        let _ = app.run(&f, json!({"rows": rows}));
+                    }
+                    None => {
+                        let _ = app.run(&f, json!({}));
+                    }
+                }
+                continue;
+            }
             // an export that can't start (e.g. no folder) says why instead of doing nothing
             if let Err(e) = app.run(&f, json!({}))
                 && matches!(f.as_str(), "app.export" | "app.exportPrevious")

docs/parity-checklist.md

@@ -651,7 +651,7 @@ From `06-shortcuts.md` part 1. Evidence is our binding; conflicts are explained
 | KEY-ERASE | Erase while held — ⌥ | P0 | ✅ | `crates/ui-egui/src/panels/detail.rs`, `crates/ui-egui/src/panels/masking.rs` | ⌥ while painting flips Add ↔ Erase for that stroke; Add/Erase buttons too |
 | KEY-RATEADVANCE | Rate and advance — ⇧0…5 | P1 | ✅ | `crates/ui-egui/src/shortcuts.rs` → `cmd:photo.rate` (`advance`) | |
 | KEY-FLAGADVANCE | Flag and advance — ⇧Z / ⇧X / ⇧U | P1 | ✅ | `cmd:photo.flag` (`advance`), `crates/ui-egui/src/shortcuts.rs` | Shift+Z pick, Shift+X reject, Shift+U clear; Shift+P also picks and advances in the library grids; exactly one step with Auto Advance enabled |
-| KEY-NEXTPREV | Next / previous — → / ← | P0 | ✅ | `cmd:library.next`, `cmd:library.previous` | |
+| KEY-NEXTPREV | Next / previous — → / ←; above / below — ↑ / ↓ | P0 | ✅ | `cmd:library.next`, `cmd:library.previous`, `cmd:library.up`, `cmd:library.down` | ↑ / ↓ go to the cell above / below in the grids (nearest by position in justified rows; the top / bottom row stays put), one photo in the filmstrip (issue #1171); no vim keys by default (H, J, K, L are taken), but they can be assigned in the keyboard shortcuts editor |
 | KEY-BA-CYCLE | Before/after — Y | P0 | ✅ | `cmd:view.beforeAfter` | toggles side by side (no cycling) |
 | KEY-BA-TOPBOTTOM | Before/after top/bottom — ⌥Y | P1 | ✅ | `cmd:view.beforeAfterTopBottom`, `cmd:view.beforeAfterSplitTopBottom` | ⌥Y, ⇧⌥Y |
 | KEY-BA-SPLIT | Split before/after — ⇧Y | P0 | ✅ | `cmd:view.beforeAfterSplit` | |