From 8359aaa04c65d14d8a7a55e432823fa66aeb04f7 Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Fri, 9 Oct 2026 10:49:27 -0400 Subject: [PATCH] LRC-LIB-SMARTCOLL: rule editor: calendars only on date fields; in-the-last N as text From final review: - Every non-number "between" got date pickers, so a text or choice rule written with "between" (by an agent; the check marks it) offered calendars that would write dates into it. Only date fields get them now; a test checks a "Title between" rule. - An "in the last" N stored as text ("7") showed as 30, and changing only the unit wrote 30. It is now read as the catalog reads it. - take_escape's comment no longer claims later egui passes see the key press (they don't); keying it to the frame just lets it lapse. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/panels/rules_editor.rs | 13 +++++++++++-- crates/ui-egui/src/tests_date_picker.rs | 9 +++++++++ crates/ui-egui/src/widgets.rs | 3 +-- 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/crates/ui-egui/src/panels/rules_editor.rs b/crates/ui-egui/src/panels/rules_editor.rs index 9b8c994f1..006d42667 100644 --- a/crates/ui-egui/src/panels/rules_editor.rs +++ b/crates/ui-egui/src/panels/rules_editor.rs @@ -137,7 +137,11 @@ fn value_editor(ui: &mut egui::Ui, field: &str, op: &str, v: &mut Value, salt: & // shown as stored ({n, unit}, or a plain number of days) and only written when changed: // drawing never turns "0 days" into 1 or 20,000 into the spinner's 10,000 let (stored_n, stored_unit) = match &*v { - Value::Object(o) => (o.get("n").and_then(Value::as_f64), o.get("unit").and_then(Value::as_str).unwrap_or("days").to_string()), + // n as the catalog reads it: a number, or one written as text ("7") + Value::Object(o) => ( + o.get("n").and_then(|n| n.as_f64().or_else(|| n.as_str().and_then(|t| t.trim().parse().ok()))), + o.get("unit").and_then(Value::as_str).unwrap_or("days").to_string(), + ), Value::Number(n) => (n.as_f64(), "days".to_string()), _ => (None, "days".to_string()), }; @@ -171,7 +175,7 @@ fn value_editor(ui: &mut egui::Ui, field: &str, op: &str, v: &mut Value, salt: & number_value(ui, lo, field); ui.label(crate::i18n::tr("and")); number_value(ui, hi, field); - } else { + } else if k == Kind::Date { // each date's picker stops at the other one, so the two stay in order let (lo_date, hi_date) = (lo.as_str().and_then(PickedDate::parse), hi.as_str().and_then(PickedDate::parse)); text_value(ui, lo, 86.0, "2026-01-01", &format!("{salt}-a")); @@ -179,6 +183,11 @@ fn value_editor(ui: &mut egui::Ui, field: &str, op: &str, v: &mut Value, salt: & ui.label(crate::i18n::tr("and")); text_value(ui, hi, 86.0, "2026-12", &format!("{salt}-b")); date_picker(ui, hi, &format!("{salt}-b"), lo_date, None, env); + } else { + // a field without "between" (the check marks it): its two values, as text + text_value(ui, lo, 86.0, "", &format!("{salt}-a")); + ui.label(crate::i18n::tr("and")); + text_value(ui, hi, 86.0, "", &format!("{salt}-b")); } if pair != before { *v = pair; diff --git a/crates/ui-egui/src/tests_date_picker.rs b/crates/ui-egui/src/tests_date_picker.rs index 0ea04f0ce..0f7017675 100644 --- a/crates/ui-egui/src/tests_date_picker.rs +++ b/crates/ui-egui/src/tests_date_picker.rs @@ -114,3 +114,12 @@ fn escape_closes_only_a_dropdown() { assert!(!has(&h, "ruleFieldItem:rating:rules-0"), "the menu closed"); assert!(h.app.ui.dialog.is_some(), "the editor stays open"); } + +/// Only date rules get calendars: a "between" on a text field (written by an agent; the check marks +/// it) shows its two values as text, without date pickers that would write dates into it. +#[test] +fn only_dates_get_calendars() { + let h = editor(json!([{"field": "title", "op": "between", "value": ["a", "b"]}])); + assert!(!has(&h, "datePicker:rules-0-a") && !has(&h, "datePicker:rules-0-b")); + assert!(has(&h, "ruleProblem:rules-0"), "and the operator is marked"); +} diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index b221f725e..1d618cbeb 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -20,8 +20,7 @@ pub fn register(ctx: &egui::Context, id: impl Into, rect: 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. +/// as dialogs shouldn't close too. Keyed to the frame, so it lapses by itself on the next one. 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));