feat(recurrence): per-rule fire time on every tab; drop redundant labels
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 `<p>` 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 <tz>" inline in the picker is noise.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
797917a4ba
commit
08435988c2
@ -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<Draft>) {
|
||||
setDrafts((prev) => prev.map((d, i) => (i === idx ? { ...d, ...patch } : d)));
|
||||
@ -263,12 +301,11 @@ export function RecurrencePicker({ firstFire, value, onChange }: RecurrencePicke
|
||||
|
||||
<RuleEditor
|
||||
draft={draft}
|
||||
firstFire={firstFire}
|
||||
onChange={(patch) => updateDraft(idx, patch)}
|
||||
/>
|
||||
|
||||
<p className="rounded-lg bg-primary/5 px-2.5 py-1.5 text-xs text-primary/80">
|
||||
{describeDraft(draft, firstFire)}
|
||||
{describeDraft(draft)}
|
||||
</p>
|
||||
</div>
|
||||
))}
|
||||
@ -303,11 +340,10 @@ export function RecurrencePicker({ firstFire, value, onChange }: RecurrencePicke
|
||||
|
||||
interface RuleEditorProps {
|
||||
draft: Draft;
|
||||
firstFire: DateTime;
|
||||
onChange: (patch: Partial<Draft>) => void;
|
||||
}
|
||||
|
||||
function RuleEditor({ draft, firstFire, onChange }: RuleEditorProps) {
|
||||
function RuleEditor({ draft, onChange }: RuleEditorProps) {
|
||||
return (
|
||||
<Tabs
|
||||
value={draft.type}
|
||||
@ -320,7 +356,7 @@ function RuleEditor({ draft, firstFire, onChange }: RuleEditorProps) {
|
||||
<TabsTrigger value="yearly">Yearly</TabsTrigger>
|
||||
</TabsList>
|
||||
|
||||
<TabsContent value="daily" className="space-y-2 pt-3">
|
||||
<TabsContent value="daily" className="space-y-3 pt-3">
|
||||
<RadioRow
|
||||
name={`daily-${draft.type}`}
|
||||
checked={draft.dailyMode === "every_day"}
|
||||
@ -333,58 +369,65 @@ function RuleEditor({ draft, firstFire, onChange }: RuleEditorProps) {
|
||||
onChange={() => onChange({ dailyMode: "weekdays" })}
|
||||
label="Every weekday (Mon – Fri)"
|
||||
/>
|
||||
<TimeField draft={draft} onChange={onChange} />
|
||||
</TabsContent>
|
||||
|
||||
<TabsContent value="weekly" className="space-y-2 pt-3">
|
||||
<Label className="text-sm">On these days</Label>
|
||||
<div className="flex flex-wrap gap-1.5">
|
||||
{WEEKDAY_LABELS.map(({ iso, short }) => {
|
||||
const cronDow = isoWeekdayToCron(iso);
|
||||
const active = draft.weekdays.includes(cronDow);
|
||||
return (
|
||||
<button
|
||||
key={iso}
|
||||
type="button"
|
||||
onClick={() =>
|
||||
onChange({
|
||||
weekdays: active
|
||||
? draft.weekdays.filter((d) => d !== cronDow)
|
||||
: [...draft.weekdays, cronDow].sort((a, b) => a - b),
|
||||
})
|
||||
}
|
||||
aria-pressed={active}
|
||||
className={cn(
|
||||
"inline-flex h-8 min-w-12 items-center justify-center rounded-lg border px-2.5 text-xs font-medium transition-colors",
|
||||
active
|
||||
? "border-primary bg-primary text-primary-foreground"
|
||||
: "border-border bg-background hover:border-primary/50 hover:bg-primary/5 hover:text-primary",
|
||||
)}
|
||||
>
|
||||
{short}
|
||||
</button>
|
||||
);
|
||||
})}
|
||||
<TabsContent value="weekly" className="space-y-3 pt-3">
|
||||
<div className="space-y-2">
|
||||
<Label className="text-sm">On these days</Label>
|
||||
<div className="flex flex-wrap gap-1.5">
|
||||
{WEEKDAY_LABELS.map(({ iso, short }) => {
|
||||
const cronDow = isoWeekdayToCron(iso);
|
||||
const active = draft.weekdays.includes(cronDow);
|
||||
return (
|
||||
<button
|
||||
key={iso}
|
||||
type="button"
|
||||
onClick={() =>
|
||||
onChange({
|
||||
weekdays: active
|
||||
? draft.weekdays.filter((d) => d !== cronDow)
|
||||
: [...draft.weekdays, cronDow].sort((a, b) => a - b),
|
||||
})
|
||||
}
|
||||
aria-pressed={active}
|
||||
className={cn(
|
||||
"inline-flex h-8 min-w-12 items-center justify-center rounded-lg border px-2.5 text-xs font-medium transition-colors",
|
||||
active
|
||||
? "border-primary bg-primary text-primary-foreground"
|
||||
: "border-border bg-background hover:border-primary/50 hover:bg-primary/5 hover:text-primary",
|
||||
)}
|
||||
>
|
||||
{short}
|
||||
</button>
|
||||
);
|
||||
})}
|
||||
</div>
|
||||
</div>
|
||||
<TimeField draft={draft} onChange={onChange} />
|
||||
</TabsContent>
|
||||
|
||||
<TabsContent value="monthly" className="space-y-2 pt-3">
|
||||
<Label className="text-sm">Day of the month</Label>
|
||||
<div className="flex items-center gap-2">
|
||||
<Input
|
||||
type="number"
|
||||
min={1}
|
||||
max={31}
|
||||
value={draft.monthDay}
|
||||
onChange={(e) => onChange({ monthDay: Number(e.target.value) || 1 })}
|
||||
className="h-8 w-24"
|
||||
/>
|
||||
<span className="text-xs text-muted-foreground">
|
||||
Months without this day skip naturally (e.g. 31st)
|
||||
</span>
|
||||
<TabsContent value="monthly" className="space-y-3 pt-3">
|
||||
<div className="space-y-2">
|
||||
<Label className="text-sm">Day of the month</Label>
|
||||
<div className="flex items-center gap-2">
|
||||
<Input
|
||||
type="number"
|
||||
min={1}
|
||||
max={31}
|
||||
value={draft.monthDay}
|
||||
onChange={(e) => onChange({ monthDay: Number(e.target.value) || 1 })}
|
||||
className="h-8 w-24"
|
||||
/>
|
||||
<span className="text-xs text-muted-foreground">
|
||||
Months without this day skip naturally (e.g. 31st)
|
||||
</span>
|
||||
</div>
|
||||
</div>
|
||||
<TimeField draft={draft} onChange={onChange} />
|
||||
</TabsContent>
|
||||
|
||||
<TabsContent value="yearly" className="pt-3">
|
||||
<TabsContent value="yearly" className="space-y-3 pt-3">
|
||||
<div className="flex flex-wrap items-end gap-3">
|
||||
<div className="space-y-1">
|
||||
<Label className="text-xs text-muted-foreground">Month</Label>
|
||||
@ -412,11 +455,36 @@ function RuleEditor({ draft, firstFire, onChange }: RuleEditorProps) {
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
<TimeField draft={draft} onChange={onChange} />
|
||||
</TabsContent>
|
||||
</Tabs>
|
||||
);
|
||||
}
|
||||
|
||||
interface TimeFieldProps {
|
||||
draft: Draft;
|
||||
onChange: (patch: Partial<Draft>) => 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 (
|
||||
<div className="space-y-1">
|
||||
<Label className="text-xs text-muted-foreground">Fires at</Label>
|
||||
<Input
|
||||
type="time"
|
||||
value={value}
|
||||
onChange={(e) => {
|
||||
const parsed = parseHHMM(e.target.value);
|
||||
if (parsed) onChange(parsed);
|
||||
}}
|
||||
className="h-8 w-32"
|
||||
/>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
interface RadioRowProps {
|
||||
name: string;
|
||||
checked: boolean;
|
||||
|
||||
@ -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 (
|
||||
<div className="space-y-5">
|
||||
<div className="grid grid-cols-1 sm:grid-cols-2 gap-3">
|
||||
@ -162,12 +156,6 @@ export function EditWhenForm({
|
||||
|
||||
<RecurrencePicker firstFire={previewDt} value={spec} onChange={setSpec} />
|
||||
|
||||
{previewSentence && (
|
||||
<p className="rounded-lg bg-primary/5 px-3 py-2 text-xs text-primary/80">
|
||||
{previewSentence}
|
||||
</p>
|
||||
)}
|
||||
|
||||
{error && (
|
||||
<div className="flex items-center gap-1.5 rounded-lg bg-destructive/10 px-3 py-2 text-xs text-destructive">
|
||||
<AlertCircleIcon className="size-3.5 shrink-0" />
|
||||
@ -175,10 +163,6 @@ export function EditWhenForm({
|
||||
</div>
|
||||
)}
|
||||
|
||||
<p className="text-xs text-muted-foreground">
|
||||
Times are in <span className="font-medium">{timezone}</span>.
|
||||
</p>
|
||||
|
||||
<div className="flex justify-end">
|
||||
<Button type="button" onClick={handleSave} disabled={submitting} className="gap-2">
|
||||
{submitting ? <Loader2Icon className="size-4 animate-spin" /> : <SaveIcon className="size-4" />}
|
||||
|
||||
@ -7,12 +7,7 @@ import { CalendarIcon, ClockIcon, AlertCircleIcon } from "lucide-react";
|
||||
import { Button } from "@/components/ui/button";
|
||||
import { Input } from "@/components/ui/input";
|
||||
import { Label } from "@/components/ui/label";
|
||||
import {
|
||||
buildRrule,
|
||||
describeRecurrence,
|
||||
DEFAULT_RECURRENCE,
|
||||
type RecurrenceSpec,
|
||||
} from "@/lib/recurrence";
|
||||
import { buildRrule, DEFAULT_RECURRENCE, type RecurrenceSpec } from "@/lib/recurrence";
|
||||
import { splitDateTime, validateScheduledAt } from "@/lib/date-picker";
|
||||
import { RecurrencePicker } from "@/components/recurrence-picker";
|
||||
|
||||
@ -124,8 +119,6 @@ export function WhenFormClient({
|
||||
router.push(`/reminders/new?${sp.toString()}` as any);
|
||||
}
|
||||
|
||||
const previewSentence = describeRecurrence(spec, previewDt);
|
||||
|
||||
return (
|
||||
<div className="space-y-5">
|
||||
{/* Date + time */}
|
||||
@ -166,12 +159,6 @@ export function WhenFormClient({
|
||||
|
||||
<RecurrencePicker firstFire={previewDt} value={spec} onChange={setSpec} />
|
||||
|
||||
{previewSentence && (
|
||||
<p className="rounded-lg bg-primary/5 px-3 py-2 text-xs text-primary/80">
|
||||
{previewSentence}
|
||||
</p>
|
||||
)}
|
||||
|
||||
{error && (
|
||||
<div className="flex items-center gap-1.5 rounded-lg bg-destructive/10 px-3 py-2 text-xs text-destructive">
|
||||
<AlertCircleIcon className="size-3.5 shrink-0" />
|
||||
@ -179,10 +166,6 @@ export function WhenFormClient({
|
||||
</div>
|
||||
)}
|
||||
|
||||
<p className="text-xs text-muted-foreground">
|
||||
Times are in <span className="font-medium">{timezone}</span>.
|
||||
</p>
|
||||
|
||||
<div className="flex justify-end pt-1">
|
||||
<Button type="button" onClick={handleContinue}>
|
||||
Continue
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user