From a4b9deaf961947c384175251245b1204fecb5aaf Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Sat, 29 Aug 2026 22:32:10 +0200 Subject: [PATCH] Put the people filter where the filters are MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Narrowing the grid to two people at once has worked since people became a selector term, and it was effectively unreachable. The only control that could add a second person lived on the People screen, behind selecting them there, and it appeared only once the grid was already narrowed to somebody — so "photographs with both of them" needed a two-screen round trip the user had to guess at. A filter belongs on the filter bar. A "People" chip there opens a tray of everyone the library knows; tapping a name adds or removes them, and the any/all chip beside it — already there, and already the thing nobody found — now has something to sit next to that explains it. The caption leads the row so a pair of chips means something before either is pressed. The tray is a strip under the bar rather than a popup, the way the develop column's film picker is: the view scrolls as one, so an inline strip is taller content and not a second overlay to dismiss. It scrolls horizontally for the same hard reason the bar above it does — a layout cannot be narrower than its children's minimums, and forty people would otherwise set the minimum width of the whole view. The roster is built on open, not kept in step: indexing and regrouping change who exists, and a list cached at startup would be stale for exactly the user who has just been naming people. Named first, then by how much of them the library holds — the catalog orders by face count alone, which puts a dozen unnamed strangers ahead of the two people the user actually cares about. Co-Authored-By: Claude Opus 5 (1M context) --- ui/dr-ui/src/library_ui.rs | 102 ++++++++++++++++++++++++++++ ui/dr-ui/ui/app.slint | 7 ++ ui/dr-ui/ui/library.slint | 132 ++++++++++++++++++++++++++++++++++++- 3 files changed, 240 insertions(+), 1 deletion(-) diff --git a/ui/dr-ui/src/library_ui.rs b/ui/dr-ui/src/library_ui.rs index 248809c..348273c 100644 --- a/ui/dr-ui/src/library_ui.rs +++ b/ui/dr-ui/src/library_ui.rs @@ -5097,6 +5097,34 @@ pub fn wire( }); } + { + let weak = window.as_weak(); + let ctl = ctl.clone(); + window.on_library_people_listed(move || { + let Some(w) = weak.upgrade() else { return }; + push_people_roster(&w, &ctl); + }); + } + + { + let weak = window.as_weak(); + let ctl = ctl.clone(); + window.on_library_filter_person_toggled(move |id| { + let Some(w) = weak.upgrade() else { return }; + let person = id.max(0) as u64; + { + let mut f = ctl.filter.borrow_mut(); + if let Some(at) = f.people.iter().position(|p| *p == person) { + f.people.remove(at); + } else { + f.people.push(person); + } + } + push_people_chips(&w, &ctl); + refilter(&w, &ctl); + }); + } + { let weak = window.as_weak(); let ctl = ctl.clone(); @@ -5459,6 +5487,9 @@ fn push_people_chips(window: &AppWindow, ctl: &Rc) { .set_library_filter_people(slint::ModelRc::new(slint::VecModel::from( Vec::::new(), ))); + // The tray, if it is open, has to lose its ticks with the chips: the + // roster carries `picked` and is the same fact drawn a second time. + push_people_roster(window, ctl); return; } @@ -5487,6 +5518,12 @@ fn push_people_chips(window: &AppWindow, ctl: &Rc) { PersonChip { id: *id as i32, name: name.into(), + // Not asked for on this side. The bar's job here is to say who + // the grid is narrowed to and offer a way out of it; a face + // count beside each would be a second number competing with the + // image counts already on the bar. + faces: -1, + picked: true, } }) .collect(); @@ -5496,6 +5533,71 @@ fn push_people_chips(window: &AppWindow, ctl: &Rc) { ctl.filter.borrow().people_mode, library::PeopleMode::All )); + push_people_roster(window, ctl); +} + +/// Fill the filter bar's people tray. +/// +/// Rebuilt whole rather than patched, because `picked` is on every row and a +/// toggle changes two things at once — the chip that was pressed and, under +/// `All`, what the whole filter means. +/// +/// **Named people first, then by how much of them the library holds.** The +/// catalog orders by face count alone, which on a real library puts a dozen +/// unnamed strangers ahead of the two people the user has actually named — and +/// the tray scrolls horizontally, so anything past the first few chips costs a +/// gesture to reach. Naming somebody is the user saying they matter; the order +/// says it back. +fn push_people_roster(window: &AppWindow, ctl: &Rc) { + let picked = ctl.filter.borrow().people.clone(); + let borrow = ctl.catalog.borrow(); + let people = borrow + .as_ref() + .and_then(|cat| dr_catalog::faces::people(cat.connection()).ok()) + .unwrap_or_default(); + + // Sorted as (unnamed, -faces) pairs beside the chip rather than by + // re-reading the drawn label: "is this person named" is a fact about the + // record, and recovering it from the `Unnamed (n faces)` wording would put + // a sort key inside a string meant for a human to read. + let mut rows: Vec<(bool, i64, PersonChip)> = people + .iter() + .filter(|p| { + let on = picked.contains(&p.id.0); + // A person already in the filter always has a chip, whatever else + // is true of them: the tray is where the filter is taken apart, and + // a term with no control is a term the user cannot remove. + // + // Otherwise: nobody the user set aside, and nobody with no faces — + // a named person emptied by a split would be a chip that narrows + // the grid to nothing whatever else is on the bar. + on || (!p.ignored && p.confirmed_faces + p.suggested_faces > 0) + }) + .map(|p| { + let faces = p.confirmed_faces + p.suggested_faces; + let unnamed = p.name.trim().is_empty(); + ( + unnamed, + faces as i64, + PersonChip { + id: p.id.0 as i32, + name: if unnamed { + format!("Unnamed ({faces} faces)").into() + } else { + p.name.clone().into() + }, + faces: faces as i32, + picked: picked.contains(&p.id.0), + }, + ) + }) + .collect(); + // Stable, so the catalog's own tiebreak by name survives inside each of the + // two blocks. + rows.sort_by_key(|(unnamed, faces, _)| (*unnamed, -faces)); + + let rows: Vec = rows.into_iter().map(|(_, _, chip)| chip).collect(); + window.set_library_people(slint::ModelRc::new(slint::VecModel::from(rows))); } fn refilter(window: &AppWindow, ctl: &Rc) { diff --git a/ui/dr-ui/ui/app.slint b/ui/dr-ui/ui/app.slint index fd5c1a5..97beb6f 100644 --- a/ui/dr-ui/ui/app.slint +++ b/ui/dr-ui/ui/app.slint @@ -588,6 +588,10 @@ export component AppWindow inherits Window { in property library-filter-people-all: false; callback library-filter-person-cleared(int); callback library-filter-people-mode-toggled(); + /// Everyone the library knows, for the filter bar's people tray. + in property <[PersonChip]> library-people; + callback library-people-listed(); + callback library-filter-person-toggled(int); in property library-filter-min-rating: 0; in property library-filter-unjudged: false; in property library-filter-flag: 0; @@ -1499,6 +1503,7 @@ in property panel-visible: true; filter-people: root.library-filter-people; filter-people-all: root.library-filter-people-all; + people: root.library-people; filter-min-rating: root.library-filter-min-rating; filter-unjudged: root.library-filter-unjudged; filter-flag: root.library-filter-flag; @@ -1531,6 +1536,8 @@ in property panel-visible: true; filter-flag-changed(f) => { root.library-filter-flag-changed(f); } filter-person-cleared(id) => { root.library-filter-person-cleared(id); } filter-people-mode-toggled() => { root.library-filter-people-mode-toggled(); } + people-listed() => { root.library-people-listed(); } + filter-person-toggled(id) => { root.library-filter-person-toggled(id); } } } } diff --git a/ui/dr-ui/ui/library.slint b/ui/dr-ui/ui/library.slint index dd714db..5a16a5d 100644 --- a/ui/dr-ui/ui/library.slint +++ b/ui/dr-ui/ui/library.slint @@ -414,10 +414,20 @@ export component Timeline inherits Rectangle { } } -/// One person the grid is narrowed to, as drawn on the filter bar. +/// One person on the filter bar, in either of the two roles it plays there. +/// +/// The chips for who the grid is *currently* narrowed to, and the roster in the +/// tray the user picks from, are the same thing drawn twice — same id, same +/// wording — so they are one struct. `faces` and `picked` are the tray's half +/// and are simply not read by the narrowed-to chips. export struct PersonChip { id: int, name: string, + /// How many faces the library holds of them. `-1` where it was not asked + /// for, which `FilterChip` draws as no number at all rather than as zero. + faces: int, + /// Already one of the people the grid is narrowed to. + picked: bool, } export struct LibraryCell { @@ -1335,6 +1345,24 @@ export component LibraryGrid inherits Rectangle { /// Whose photographs the grid is narrowed to — one chip each, so a /// selection of three can be taken apart one person at a time. in property <[PersonChip]> filter-people; + /// Everyone the library knows, for the tray. Filled on demand — see the + /// tray itself for why it is not simply pushed alongside the chips. + in property <[PersonChip]> people; + /// Asked for when the tray opens, so a roster is never built for a bar + /// nobody has opened and never goes stale in one that is. + callback people-listed(); + /// Add or remove one person from the filter, keeping the rest. + /// + /// The gesture the whole tray exists for: `filter-person-cleared` can only + /// take somebody *out*, so before this the only way to narrow to two people + /// at once was to visit the People screen twice. + callback filter-person-toggled(int); + /// Whether the people tray is open. + /// + /// Private to the view, like every other disclosure here: nothing in Rust + /// needs to know, and a strip whose open state round-tripped through a + /// callback would flicker on every press. + property people-tray: false; /// Whether those people are an intersection rather than a union. in property filter-people-all: false; callback filter-person-cleared(int); @@ -1979,6 +2007,38 @@ export component LibraryGrid inherits Rectangle { clicked => { root.filter-people-mode-toggled(); } } + // The way in to the people tray, and the reason it exists. + // + // Narrowing to *two* people at once was already possible and + // effectively unreachable: the only control that could add a + // second one lived on the People screen, behind selecting them + // there, and it only appeared once the grid was already + // narrowed to somebody. So a user who wanted "photographs with + // both of them" had to guess a two-screen round trip. A filter + // belongs on the filter bar; this chip is the whole feature's + // front door and the tray below is where both terms and the + // any/all choice actually are. + FilterChip { + icon: root.people-tray ? "chevron-down" : "chevron-right"; + label: "People"; + // The number narrowed to, not the size of the roster: it + // says what the filter is doing, which is what every other + // count on this bar says. + count: root.filter-people.length > 0 + ? root.filter-people.length : -1; + active: root.people-tray; + y: (parent.height - self.height) / 2; + clicked => { + root.people-tray = !root.people-tray; + // Asked for on open rather than kept in step, so the + // roster is current — indexing and regrouping change + // who exists, and a list cached at startup would be + // stale for exactly the user who has just been naming + // people. + if (root.people-tray) { root.people-listed(); } + } + } + Caption { text: "Show"; vertical-alignment: center; @@ -2103,6 +2163,76 @@ export component LibraryGrid inherits Rectangle { } } + // --- the people tray ---------------------------------------------- + // + // A second strip under the filter bar rather than a popup, for the + // reason the develop column's film picker gives: this view already + // scrolls as one, so an inline strip is taller content and not a second + // overlay with its own dismiss gesture to lose a drag to. + // + // Horizontally scrolling, exactly like the bar above it and for the + // same hard reason — **a layout cannot be narrower than its children's + // minimums**, and a library with forty people would otherwise report a + // minimum width of forty chips and inflate the whole view. See the bar + // above for the full account of that fault. + if root.people-tray: Rectangle { + height: 38px; + background: Theme.surface; + + Rectangle { + y: parent.height - 1px; + height: 1px; + background: Theme.rule; + } + + Flickable { + width: 100%; + height: 100%; + viewport-height: self.height; + viewport-width: max(self.width, people-row.preferred-width); + + people-row := HorizontalLayout { + width: parent.viewport-width; + height: parent.viewport-height; + padding-left: Theme.gap; + padding-right: Theme.gap; + spacing: 4px; + alignment: start; + + Caption { + // Says what a *pair* of chips will mean before either + // is pressed, which is the thing the old design never + // said anywhere. + text: root.filter-people-all + ? "In every one:" : "In the picture:"; + vertical-alignment: center; + } + + // The roster. Ordered most-photographed-first with the + // named ahead of the rest, so the people a user actually + // intersects are the ones under the thumb without + // scrolling. + for p[i] in root.people: FilterChip { + icon: p.picked ? "check" : ""; + label: p.name; + count: p.faces; + active: p.picked; + y: (parent.height - self.height) / 2; + clicked => { root.filter-person-toggled(p.id); } + } + + // Not an error and not empty chrome: face indexing is an + // opt-in overnight pass, so "nobody yet" is the ordinary + // state of a library nobody has run it on, and it should + // say where the pass lives. + if root.people.length == 0: Caption { + text: "Nobody indexed yet — find faces on the People screen."; + vertical-alignment: center; + } + } + } + } + // --- what is selected, and what to do with it -------------------- // // The count and these actions used to live only in the header row,