From 034ef489ed488f4c624f966627871f05782f7eea Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Fri, 9 Oct 2026 09:48:59 -0400 Subject: [PATCH] LRC-LIB-SMARTCOLL: date picker review fixes: Esc, paging limits, strict parse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From review: - Esc with the calendar open also closed the rule editor and lost its edits. The shortcut layer runs before the UI, and Esc's command (Back) closes any dialog. Shortcuts now leave Esc to an open popup (any menu or calendar). The picker marks Esc as handled for the frame (widgets::take_escape / escape_taken; the mark lasts the whole frame, since egui may run more than one pass), and the dialog's own Esc check defers to it. New headless test: Esc closes only the calendar. - Paging wrapped at the ends (‹ on January 0000 showed December 0000). step() is now a pure function that stops at years 0000 / 9999; year pages are aligned blocks of twelve, and the last one holds 9996–9999. View::open clamps a caller's year and month, so a huge `today` can't overflow. - PickedDate::parse reads a time after the day only if it is one (T10, T10:00, T10:00:00), as the rule check does; "2026-08-14Tjunk" is no longer shown as a valid day. - Today is marked in the month and year views too (holds_today). - Documented: ids must be unique among the pickers on screen; the picker picks dates, not times (picking a day replaces a typed time). The i18n test also covers Year / Month / Day. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/date_picker.rs | 123 ++++++++++++++++++---- crates/ui-egui/src/i18n.rs | 2 +- crates/ui-egui/src/panels/dialogs.rs | 5 +- crates/ui-egui/src/panels/rules_editor.rs | 3 +- crates/ui-egui/src/shortcuts.rs | 5 + crates/ui-egui/src/tests_date_picker.rs | 13 +++ crates/ui-egui/src/widgets.rs | 13 +++ docs/whats-new.md | 3 +- 8 files changed, 139 insertions(+), 28 deletions(-) diff --git a/crates/ui-egui/src/date_picker.rs b/crates/ui-egui/src/date_picker.rs index ef801ac08..c04c309af 100644 --- a/crates/ui-egui/src/date_picker.rs +++ b/crates/ui-egui/src/date_picker.rs @@ -71,12 +71,16 @@ impl PickedDate { return Some(PickedDate { year, month: Some(month), day: None }); } let day = u8::try_from(num(8, 10)?).ok().filter(|d| (1..=days_in_month(year, month)).contains(d))?; - let time = s.get(10..11); - (s.get(7..8) == Some("-") && (s.len() == 10 || matches!(time, Some("T" | " ")))).then_some(PickedDate { - year, - month: Some(month), - day: Some(day), - }) + // a time after the day (read past) must be one: T10, T10:00 or T10:00:00 (or a space for the T) + let time_ok = match s.len() { + 10 => true, + 13 | 16 | 19 => { + matches!(s.get(10..11), Some("T" | " ")) + && s.char_indices().skip(11).all(|(i, c)| if i == 13 || i == 16 { c == ':' } else { c.is_ascii_digit() }) + } + _ => false, + }; + (s.get(7..8) == Some("-") && time_ok).then_some(PickedDate { year, month: Some(month), day: Some(day) }) } /// `2026`, `2026-08` or `2026-08-14`. @@ -149,6 +153,42 @@ struct View { month: u8, } +impl View { + /// The view opened on `at`: years kept to 0000–9999 and months to 1–12, whatever a caller's + /// value holds. + fn open(at: PickedDate, precision: Precision) -> View { + View { precision, year: at.year.clamp(0, 9999), month: at.month.unwrap_or(1).clamp(1, 12) } + } +} + +/// The page before (`forward` false) or after `v`: a month, a year or twelve years. Paging stops at +/// years 0000 and 9999 rather than wrapping or leaving four-digit years. +fn step(v: View, forward: bool) -> View { + let (year, month) = match v.precision { + Precision::Day => { + let m = i32::from(v.month) - 1 + if forward { 1 } else { -1 }; + (v.year.saturating_add(m.div_euclid(12)), u8::try_from(m.rem_euclid(12) + 1).unwrap_or(1)) + } + Precision::Month => (v.year.saturating_add(if forward { 1 } else { -1 }), v.month), + Precision::Year => { + // pages of twelve years (2016–2027); the first and last pages hold the ends + let page = v.year.div_euclid(12).saturating_add(if forward { 1 } else { -1 }); + (page.saturating_mul(12), v.month) + } + }; + let years = 0..=9999; + let fits = match v.precision { + Precision::Year => years.contains(&year) || years.contains(&year.saturating_add(11)), + _ => years.contains(&year), + }; + if fits { View { year, month, ..v } } else { v } +} + +/// Whether picking `cell` would cover today: its day, its month or its year. +fn holds_today(cell: &PickedDate, today: Option) -> bool { + today.is_some_and(|t| t.first_day() >= cell.first_day() && t.first_day() <= cell.last_day()) +} + /// A calendar button that edits a [`PickedDate`]; see the module docs. pub struct DatePicker<'a> { id: String, @@ -160,8 +200,9 @@ pub struct DatePicker<'a> { } impl<'a> DatePicker<'a> { - /// `id` keeps pickers apart (and names their widgets); `value` is what they edit (`None`: no - /// date yet, or one that couldn't be read). + /// `id` keeps pickers apart and names their widgets, so it must be unique among the pickers + /// on screen (two with the same id share their open page); `value` is what they edit (`None`: + /// no date yet, or one that couldn't be read). pub fn new(id: impl Into, value: &'a mut Option) -> Self { DatePicker { id: id.into(), value, precisions: &Precision::ALL, min: None, max: None, today: None } } @@ -204,7 +245,7 @@ impl<'a> DatePicker<'a> { let at = value.or(today).unwrap_or(PickedDate::year(2026)); let fallback = if precisions.contains(&Precision::Day) { Precision::Day } else { precisions.first().copied().unwrap_or(Precision::Day) }; let precision = value.map(|v| v.precision()).filter(|p| precisions.contains(p)).unwrap_or(fallback); - ui.data_mut(|d| d.insert_temp(state, View { precision, year: at.year, month: at.month.unwrap_or(1) })); + ui.data_mut(|d| d.insert_temp(state, View::open(at, precision))); } let mut picked: Option = None; let mut view = ui.data(|d| d.get_temp::(state)).unwrap_or(View { precision: Precision::Day, year: 2026, month: 1 }); @@ -239,28 +280,17 @@ impl<'a> DatePicker<'a> { ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { let next = ui.small_button("›"); register(ui.ctx(), format!("datePickerNext:{id}"), next.rect); - let step = |v: &mut View, forward: bool| match v.precision { - Precision::Day => { - let m = i32::from(v.month) - 1 + if forward { 1 } else { -1 }; - v.year += m.div_euclid(12); - v.month = u8::try_from(m.rem_euclid(12) + 1).unwrap_or(1); - } - Precision::Month => v.year += if forward { 1 } else { -1 }, - Precision::Year => v.year += if forward { 12 } else { -12 }, - }; if next.clicked() { - step(&mut view, true); + view = step(view, true); } if prev.clicked() { - step(&mut view, false); + view = step(view, false); } }); }); - // four-digit years, as dates are written - view.year = view.year.clamp(0, 9999); let cell = |ui: &mut Ui, date: PickedDate, text: String, picked: &mut Option| { let allowed = in_bounds(&date, min.as_ref(), max.as_ref()); - let text = if today == Some(date) { RichText::new(text).strong().underline() } else { RichText::new(text) }; + let text = if holds_today(&date, today) { RichText::new(text).strong().underline() } else { RichText::new(text) }; let r = ui.add_enabled(allowed, egui::Button::selectable(*value == Some(date), text).min_size(vec2(26.0, 0.0))); register(ui.ctx(), format!("datePickerCell:{}:{id}", date.iso()), r.rect); if r.clicked() { @@ -310,6 +340,11 @@ impl<'a> DatePicker<'a> { } } }); + // Esc closes the calendar and stops there: the dialog around it keeps its edits + if ui.input(|i| i.key_pressed(egui::Key::Escape)) { + crate::widgets::take_escape(ui.ctx()); + ui.close(); + } if picked.is_some() { ui.close(); } @@ -358,6 +393,48 @@ mod tests { assert_eq!(month_grid(2026, 6).first(), Some(&Some(1)), "June 2026 starts on a Monday"); } + /// Paging stops at years 0000 and 9999 instead of wrapping (‹ on January 0000 stays there), + /// and a view opened on a year beyond them starts at the nearest end. + #[test] + fn paging_stops_at_the_ends() { + let v = |precision, year, month| View { precision, year, month }; + let at = |v: View| (v.year, v.month); + assert_eq!(at(step(v(Precision::Day, 2026, 12), true)), (2027, 1)); + assert_eq!(at(step(v(Precision::Day, 2026, 1), false)), (2025, 12)); + assert_eq!(at(step(v(Precision::Day, 0, 1), false)), (0, 1)); + assert_eq!(at(step(v(Precision::Day, 9999, 12), true)), (9999, 12)); + assert_eq!(at(step(v(Precision::Month, 0, 5), false)), (0, 5)); + assert_eq!(at(step(v(Precision::Month, 9999, 5), true)), (9999, 5)); + assert_eq!(at(step(v(Precision::Year, 5, 1), false)), (5, 1), "the first page of years"); + assert_eq!(at(step(v(Precision::Year, 9990, 1), true)), (9996, 1), "the page holding 9996–9999"); + assert_eq!(at(step(v(Precision::Year, 9996, 1), true)), (9996, 1), "the last page"); + assert_eq!(at(step(v(Precision::Year, 2026, 1), false)), (2004, 1), "the page 2004–2015"); + let huge = PickedDate { year: i32::MAX, month: Some(13), day: None }; + assert_eq!(at(View::open(huge, Precision::Day)), (9999, 12)); + assert_eq!(at(View::open(PickedDate { year: i32::MIN, month: Some(0), day: None }, Precision::Day)), (0, 1)); + } + + /// The picker reads a date the way the rule check does: a time after it must be a time. + #[test] + fn a_trailing_time_must_be_a_time() { + for ok in ["2026-08-14T10", "2026-08-14T10:00", "2026-08-14 10:00:00"] { + assert!(PickedDate::parse(ok).is_some(), "{ok}"); + } + for bad in ["2026-08-14Tjunk", "2026-08-14T", "2026-08-14T1", "2026-08-14T10:0", "2026-08-14T10:00Z"] { + assert_eq!(PickedDate::parse(bad), None, "{bad}"); + } + } + + /// Today is marked in every view: its day, its month, its year. + #[test] + fn today_is_marked_at_every_precision() { + let d = |s: &str| PickedDate::parse(s).unwrap(); + let today = Some(d("2026-10-09")); + assert!(holds_today(&d("2026-10-09"), today) && holds_today(&d("2026-10"), today) && holds_today(&d("2026"), today)); + assert!(!holds_today(&d("2026-10-08"), today) && !holds_today(&d("2026-09"), today) && !holds_today(&d("2025"), today)); + assert!(!holds_today(&d("2026"), None)); + } + #[test] fn bounds_keep_whole_values_in_range() { let d = |s: &str| PickedDate::parse(s).unwrap(); diff --git a/crates/ui-egui/src/i18n.rs b/crates/ui-egui/src/i18n.rs index 445028562..7ba6a47e2 100644 --- a/crates/ui-egui/src/i18n.rs +++ b/crates/ui-egui/src/i18n.rs @@ -779,7 +779,7 @@ mod tests { #[test] fn date_picker_words_are_translated() { for language in Locale::ALL.iter().filter(|l| **l != Locale::En) { - for word in crate::date_picker::WEEKDAY_SHORT.iter().chain(&["Pick a date"]) { + for word in crate::date_picker::WEEKDAY_SHORT.iter().chain(&["Pick a date", "Year", "Month", "Day"]) { assert!(language.catalog().contains_key(*word), "{} lacks {word:?}", language.code()); } } diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs index d540baa9d..91d1af6ef 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/crates/ui-egui/src/panels/dialogs.rs @@ -877,8 +877,9 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) { ctx.move_to_top(w.response.layer_id); crate::widgets::register(ctx, "dialog:window", w.response.rect); } - // (while the shortcuts editor records a key, it takes Esc itself to cancel) - if ctx.input(|i| i.key_pressed(egui::Key::Escape)) { + // (while the shortcuts editor records a key, it takes Esc itself to cancel; an open popup in + // the dialog, such as a date picker's calendar, takes it to close itself) + if ctx.input(|i| i.key_pressed(egui::Key::Escape)) && !crate::widgets::escape_taken(ctx) { close = true; } if confirm { diff --git a/crates/ui-egui/src/panels/rules_editor.rs b/crates/ui-egui/src/panels/rules_editor.rs index a29487ba6..7d7b2c4b7 100644 --- a/crates/ui-egui/src/panels/rules_editor.rs +++ b/crates/ui-egui/src/panels/rules_editor.rs @@ -91,7 +91,8 @@ fn number_value(ui: &mut egui::Ui, v: &mut Value, field: &str) { } /// A rule date's calendar ([`DatePicker`]): opens on the date as written (or today), keeps its -/// precision, and writes what is picked as `2026`, `2026-08` or `2026-08-14`. +/// precision, and writes what is picked as `2026`, `2026-08` or `2026-08-14`. It picks dates, not +/// times: picking a day for "2026-08-14T10:00" leaves "2026-08-14" (a time is typed). fn date_picker(ui: &mut egui::Ui, v: &mut Value, salt: &str, min: Option, max: Option, env: &Env) { let mut date = v.as_str().and_then(PickedDate::parse); let today = PickedDate::parse(&env.today); diff --git a/crates/ui-egui/src/shortcuts.rs b/crates/ui-egui/src/shortcuts.rs index cd69b2daa..0ebb0808c 100644 --- a/crates/ui-egui/src/shortcuts.rs +++ b/crates/ui-egui/src/shortcuts.rs @@ -296,11 +296,15 @@ pub fn handle(app: &mut LightcraftApp, ctx: &egui::Context) { let keymap = &app.ui.settings.keymap; // keys the user gave to a command: the fixed bindings below (aliases, ratings) yield to them let taken: Vec<(Modifiers, Key)> = keymap.values().filter_map(|s| parse(s)).collect(); + // an open popup (a menu, a date picker's calendar) closes on Esc itself: Esc's command (Back, + // which also closes dialogs) waits until nothing is open + let popup_open = egui::Popup::is_any_open(ctx); ctx.input(|i| { for b in bindable() { if let Some(sc) = binding(keymap, b.id, b.default) && let Some((m, k)) = parse(sc) && !native(sc) + && !(k == Key::Escape && popup_open) && matches(i, m, k) { fire.push(b.id.to_string()); @@ -308,6 +312,7 @@ pub fn handle(app: &mut LightcraftApp, ctx: &egui::Context) { } for (sc, id, params) in ALIASES { if let Some((m, k)) = parse(sc).filter(|_| !native(sc)) + && !(k == Key::Escape && popup_open) && !taken.contains(&(m, k)) && matches(i, m, k) { diff --git a/crates/ui-egui/src/tests_date_picker.rs b/crates/ui-egui/src/tests_date_picker.rs index 63565b22d..ba5e28113 100644 --- a/crates/ui-egui/src/tests_date_picker.rs +++ b/crates/ui-egui/src/tests_date_picker.rs @@ -88,3 +88,16 @@ fn an_unreadable_date_opens_on_this_month() { assert!(has(&h, "datePickerCell:2026-10-09:rules-0")); assert_eq!(value(&h, 0), json!("banana"), "opening doesn't change the rule"); } + +/// Esc closes the calendar only, not the rule editor around it (and its unsaved edits). +#[test] +fn escape_closes_only_the_calendar() { + let mut h = editor(json!([{"field": "captureDate", "op": "is", "value": "2026-01-20"}])); + click(&mut h, "datePicker:rules-0"); + assert!(has(&h, "datePickerCell:2026-01-20:rules-0")); + let r = h.request("ui.key", json!({"key": "Escape"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + assert!(!has(&h, "datePickerCell:2026-01-20:rules-0"), "the calendar closed"); + assert!(h.app.ui.dialog.is_some(), "the editor stays open"); +} diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index c98baf41f..b221f725e 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -19,6 +19,19 @@ pub fn register(ctx: &egui::Context, id: impl Into, rect: Rect) { ctx.data_mut(|d| d.get_temp_mut_or_default::(egui::Id::new("lc-registry")).0.push((id, rect))); } +/// Esc was handled this frame by something nested (an open popup closed on it): containers such +/// as dialogs shouldn't close too. Kept for the whole frame, since egui may run it in more than one +/// pass and the popup is already closed in the later ones. +pub fn take_escape(ctx: &egui::Context) { + let frame = ctx.cumulative_frame_nr(); + ctx.data_mut(|d| d.insert_temp(egui::Id::new("lc-escape-taken"), frame)); +} + +/// Whether [`take_escape`] was called this frame. +pub fn escape_taken(ctx: &egui::Context) -> bool { + ctx.data(|d| d.get_temp::(egui::Id::new("lc-escape-taken"))) == Some(ctx.cumulative_frame_nr()) +} + pub fn take_registry(ctx: &egui::Context) -> Vec<(String, Rect)> { ctx.data_mut(|d| std::mem::take(&mut d.get_temp_mut_or_default::(egui::Id::new("lc-registry")).0)) } diff --git a/docs/whats-new.md b/docs/whats-new.md index 0db41edb3..2f5799b9e 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -18,7 +18,8 @@ album that is gone or smart, or an empty group is reported with its rule ("rule 2.1: rating is 0–5, not 9") instead of quietly matching nothing or everything. New date rules start at the current year. - Date rules have a calendar button: pick a day, a month or a whole year (Year / Month / Day), in your language; the - two dates of a "between" can't be picked out of order. Typing still works, and "2026-10-01 10:00" (a space for the + two dates of a "between" can't be picked out of order; Esc closes just the calendar. It picks dates, not times (a + time is typed, and picking a day replaces one). Typing still works, and "2026-10-01 10:00" (a space for the T) now matches; a time zone is refused, since capture times are local. - Rule problems are shown in your language and name the field the way the menu does ("Title: needs something to look for").