From cb5a77a472117f711e23829d0f058b3286588210 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Mon, 5 Oct 2026 13:01:34 -0400 Subject: [PATCH] Date & Time: review r2 fixes --- date_picker.go | 2 +- date_range_picker.go | 23 ++++++- datetime_review2_test.go | 142 +++++++++++++++++++++++++++++++++++++++ time_picker.go | 104 ++++++---------------------- time_picker_popup.go | 90 +++++++++++++++++++++++++ 5 files changed, 275 insertions(+), 86 deletions(-) create mode 100644 datetime_review2_test.go create mode 100644 time_picker_popup.go diff --git a/date_picker.go b/date_picker.go index 26644bf..bec7d78 100644 --- a/date_picker.go +++ b/date_picker.go @@ -113,7 +113,7 @@ func DatePicker(c *ui.Context, value *Date, opts DatePickerOptions) DateTimeResu } in.Label(label).Error(*problem) focused := in.Focused() - if !opts.ReadOnly && (in.Submitted() || *wasFocused && !focused && *text != format(*value)) { + if !opts.ReadOnly && !in.IsDisabled() && (in.Submitted() || *wasFocused && !focused && *text != format(*value)) { submit() } *wasFocused = focused diff --git a/date_range_picker.go b/date_range_picker.go index 096e95b..ec6f51a 100644 --- a/date_range_picker.go +++ b/date_range_picker.go @@ -135,7 +135,12 @@ func DateRangePicker(c *ui.Context, value *DateRange, opts DateRangePickerOption } ready := !draft.Start.IsZero() && !draft.End.IsZero() if Button(c, msg(c, "calendar.confirm"), ButtonOptions{Disabled: !ready}).Clicked() { - finish(*draft, "confirm") + if p := dateRangeProblem(c, *draft, opts.Min, opts.Max, enabled, layout); p != "" { + slog.Debug("mujica: date_range_picker invalid confirm", "draft", *draft) + *problem = p + } else { + finish(*draft, "confirm") + } } }) }) @@ -151,6 +156,22 @@ func DateRangePicker(c *ui.Context, value *DateRange, opts DateRangePickerOption return DateTimeResult{Element: col, changed: changed, cancelled: cancelled} } +// dateRangeProblem checks a whole draft against the current bounds and filter. +func dateRangeProblem(c *ui.Context, r DateRange, lo, hi Date, enabled func(Date) bool, layout string) string { + format := func(d Date) string { return d.Format(layout) } + for _, d := range []Date{r.Start, r.End} { + if p := datePickerProblem(c, d, lo, hi, enabled, format); p != "" { + return p + } + } + for x := r.Start; x.Compare(r.End) <= 0; x = x.AddDays(1) { + if !enabled(x) { + return fmt.Sprintf(msg(c, "date_range_picker.blocked"), format(x)) + } + } + return "" +} + // dateRangePick applies a picked day to the draft and returns any problem. func dateRangePick(c *ui.Context, draft *DateRange, day Date, enabled func(Date) bool, layout string) string { if draft.Start.IsZero() || !draft.End.IsZero() || day.Compare(draft.Start) < 0 { diff --git a/datetime_review2_test.go b/datetime_review2_test.go new file mode 100644 index 0000000..fbc1241 --- /dev/null +++ b/datetime_review2_test.go @@ -0,0 +1,142 @@ +package mujica + +import ( + "testing" + "time" + + "github.com/egoist/mygo/ui" +) + +func TestTimePickerDisabledIgnoresSegmentKeys(t *testing.T) { + v, n := TimeOfDay{9, 30, 0}, 0 + opts := TimePickerOptions{Label: "Alarm"} + tt := ui.NewTester(func(c *ui.Context) { + Use(c, Settings{Mode: Light}) + if TimePicker(c, &v, opts).Changed() { + n++ + } + }, 420, 300) + tt.Key(0, ui.KeyTab) + if !tt.Focused("Alarm: Hours") { + t.Fatal("setup: hours not focused") + } + opts.Disabled = true + tt.Frame() + tt.Key(0, ui.KeyUp) + tt.Key(0, ui.Key1) + tt.Key(0, ui.Key5) + if v != (TimeOfDay{9, 30, 0}) || n != 0 { + t.Fatalf("disabled picker took keys: %v (%d)", v, n) + } +} + +func TestTimeRangePickerDisabledIgnoresSegmentKeys(t *testing.T) { + v, n := TimeRange{TimeOfDay{9, 0, 0}, TimeOfDay{17, 0, 0}}, 0 + opts := TimeRangePickerOptions{} + tt := ui.NewTester(func(c *ui.Context) { + Use(c, Settings{Mode: Light}) + if TimeRangePicker(c, &v, opts).Changed() { + n++ + } + }, 520, 300) + _ = tt.Click("End time: Hours") + opts.Disabled = true + tt.Frame() + tt.Key(0, ui.KeyUp) + tt.Key(0, ui.KeyDown) + tt.Key(0, ui.KeyDown) + if v != (TimeRange{TimeOfDay{9, 0, 0}, TimeOfDay{17, 0, 0}}) || n != 0 { + t.Fatalf("disabled range took keys: %v (%d)", v, n) + } +} + +func TestDatePickerDisabledBlurDoesNotCommit(t *testing.T) { + v, n := oct(5), 0 + opts := DatePickerOptions{} + tt := datePickerLive(&v, &opts, &n) + datePickerType(tt, "Due", "2026-10-20") + opts.Disabled = true + tt.Frame() + _ = tt.Click("Open calendar") + tt.Key(0, ui.KeyTab) + tt.Frame() + if v != oct(5) || n != 0 { + t.Fatalf("disabled picker committed its draft on blur: %v (%d)", v, n) + } +} + +func TestTimePickerSegmentErrorSurvivesOtherSegments(t *testing.T) { + v, n := TimeOfDay{9, 30, 0}, 0 + tt := timePickerTester(&v, TimePickerOptions{}, &n) + _ = tt.Click("Alarm: Hours") + tt.Key(0, ui.Key2) + tt.Key(0, ui.Key9) + _ = tt.Click("Alarm: Minutes") + tt.Key(0, ui.KeyUp) + if !tt.HasText("29") || !tt.HasText("Hours must be from 0 to 23.") { + t.Fatalf("editing minutes cleared the hour error: %v", tt.Texts()) + } + if v != (TimeOfDay{2, 30, 0}) { + t.Fatalf("value written while a segment is invalid: %v", v) + } + _ = tt.Click("Alarm: Hours") + tt.Key(0, ui.Key0) + tt.Key(0, ui.Key8) + if v != (TimeOfDay{8, 31, 0}) || tt.HasText("Hours must be") { + t.Fatalf("fixing the hour should commit both: %v", v) + } +} + +func TestDateTimePickerBlocksConfirmWithInvalidSegment(t *testing.T) { + v := time.Date(2026, time.October, 5, 9, 30, 0, 0, time.UTC) + n, x := 0, 0 + tt := dateTimeTester(t, &v, DateTimePickerOptions{Location: time.UTC}, &n, &x) + _ = tt.Click("Meeting") + _ = tt.Click("Time: Hours") + tt.Key(0, ui.Key2) + tt.Key(0, ui.Key9) + _ = tt.Click("Time: Minutes") + tt.Key(0, ui.KeyUp) + _ = tt.Click("Confirm") + if n != 0 || !v.Equal(time.Date(2026, time.October, 5, 9, 30, 0, 0, time.UTC)) { + t.Fatalf("confirmed with an invalid hour: %v", v) + } + if !tt.HasText("Hours must be from 0 to 23.") || !tt.HasText("Confirm") { + t.Fatalf("expected the popup to stay with the error: %v", tt.Texts()) + } +} + +func TestDateRangePickerConfirmChecksCurrentBounds(t *testing.T) { + v, n := DateRange{}, 0 + opts := DateRangePickerOptions{} + tt := ui.NewTester(func(c *ui.Context) { + Use(c, Settings{Mode: Light}) + o := opts + o.Today, o.Label = calendarToday, "Stay" + if DateRangePicker(c, &v, o).Changed() { + n++ + } + }, 420, 520) + _ = tt.Click("Stay") + _ = tt.Click(datetimeTestLabel(oct(10))) + _ = tt.Click(datetimeTestLabel(oct(15))) + opts.Max = oct(12) + tt.Frame() + _ = tt.Click("Confirm") + if n != 0 || !v.IsZero() || !tt.HasText("Choose 2026-10-12 or earlier.") { + t.Fatalf("out-of-range draft committed: %v %v", v, tt.Texts()) + } + opts.Max = Date{} + opts.DisableDate = func(d Date) bool { return d == oct(11) } + tt.Frame() + _ = tt.Click("Confirm") + if n != 0 || !tt.HasText("The range cannot include 2026-10-11.") { + t.Fatalf("draft over a newly disabled day committed: %v", v) + } + opts.DisableDate = nil + tt.Frame() + _ = tt.Click("Confirm") + if n != 1 || v != (DateRange{oct(10), oct(15)}) { + t.Fatalf("valid draft not committed: %v", v) + } +} diff --git a/time_picker.go b/time_picker.go index 57f4eac..b8dd80a 100644 --- a/time_picker.go +++ b/time_picker.go @@ -259,11 +259,11 @@ func timeSegment(c *ui.Context, draft *TimeOfDay, part timePart, cfg timeConfig, internal.Ring(seg, k.Focus, theme.ControlRadius) typed := ui.Local(seg, "time-typed", func() string { return "" }) badText := ui.Local(seg, "time-bad", func() string { return "" }) - badFor := ui.Local(seg, "time-bad-for", func() TimeOfDay { return TimeOfDay{} }) + badFor := ui.Local(seg, "time-bad-for", func() int { return 0 }) if !seg.Focused() { *typed = "" } - if *badFor != *draft { + if *badFor != timePartValue(*draft, part, cfg) { *badText = "" } set := func(t TimeOfDay) { @@ -275,7 +275,7 @@ func timeSegment(c *ui.Context, draft *TimeOfDay, part timePart, cfg timeConfig, } t := *draft lo, hi := timeLimits(part, cfg) - if !cfg.readOnly { + if !cfg.readOnly && !seg.IsDisabled() { switch { case seg.Shortcut(0, ui.KeyUp): set(timeStepPart(t, part, cfg, 1)) @@ -299,7 +299,7 @@ func timeSegment(c *ui.Context, draft *TimeOfDay, part timePart, cfg timeConfig, set(timeWithPart(t, part, v, cfg)) done = complete case complete: - *badText, *badFor = fmt.Sprintf("%02d", v), *draft + *badText, *badFor = fmt.Sprintf("%02d", v), timePartValue(*draft, part, cfg) slog.Debug("mujica: time_picker invalid segment", "part", int(part), "typed", v) } } @@ -349,6 +349,22 @@ func timeSegmentDescription(c *ui.Context, part timePart, text string, lo, hi in return fmt.Sprintf(msg(c, "time_picker.segment"), strings.TrimSpace(text), lo, hi) } +// timePartValue is the value a segment shows, so other segments' edits keep its error. +func timePartValue(t TimeOfDay, part timePart, cfg timeConfig) int { + switch part { + case timeHourPart: + if cfg.hour12 { + return timeHour12(t.Hour) + } + return t.Hour + case timeMinutePart: + return t.Minute + case timeSecondPart: + return t.Second + } + return t.Hour / 12 +} + // timeStepPart steps one segment of t by dir, wrapping round its cycle. func timeStepPart(t TimeOfDay, part timePart, cfg timeConfig, dir int) TimeOfDay { switch part { @@ -408,83 +424,3 @@ func timeType(typed *string, n, lo, hi int) (v int, ok, complete bool) { *typed = s return v, v >= lo && v <= hi, complete } - -// timeColumns lists choosable values per segment and reports whether a choice edited *draft. -func timeColumns(c *ui.Context, draft *TimeOfDay, cfg timeConfig, fresh bool) (edited bool) { - k := Tokens(c) - d := Density(c) - h := d.ControlHeight() - 4 - within := func(lo, hi TimeOfDay) bool { return hi.Compare(cfg.min) >= 0 && lo.Compare(cfg.max) <= 0 } - ui.Row(c).Gap(d.Unit()).Children(func() { - for _, part := range timeParts(cfg) { - t := *draft - var options []TimeOfDay - switch part { - case timeHourPart: - for v := range 24 { - if !cfg.hour12 || v/12 == t.Hour/12 { - options = append(options, TimeOfDay{v, t.Minute, t.Second}) - } - } - case timeMinutePart: - for v := 0; v < 60; v += cfg.minuteStep { - options = append(options, TimeOfDay{t.Hour, v, t.Second}) - } - case timeSecondPart: - for v := 0; v < 60; v += cfg.secondStep { - options = append(options, TimeOfDay{t.Hour, t.Minute, v}) - } - case timePeriodPart: - options = []TimeOfDay{{t.Hour % 12, t.Minute, t.Second}, {t.Hour%12 + 12, t.Minute, t.Second}} - } - name := msg(c, timePartNames[part]) - ui.Scroll(c).Height(h * 6).Width(d.ControlHeight() * 1.5).Children(func() { - ui.Column(c).FocusGroup(ui.Vertical).Role(ui.RoleRadioGroup).Label(name).Children(func() { - for _, o := range options { - var lo, hi TimeOfDay - text := "" - switch part { - case timeHourPart: - lo, hi = TimeOfDay{o.Hour, 0, 0}, TimeOfDay{o.Hour, 59, 59} - text = fmt.Sprintf("%02d", o.Hour) - if cfg.hour12 { - text = fmt.Sprintf("%d", timeHour12(o.Hour)) - } - case timeMinutePart: - lo, hi = TimeOfDay{o.Hour, o.Minute, 0}, TimeOfDay{o.Hour, o.Minute, 59} - text = fmt.Sprintf("%02d", o.Minute) - case timeSecondPart: - lo, hi = o, o - text = fmt.Sprintf("%02d", o.Second) - case timePeriodPart: - lo, hi = TimeOfDay{o.Hour / 12 * 12, 0, 0}, TimeOfDay{o.Hour/12*12 + 11, 59, 59} - text = timePeriod(c, o.Hour) - } - sel := t - item := ui.RadioBase(c, &sel, o).Height(h).FillWidth().Center().Radius(theme.ControlRadius). - Disabled(!within(lo, hi)).Label(text) - fg := k.Text - switch { - case o == t: - item.Background(k.Accent) - fg = k.OnAccent - if fresh { - item.ScrollIntoView() - } - case item.Pressed(): - item.Background(k.Selection) - case item.Hovered(): - item.Background(k.SurfaceHover) - } - internal.Ring(item, k.Focus, theme.ControlRadius) - item.Children(func() { ui.Text(c, text).TextColor(fg).FontFeatures("tnum") }) - if item.Clicked() && o != *draft { - *draft, edited = o, true - } - } - }) - }) - } - }) - return edited -} diff --git a/time_picker_popup.go b/time_picker_popup.go new file mode 100644 index 0000000..e08dc9e --- /dev/null +++ b/time_picker_popup.go @@ -0,0 +1,90 @@ +package mujica + +import ( + "fmt" + + "github.com/egoist/mygo/ui" + + "mujicaui/internal" + "mujicaui/theme" +) + +// timeColumns lists choosable values per segment and reports whether a choice edited *draft. +func timeColumns(c *ui.Context, draft *TimeOfDay, cfg timeConfig, fresh bool) (edited bool) { + k := Tokens(c) + d := Density(c) + h := d.ControlHeight() - 4 + within := func(lo, hi TimeOfDay) bool { return hi.Compare(cfg.min) >= 0 && lo.Compare(cfg.max) <= 0 } + ui.Row(c).Gap(d.Unit()).Children(func() { + for _, part := range timeParts(cfg) { + t := *draft + var options []TimeOfDay + switch part { + case timeHourPart: + for v := range 24 { + if !cfg.hour12 || v/12 == t.Hour/12 { + options = append(options, TimeOfDay{v, t.Minute, t.Second}) + } + } + case timeMinutePart: + for v := 0; v < 60; v += cfg.minuteStep { + options = append(options, TimeOfDay{t.Hour, v, t.Second}) + } + case timeSecondPart: + for v := 0; v < 60; v += cfg.secondStep { + options = append(options, TimeOfDay{t.Hour, t.Minute, v}) + } + case timePeriodPart: + options = []TimeOfDay{{t.Hour % 12, t.Minute, t.Second}, {t.Hour%12 + 12, t.Minute, t.Second}} + } + name := msg(c, timePartNames[part]) + ui.Scroll(c).Height(h * 6).Width(d.ControlHeight() * 1.5).Children(func() { + ui.Column(c).FocusGroup(ui.Vertical).Role(ui.RoleRadioGroup).Label(name).Children(func() { + for _, o := range options { + var lo, hi TimeOfDay + text := "" + switch part { + case timeHourPart: + lo, hi = TimeOfDay{o.Hour, 0, 0}, TimeOfDay{o.Hour, 59, 59} + text = fmt.Sprintf("%02d", o.Hour) + if cfg.hour12 { + text = fmt.Sprintf("%d", timeHour12(o.Hour)) + } + case timeMinutePart: + lo, hi = TimeOfDay{o.Hour, o.Minute, 0}, TimeOfDay{o.Hour, o.Minute, 59} + text = fmt.Sprintf("%02d", o.Minute) + case timeSecondPart: + lo, hi = o, o + text = fmt.Sprintf("%02d", o.Second) + case timePeriodPart: + lo, hi = TimeOfDay{o.Hour / 12 * 12, 0, 0}, TimeOfDay{o.Hour/12*12 + 11, 59, 59} + text = timePeriod(c, o.Hour) + } + sel := t + item := ui.RadioBase(c, &sel, o).Height(h).FillWidth().Center().Radius(theme.ControlRadius). + Disabled(!within(lo, hi)).Label(text) + fg := k.Text + switch { + case o == t: + item.Background(k.Accent) + fg = k.OnAccent + if fresh { + item.ScrollIntoView() + } + case item.Pressed(): + item.Background(k.Selection) + case item.Hovered(): + item.Background(k.SurfaceHover) + } + internal.Ring(item, k.Focus, theme.ControlRadius) + item.Children(func() { ui.Text(c, text).TextColor(fg).FontFeatures("tnum") }) + if item.Clicked() && o != *draft { + *draft, edited = o, true + } + } + }) + }) + } + }) + return edited +}