diff --git a/ui/dr-ui/src/library_ui.rs b/ui/dr-ui/src/library_ui.rs index 38a6451..e8b2dc8 100644 --- a/ui/dr-ui/src/library_ui.rs +++ b/ui/dr-ui/src/library_ui.rs @@ -22,7 +22,7 @@ use dr_types::FormatFilter; use slint::{ComponentHandle, Model as _}; use crate::library::{self, ScanMessage, ThumbnailMessage}; -use crate::{AppWindow, LibraryCell, TimelineBar}; +use crate::{AppWindow, KeywordRow, LibraryCell, TimelineBar}; /// Window size before the grid has reported its geometry. /// @@ -2117,6 +2117,170 @@ fn refresh_rating_counts(window: &AppWindow, catalog: &Catalog) { window.set_library_local_count(library::local_original_count(catalog).unwrap_or(0) as i32); } +// --- keywords (FR-CAT-5, FR-CAT-6) --------------------------------------- +// +// `dr_catalog::keywords` owns the data rules — the vocabulary, the many-to-many +// join, what a rename does to the assignments. This part owns the *interaction*: +// which photographs the sheet is acting on, and keeping what it draws honest +// about what actually landed. + +/// Redraw the keywording sheet against whatever is selected now. +/// +/// Called when the sheet opens and after every assignment, rather than on every +/// selection change: the selection moves on each arrow key and the sheet is shut +/// for almost all of them, so computing coverage over a forty-image selection +/// on each one would be work nobody is looking at. +/// +/// Re-read from the catalog rather than patched in place after a write. A word +/// applied to a selection that partly already had it moves from "3 of 12" to +/// "12 of 12", and a model updated by hand would have to reproduce the rule +/// that decides that — which is exactly the rule the catalog has just applied. +fn refresh_keywords(window: &AppWindow, ctl: &Rc, images: &[dr_types::ImageId]) { + let borrow = ctl.catalog.borrow(); + let Some(catalog) = borrow.as_ref() else { + return; + }; + + let rows = match dr_catalog::keywords::for_images(catalog.connection(), images) { + Ok(rows) => rows, + Err(e) => { + // The grid is entirely usable without the sheet, so this is logged + // rather than surfaced: a keyword read that failed must not put an + // error banner over a library the user is browsing. + log::debug!("reading keywords: {e}"); + return; + } + }; + + let model: Vec = rows + .into_iter() + .map(|row| KeywordRow { + id: row.keyword.id.0 as i32, + name: row.keyword.name.into(), + coverage: match row.coverage { + dr_catalog::Coverage::None => 0, + dr_catalog::Coverage::Some => 1, + dr_catalog::Coverage::All => 2, + }, + selected_count: row.selected_count as i32, + image_count: row.keyword.image_count as i32, + }) + .collect(); + + window.set_library_keywords(slint::ModelRc::new(slint::VecModel::from(model))); +} + +/// Put a keyword on the selection, or take it off. +/// +/// # Why this does not write a sidecar +/// +/// Every other judgement in this file — a star, a flag — is written to the +/// catalog and then queued to the image's sidecar, because the sidecar is what +/// makes it survive a catalog rebuild (ARCH §6.12). A keyword has no place in +/// the sidecar format yet: `dr_pipeline::sidecar::Version` carries `rating` and +/// `flag` and nothing else that is not an edit-graph parameter. +/// +/// So a keyword is, for now, catalog state that reaches the user's other +/// devices through the *catalog* merge ([`dr_catalog::merge`]) rather than +/// through the sidecar. That is a real limitation and not a silent one: a +/// deleted catalog loses keywords where it would keep ratings, until the +/// sidecar gains a `dc:subject` field (FR-CAT-13) and this grows the same +/// queued write the stars have. +fn apply_keyword(window: &AppWindow, ctl: &Rc, word: &str, assigning: bool) { + let Some(coll) = ctl.coll_ctl.borrow().as_ref().and_then(|c| c.upgrade()) else { + return; + }; + let images = coll.selected(); + + // The word as it will be *stored*, resolved before anything is written. + // The status line below quotes it back, and quoting what was typed would + // report a leading space the catalog is about to drop — leaving the user to + // wonder whether it mattered. + // + // This is also where a blank keyword is caught, which is why it happens + // before the selection check: "you typed nothing" is a better answer than + // "select an image first" to someone who pressed return on an empty field. + let word = match dr_catalog::keywords::normalise(word) { + Ok(word) => word, + Err(e) => { + // `BadName` carries text written to be read by the user rather than + // by a developer, so it is shown as it is. + window.set_library_error(format!("{e}").into()); + return; + } + }; + + // Assigning with nothing selected still means something — it puts the word + // in the vocabulary, ready for the photographs it was typed for — so only + // the removal half needs a selection to act on. + if images.is_empty() && !assigning { + window.set_library_status("Select an image first".into()); + return; + } + + let outcome = { + let borrow = ctl.catalog.borrow(); + let Some(catalog) = borrow.as_ref() else { + return; + }; + let conn = catalog.connection(); + if assigning { + dr_catalog::keywords::assign(conn, &images, &word) + } else { + dr_catalog::keywords::unassign(conn, &images, &word) + } + }; + + let n = match outcome { + Ok(n) => n, + Err(e) => { + window.set_library_error(format!("{e}").into()); + return; + } + }; + + window.set_library_error(slint::SharedString::new()); + window.set_library_status(keyword_summary(&word, n, images.len(), assigning).into()); + refresh_keywords(window, ctl, &images); + + // A filtered grid may no longer hold what was just keyworded — taking + // "puffin" off an image while showing only puffins means it belongs + // elsewhere now. The same reasoning as a rating that falls below the star + // filter. + if !ctl.filter.borrow().is_unfiltered() { + load_window(window, ctl); + } +} + +/// What the status line says about a keyword that just landed. +/// +/// The honest count, not the requested one: "added to 3 of 12" is what +/// happened when nine of them already carried the word, and a message that +/// claimed twelve would be teaching the user that the counts are decorative. +fn keyword_summary(word: &str, changed: usize, selected: usize, assigning: bool) -> String { + if selected == 0 { + return format!("Added “{word}” to the keyword list"); + } + let verb = if assigning { "Added" } else { "Removed" }; + let preposition = if assigning { "to" } else { "from" }; + if changed == 0 { + return if assigning { + format!("Every selected photograph already had “{word}”") + } else { + format!("None of the selected photographs had “{word}”") + }; + } + if changed == selected { + let what = if selected == 1 { + "1 photograph".to_string() + } else { + format!("{selected} photographs") + }; + return format!("{verb} “{word}” {preposition} {what}"); + } + format!("{verb} “{word}” {preposition} {changed} of {selected}") +} + /// Apply a judgement to a set of images: catalog first, then sidecars. /// /// # Order matters @@ -4152,6 +4316,40 @@ pub fn wire( }); } + // --- keywords (FR-CAT-5, FR-CAT-6) ------------------------------------ + // + // Three callbacks and no state of their own: the sheet's open/shut is local + // to the `.slint` file, and what a keyword applies to is the grid selection + // the collections controller already owns. A second copy of either here is + // a second thing that can disagree with the first. + { + let weak = window.as_weak(); + let ctl = ctl.clone(); + let coll_for_keywords = coll_ctl.clone(); + window.on_library_keywords_opened(move || { + let Some(w) = weak.upgrade() else { return }; + refresh_keywords(&w, &ctl, &coll_for_keywords.selected()); + }); + } + + { + let weak = window.as_weak(); + let ctl = ctl.clone(); + window.on_library_assign_keyword(move |word| { + let Some(w) = weak.upgrade() else { return }; + apply_keyword(&w, &ctl, word.as_str(), true); + }); + } + + { + let weak = window.as_weak(); + let ctl = ctl.clone(); + window.on_library_unassign_keyword(move |word| { + let Some(w) = weak.upgrade() else { return }; + apply_keyword(&w, &ctl, word.as_str(), false); + }); + } + // --- the filter bar --------------------------------------------------- // // Each of these narrows what the grid *queries*, so all three reset the @@ -5017,6 +5215,71 @@ mod tests { assert_eq!(paths, vec!["c.CR2", "a.CR2"]); } + // --- what the status line says about a keyword (FR-CAT-5) ------------- + // + // Split out from the callback for the same reason `decide_drop` is: the + // sheet cannot be driven from a test, and this is the part that can + // actually mislead someone. + + /// TRACES: FR-CAT-5 + #[test] + fn a_partly_applied_keyword_reports_the_honest_count() { + // Nine of the twelve already had it. Claiming twelve is how a user + // learns that the counts are decorative. + assert_eq!( + keyword_summary("puffin", 3, 12, true), + "Added “puffin” to 3 of 12" + ); + } + + /// TRACES: FR-CAT-5 + #[test] + fn a_keyword_that_changed_nothing_says_so_rather_than_claiming_success() { + assert_eq!( + keyword_summary("puffin", 0, 12, true), + "Every selected photograph already had “puffin”" + ); + assert_eq!( + keyword_summary("puffin", 0, 12, false), + "None of the selected photographs had “puffin”" + ); + } + + /// TRACES: FR-CAT-5 + #[test] + fn one_photograph_is_singular() { + // "Added to 1 photographs" is the kind of small wrongness that makes + // the rest of the interface look unfinished. + assert_eq!( + keyword_summary("puffin", 1, 1, true), + "Added “puffin” to 1 photograph" + ); + assert_eq!( + keyword_summary("puffin", 2, 2, true), + "Added “puffin” to 2 photographs" + ); + } + + /// TRACES: FR-CAT-5 + #[test] + fn removing_a_keyword_reads_as_removal() { + assert_eq!( + keyword_summary("blurry", 4, 4, false), + "Removed “blurry” from 4 photographs" + ); + } + + /// TRACES: FR-CAT-5 + #[test] + fn typing_a_word_with_nothing_selected_says_what_it_did_do() { + // It builds the vocabulary, which is a legitimate thing to do ahead of + // a shoot — so it must not report itself as having keyworded nothing. + assert_eq!( + keyword_summary("puffin", 0, 0, true), + "Added “puffin” to the keyword list" + ); + } + /// TRACES: FR-EXP-7 #[test] fn a_selection_outside_the_loaded_window_still_resolves() { diff --git a/ui/dr-ui/ui/app.slint b/ui/dr-ui/ui/app.slint index 8460f8e..fff578a 100644 --- a/ui/dr-ui/ui/app.slint +++ b/ui/dr-ui/ui/app.slint @@ -2,7 +2,7 @@ import { Theme } from "theme.slint"; import { AdjustPanel, GeometryPanel, GroupStrip, ParamRow, TransferPanel } from "adjust.slint"; import { MaskPanel, MaskRow, SubjectRow } from "masks.slint"; import { LaunchScreen } from "launch.slint"; -import { LibraryGrid, LibraryCell, TimelineBar, PhotoRoll } from "library.slint"; +import { LibraryGrid, LibraryCell, TimelineBar, PhotoRoll, KeywordRow } from "library.slint"; import { Button, PanelHeading, Label, Value, Caption, Panel, EmptyState, ProgressBar, ActivityRow } from "widgets.slint"; import { CollectionsPanel, CollectionRow, OfflinePrompt } from "collections.slint"; import { HistogramPanel, HistogramView } from "histogram.slint"; @@ -572,6 +572,25 @@ export component AppWindow inherits Window { /// the target's id, and whether to take the images out of the collection /// currently being shown. callback library-file-in-collection(int, bool); + /// TRACES: FR-CAT-5 | FR-CAT-6 + /// Keywording the grid's selection. The catalog has been searchable by + /// keyword since it existed and there was nowhere to type one; this is it. + /// + /// The vocabulary arrives already answered against the selection — each row + /// says how many of the selected photographs carry that word — because only + /// Rust knows what is selected, and a `.slint` file counting it would need + /// the selection as a second model that could disagree with the first. + in property <[KeywordRow]> library-keywords; + /// The sheet is opening: recompute the rows against the selection as it + /// stands now. Pulled rather than pushed, because the selection changes on + /// every arrow key and the sheet is shut for almost all of them. + callback library-keywords-opened(); + /// Put a keyword on the selection, creating it if it is new. By name, so a + /// word typed into the field and a word tapped in the list are one path. + callback library-assign-keyword(string); + /// Take a keyword off the selection. Never deletes the keyword itself — + /// it stays in the vocabulary and on every other photograph that carries it. + callback library-unassign-keyword(string); /// TRACES: FR-UI-2 /// Whether a tap in the grid selects rather than opens, and the button /// that turns it on. The long press does the same thing without it. @@ -1229,6 +1248,10 @@ in property panel-visible: true; file-in-collection(id, moves) => { root.library-file-in-collection(id, moves); } + keywords: root.library-keywords; + keywords-opened() => { root.library-keywords-opened(); } + assign-keyword(word) => { root.library-assign-keyword(word); } + unassign-keyword(word) => { root.library-unassign-keyword(word); } cursor: root.library-cursor; move-cursor(delta, extend) => { root.library-move-cursor(delta, extend); diff --git a/ui/dr-ui/ui/library.slint b/ui/dr-ui/ui/library.slint index 5089a45..4e1a50f 100644 --- a/ui/dr-ui/ui/library.slint +++ b/ui/dr-ui/ui/library.slint @@ -9,12 +9,38 @@ // must not look identical (FR-NC-6c). import { Theme } from "theme.slint"; -import { Button, IconButton, Label, Value, Caption, EmptyState, FilterChip, ProgressBar, Icon } from "widgets.slint"; +import { Button, IconButton, Label, Value, Caption, EmptyState, FilterChip, ProgressBar, Icon, Field } from "widgets.slint"; // The filing sheet lists the same rows the sidebar draws, from the same model: // two lists of collections that could disagree about what exists is one list // too many. import { CollectionRow } from "collections.slint"; +// TRACES: FR-CAT-5 +// One keyword in the keywording sheet, already answered against the selection. +// +// The three-way `coverage` is the whole reason this is a struct rather than a +// list of strings. Applying a word to forty photographs where thirty already +// carry it must not look like applying it to forty that carry none, and +// removing one that only some of them carry must not silently claim to have +// taken it off all forty. Rust computes it, because only Rust knows how big the +// selection is and how many of it each word covers. +export struct KeywordRow { + // Row id in `keyword_terms`, or 0 for a word an image carries that the + // vocabulary has no identity for yet. The sheet acts on `name`, never on + // this, so a 0 costs nothing — it is here so a future rename gesture has + // something to name. + id: int, + name: string, + // 0 none of the selection, 1 some of it, 2 all of it. + coverage: int, + // How many of the selected photographs carry it, for the "3 of 12" that + // makes `coverage: 1` a number rather than a shrug. + selected-count: int, + // How many photographs in the whole library carry it. Lets a word in + // regular use be told from one typed once by mistake. + image-count: int, +} + // One bar of the capture-time histogram. export struct TimelineBar { // 0..1, relative to the tallest bucket. Square-rooted in Rust so a quiet @@ -656,6 +682,9 @@ component HeaderActions inherits HorizontalLayout { callback remove-from-collection(); /// Open the sheet that files the selection in a collection. callback add-to-collection(); + /// TRACES: FR-CAT-5 + /// Open the sheet that keywords the selection. + callback add-keyword(); callback toggle-select-mode(); callback change-library(); callback toggle-pin-scope(); @@ -694,6 +723,17 @@ component HeaderActions inherits HorizontalLayout { clicked => { root.add-to-collection(); } } + // TRACES: FR-CAT-5 | FR-CAT-6 + // Keyword the selection. Beside "Add to collection" because they are the + // same thought — these photographs are *of* something, and they belong + // *with* something — and appearing under the same condition, because + // neither means anything without a selection to act on. + if root.selected-count > 0: Button { + text: "Keywords"; + y: root.centred ? (root.row-height - self.height) / 2 : 0; + clicked => { root.add-keyword(); } + } + // TRACES: FR-DEV-6 // Batch-apply the copied settings. Shown only with both a selection and a // clipboard, because it is meaningless without either — and because a @@ -1105,6 +1145,34 @@ export component LibraryGrid inherits Rectangle { /// out of the one currently being shown. callback file-in-collection(int, bool); + // --- keywording the selection (FR-CAT-5, FR-CAT-6) ---------------------- + // + // The catalog has been searchable by keyword since it existed and there was + // never anywhere to type one. This sheet is that place, and it sits beside + // the filing sheet above because the two are the same gesture applied to + // two different kinds of label — pick the photographs, then say what they + // are — and a user who has learnt one should not have to learn the other. + // + // Assign and unassign travel by **name**, not by id. A word typed into the + // field and a word tapped in the list are then one path through Rust rather + // than two, and the sheet does not have to invent an id for a keyword that + // does not exist yet. + /// The vocabulary, already answered against the current selection. + in property <[KeywordRow]> keywords; + /// The sheet is opening: Rust answers by refreshing `keywords` against + /// whatever is selected *now*. + /// + /// Pulled on open rather than pushed on every selection change, because the + /// selection changes on every arrow key and the sheet is shut for almost + /// all of them — recomputing coverage over a forty-image selection for a + /// panel nobody is looking at is work the grid cannot afford. + callback keywords-opened(); + callback assign-keyword(string); + callback unassign-keyword(string); + /// Whether the sheet is up. Local, for the same reason `filing` is: it is a + /// disclosure rather than a preference, and what closes it is dismissing it. + property keywording: false; + // Cell geometry. Columns are derived from the available width so the grid // reflows with the window rather than fixing a count (FR-UI-1). // Zoomable, so the grid serves both jobs: fewer, larger images for @@ -1355,6 +1423,14 @@ export component LibraryGrid inherits Rectangle { // to is still there when the sheet closes. root.actions-open = false; } + add-keyword => { + // Ask for the vocabulary before showing the sheet, so + // it is answered against the selection as it stands now + // rather than as it stood when the grid last loaded. + root.keywords-opened(); + root.keywording = true; + root.actions-open = false; + } change-library => { root.change-library(); } toggle-pin-scope => { root.toggle-pin-scope(); } sync-now => { root.sync-now(); } @@ -1429,6 +1505,14 @@ export component LibraryGrid inherits Rectangle { // to is still there when the sheet closes. root.actions-open = false; } + add-keyword => { + // Ask for the vocabulary before showing the sheet, so + // it is answered against the selection as it stands now + // rather than as it stood when the grid last loaded. + root.keywords-opened(); + root.keywording = true; + root.actions-open = false; + } change-library => { root.change-library(); } toggle-pin-scope => { root.toggle-pin-scope(); } sync-now => { root.sync-now(); } @@ -1788,6 +1872,10 @@ export component LibraryGrid inherits Rectangle { // button, and a sheet it walked straight past would leave // the user out of the grid with their selection gone. if (event.text == Key.Back || event.text == Key.Escape) { + if (root.keywording) { + root.keywording = false; + return accept; + } if (root.filing) { root.filing = false; return accept; @@ -2486,4 +2574,184 @@ export component LibraryGrid inherits Rectangle { } } } + + // --- the keywording sheet (FR-CAT-5, FR-CAT-6) -------------------------- + // + // "These are of…". Deliberately the same card, scrim and dismissal as the + // filing sheet above: a user who has filed a selection already knows how + // this works, and a second idiom for the same gesture would be a second + // thing to learn for no gain. + // + // It stays open after each word, where the filing sheet closes. Filing is + // one choice; keywording is usually several — "puffin", "Látrabjarg", + // "2026" — and a sheet that shut after each one would have to be reopened, + // and the selection re-confirmed, three times over. + if root.keywording: Rectangle { + background: #000000CC; + + // Swallows the taps that miss the card, and closes. First, so the + // card's own controls sit above it. + TouchArea { + clicked => { root.keywording = false; } + } + + Rectangle { + width: min(420px, parent.width - 2 * Theme.gap-lg); + height: min(kw-sheet.preferred-height, parent.height - 2 * Theme.gap-lg); + x: (parent.width - self.width) / 2; + y: (parent.height - self.height) / 2; + background: Theme.surface; + border-radius: Theme.radius; + border-width: 1px; + border-color: Theme.rule; + + // Stops a press on the card reaching the scrim behind it. + TouchArea { } + + kw-sheet := VerticalLayout { + padding: Theme.gap-lg; + spacing: Theme.gap; + + Text { + text: root.selected-count == 1 + ? "Keywords for 1 photograph" + : "Keywords for " + root.selected-count + " photographs"; + color: Theme.ink; + font-size: Theme.text-lg; + font-weight: 600; + wrap: word-wrap; + } + + // Typing a word applies it, whether or not it already exists. + // One field for both, because "is this keyword new?" is a + // question about the catalog and not about what the user meant, + // and Rust can answer it without being asked. + // + // The field clears itself on accept so the next word can be + // typed straight after — keywording a shoot is a run of them. + new-keyword := Field { + placeholder: "Type a keyword and press return"; + accepted(text) => { + root.assign-keyword(text); + self.text = ""; + } + } + + Rectangle { height: 1px; background: Theme.rule; } + + Flickable { + vertical-stretch: 1; + // A floor, so the list is not squeezed out of existence by + // the field and the button around it on a short window. + min-height: 120px; + viewport-height: root.keywords.length * (Theme.touch-target + 2px); + + for word[i] in root.keywords: Rectangle { + y: i * (Theme.touch-target + 2px); + width: parent.width; + // A full touch target per row, for the same reason the + // filing sheet uses one: this is a place to hit once, + // with a thumb, holding a selection that took a minute + // to build (FR-UI-3). + height: Theme.touch-target; + background: kw-touch.pressed ? Theme.pressed + : (kw-touch.has-hover ? Theme.hover : transparent); + border-radius: Theme.radius-sm; + + HorizontalLayout { + padding-left: Theme.gap-sm; + padding-right: Theme.gap-sm; + spacing: Theme.gap-sm; + + // Tick, dash, or nothing — the three states of + // `coverage`, drawn as three different marks rather + // than as two. A half-applied keyword shown as + // applied is a lie about photographs the user + // cannot see from here. + Rectangle { + width: 16px; + y: (parent.height - self.height) / 2; + height: 16px; + border-radius: Theme.radius-sm; + border-width: 1px; + border-color: word.coverage == 0 ? Theme.rule : Theme.active; + background: word.coverage == 2 ? Theme.active : transparent; + + // The dash for "some of them". A bar rather + // than a tick, because a tick at half strength + // reads as a rendering artefact. + if word.coverage == 1: Rectangle { + width: 8px; + height: 2px; + x: (parent.width - self.width) / 2; + y: (parent.height - self.height) / 2; + background: Theme.active; + } + if word.coverage == 2: Icon { + name: "check"; + ink: Theme.surface; + size: 12px; + x: (parent.width - self.width) / 2; + y: (parent.height - self.height) / 2; + } + } + + Text { + text: word.name; + color: Theme.ink; + font-size: Theme.text; + vertical-alignment: center; + overflow: elide; + horizontal-stretch: 1; + } + + // "3 of 12" only where it says something the mark + // does not. For a word the whole selection carries, + // or none of it, the mark has already said it and + // the number would be noise on every row. + Text { + text: word.coverage == 1 + ? word.selected-count + " of " + root.selected-count + : (word.image-count > 0 ? word.image-count + "" : ""); + color: Theme.ink-faint; + font-size: Theme.text-sm; + vertical-alignment: center; + } + } + + // One target for both directions. A word the selection + // fully carries comes off; anything else goes on — so a + // partly-applied keyword is completed rather than + // removed, which is what a user tapping a dash means + // nine times in ten, and the tenth is one more tap + // away. + kw-touch := TouchArea { + clicked => { + if (word.coverage == 2) { + root.unassign-keyword(word.name); + } else { + root.assign-keyword(word.name); + } + } + } + } + + if root.keywords.length == 0: Text { + text: "No keywords yet. Type one above to make the first."; + color: Theme.ink-faint; + font-size: Theme.text-sm; + wrap: word-wrap; + width: parent.width; + } + } + + Rectangle { height: 1px; background: Theme.rule; } + + Button { + text: "Done"; + clicked => { root.keywording = false; } + } + } + } + } }