diff --git a/crates/engine/src/cmd/library.rs b/crates/engine/src/cmd/library.rs index 2d06d73f6..3a5575ea8 100644 --- a/crates/engine/src/cmd/library.rs +++ b/crates/engine/src/cmd/library.rs @@ -219,6 +219,12 @@ fn step(s: &mut Session, d: isize) -> Result { 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 { let targets = s.targets(p); let mut ops = Vec::new(); @@ -641,6 +647,24 @@ pub fn specs() -> Vec { }), 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; diff --git a/crates/engine/src/tests_spots.rs b/crates/engine/src/tests_spots.rs index 25fd482b7..988e7aac1 100644 --- a/crates/engine/src/tests_spots.rs +++ b/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 { s.develop_of(s.active().unwrap()).unwrap().spots.clone() } diff --git a/crates/ui-egui/src/headless.rs b/crates/ui-egui/src/headless.rs index 8cb91b9b3..45414f29c 100644 --- a/crates/ui-egui/src/headless.rs +++ b/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] diff --git a/crates/ui-egui/src/panels/grid.rs b/crates/ui-egui/src/panels/grid.rs index 7b6d5cf61..1f61a51e1 100644 --- a/crates/ui-egui/src/panels/grid.rs +++ b/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 { + 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 { + 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 { diff --git a/crates/ui-egui/src/shortcuts.rs b/crates/ui-egui/src/shortcuts.rs index 129fb1d9e..3c7cea58c 100644 --- a/crates/ui-egui/src/shortcuts.rs +++ b/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") diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 77ae26e4c..0134017b2 100644 --- a/docs/parity-checklist.md +++ b/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` | |