Walk the grid with the arrow keys, and open with Return

A cull is thousands of decisions. The grid already bound 0-5, P, X and U so
the judgement itself needs no mouse, and then made the photographer reach
for one to move to the next frame — which is most of the gesture, most of
the time. Arrows now move a cursor, shift+arrow extends the selection,
Home/End and PageUp/PageDown cover ground, and Return opens what the cursor
is on.

The cursor is a library ordinal, not a row of the loaded window. The window
is a few screenfuls around wherever the user is looking, so a cursor held as
a row would stop at its edge or keep counting into cells belonging to other
photographs; walking out of the window reloads it around the new position,
exactly as scrolling does. This is the argument the selection already made
by keying on ids rather than rows.

The anchor had that fault for real: it was a window row, so a shift-click
after a scroll extended from whatever image had since drifted into it. It is
an ordinal now, and apply_press takes the window's offset to map between the
two. A range longer than the loaded window truncates to what is loaded,
since selection is by id and an image the catalog has not been asked for has
none.

The pointer and the keyboard share select_row so the two cannot drift apart.
The one deliberate difference: a plain arrow collapses the selection onto
the cursor, where a plain click on an already-selected cell leaves it alone
— that exception exists so a multi-image drag can start from one of its
members, and there is no drag behind a keystroke.

The grid scrolls to the cursor only when the cursor leaves the viewport, and
then by as little as will do it. Reusing the scrub's seek() would put the
cursor's row at the top on every press, which makes a row unreadable as you
walk along it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

ui/dr-ui/src/collections_ui.rs

