From 08435988c2e5a986fedba494a3639a114d498bdc Mon Sep 17 00:00:00 2001 From: yiekheng Date: Sun, 10 May 2026 11:18:23 +0800 Subject: [PATCH] feat(recurrence): per-rule fire time on every tab; drop redundant labels MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each schedule row in the recurrence picker now owns its own HH:MM fire time. Previously every row inherited the time from the date+time inputs above, which meant you couldn't say "every Monday at 09:00 AND every Friday at 17:00" — both rules shared whatever the form-level time was. The Daily / Weekly / Monthly / Yearly tabs each render a small "Fires at" time field. The picker still seeds new rules with the form's date+time so the most common case (one rule, time matches the first fire) doesn't need extra clicks. Round-trip: the cron line `35 17 * * 1` now restores hour=17, minute=35 on a weekly draft. The parser pulls MM HH off the front of every expression and feeds the rest of the pattern matchers as before. Also clean up two pieces of duplicated/obsolete UI feedback in both the wizard When step and the edit When form: - Removed the `

` showing "Cron: 0 9 * * *" — the per-row description ("Every day at 09:00") already says it, in human form. - Removed the standalone "Times are in Asia/Kuala_Lumpur" footer. The timezone is shown elsewhere (header) and the cron output is always evaluated in the configured zone — telling the user "times are in " inline in the picker is noise. Co-Authored-By: Claude Opus 4.7 (1M context) --- apps/web/src/components/recurrence-picker.tsx | 214 ++++++++++++------ .../reminder-edit/edit-when-form.tsx | 18 +- .../reminder-wizard/when-form-client.tsx | 19 +- 3 files changed, 143 insertions(+), 108 deletions(-) diff --git a/apps/web/src/components/recurrence-picker.tsx b/apps/web/src/components/recurrence-picker.tsx index dcf67d5..22f8a8b 100644 --- a/apps/web/src/components/recurrence-picker.tsx +++ b/apps/web/src/components/recurrence-picker.tsx @@ -15,7 +15,12 @@ import { } from "@/lib/recurrence"; interface RecurrencePickerProps { - /** First fire — drives the HH:MM in every row's cron output. */ + /** + * First fire — only used to seed defaults for newly-added rules + * (initial weekday, monthday, time). Each rule then carries its + * own hour/minute, so changing the date+time inputs above no + * longer reshapes existing rules. + */ firstFire: DateTime; value: RecurrenceSpec; onChange: (next: RecurrenceSpec) => void; @@ -34,6 +39,10 @@ interface Draft { weekdays: number[]; monthDay: number; month: number; + /** Hour-of-day (0-23) for this rule's fire time. */ + hour: number; + /** Minute-of-hour (0-59). */ + minute: number; } const MAX_RULES = 8; @@ -45,9 +54,24 @@ function defaultDraft(firstFire: DateTime): Draft { weekdays: [isoWeekdayToCron(firstFire.weekday)], monthDay: firstFire.day, month: firstFire.month, + hour: firstFire.hour, + minute: firstFire.minute, }; } +function pad2(n: number): string { + return n.toString().padStart(2, "0"); +} + +function parseHHMM(s: string): { hour: number; minute: number } | null { + const m = s.match(/^(\d{1,2}):(\d{2})$/); + if (!m) return null; + const h = Number(m[1]); + const min = Number(m[2]); + if (h < 0 || h > 23 || min < 0 || min > 59) return null; + return { hour: h, minute: min }; +} + function clamp(n: number, lo: number, hi: number): number { if (!Number.isFinite(n)) return lo; return Math.min(Math.max(Math.floor(n), lo), hi); @@ -58,9 +82,9 @@ const MONTH_NAMES = [ "July", "August", "September", "October", "November", "December", ]; -function draftToCron(d: Draft, firstFire: DateTime): string | null { - const m = firstFire.minute; - const h = firstFire.hour; +function draftToCron(d: Draft): string | null { + const m = clamp(d.minute, 0, 59); + const h = clamp(d.hour, 0, 23); switch (d.type) { case "daily": return d.dailyMode === "weekdays" ? `${m} ${h} * * 1-5` : `${m} ${h} * * *`; @@ -74,8 +98,8 @@ function draftToCron(d: Draft, firstFire: DateTime): string | null { } } -function describeDraft(d: Draft, firstFire: DateTime): string { - const t = firstFire.toFormat("HH:mm"); +function describeDraft(d: Draft): string { + const t = `${pad2(clamp(d.hour, 0, 23))}:${pad2(clamp(d.minute, 0, 59))}`; switch (d.type) { case "daily": return d.dailyMode === "weekdays" @@ -105,14 +129,21 @@ function describeDraft(d: Draft, firstFire: DateTime): string { * daily-every-day if the expression doesn't match a known shape. */ function draftFromCronExpr(expr: string, firstFire: DateTime): Draft { const base = defaultDraft(firstFire); + // Pull MM HH off the front so each rule restores its own time. + const head = expr.match(/^(\d+)\s+(\d+)\s+(.*)$/); + if (!head) return base; + const minute = clamp(Number(head[1]), 0, 59); + const hour = clamp(Number(head[2]), 0, 23); + const rest = head[3]!.trim(); + let m: RegExpMatchArray | null; - if (expr.match(/^\d+ \d+ \* \* 1-5$/)) { - return { ...base, type: "daily", dailyMode: "weekdays" }; + if (rest === "* * 1-5") { + return { ...base, type: "daily", dailyMode: "weekdays", hour, minute }; } - if (expr.match(/^\d+ \d+ \* \* \*$/)) { - return { ...base, type: "daily", dailyMode: "every_day" }; + if (rest === "* * *") { + return { ...base, type: "daily", dailyMode: "every_day", hour, minute }; } - if ((m = expr.match(/^\d+ \d+ \* \* ([0-9,\-]+)$/))) { + if ((m = rest.match(/^\* \* ([0-9,\-]+)$/))) { const days = m[1]! .split(",") .flatMap((p) => { @@ -125,15 +156,22 @@ function draftFromCronExpr(expr: string, firstFire: DateTime): Draft { return [Number(p)]; }) .filter((n) => n >= 0 && n <= 6); - return { ...base, type: "weekly", weekdays: days }; + return { ...base, type: "weekly", weekdays: days, hour, minute }; } - if ((m = expr.match(/^\d+ \d+ (\d+) \* \*$/))) { - return { ...base, type: "monthly", monthDay: Number(m[1]) }; + if ((m = rest.match(/^(\d+) \* \*$/))) { + return { ...base, type: "monthly", monthDay: Number(m[1]), hour, minute }; } - if ((m = expr.match(/^\d+ \d+ (\d+) (\d+) \*$/))) { - return { ...base, type: "yearly", monthDay: Number(m[1]), month: Number(m[2]) }; + if ((m = rest.match(/^(\d+) (\d+) \*$/))) { + return { + ...base, + type: "yearly", + monthDay: Number(m[1]), + month: Number(m[2]), + hour, + minute, + }; } - return base; + return { ...base, hour, minute }; } /** Parse a (possibly multi-line) cron rule into an array of drafts. */ @@ -147,10 +185,10 @@ function draftsFromRule(rule: string | null | undefined, firstFire: DateTime): D /** Compile an array of drafts to a single multi-line CRON: rule, or null * if there are no rules (= one-off). */ -function draftsToRule(drafts: Draft[], firstFire: DateTime): string | null { +function draftsToRule(drafts: Draft[]): string | null { if (drafts.length === 0) return null; const exprs = drafts - .map((d) => draftToCron(d, firstFire)) + .map((d) => draftToCron(d)) .filter((s): s is string => Boolean(s)); if (exprs.length === 0) return null; return exprs.join("\n"); @@ -190,11 +228,11 @@ export function RecurrencePicker({ firstFire, value, onChange }: RecurrencePicke draftsFromRule(value.kind === "cron" ? value.cron ?? null : null, firstFire), ); - // Compile drafts back to the parent value whenever they change. Also - // re-runs when first-fire moves (changing the time picker reshapes - // every row's cron output). + // Compile drafts back to the parent value whenever they change. Each + // rule carries its own hour/minute now, so the date+time inputs above + // no longer drive cron output — they only set the very first fire. useEffect(() => { - const rule = draftsToRule(drafts, firstFire); + const rule = draftsToRule(drafts); if (!rule) { if (value.kind !== "none") { onChange({ kind: "none", interval: 1, weeklyDays: [], end: { kind: "never" } }); @@ -211,7 +249,7 @@ export function RecurrencePicker({ firstFire, value, onChange }: RecurrencePicke }); } // eslint-disable-next-line react-hooks/exhaustive-deps - }, [drafts, firstFire.minute, firstFire.hour, firstFire.day, firstFire.month, firstFire.weekday]); + }, [drafts]); function updateDraft(idx: number, patch: Partial) { setDrafts((prev) => prev.map((d, i) => (i === idx ? { ...d, ...patch } : d))); @@ -263,12 +301,11 @@ export function RecurrencePicker({ firstFire, value, onChange }: RecurrencePicke updateDraft(idx, patch)} />

- {describeDraft(draft, firstFire)} + {describeDraft(draft)}

))} @@ -303,11 +340,10 @@ export function RecurrencePicker({ firstFire, value, onChange }: RecurrencePicke interface RuleEditorProps { draft: Draft; - firstFire: DateTime; onChange: (patch: Partial) => void; } -function RuleEditor({ draft, firstFire, onChange }: RuleEditorProps) { +function RuleEditor({ draft, onChange }: RuleEditorProps) { return ( Yearly - + onChange({ dailyMode: "weekdays" })} label="Every weekday (Mon – Fri)" /> + - - -
- {WEEKDAY_LABELS.map(({ iso, short }) => { - const cronDow = isoWeekdayToCron(iso); - const active = draft.weekdays.includes(cronDow); - return ( - - ); - })} + +
+ +
+ {WEEKDAY_LABELS.map(({ iso, short }) => { + const cronDow = isoWeekdayToCron(iso); + const active = draft.weekdays.includes(cronDow); + return ( + + ); + })} +
+ - - -
- onChange({ monthDay: Number(e.target.value) || 1 })} - className="h-8 w-24" - /> - - Months without this day skip naturally (e.g. 31st) - + +
+ +
+ onChange({ monthDay: Number(e.target.value) || 1 })} + className="h-8 w-24" + /> + + Months without this day skip naturally (e.g. 31st) + +
+ - +
@@ -412,11 +455,36 @@ function RuleEditor({ draft, firstFire, onChange }: RuleEditorProps) { />
+ ); } +interface TimeFieldProps { + draft: Draft; + onChange: (patch: Partial) => void; +} + +/** Per-rule time picker (HH:MM). Drives the cron expression's MM HH columns. */ +function TimeField({ draft, onChange }: TimeFieldProps) { + const value = `${pad2(clamp(draft.hour, 0, 23))}:${pad2(clamp(draft.minute, 0, 59))}`; + return ( +
+ + { + const parsed = parseHHMM(e.target.value); + if (parsed) onChange(parsed); + }} + className="h-8 w-32" + /> +
+ ); +} + interface RadioRowProps { name: string; checked: boolean; diff --git a/apps/web/src/components/reminder-edit/edit-when-form.tsx b/apps/web/src/components/reminder-edit/edit-when-form.tsx index c0392d8..dc258d8 100644 --- a/apps/web/src/components/reminder-edit/edit-when-form.tsx +++ b/apps/web/src/components/reminder-edit/edit-when-form.tsx @@ -14,11 +14,7 @@ import { Button } from "@/components/ui/button"; import { Input } from "@/components/ui/input"; import { Label } from "@/components/ui/label"; import { splitDateTime, validateScheduledAt } from "@/lib/date-picker"; -import { - buildRrule, - describeRecurrence, - type RecurrenceSpec, -} from "@/lib/recurrence"; +import { buildRrule, type RecurrenceSpec } from "@/lib/recurrence"; import { RecurrencePicker } from "@/components/recurrence-picker"; import { updateReminderAction } from "@/actions/reminders"; @@ -121,8 +117,6 @@ export function EditWhenForm({ } } - const previewSentence = describeRecurrence(spec, previewDt); - return (
@@ -162,12 +156,6 @@ export function EditWhenForm({ - {previewSentence && ( -

- {previewSentence} -

- )} - {error && (
@@ -175,10 +163,6 @@ export function EditWhenForm({
)} -

- Times are in {timezone}. -

-