From e7fa046d1183b24d5e613bc73ed38031a8217e81 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Fri, 23 Oct 2026 11:22:19 +0200 Subject: [PATCH] Flag chip and filter bar show what flag and flags let through together With both set (only via library.filter) they AND, but the chip and the bar's buttons showed their union. Filter::flags_shown gives the flags that pass both; the chip names both constraints when none does. --- crates/catalog/src/query.rs | 11 +++++++++++ crates/engine/src/tests/library.rs | 21 +++++++++++++++++++++ crates/engine/src/view.rs | 11 ++++++----- crates/ui-egui/src/headless/tests.rs | 5 +++++ crates/ui-egui/src/i18n/tests.rs | 2 ++ crates/ui-egui/src/panels/chips.rs | 13 +++++++++---- crates/ui-egui/src/panels/filterbar.rs | 7 +------ 7 files changed, 55 insertions(+), 15 deletions(-) diff --git a/crates/catalog/src/query.rs b/crates/catalog/src/query.rs index 05c2ee35e..c918c53fb 100644 --- a/crates/catalog/src/query.rs +++ b/crates/catalog/src/query.rs @@ -270,6 +270,17 @@ impl Filter { self.describe_in(Some(cat)) } + /// The flags that `flag` and `flags` let through together (they AND), in the filter bar's + /// order; `None` when neither is set, empty when they exclude each other. The chip and the + /// filter bar's buttons show this, so they match what is filtered. + pub fn flags_shown(&self) -> Option> { + if self.flag.is_none() && self.flags.is_empty() { + return None; + } + let passes = |f: &Flag| self.flag.is_none_or(|x| x == *f) && (self.flags.is_empty() || self.flags.contains(f)); + Some([Flag::Pick, Flag::Reject, Flag::None].into_iter().filter(passes).collect()) + } + fn describe_in(&self, cat: Option<&crate::Catalog>) -> String { let mut v: Vec = Vec::new(); if self.rating > 0 { diff --git a/crates/engine/src/tests/library.rs b/crates/engine/src/tests/library.rs index 5b9f27cef..29af6d332 100644 --- a/crates/engine/src/tests/library.rs +++ b/crates/engine/src/tests/library.rs @@ -391,6 +391,27 @@ fn filter_any_of_several_flags() { assert_eq!(s.visible_cloned().len(), picks); } +/// `flag` and `flags` set together (only through `library.filter`) AND, and the chip says what +/// they let through, not their union. +#[test] +fn filter_flag_and_flags_together_chip() { + use lightcraft_catalog::Flag; + let mut s = crate::Session::with_demo(); + let ids: Vec = s.catalog.photos().take(3).map(|p| p.id.0).collect(); + for (id, f) in ids.iter().zip(["pick", "reject", "none"]) { + s.execute("photo.flag", &json!({"ids": [id], "flag": f})).unwrap(); + } + let picks = s.catalog.photos().filter(|p| p.in_library() && p.flag == Flag::Pick).count(); + let chip = |s: &Session| crate::filter_chips(&s.filter, &s.catalog).into_iter().find(|c| c.label.starts_with("Flag")).expect("a flag chip").label; + s.execute("library.filter", &json!({"flag": "pick", "flags": ["pick", "none"]})).unwrap(); + assert_eq!(s.visible_cloned().len(), picks); + assert_eq!(chip(&s), "Flag: pick"); + // nothing passes both: the chip names both constraints + s.execute("library.filter", &json!({"flag": "pick", "flags": ["reject", "none"]})).unwrap(); + assert_eq!(s.visible_cloned().len(), 0); + assert_eq!(chip(&s), "Flag: pick and reject or none"); +} + /// Quick Collection / target album: B toggles the selection in it (created on first use); /// another album can be the target; clear empties it; all undoable. #[test] diff --git a/crates/engine/src/view.rs b/crates/engine/src/view.rs index be05989e1..ae51ed52c 100644 --- a/crates/engine/src/view.rs +++ b/crates/engine/src/view.rs @@ -158,11 +158,12 @@ pub fn filter_chips(f: &Filter, cat: &Catalog) -> Vec { }; add(format!("Rating {op} {}★", f.rating), json!({"rating": 0})); } - let mut flags = f.flags.clone(); - flags.extend(f.flag.filter(|fl| !flags.contains(fl))); - if !flags.is_empty() { - let names: Vec = flags.iter().map(|fl| format!("{fl:?}").to_lowercase()).collect(); - add(format!("Flag: {}", names.join(" or ")), json!({"flag": Null, "flags": []})); + if let Some(shown) = f.flags_shown() { + let name = |fl: &lightcraft_catalog::Flag| format!("{fl:?}").to_lowercase(); + let or = |fls: &[lightcraft_catalog::Flag]| fls.iter().map(name).collect::>().join(" or "); + // empty: `flag` and `flags` exclude each other, so name both constraints + let names = if shown.is_empty() { format!("{} and {}", f.flag.as_ref().map(name).unwrap_or_default(), or(&f.flags)) } else { or(&shown) }; + add(format!("Flag: {names}"), json!({"flag": Null, "flags": []})); } if let Some(shown) = f.labels_shown() { let or = |ls: &[lightcraft_catalog::ColorLabel]| ls.iter().map(|l| cat.label_name(*l)).collect::>().join(" or "); diff --git a/crates/ui-egui/src/headless/tests.rs b/crates/ui-egui/src/headless/tests.rs index 43c0da437..584fadd4d 100644 --- a/crates/ui-egui/src/headless/tests.rs +++ b/crates/ui-egui/src/headless/tests.rs @@ -1611,6 +1611,11 @@ fn filter_bar_flags_combine() { h.request("ui.clickWidget", json!({"id": "filter:none"}), t); let f = h.app.session.filter.clone(); assert_eq!((f.flag, f.flags), (None, vec![Flag::Reject, Flag::None])); + // both set through the API: only Picked passes, so only Picked is lit and Unflagged joins it + h.app.session.execute("library.filter", &json!({"flag": "pick", "flags": ["pick", "none"]})).unwrap(); + h.request("ui.clickWidget", json!({"id": "filter:none"}), t); + let f = h.app.session.filter.clone(); + assert_eq!((f.flag, f.flags), (None, vec![Flag::Pick, Flag::None])); h.settle(SETTLE); } diff --git a/crates/ui-egui/src/i18n/tests.rs b/crates/ui-egui/src/i18n/tests.rs index 06b70178f..b1b132ae4 100644 --- a/crates/ui-egui/src/i18n/tests.rs +++ b/crates/ui-egui/src/i18n/tests.rs @@ -605,6 +605,8 @@ fn german_switches_via_menu_and_control_and_persists() { assert!(label.contains("Color") && !label.contains(&red) && label.contains(&blue), "{label}"); let filter = lightcraft_catalog::Filter { flags: vec![lightcraft_catalog::Flag::Pick, lightcraft_catalog::Flag::None], ..Default::default() }; assert_eq!(filter_label(&filter, &app.session.catalog), "Markierung: ausgewählt oder keine"); + let filter = lightcraft_catalog::Filter { flag: Some(lightcraft_catalog::Flag::Pick), ..filter }; + assert_eq!(filter_label(&filter, &app.session.catalog), "Markierung: ausgewählt"); set_language(Locale::En); } diff --git a/crates/ui-egui/src/panels/chips.rs b/crates/ui-egui/src/panels/chips.rs index ca7addfd4..4a95dfbaa 100644 --- a/crates/ui-egui/src/panels/chips.rs +++ b/crates/ui-egui/src/panels/chips.rs @@ -69,10 +69,15 @@ pub(crate) fn display_label(chip: &FilterChip, filter: &lightcraft_catalog::Filt return format!("{}: {}", tr("Label"), label_names(filter, catalog)); } if chip.clear.get("flags").is_some() { - let mut flags = filter.flags.clone(); - flags.extend(filter.flag.filter(|flag| !flags.contains(flag))); - let names = flags.iter().map(|flag| tr(&format!("{flag:?}").to_lowercase()).to_string()).collect::>(); - return format!("{}: {}", tr("Flag"), names.join(&format!(" {} ", tr("or")))); + let shown = filter.flags_shown().unwrap_or_default(); + let name = |flag: &lightcraft_catalog::Flag| tr(&format!("{flag:?}").to_lowercase()).to_string(); + let or = |flags: &[lightcraft_catalog::Flag]| flags.iter().map(name).collect::>().join(&format!(" {} ", tr("or"))); + let names = if shown.is_empty() { + format!("{} {} {}", filter.flag.as_ref().map(name).unwrap_or_default(), tr("and"), or(&filter.flags)) + } else { + or(&shown) + }; + return format!("{}: {names}", tr("Flag")); } if chip.clear.get("dateFrom").is_some() || chip.clear.get("dateTo").is_some() { return match (&filter.date_from, &filter.date_to) { diff --git a/crates/ui-egui/src/panels/filterbar.rs b/crates/ui-egui/src/panels/filterbar.rs index 6d72c5864..9de3c4ef0 100644 --- a/crates/ui-egui/src/panels/filterbar.rs +++ b/crates/ui-egui/src/panels/filterbar.rs @@ -137,12 +137,7 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { // flags: several can be on at once (any of them matches), as in Lightroom Classic caption(ui, "Flag"); - let mut chosen: Vec = f.flags.clone(); - if let Some(fl) = f.flag - && !chosen.contains(&fl) - { - chosen.push(fl); - } + let chosen: Vec = f.flags_shown().unwrap_or_default(); for (id, icon, flag, tip) in [ ("pick", Icon::FlagPick, Flag::Pick, "Picked"), ("reject", Icon::FlagReject, Flag::Reject, "Rejected"),