From 1f1d608e95f62cbca3d51ddccaa4439ef65b1196 Mon Sep 17 00:00:00 2001 From: Lucas Winther Date: Thu, 27 Aug 2026 16:47:18 +0200 Subject: [PATCH] Ask how it repeats, not how often MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three answers — never, forever, or after a delay — because "how often" is rarely the question a reader has. They know it comes back the moment it ends, or that it comes back after a wait. The cadence follows from that and from dates they have already typed, so the form reports what it measured instead of asking for a number, and says it out loud rather than filling a field silently. A delay is expressed by where it lands: the next opening is the wait added to the close, so a delay cannot produce a rule that comes round before it ends. Only a hand-stated cadence can, and stating one by hand stays available — measuring is the convenience, not a cage. Both readings are shown for a delay, since a week's wait after a week's window is a fortnightly rule and seeing that spelled out is how a wrong number gets caught. contiguousOpening is the load-bearing detail. An end given as a date is stored as 23:59:59, so a successor opening "the moment it closes" opens at the following midnight, a second later. Comparing against the stored end instead read every day-precision rule as having a gap it does not have — and the form has to measure the instants it will save, not the ones the record happens to hold, or it opens describing a rule it would not write. Co-Authored-By: Claude Opus 5 (1M context) --- src/client/components/CustomForms.tsx | 310 +++++++++++++++++++++++--- src/shared/recurrence.ts | 15 +- test/custom-ui.test.tsx | 82 ++++++- 3 files changed, 362 insertions(+), 45 deletions(-) diff --git a/src/client/components/CustomForms.tsx b/src/client/components/CustomForms.tsx index 7af555f..66a3e10 100644 --- a/src/client/components/CustomForms.tsx +++ b/src/client/components/CustomForms.tsx @@ -6,10 +6,14 @@ import { type LaneId, } from "../../shared/custom.ts"; import { + addUnits, comesRoundEarly, movesOccurrences, + repeatModeOf, + repeatSpanning, RepeatUnit, type Repeat, + type RepeatMode, } from "../../shared/recurrence.ts"; import { EventType } from "../../shared/schema.ts"; import { useGameMeta } from "../state/gameMeta.tsx"; @@ -95,6 +99,99 @@ export function repeatFrom( return { unit, interval, until: existingUntil }; } +/** + * The instant a successor opens if it opens the moment this window closes. + * + * `readerInstant` resolves an end the reader gave no time to as 23:59:59 — + * the last second of the day they named, because that is what "runs until the + * 8th" means to a person. The next window therefore opens at midnight, one + * second later, not on that final second. An end they *did* give a time to is + * an instant they chose, and a successor opens on it exactly. + * + * One second is not a fudge factor: it is the exact distance between this + * form's end-of-day convention and the midnight that follows it. The + * convention lives here rather than in `recurrence.ts` because this form is + * what wrote the boundary in the first place. + */ +function contiguousOpening(endsMs: number, endHasTime: boolean): number { + return endHasTime ? endsMs : endsMs + 1000; +} + +/** + * What to put in the number-and-unit pair when the form opens. + * + * Both the delay control and the hand-stated cadence share one pair, because + * only one of them is ever on screen and carrying the number across when the + * reader changes their mind is kinder than resetting it to 1. + * + * For a rule that already has a gap this recovers the gap itself — the stored + * interval spans the window *and* the wait, and the reader entered the wait — + * so reopening shows them the number they typed rather than the one derived + * from it. A rule with no gap has none to recover and falls back to its own + * cadence, which is the sensible starting point if they switch to a delay. + */ +function openingControls( + startsAt: string | null, + endsAt: string | null, + endHasTime: boolean, + rule: Repeat | null, +): { unit: RepeatUnit; amount: number } { + if (rule === null || startsAt === null) return { unit: "weeks", amount: 1 }; + + const contiguousMs = + endsAt === null ? null : contiguousOpening(Date.parse(endsAt), endHasTime); + const gap = + contiguousMs === null + ? null + : repeatSpanning( + contiguousMs, + addUnits(Date.parse(startsAt), rule.unit, rule.interval), + ); + return gap === null + ? { unit: rule.unit, amount: rule.interval } + : { unit: gap.unit, amount: gap.interval }; +} + +/** + * The rule the three-way control currently describes. + * + * Pulled out of the component because it is the one place the three answers + * become the single `{unit, interval}` the schema stores, and that translation + * is worth reading in one piece rather than spread through the render. + * + * A delay is expressed by where it lands: the next opening is the wait added + * to the close, and the interval is whatever spans the anchor to there. That + * is why a delay can never produce a rule that comes round before it ends — + * the next opening is at or after the close by construction. Only a + * hand-stated cadence can, which is why `comesRoundEarly` still guards the + * form. + */ +function repeatOf(input: { + mode: RepeatMode; + measuring: boolean; + measured: Repeat | null; + startMs: number | null; + contiguousMs: number | null; + unit: RepeatUnit; + amount: number; + until: string | null; +}): Repeat | null { + const { mode, measuring, measured, startMs, contiguousMs, unit, amount, until } = + input; + + if (mode === "never") return null; + if (mode === "forever") { + return measuring && measured !== null + ? repeatFrom(measured.unit, measured.interval, until) + : repeatFrom(unit, amount, until); + } + + if (startMs === null || contiguousMs === null) return null; + if (!Number.isInteger(amount) || amount < 1 || amount > 365) return null; + const span = repeatSpanning(startMs, addUnits(contiguousMs, unit, amount)); + return span === null ? null : repeatFrom(span.unit, span.interval, until); +} + function labelClass(): string { return "block text-xs font-medium text-muted"; } @@ -211,6 +308,24 @@ export function EventForm({ const start = fields(initial?.startsAt ?? null); const end = fields(initial?.endsAt ?? null); + // Round-tripped through the same fields-then-readerInstant path the live + // form uses, rather than read straight off the record. The two disagree: a + // day-precision end is stored as whatever instant it was written at, and the + // form re-resolves it to the end of the reader's own day. Deriving the + // opening state from the stored value and everything after it from the + // re-resolved one is how the control opens saying "with a delay" about a + // rule that has no gap at all. + const initialStartTime = initial?.startPrecision === "exact" ? start.time : ""; + const initialEndTime = initial?.endPrecision === "exact" ? end.time : ""; + const initialStartsAt = + start.date === "" + ? null + : (readerInstant(start.date, initialStartTime, "start") ?? null); + const initialEndsAt = + initial?.endsAt == null || end.date === "" + ? null + : (readerInstant(end.date, initialEndTime, "end") ?? null); + // The reader's own games first, and so the default too. Someone filling this // in by hand is usually doing it *because* the game isn't tracked; making // them scroll past nine that are gets the common case backwards. Stable @@ -227,23 +342,36 @@ export function EventForm({ const [summary, setSummary] = useState(initial?.summary ?? ""); const [startDate, setStartDate] = useState(start.date); const [startTime, setStartTime] = useState( - initial?.startPrecision === "exact" ? start.time : "", + initialStartTime, ); // 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); const [endDate, setEndDate] = useState(end.date); const [endTime, setEndTime] = useState( - initial?.endPrecision === "exact" ? end.time : "", + initialEndTime, ); - // "never" rather than a null unit, so the select has one vocabulary and the - // default reads as an answer the reader gave rather than a field they left. - const [repeatUnit, setRepeatUnit] = useState( - initial?.repeat?.unit ?? "never", - ); - const [repeatInterval, setRepeatInterval] = useState( - String(initial?.repeat?.interval ?? 1), + // Three answers rather than a unit and a number, because "how often" is + // rarely the question a reader actually has. They know it comes back the + // moment it ends, or that it comes back after a wait; the cadence follows + // from that and from dates they have already typed. + const [repeatMode, setRepeatMode] = useState(() => + initial === undefined + ? "never" + : repeatModeOf( + Date.parse(initialStartsAt ?? initial.startsAt), + initialEndsAt === null + ? null + : contiguousOpening(Date.parse(initialEndsAt), initialEndTime !== ""), + initial.repeat, + ), ); + // Measuring is the convenience, not a cage: an irregular rotation still has + // to be sayable when the first window does not describe it. + const [ownCadence, setOwnCadence] = useState(false); + const opening = openingControls(initialStartsAt, initialEndsAt, initialEndTime !== "", initial?.repeat ?? null); + const [cadenceUnit, setCadenceUnit] = useState(opening.unit); + const [cadenceAmount, setCadenceAmount] = useState(String(opening.amount)); const startsAt = startDate === "" ? null : readerInstant(startDate, startTime, "start"); const endsAt = @@ -252,10 +380,44 @@ export function EventForm({ const endMissing = endKnown && endDate !== "" && endsAt === null; const backwards = startsAt !== null && endsAt !== null && endsAt <= startsAt; - const interval = Number(repeatInterval); - const intervalValid = - Number.isInteger(interval) && interval >= 1 && interval <= 365; - const repeat = repeatFrom(repeatUnit, interval, initial?.repeat?.until ?? null); + const startMs = startsAt === null ? null : Date.parse(startsAt); + const endMs = endsAt === null ? null : Date.parse(endsAt); + + const amount = Number(cadenceAmount); + const amountValid = Number.isInteger(amount) && amount >= 1 && amount <= 365; + const existingUntil = initial?.repeat?.until ?? null; + + // What the dates already say, when they say anything. Null covers both "no + // end given yet" and a window that is no whole number of any unit — two + // exact times a few hours apart — where rounding would move a boundary the + // reader chose, so the form asks instead. + const contiguousMs = + endMs === null ? null : contiguousOpening(endMs, endTime !== ""); + const measured = + startMs === null || contiguousMs === null + ? null + : repeatSpanning(startMs, contiguousMs); + const measuring = repeatMode === "forever" && !ownCadence && measured !== null; + + // A delay is measured from the end, so it has nothing to work with until one + // is given. Forever still does: the reader states the cadence and each + // 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 repeatIncomplete = + (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. @@ -275,7 +437,7 @@ export function EventForm({ !endMissing && (!endKnown || endDate !== "") && !earlyReturn && - (repeatUnit === "never" || intervalValid); + !repeatIncomplete; const draft: EventDraft | null = startsAt === null @@ -414,36 +576,112 @@ export function EventForm({ )} -
- + + {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" && ( + <> +
+ + +
+ {/* 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)} in all` : ""} +

+ + )} {earlyReturn && (

@@ -462,13 +700,13 @@ export function EventForm({ /> - {!endKnown && repeatUnit === "never" && ( + {!endKnown && repeatMode === "never" && (

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

)} - {!endKnown && repeatUnit !== "never" && ( + {!endKnown && repeatMode !== "never" && ( /* 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 5f2f776..6e0f11a 100644 --- a/src/shared/recurrence.ts +++ b/src/shared/recurrence.ts @@ -161,18 +161,27 @@ export type RepeatMode = "never" | "forever" | "delay"; * state describes it, rather than in whichever state happens to be the * default. * + * **`contiguousMs` is the instant a successor would open if it opened the + * moment this one closed — which is not always the stored end.** A boundary + * the reader gave a time to is an instant, and a successor opens on it. One + * they gave only a date to is stored as the last second of that day, so its + * successor opens a second later, at the following midnight. Passing the + * stored end for both would read every day-precision rule as having a gap it + * does not have. The caller owns that convention because the caller is what + * wrote the boundary. + * * An unstated end is `forever`: the reader gave a cadence and no end, so each * occurrence runs until the next opens. There is no gap to describe, and a * delay would have nothing to be measured from. */ export function repeatModeOf( startsMs: number, - endsMs: number | null, + contiguousMs: number | null, repeat: Repeat | null, ): RepeatMode { if (repeat === null) return "never"; - if (endsMs === null) return "forever"; - return addUnits(startsMs, repeat.unit, repeat.interval) === endsMs + if (contiguousMs === null) return "forever"; + return addUnits(startsMs, repeat.unit, repeat.interval) === contiguousMs ? "forever" : "delay"; } diff --git a/test/custom-ui.test.tsx b/test/custom-ui.test.tsx index 6643ba0..c797da0 100644 --- a/test/custom-ui.test.tsx +++ b/test/custom-ui.test.tsx @@ -368,17 +368,58 @@ describe("stating a repeat", () => { ...over, }); - test("a fresh form offers a repeat, set to never", () => { + // The fixture runs 1-8 Sep inclusive with no times given, so it closes at + // 23:59:59 on the 8th and a successor opening "the moment it closes" opens + // at midnight on the 9th — eight days after the anchor, not seven. Its rule + // is every two weeks, which leaves a gap, so `repeating()` is the delay + // 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", () => { const html = renderToStaticMarkup( {}} onCancel={() => {}} />, ); - expect(html).toContain("Repeats"); - // The interval field is hidden until there is something to count, so the - // form a reader already knows is unchanged until they reach for this. + 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("editing a rule shows the rule it already has", () => { + test("a rule that reopens as it closes shows its cadence rather than a control", () => { + // The whole point of "forever": the dates already say how often, so the + // form reports what it measured instead of asking for a number. + const html = renderToStaticMarkup( + {}} + onCancel={() => {}} + />, + ); + expect(html).toContain("every 8 days"); + expect(html).toContain("from your dates"); + expect(html).not.toContain("Wait"); + }); + + test("a measured cadence can still be overridden by hand", () => { + // Measuring is the convenience, not a cage — an irregular rotation has to + // be sayable even when the first window does not describe it. + const html = renderToStaticMarkup( + {}} + onCancel={() => {}} + />, + ); + expect(html).toContain("state it myself"); + }); + + test("a rule with a gap opens as a delay, showing the gap", () => { const html = renderToStaticMarkup( { onCancel={() => {}} />, ); + expect(html).toContain("Wait"); + expect(html).toContain("after it ends"); + // A week's gap after a week's window is a fortnightly rule; both readings + // are shown so the reader can check the one against the other. + expect(html).toContain("every 2 weeks"); + }); + + test("with no end date there is nothing to measure, so it asks", () => { + const html = renderToStaticMarkup( + {}} + onCancel={() => {}} + />, + ); expect(html).toContain("Every"); - expect(html).toContain('value="2"'); + }); + + test("a delay needs an end date to be measured from", () => { + const html = renderToStaticMarkup( + {}} + onCancel={() => {}} + />, + ); + expect(html).toContain("needs an end date"); }); test("an unknown end with a rule stops claiming there is no countdown", () => {