From 6dad359b2a215043ec7514c268d05afd859b4452 Mon Sep 17 00:00:00 2001 From: Lucas Winther Date: Fri, 28 Aug 2026 04:43:56 +0200 Subject: [PATCH] Ask the cadence first, then only the dates it needs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The repeat controls sat after the dates and applied to whatever was there, which meant a weekly chore still had to answer an end date it has no honest answer to. Asking first inverts that: the cadence decides which dates are even questions. A preset carries no window. Weekly means the week is the window, so there is no end to type and no ignorance to admit — five fields instead of eight, and it stores exactly what the model already renders as back-to-back occurrences. A dated recurring event is therefore a custom, which is where the forever/delay control now lives; a one-off shows no repeat machinery at all. cadenceOf derives which of the five a saved event opens in, so a rule made before this control existed opens in whichever answer describes it. Nothing about the schema moved. Switching cadence hides the end date rather than clearing it: hiding a field and quietly discarding what is in it is how a form loses somebody's work when they change their mind back. The duplicate note was found by rendering the form and looking at it. Both of the older notes explain the end-date field, so a preset — which has no such field — was showing two sentences that said nearly the same thing. Co-Authored-By: Claude Opus 5 (1M context) --- docs/PRD.md | 32 ++- src/client/components/CustomForms.tsx | 314 +++++++++++++++----------- src/shared/recurrence.ts | 46 ++++ test/custom-ui.test.tsx | 141 +++++++++++- test/recurrence.test.ts | 41 +++- 5 files changed, 434 insertions(+), 140 deletions(-) diff --git a/docs/PRD.md b/docs/PRD.md index 6520c23..b1ec5a4 100644 --- a/docs/PRD.md +++ b/docs/PRD.md @@ -323,10 +323,34 @@ Four constraints, each protecting something that already exists: which is the same argument the code already makes for streaks. This is the *only* copy — there is no server to restore from. -A reader's event may also state how it comes round again — every N days, weeks -or months. The rule is stored; its occurrences are derived, and each one is an -ordinary event everywhere in the app: its own countdown, its own completion, -its own daily checklist. +A reader's event may also state how it comes round again. The form asks that +directly after **Kind**, before it asks for any date, because the answer +decides which dates are even questions: + +| Cadence | What it asks for | What it stores | +|---|---|---| +| one-off | start, end, or "I don't know when it ends" | no rule | +| daily / weekly / monthly | a start, and nothing else | no end, a one-unit cycle | +| custom | start, end, and how it repeats | whatever the reader states | + +**A preset carries no window.** Choosing weekly says the week *is* the window — +each occurrence runs until the next opens — so there is no end date to type and +no ignorance to admit. A dated recurring event ("1–16 September, and again +every month") is therefore a `custom`, not a preset. + +Under `custom` the reader says whether it repeats **forever** — reopening the +moment it closes — or **after a delay**, and the cadence is measured from the +dates they already gave rather than asked for again. A delay is stated as the +wait between closing and reopening, so it cannot describe a rule that comes +round before it ends; a hand-stated cycle length can, and is refused. + +Which of the five a saved event opens in is derived from the rule itself, so an +event made before this control existed, or one that arrived by import, opens in +whichever answer actually describes it. + +The rule is stored; its occurrences are derived, and each one is an ordinary +event everywhere in the app: its own countdown, its own completion, its own +daily checklist. **The schedule can stop on a date (`until`), but the form has no control for setting one.** The field exists in the schema — descoped from the form during diff --git a/src/client/components/CustomForms.tsx b/src/client/components/CustomForms.tsx index 495b679..ef89a2d 100644 --- a/src/client/components/CustomForms.tsx +++ b/src/client/components/CustomForms.tsx @@ -7,11 +7,14 @@ import { } from "../../shared/custom.ts"; import { addUnits, + cadenceOf, comesRoundEarly, movesOccurrences, + PRESET_UNIT, repeatModeOf, repeatSpanning, RepeatUnit, + type Cadence, type Repeat, type RepeatMode, } from "../../shared/recurrence.ts"; @@ -375,6 +378,11 @@ export function EventForm({ // Separate from an empty end date so "I don't know" is a thing the reader // states, not a field they leave blank and hope about. const [endKnown, setEndKnown] = useState(initial ? initial.endsAt !== null : true); + // Asked before the dates, because it decides which of them are even + // questions: a preset's period is its window, so there is no end to type. + const [cadence, setCadence] = useState(() => + cadenceOf(initial?.endsAt ?? null, initial?.repeat ?? null), + ); const [endDate, setEndDate] = useState(end.date); const [endTime, setEndTime] = useState( initialEndTime, @@ -401,11 +409,20 @@ export function EventForm({ const [cadenceUnit, setCadenceUnit] = useState(opening.unit); const [cadenceAmount, setCadenceAmount] = useState(String(opening.amount)); + // A preset's period is its window, so it has no end to state. The end the + // reader may have typed under a different cadence is kept in state rather + // than cleared — hiding a field and quietly discarding what is in it is how + // a form loses somebody's work when they change their mind back. + const preset = cadence === "daily" || cadence === "weekly" || cadence === "monthly"; + const datedWindow = cadence === "one-off" || cadence === "custom"; + const startsAt = startDate === "" ? null : readerInstant(startDate, startTime, "start"); const endsAt = - !endKnown || endDate === "" ? null : readerInstant(endDate, endTime, "end"); + !datedWindow || !endKnown || endDate === "" + ? null + : readerInstant(endDate, endTime, "end"); - const endMissing = endKnown && endDate !== "" && endsAt === null; + const endMissing = datedWindow && endKnown && endDate !== "" && endsAt === null; const backwards = startsAt !== null && endsAt !== null && endsAt <= startsAt; const startMs = startsAt === null ? null : Date.parse(startsAt); @@ -432,20 +449,27 @@ export function EventForm({ // occurrence runs until the next opens. const delayNeedsEnd = repeatMode === "delay" && endMs === null; - const repeat = repeatOf({ - mode: repeatMode, - measuring, - measured, - startMs, - contiguousMs, - unit: cadenceUnit, - amount, - until: existingUntil, - }); + const repeat = preset + ? repeatFrom(PRESET_UNIT[cadence], 1, existingUntil) + : cadence === "one-off" + ? null + : repeatOf({ + mode: repeatMode, + measuring, + measured, + startMs, + contiguousMs, + unit: cadenceUnit, + amount, + until: existingUntil, + }); + // Only `custom` can be incomplete. A preset is one unit with no window and + // is therefore always sayable, and a one-off has no repeat to get wrong. const repeatIncomplete = - (repeatMode === "forever" && !measuring && !amountValid) || - (repeatMode === "delay" && (delayNeedsEnd || repeat === null)); + cadence === "custom" && + ((repeatMode === "forever" && !measuring && !amountValid) || + (repeatMode === "delay" && (delayNeedsEnd || repeat === null))); // The same predicate the schema refines on, so the form cannot start // refusing saves the schema would accept or promising ones it will reject. @@ -547,6 +571,31 @@ export function EventForm({ + {/* Asked before the dates because it decides which of them are even + questions. A weekly chore has no end date worth typing — the week is + the window — and a form that asks anyway is asking something with no + honest answer. */} + + + {preset && ( +

+ Each one runs until the next opens, so there is no end date to give. +

+ )} +
- {/* The end is allowed to be unknown, and says so out loud. Making it + {/* Only a cadence that carries its own window asks about an end. The end + is allowed to be unknown there, and says so out loud: making it mandatory would push the reader into inventing a date, which is exactly the failure the parsers are forbidden from committing. */} - - - {endKnown && ( -
- - -
- )} - - - - {repeatMode !== "never" && endMs === null && ( -

- A delay needs an end date to be measured from. With none, each one - just runs until the next opens. -

- )} - - {/* Measured, and said out loud — a cadence the form worked out silently - would be a date the reader never agreed to, which is the one thing - this product does not do. */} - {measuring && measured !== null && ( -

- {cadenceLabel(measured)} · worked out from your dates.{" "} - -

- )} - - {repeatMode === "forever" && !measuring && ( -
- - -
- )} - - {repeatMode === "delay" && ( + {datedWindow && ( <> + + + {endKnown && ( +
+ + +
+ )} + + )} + + {/* Only a custom cadence needs any of this: a preset answers it by + construction, and a one-off has nothing to answer. */} + {cadence === "custom" && ( + <> + + + {repeatMode !== "never" && endMs === null && ( +

+ A delay needs an end date to be measured from. With none, each one + just runs until the next opens. +

+ )} + + {/* Measured, and said out loud — a cadence the form worked out silently + would be a date the reader never agreed to, which is the one thing + this product does not do. */} + {measuring && measured !== null && ( +

+ {cadenceLabel(measured)} · worked out from your dates.{" "} + +

+ )} + + {repeatMode === "forever" && !measuring && (
- {/* Both readings, so the reader can check one against the other: a - week's wait after a week's window is a fortnightly rule, and - seeing that spelled out is how they catch a wrong number. */} -

- after it ends - {repeat !== null ? ` · ${cadenceLabel(repeat)}` : ""} -

+ )} + + {repeatMode === "delay" && ( + <> +
+ + +
+ {/* Both readings, so the reader can check one against the other: a + week's wait after a week's window is a fortnightly rule, and + seeing that spelled out is how they catch a wrong number. */} +

+ after it ends + {repeat !== null ? ` · ${cadenceLabel(repeat)}` : ""} +

+ + )} )} @@ -728,13 +788,13 @@ export function EventForm({ /> - {!endKnown && repeatMode === "never" && ( + {datedWindow && !endKnown && cadence === "one-off" && (

It'll show with no countdown and no daily checklist, the same as an event whose source hasn't announced an end.

)} - {!endKnown && repeatMode !== "never" && ( + {datedWindow && !endKnown && cadence === "custom" && ( /* Not a degraded answer here — the interval bounds it. */

Each one runs until the next one opens, so it still counts down. diff --git a/src/shared/recurrence.ts b/src/shared/recurrence.ts index 6e0f11a..2a5597f 100644 --- a/src/shared/recurrence.ts +++ b/src/shared/recurrence.ts @@ -151,6 +151,52 @@ export function repeatSpanning(fromMs: number, toMs: number): Repeat | null { /** The three answers the form offers for "does this come round again?". */ export type RepeatMode = "never" | "forever" | "delay"; +/** The five answers the form offers for "how often is this?". */ +export type Cadence = "one-off" | "daily" | "weekly" | "monthly" | "custom"; + +const PRESET_OF: Record = { + days: "daily", + weeks: "weekly", + months: "monthly", +}; + +/** + * The unit each preset stands for — the inverse of the map above, kept beside + * it so the two cannot drift apart. A preset is always an interval of one: + * that is what makes it a preset rather than a cycle length. + */ +export const PRESET_UNIT: Record<"daily" | "weekly" | "monthly", RepeatUnit> = { + daily: "days", + weekly: "weeks", + monthly: "months", +}; + +/** + * Which of the form's five answers describes a saved event. + * + * Derived rather than stored, for the reason `repeatModeOf` is: a rule made + * before this control existed, or one that arrived by import, has to open in + * whichever answer actually fits it rather than in whichever happens to be the + * default. + * + * **A preset carries no window.** Choosing daily, weekly or monthly says the + * period *is* the window — each occurrence runs until the next opens — so an + * event that states its own end is saying something no preset can, and belongs + * under `custom` where that is sayable. The same goes for a series that stops: + * there is no control for `until` in a preset, and opening one there would + * offer to save a rule quietly stripped of the date it ends on. + */ +export function cadenceOf( + endsAt: string | null, + repeat: Repeat | null, +): Cadence { + if (repeat === null) return "one-off"; + if (endsAt !== null || repeat.until !== null || repeat.interval !== 1) { + return "custom"; + } + return PRESET_OF[repeat.unit]; +} + /** * Which of the three states a saved rule belongs to. * diff --git a/test/custom-ui.test.tsx b/test/custom-ui.test.tsx index c168676..088b147 100644 --- a/test/custom-ui.test.tsx +++ b/test/custom-ui.test.tsx @@ -375,16 +375,21 @@ describe("stating a repeat", () => { // case; `forever()` narrows the interval to exactly that eight-day step. const forever = () => repeating({ repeat: { unit: "days", interval: 8, until: null } }); - test("a fresh form offers the three answers, set to never", () => { + test("a custom cadence offers the three answers", () => { + // The three-way control lives under `custom` now: a preset answers the + // question by construction and a one-off has nothing to answer, so this + // is the only cadence that has to ask. const html = renderToStaticMarkup( - {}} onCancel={() => {}} />, + {}} + onCancel={() => {}} + />, ); expect(html).toContain("Repeat"); expect(html).toContain("with a delay"); - // Nothing to configure until they pick one, so the form a reader already - // knows is unchanged until they reach for this. - expect(html).not.toContain("Every"); - expect(html).not.toContain("Wait"); }); test("a rule that reopens as it closes shows its cadence rather than a control", () => { @@ -441,7 +446,7 @@ describe("stating a repeat", () => { {}} onCancel={() => {}} />, @@ -454,7 +459,7 @@ describe("stating a repeat", () => { {}} onCancel={() => {}} />, @@ -675,3 +680,123 @@ describe("the derived-boundary note", () => { expect(html).not.toContain("server reset"); }); }); + +describe("the cadence control", () => { + const event = (over: Record = {}) => + CustomEvent.parse({ + id: "myevent:k3f9qa2m01", + game: "mygame:limbus-company", + title: "Mirror Dungeon", + type: "challenge", + summary: null, + startsAt: "2026-09-01T00:00:00.000Z", + startPrecision: "day", + endsAt: "2026-09-08T00:00:00.000Z", + endPrecision: "day", + repeat: null, + at: AT, + updatedAt: AT, + ...over, + }); + + const render = (initial?: ReturnType) => + renderToStaticMarkup( + {}} + onCancel={() => {}} + />, + ); + + test("a fresh form offers all five answers and opens on one-off", () => { + const html = render(); + expect(html).toContain("Cadence"); + for (const answer of ["one-off", "daily", "weekly", "monthly", "custom"]) { + expect(html).toContain(answer); + } + // A one-off is what the form was before any of this, so it still asks for + // an end and still lets the reader say they do not know it. + expect(html).toContain("Ends"); + expect(html).toContain("I don't know when it ends"); + }); + + test("a preset asks for a start and nothing else", () => { + // The period is the window, so there is no end to type and no ignorance + // to admit. This is the whole reason the presets exist. + const html = render( + event({ + endsAt: null, + endPrecision: "unknown", + repeat: { unit: "weeks", interval: 1, until: null }, + }), + ); + expect(html).toContain("Starts"); + expect(html).not.toContain("Ends"); + expect(html).not.toContain("I don't know when it ends"); + expect(html).not.toContain("Cycle length"); + expect(html).not.toContain("Wait"); + }); + + test("each preset reopens as itself", () => { + const daily = render( + event({ endsAt: null, endPrecision: "unknown", repeat: { unit: "days", interval: 1, until: null } }), + ); + expect(daily).toContain('value="daily" selected=""'); + const monthly = render( + event({ endsAt: null, endPrecision: "unknown", repeat: { unit: "months", interval: 1, until: null } }), + ); + expect(monthly).toContain('value="monthly" selected=""'); + }); + + test("a dated recurring event reopens as a custom, with its window intact", () => { + // Presets carry no window, so anything that states one has to land here — + // and the end date it stated has to still be on screen. + const html = render(event({ repeat: { unit: "days", interval: 26, until: null } })); + expect(html).toContain('value="custom" selected=""'); + expect(html).toContain("Ends"); + expect(html).toContain("Repeat"); + }); + + test("a one-off shows no repeat machinery at all", () => { + const html = render(event()); + expect(html).toContain('value="one-off" selected=""'); + expect(html).not.toContain("Cycle length"); + expect(html).not.toContain("Wait"); + expect(html).not.toContain("worked out from your dates"); + }); +}); + +describe("a preset says its piece exactly once", () => { + test("no end-date note tags along when there is no end-date field", () => { + // Both of the older notes explain the end-date field. A preset has no such + // field, so rendering them there left two sentences saying almost the same + // thing — caught by looking at the form, not by any assertion on content. + const html = renderToStaticMarkup( + {}} + onCancel={() => {}} + />, + ); + expect(html).toContain("no end date to give"); + expect(html).not.toContain("still counts down"); + expect(html).not.toContain("no countdown"); + }); +}); diff --git a/test/recurrence.test.ts b/test/recurrence.test.ts index ee79e11..068f119 100644 --- a/test/recurrence.test.ts +++ b/test/recurrence.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { addUnits, comesRoundEarly, Repeat, repeatSpanning, repeatModeOf, isOccurrenceId, occurrenceId, occurrenceForId, ruleIdOf, movesOccurrences, nextOccurrences, occurrencesOf, strandedOccurrences, type RepeatingEvent } from "../src/shared/recurrence.ts"; +import { addUnits, comesRoundEarly, Repeat, repeatSpanning, repeatModeOf, cadenceOf, isOccurrenceId, occurrenceId, occurrenceForId, ruleIdOf, movesOccurrences, nextOccurrences, occurrencesOf, strandedOccurrences, type RepeatingEvent } from "../src/shared/recurrence.ts"; import { CustomEventId, isCustomEventId } from "../src/shared/custom.ts"; // Pinned so the DST cases mean something. Copenhagen is UTC+1 in winter and @@ -551,3 +551,42 @@ describe("repeatModeOf", () => { .toBe("forever"); }); }); + +describe("cadenceOf", () => { + // Which of the form's five answers describes a saved event. Derived rather + // than stored, like `repeatModeOf`, so a rule made before the control + // existed — or one that arrived by import — opens in whichever answer + // actually fits it rather than in whichever happens to be the default. + const weekly = { unit: "weeks", interval: 1, until: null } as const; + + test("no rule at all is a one-off", () => { + expect(cadenceOf(null, null)).toBe("one-off"); + expect(cadenceOf("2026-09-08T00:00:00.000Z", null)).toBe("one-off"); + }); + + test("a single unit with no end is the matching preset", () => { + expect(cadenceOf(null, { unit: "days", interval: 1, until: null })).toBe("daily"); + expect(cadenceOf(null, weekly)).toBe("weekly"); + expect(cadenceOf(null, { unit: "months", interval: 1, until: null })).toBe("monthly"); + }); + + test("a longer cycle is a custom", () => { + expect(cadenceOf(null, { unit: "days", interval: 26, until: null })).toBe("custom"); + expect(cadenceOf(null, { unit: "weeks", interval: 2, until: null })).toBe("custom"); + }); + + test("a stated end is a custom, whatever the cycle", () => { + // The presets carry no window — picking one means the period *is* the + // window. An event that states its own end is saying something the preset + // cannot, so it belongs where that is sayable. + expect(cadenceOf("2026-09-08T00:00:00.000Z", weekly)).toBe("custom"); + }); + + test("a series that stops is a custom", () => { + // There is no control for `until` in a preset, so opening one there would + // offer to save a rule quietly stripped of the date it stops on. + expect( + cadenceOf(null, { unit: "weeks", interval: 1, until: "2027-01-01T00:00:00.000Z" }), + ).toBe("custom"); + }); +});