@@ -59,7 +59,22 @@ pub struct CollectionsController {
     /// Where a shift-click extends from. The last cell *clicked*, not the last
     /// added — extending from the far end of a previous range is not what the
     /// gesture means anywhere else.
+    ///
+    /// An **image ordinal in the library**, not a row of the loaded window.
+    /// The grid is a window over the catalog, so row 7 names a different
+    /// photograph after every scroll: held as a row, an anchor set before a
+    /// scroll described a range from wherever that row had since drifted to.
+    /// Selection is by id for the same reason (see the preamble); this is the
+    /// same argument applied to the one index that has to survive a move.
     anchor: RefCell<Option<usize>>,
+    /// Where the keyboard is, as an image ordinal.
+    ///
+    /// Distinct from the anchor, and it has to be: shift+arrow grows a range
+    /// *from* the anchor *to* the cursor, so the two are the two ends and
+    /// cannot be one field. `None` until the user has taken hold of the grid,
+    /// so the first arrow press starts from what is on screen rather than
+    /// jumping to the top of the library.
+    cursor: std::cell::Cell<Option<usize>>,
     /// Whether the press that began the current gesture carried ctrl or shift.
     ///
     /// A modified click is a *selection* gesture and must not also open the
@@ -166,6 +181,16 @@ impl CollectionsController {
     pub fn clear_selection(&self) {
         self.selection.borrow_mut().clear();
         *self.anchor.borrow_mut() = None;
+        self.cursor.set(None);
+    }
+
+    /// Where the keyboard cursor is, as an image ordinal.
+    pub fn cursor(&self) -> Option<usize> {
+        self.cursor.get()
+    }
+
+    pub fn set_cursor(&self, at: Option<usize>) {
+        self.cursor.set(at);
     }
 }
 
@@ -185,15 +210,21 @@ impl CollectionsController {
 /// A plain press on an image that is *already* selected leaves the selection
 /// alone. That is what makes dragging a multi-selection possible at all — the
 /// press that begins the drag would otherwise collapse the selection to one.
+///
+/// `ids` is the **loaded window**, `offset` where it starts in the library and
+/// `row` a position within it; the anchor is kept as `offset + row`, an
+/// ordinal that still names the same photograph after the window has moved.
 pub fn apply_press(
     selection: &mut BTreeSet<ImageId>,
     anchor: &mut Option<usize>,
     ids: &[ImageId],
+    offset: usize,
     row: usize,
     ctrl: bool,
     shift: bool,
 ) {
     let Some(&id) = ids.get(row) else { return };
+    let here = offset + row;
 
     if shift {
         let Some(from) = *anchor else {
@@ -201,7 +232,7 @@ pub fn apply_press(
             // the anchor, so the *next* shift-click has a range to describe.
             selection.clear();
             selection.insert(id);
-            *anchor = Some(row);
+            *anchor = Some(here);
             return;
         };
 
@@ -219,13 +250,26 @@ pub fn apply_press(
             selection.clear();
         }
 
-        let (lo, hi) = if from <= row {
-            (from, row)
+        let (lo, hi) = if from <= here {
+            (from, here)
         } else {
-            (row, from)
+            (here, from)
         };
-        let hi = hi.min(ids.len().saturating_sub(1));
-        for id in &ids[lo..=hi] {
+
+        // The range is described in ordinals and applied to the window: only
+        // ids that are loaded can be selected, because selection is by id and
+        // an image the catalog has not been asked for has no id here. A range
+        // longer than the loaded window is therefore truncated to it rather
+        // than silently selecting the wrong photographs — which is what
+        // holding the anchor as a window row used to do.
+        //
+        // The slice is always in range and needs no guard: `ids.get(row)`
+        // succeeded, so the window is non-empty and `here` is inside it, and
+        // `here` is one of the two bounds.
+        let last = ids.len() - 1;
+        let lo_row = lo.saturating_sub(offset);
+        let hi_row = hi.saturating_sub(offset).min(last);
+        for id in &ids[lo_row..=hi_row] {
             selection.insert(*id);
         }
         return;
@@ -235,7 +279,7 @@ pub fn apply_press(
         if !selection.remove(&id) {
             selection.insert(id);
         }
-        *anchor = Some(row);
+        *anchor = Some(here);
         return;
     }
 
@@ -243,13 +287,45 @@ pub fn apply_press(
     // follow carries the whole selection, and collapsing it here would make a
     // multi-image drag impossible to start.
     if selection.contains(&id) {
-        *anchor = Some(row);
+        *anchor = Some(here);
         return;
     }
 
     selection.clear();
     selection.insert(id);
-    *anchor = Some(row);
+    *anchor = Some(here);
+}
+
+/// Apply a press and push the result into the grid — the whole of what a
+/// click, or an arrow key, does to the selection.
+///
+/// Shared so the keyboard and the pointer cannot drift: they are the same
+/// gesture reached two ways, and the moment one of them grew its own copy of
+/// the selection policy, "click here, shift+down twice" would stop meaning
+/// what "click here, shift-click there" means.
+pub fn select_row(
+    window: &AppWindow,
+    ctl: &Rc<CollectionsController>,
+    ids: &[ImageId],
+    offset: usize,
+    row: usize,
+    ctrl: bool,
+    shift: bool,
+) {
+    apply_press(
+        &mut ctl.selection.borrow_mut(),
+        &mut ctl.anchor.borrow_mut(),
+        ids,
+        offset,
+        row,
+        ctrl,
+        shift,
+    );
+    // The cursor follows the press, so an arrow key after a click continues
+    // from the cell that was clicked rather than from wherever the keyboard
+    // was last.
+    ctl.set_cursor(Some(offset + row));
+    sync_selection(window, ctl, ids);
 }
 
 /// Rebuild the sidebar from the catalog.
@@ -973,15 +1049,11 @@ pub fn wire<S, R, C>(
             // a selection and must not also navigate to develop.
             ctl.modified_press.set(ctrl_held || shift_held);
 
-            apply_press(
-                &mut ctl.selection.borrow_mut(),
-                &mut ctl.anchor.borrow_mut(),
-                &ids,
-                row as usize,
-                ctrl_held,
-                shift_held,
-            );
-            sync_selection(&w, &ctl, &ids);
+            // Where the window starts, so the press is recorded as the ordinal
+            // it is rather than as a row that stops meaning this photograph on
+            // the next scroll.
+            let offset = w.get_library_offset().max(0) as usize;
+            select_row(&w, &ctl, &ids, offset, row as usize, ctrl_held, shift_held);
         });
     }
 
@@ -1753,8 +1825,8 @@ mod tests {
         let mut sel = BTreeSet::new();
         let mut anchor = None;
 
-        apply_press(&mut sel, &mut anchor, &all, 0, false, false);
-        apply_press(&mut sel, &mut anchor, &all, 2, false, false);
+        apply_press(&mut sel, &mut anchor, &all, 0, 0, false, false);
+        apply_press(&mut sel, &mut anchor, &all, 0, 2, false, false);
 
         assert_eq!(sel.iter().copied().collect::<Vec<_>>(), vec![ImageId(3)]);
     }
@@ -1765,12 +1837,12 @@ mod tests {
         let mut sel = BTreeSet::new();
         let mut anchor = None;
 
-        apply_press(&mut sel, &mut anchor, &all, 0, false, false);
-        apply_press(&mut sel, &mut anchor, &all, 3, true, false);
+        apply_press(&mut sel, &mut anchor, &all, 0, 0, false, false);
+        apply_press(&mut sel, &mut anchor, &all, 0, 3, true, false);
         assert_eq!(sel.len(), 2);
 
         // Toggling: a second ctrl-press on the same cell takes it out again.
-        apply_press(&mut sel, &mut anchor, &all, 3, true, false);
+        apply_press(&mut sel, &mut anchor, &all, 0, 3, true, false);
         assert_eq!(sel.iter().copied().collect::<Vec<_>>(), vec![ImageId(1)]);
     }
 
@@ -1780,8 +1852,8 @@ mod tests {
         let mut sel = BTreeSet::new();
         let mut anchor = None;
 
-        apply_press(&mut sel, &mut anchor, &all, 2, false, false);
-        apply_press(&mut sel, &mut anchor, &all, 6, false, true);
+        apply_press(&mut sel, &mut anchor, &all, 0, 2, false, false);
+        apply_press(&mut sel, &mut anchor, &all, 0, 6, false, true);
 
         assert_eq!(sel.len(), 5, "rows 2..=6 inclusive");
         assert!(sel.contains(&ImageId(3)) && sel.contains(&ImageId(7)));
@@ -1793,8 +1865,8 @@ mod tests {
         let mut sel = BTreeSet::new();
         let mut anchor = None;
 
-        apply_press(&mut sel, &mut anchor, &all, 6, false, false);
-        apply_press(&mut sel, &mut anchor, &all, 2, false, true);
+        apply_press(&mut sel, &mut anchor, &all, 0, 6, false, false);
+        apply_press(&mut sel, &mut anchor, &all, 0, 2, false, true);
         assert_eq!(sel.len(), 5);
     }
 
@@ -1807,12 +1879,12 @@ mod tests {
         let mut sel = BTreeSet::new();
         let mut anchor = None;
 
-        apply_press(&mut sel, &mut anchor, &all, 5, false, false);
-        apply_press(&mut sel, &mut anchor, &all, 15, false, true);
+        apply_press(&mut sel, &mut anchor, &all, 0, 5, false, false);
+        apply_press(&mut sel, &mut anchor, &all, 0, 15, false, true);
         assert_eq!(sel.len(), 11, "rows 5..=15");
 
         // Corrected to a shorter range from the same anchor.
-        apply_press(&mut sel, &mut anchor, &all, 8, false, true);
+        apply_press(&mut sel, &mut anchor, &all, 0, 8, false, true);
         assert_eq!(sel.len(), 4, "rows 5..=8, and nothing from the first range");
         assert!(!sel.contains(&ImageId(16)), "row 15 is no longer selected");
     }
@@ -1825,9 +1897,9 @@ mod tests {
         let mut sel = BTreeSet::new();
         let mut anchor = None;
 
-        apply_press(&mut sel, &mut anchor, &all, 10, false, false);
-        apply_press(&mut sel, &mut anchor, &all, 14, false, true);
-        apply_press(&mut sel, &mut anchor, &all, 12, false, true);
+        apply_press(&mut sel, &mut anchor, &all, 0, 10, false, false);
+        apply_press(&mut sel, &mut anchor, &all, 0, 14, false, true);
+        apply_press(&mut sel, &mut anchor, &all, 0, 12, false, true);
 
         assert_eq!(anchor, Some(10));
         assert_eq!(sel.len(), 3, "rows 10..=12");
@@ -1841,13 +1913,13 @@ mod tests {
         let mut sel = BTreeSet::new();
         let mut anchor = None;
 
-        apply_press(&mut sel, &mut anchor, &all, 0, false, false);
-        apply_press(&mut sel, &mut anchor, &all, 2, false, true);
+        apply_press(&mut sel, &mut anchor, &all, 0, 0, false, false);
+        apply_press(&mut sel, &mut anchor, &all, 0, 2, false, true);
         assert_eq!(sel.len(), 3);
 
         // A new anchor by ctrl-click, then a ctrl+shift range from it.
-        apply_press(&mut sel, &mut anchor, &all, 10, true, false);
-        apply_press(&mut sel, &mut anchor, &all, 12, true, true);
+        apply_press(&mut sel, &mut anchor, &all, 0, 10, true, false);
+        apply_press(&mut sel, &mut anchor, &all, 0, 12, true, true);
 
         assert_eq!(sel.len(), 6, "rows 0..=2 and 10..=12");
         assert!(sel.contains(&ImageId(1)) && sel.contains(&ImageId(13)));
@@ -1861,13 +1933,13 @@ mod tests {
         let mut sel = BTreeSet::new();
         let mut anchor = None;
 
-        apply_press(&mut sel, &mut anchor, &all, 0, false, false);
-        apply_press(&mut sel, &mut anchor, &all, 1, true, false);
-        apply_press(&mut sel, &mut anchor, &all, 2, true, false);
+        apply_press(&mut sel, &mut anchor, &all, 0, 0, false, false);
+        apply_press(&mut sel, &mut anchor, &all, 0, 1, true, false);
+        apply_press(&mut sel, &mut anchor, &all, 0, 2, true, false);
         assert_eq!(sel.len(), 3);
 
         // Pressing one of the three to begin a drag.
-        apply_press(&mut sel, &mut anchor, &all, 1, false, false);
+        apply_press(&mut sel, &mut anchor, &all, 0, 1, false, false);
         assert_eq!(sel.len(), 3, "the selection survived the press");
     }
 
@@ -1878,17 +1950,84 @@ mod tests {
         let mut sel = BTreeSet::new();
         let mut anchor = None;
 
-        apply_press(&mut sel, &mut anchor, &all, 99, false, false);
+        apply_press(&mut sel, &mut anchor, &all, 0, 99, false, false);
         assert!(sel.is_empty());
     }
 
+    #[test]
+    fn an_anchor_survives_the_window_moving_under_it() {
+        // The grid is a window over the catalog, and the window moves whenever
+        // the user scrolls. Held as a row, an anchor set before a scroll
+        // described a range from whatever photograph had since drifted into
+        // that row — so shift-clicking after scrolling selected a run the user
+        // never pointed at, silently and with no way to tell.
+        let all = ids(20);
+        let first: Vec<_> = all[0..10].to_vec();
+        let later: Vec<_> = all[4..14].to_vec();
+
+        let mut sel = BTreeSet::new();
+        let mut anchor = None;
+
+        // Anchor on the sixth image, in a window starting at the beginning.
+        apply_press(&mut sel, &mut anchor, &first, 0, 5, false, false);
+        assert_eq!(anchor, Some(5), "the anchor is an ordinal, not a row");
+
+        // The user scrolls — the window now starts four images in — and
+        // shift-clicks the image at ordinal 10.
+        apply_press(&mut sel, &mut anchor, &later, 4, 6, false, true);
+
+        assert_eq!(sel.len(), 6, "ordinals 5..=10");
+        assert!(sel.contains(&ImageId(6)), "the anchored image is still in");
+        assert!(sel.contains(&ImageId(11)), "up to the one shift-clicked");
+        assert!(!sel.contains(&ImageId(5)), "and nothing before the anchor");
+    }
+
+    #[test]
+    fn a_range_reaching_outside_the_window_selects_what_is_loaded() {
+        // Selection is by id, so a range can only cover images the window
+        // holds. Truncating is the honest answer; the alternative — indexing
+        // from the window's start as though it were the library's — selects
+        // the wrong photographs and looks like it worked.
+        let all = ids(30);
+        let window: Vec<_> = all[10..20].to_vec();
+
+        let mut sel = BTreeSet::new();
+        let mut anchor = Some(0);
+
+        apply_press(&mut sel, &mut anchor, &window, 10, 3, false, true);
+
+        assert_eq!(sel.len(), 4, "ordinals 10..=13, the loaded part of 0..=13");
+        assert!(sel.contains(&ImageId(11)) && sel.contains(&ImageId(14)));
+    }
+
+    #[test]
+    fn a_range_running_off_the_far_end_stops_at_the_window() {
+        // The mirror of the case above, with the anchor *ahead* of the press
+        // instead of behind it. Both directions truncate to what is loaded,
+        // and neither may wrap round to the other end of the window.
+        let all = ids(30);
+        let window: Vec<_> = all[0..5].to_vec();
+
+        let mut sel = BTreeSet::new();
+        let mut anchor = Some(25);
+
+        apply_press(&mut sel, &mut anchor, &window, 0, 2, false, true);
+
+        assert_eq!(sel.len(), 3, "ordinals 2..=4, the loaded part of 2..=25");
+        assert!(sel.contains(&ImageId(3)) && sel.contains(&ImageId(5)));
+        assert!(
+            !sel.contains(&ImageId(1)),
+            "nothing before the pressed cell"
+        );
+    }
+
     #[test]
     fn shift_without_an_anchor_selects_just_the_one() {
         let all = ids(5);
         let mut sel = BTreeSet::new();
         let mut anchor = None;
 
-        apply_press(&mut sel, &mut anchor, &all, 3, false, true);
+        apply_press(&mut sel, &mut anchor, &all, 0, 3, false, true);
         assert_eq!(sel.iter().copied().collect::<Vec<_>>(), vec![ImageId(4)]);
     }
 

ui/dr-ui/src/library_ui.rs

@@ -2252,6 +2252,101 @@ fn to_slint_image(width: u32, height: u32, rgba: &[u8]) -> slint::Image {
     slint::Image::from_rgba8(buf)
 }
 
+/// Walk the keyboard cursor through the library — the arrow keys.
+///
+/// **The cursor is a library ordinal, not a row of the loaded window.** That is
+/// what lets it walk past the window's edge: the window is a few screenfuls
+/// around wherever the user is looking, and a cursor held as a row would stop
+/// at its end or, worse, keep counting into cells belonging to different
+/// photographs. Moving out of the window reloads it around the new position,
+/// which is the same thing scrolling does.
+///
+/// The grid supplies the step, because how far "down" is depends on how many
+/// columns the window happens to be showing, and only the grid knows that. It
+/// does not clamp: `Home` and `End` arrive as a step longer than the library
+/// and are clamped here, where the total is known.
+fn move_cursor(
+    window: &AppWindow,
+    ctl: &Rc<LibraryController>,
+    coll: &Rc<crate::collections_ui::CollectionsController>,
+    delta: i32,
+    extend: bool,
+) {
+    let total = window.get_library_total().max(0) as usize;
+    if total == 0 {
+        return;
+    }
+
+    let Some(from) = coll.cursor() else {
+        // The first press takes hold of the grid rather than moving in it. A
+        // key that jumped to image zero would lose wherever the user had
+        // scrolled to, and one that started from a cell off the top of the
+        // screen would appear to do nothing but scroll.
+        //
+        // The first *visible* ordinal, not the window's start: the loaded
+        // window deliberately begins a quarter of a screen above the view, so
+        // its first cell is one the user cannot see.
+        let at = ctl.resume_at.get().min(total - 1);
+        place_cursor(window, ctl, coll, at, false);
+        return;
+    };
+
+    // Saturating in `isize`, so a `Home` expressed as minus the library's
+    // length does not wrap round to the end.
+    let next = (from as isize)
+        .saturating_add(delta as isize)
+        .clamp(0, total as isize - 1) as usize;
+    if next == from {
+        // Already at the end being pressed toward. Nothing to move, and
+        // reloading the window would be a visible jerk for no movement.
+        return;
+    }
+    place_cursor(window, ctl, coll, next, extend);
+}
+
+/// Put the cursor on one image, bringing the window with it.
+fn place_cursor(
+    window: &AppWindow,
+    ctl: &Rc<LibraryController>,
+    coll: &Rc<crate::collections_ui::CollectionsController>,
+    at: usize,
+    extend: bool,
+) {
+    // Bring the window to the cursor if it has walked out of it. Centred a
+    // quarter in, exactly as a scroll does it, so continuing in the same
+    // direction has loaded cells to move into rather than another reload on
+    // the very next press.
+    let loaded = window.get_library_cells().row_count();
+    let offset = *ctl.offset.borrow();
+    if at < offset || at >= offset + loaded {
+        let size = *ctl.window.borrow();
+        *ctl.offset.borrow_mut() = at.saturating_sub(size / 4);
+        load_window(window, ctl);
+    }
+
+    // Re-read: `load_window` clamps the offset against the library's end, so
+    // the window may not start where it was asked to.
+    let offset = *ctl.offset.borrow();
+    let ids = ctl.visible_ids();
+    let Some(row) = at.checked_sub(offset).filter(|r| *r < ids.len()) else {
+        // The window could not be brought to the cursor — an empty or
+        // shrinking library. Leaving the cursor where it was is better than
+        // pointing it at nothing.
+        return;
+    };
+
+    if !extend {
+        // An arrow key collapses the selection onto the cursor. A *click* on
+        // an already-selected cell deliberately leaves the selection alone —
+        // that exception exists so a multi-image drag can start from one of
+        // its members — and there is no drag behind a keystroke.
+        coll.clear_selection();
+    }
+
+    crate::collections_ui::select_row(window, coll, &ids, offset, row, false, extend);
+    window.set_library_cursor(at as i32);
+}
+
 /// Connect the grid's callbacks.
 pub fn wire<F>(
     window: &AppWindow,
@@ -2261,10 +2356,15 @@ pub fn wire<F>(
 ) where
     F: Fn(String) + 'static,
 {
+    // Shared rather than moved: a click and `Return` both open an image, and
+    // they are two callbacks.
+    let on_open_image = Rc::new(on_open_image);
+
     {
         let weak = window.as_weak();
         let ctl = ctl.clone();
         let coll_for_click = coll_ctl.clone();
+        let on_open_image = on_open_image.clone();
         window.on_library_cell_clicked(move |i| {
             // A ctrl- or shift-click is a selection gesture. Opening the image
             // too would throw the user out of the grid mid-selection.
@@ -2283,6 +2383,47 @@ pub fn wire<F>(
         });
     }
 
+    // --- the keyboard cursor (FR-CULL-4) -----------------------------------
+    //
+    // Walking the grid with the arrows, and opening with `Return`. Together
+    // with the judgement keys already bound in the grid, this is what makes a
+    // culling pass a keyboard job: move, rate, move, open the doubtful one,
+    // come back. A cull is thousands of decisions, and reaching for the mouse
+    // between each of them is the difference between an hour and an evening.
+    {
+        let weak = window.as_weak();
+        let ctl = ctl.clone();
+        let coll = coll_ctl.clone();
+        window.on_library_move_cursor(move |delta, extend| {
+            let Some(w) = weak.upgrade() else { return };
+            move_cursor(&w, &ctl, &coll, delta, extend);
+        });
+    }
+
+    {
+        let weak = window.as_weak();
+        let ctl = ctl.clone();
+        let coll = coll_ctl.clone();
+        let on_open = on_open_image.clone();
+        window.on_library_open_cursor(move || {
+            let Some(w) = weak.upgrade() else { return };
+            // The cursor is a library ordinal and `paths` is the loaded
+            // window, so the row is the difference. A cursor outside the
+            // window cannot happen — moving it loads the window around it —
+            // but a shrinking library could leave one behind, and opening the
+            // wrong photograph is worse than opening none.
+            let Some(cursor) = coll.cursor() else { return };
+            let offset = *ctl.offset.borrow();
+            let path = cursor
+                .checked_sub(offset)
+                .and_then(|row| ctl.paths.borrow().get(row).cloned());
+            if let Some(path) = path {
+                w.set_show_library(false);
+                on_open(path);
+            }
+        });
+    }
+
     // Ctrl+wheel or pinch over the grid resizes the cells.
     //
     // Geometric steps rather than fixed pixels: the same gesture should feel