From 9de37bed814b45d0497971529d67b7b90e84986a Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Fri, 25 Sep 2026 22:08:47 -0400 Subject: [PATCH] Hold develop's judgement keys to the open photograph, and to staying on it Rating and flagging in develop (FR-UI-5, amended 2026-09-19) landed with the keyboard audit: 0-5, P, X and U in develop's key scope, stars and Pick/Reject in its top bar, and flag and stars on the roll's cells. Two of the amendment's rules were held by nothing. The keys must judge the photograph on screen and never a selection left behind in the grid, and judging must not move on to the next frame, which is culling's auto-advance and not develop's. The rating, flag and label callbacks that take a row each spelled the row-to-image lookup themselves. It is now one function, `image_at_row`, which answers None for a negative row as well as one past the end: the roll passes -1 when the open photograph is outside the loaded window, and the right answer then is to judge nothing. A test says so. The key bindings live in Slint, where no test can press them, so a second test reads develop's handler as the gestures gate and the canvas-order test do, and checks that each judgement key calls the row callback on `library-roll-current` and that none of them steps the roll or the cursor. The writes themselves go through `apply_judgement`, the grid's own path: one catalog statement, then the sidecar and XMP writes behind it. --- ui/dr-ui/src/library_ui/ratings_keywords.rs | 97 +++++++++++++++++---- 1 file changed, 79 insertions(+), 18 deletions(-) diff --git a/ui/dr-ui/src/library_ui/ratings_keywords.rs b/ui/dr-ui/src/library_ui/ratings_keywords.rs index d3571ec..20074c2 100644 --- a/ui/dr-ui/src/library_ui/ratings_keywords.rs +++ b/ui/dr-ui/src/library_ui/ratings_keywords.rs @@ -454,6 +454,20 @@ fn apply_judgement( start_xmp_writes(window, ctl, images); } +/// TRACES: FR-UI-5 | FR-CULL-4 +/// The photograph at one row of the loaded window, for the gestures that +/// name a single photograph: a star clicked on a cell, and develop's keys +/// and top bar, which name the open one by its roll row. +/// +/// `None` for a negative row as well as one past the end. Develop passes +/// `-1` when the open photograph is not in the window, and the answer then +/// is to judge nothing — never to fall back on the grid's selection, which +/// is not what is on screen. +fn image_at_row(ids: &[i64], row: i32) -> Option { + let row = usize::try_from(row).ok()?; + ids.get(row).map(|id| dr_types::ImageId(*id as u64)) +} + /// What the status line says about a judgement that just landed. fn judgement_summary(n: usize, rating: Option, flag: Option) -> String { let what = match (rating, flag) { @@ -975,12 +989,9 @@ pub(super) fn wire_ratings_and_flags( .global::() .on_library_cell_rated(move |row, stars| { let Some(w) = weak.upgrade() else { return }; - let id = ctl - .image_ids - .borrow() - .get(row as usize) - .map(|id| dr_types::ImageId(*id as u64)); - let Some(id) = id else { return }; + let Some(id) = image_at_row(&ctl.image_ids.borrow(), row) else { + return; + }; apply_judgement(&w, &ctl, &[id], Some(stars.clamp(0, 5) as u8), None); }); @@ -998,12 +1009,9 @@ pub(super) fn wire_ratings_and_flags( .global::() .on_library_cell_flagged(move |row, flag| { let Some(w) = weak.upgrade() else { return }; - let id = ctl - .image_ids - .borrow() - .get(row as usize) - .map(|id| dr_types::ImageId(*id as u64)); - let Some(id) = id else { return }; + let Some(id) = image_at_row(&ctl.image_ids.borrow(), row) else { + return; + }; apply_judgement(&w, &ctl, &[id], None, Some(flag_from_code(flag))); }); @@ -1158,12 +1166,9 @@ pub(super) fn wire_ratings_and_flags( let Some(gesture) = LabelGesture::from_code(code, toggle) else { return; }; - let id = ctl - .image_ids - .borrow() - .get(row as usize) - .map(|id| dr_types::ImageId(*id as u64)); - let Some(id) = id else { return }; + let Some(id) = image_at_row(&ctl.image_ids.borrow(), row) else { + return; + }; apply_label(&w, &ctl, &[id], gesture); }); } @@ -1237,6 +1242,62 @@ pub(super) fn wire_keywords( mod tests { use super::*; + // --- judging the photograph open in develop (FR-UI-5) ----------------- + + /// TRACES: FR-UI-5 + #[test] + fn a_row_names_one_photograph_and_no_row_names_none() { + let ids = [11, 22, 33]; + assert_eq!(image_at_row(&ids, 1), Some(dr_types::ImageId(22))); + assert_eq!(image_at_row(&ids, 3), None); + // The open photograph outside the loaded window: nothing is judged, + // rather than whatever the grid last had selected. + assert_eq!(image_at_row(&ids, -1), None); + assert_eq!(image_at_row(&[], 0), None); + } + + /// The `if (Keys.chord(event) == "") { … }` line of develop's + /// key handler that binds `key`. + fn develop_binding(key: &str) -> &'static str { + let app = include_str!("../../ui/app.slint"); + let scope = &app[app + .find("// KEYMAP: Develop\n key-pressed(event)") + .expect("app.slint no longer has develop's key-pressed handler")..]; + let needle = format!("if (Keys.chord(event) == \"{key}\") {{"); + let at = scope + .find(&needle) + .unwrap_or_else(|| panic!("develop does not bind `{key}`")); + scope[at..].lines().next().unwrap_or("") + } + + /// TRACES: FR-UI-5 | FR-CULL-4 + /// 0–5, P, X and U in develop judge the open photograph — its roll row — + /// and do nothing else: no step to the next frame, which belongs to + /// culling's auto-advance and not to the view where one frame is worked on. + #[test] + fn develop_judges_the_open_photograph_and_stays_on_it() { + for (key, call) in [ + ("0", "library-cell-rated(Library.library-roll-current, 0)"), + ("1", "library-cell-rated(Library.library-roll-current, 1)"), + ("2", "library-cell-rated(Library.library-roll-current, 2)"), + ("3", "library-cell-rated(Library.library-roll-current, 3)"), + ("4", "library-cell-rated(Library.library-roll-current, 4)"), + ("5", "library-cell-rated(Library.library-roll-current, 5)"), + ("P", "library-cell-flagged(Library.library-roll-current, 1)"), + ("X", "library-cell-flagged(Library.library-roll-current, 2)"), + ("U", "library-cell-flagged(Library.library-roll-current, 0)"), + ] { + let line = develop_binding(key); + assert!(line.contains(call), "`{key}` in develop: {line}"); + for step in ["step-photo", "roll-pick", "move-cursor"] { + assert!( + !line.contains(step), + "`{key}` in develop also steps: {line}" + ); + } + } + } + // --- what a judgement key reaches (FR-UI-5) ---------------------------- #[test]