LRC-LIB-SMARTCOLL: date picker review fixes: Esc, paging limits, strict parse

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 <noreply@anthropic.com>
This commit is contained in:
Nathaniel Ledford
2026-10-09 10:54:57 -04:00
co-authored by Claude Opus 5.5
parent 8d4e539d02
commit 034ef489ed
8 changed files with 139 additions and 28 deletions
+100 -23
View File
@@ -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<PickedDate>) -> 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<String>, value: &'a mut Option<PickedDate>) -> 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<PickedDate> = None;
let mut view = ui.data(|d| d.get_temp::<View>(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<PickedDate>| {
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();
+1 -1
View File
@@ -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());
}
}
+3 -2
View File
@@ -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 {
+2 -1
View File
@@ -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<PickedDate>, max: Option<PickedDate>, env: &Env) {
let mut date = v.as_str().and_then(PickedDate::parse);
let today = PickedDate::parse(&env.today);
+5
View File
@@ -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)
{
+13
View File
@@ -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");
}
+13
View File
@@ -19,6 +19,19 @@ pub fn register(ctx: &egui::Context, id: impl Into<String>, rect: Rect) {
ctx.data_mut(|d| d.get_temp_mut_or_default::<Registry>(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::<u64>(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::<Registry>(egui::Id::new("lc-registry")).0))
}
+2 -1
View File
@@ -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").