diff --git a/docs/DATA-MODEL.md b/docs/DATA-MODEL.md index 7a8ff26..8a6b079 100644 --- a/docs/DATA-MODEL.md +++ b/docs/DATA-MODEL.md @@ -291,8 +291,10 @@ out is a record marked only with that field, silently dropped on the next write. removes an unreadable goal from the record — the likeliest one is a newer shape (weekly tasks add `measure: "amount"`) met by an older cached bundle, and dropping it on load would delete it on the next write. A goal makes the event a daily regardless of `daily` and detection; setting one clears a -stored `daily: false`, and any `daily` answer clears the goal, so the two never contradict in -storage. +stored `daily: false`, and answering "this isn't a daily event" clears the goal, so the two never +contradict in storage. No other control removes a goal — not "it repeats daily", and not pressing +*Off* when no goal is readable — because either could be standing on one this build cannot see. The +"not daily" control says it removes the goal whenever *anything* is stored there, readable or not. **`effort` is load-bearing, not decorative.** Combined with the time remaining it answers "can I still finish this?" — the same two days is comfortable for a `quick` event and hopeless for a diff --git a/src/client/App.tsx b/src/client/App.tsx index 31ed848..2f8084a 100644 --- a/src/client/App.tsx +++ b/src/client/App.tsx @@ -761,6 +761,7 @@ export function App() { detectedDaily={prefs.detectDaily && isDaily(openRow.event)} dailyDays={daily.daysFor(openRow.event.id)} goal={goalOf(openRow.event.id)} + hasStoredGoal={prog.progress[openRow.event.id]?.goal !== undefined} onDaily={prog.setDaily} onGoal={prog.setGoal} onToggleDay={daily.toggleDay} diff --git a/src/client/components/EventDetail.tsx b/src/client/components/EventDetail.tsx index 020b6e3..07fc805 100644 --- a/src/client/components/EventDetail.tsx +++ b/src/client/components/EventDetail.tsx @@ -38,6 +38,7 @@ export function EventDetail({ detectedDaily, dailyDays, goal, + hasStoredGoal, onDaily, onGoal, onToggleDay, @@ -64,7 +65,13 @@ export function EventDetail({ dailyDays: string[]; /** The reader's check-in goal, already read through `readGoal`. */ goal: Goal | undefined; - onDaily: (id: string, daily: boolean | undefined) => void; + /** + * Whether anything is stored under the goal at all, readable or not. A goal + * of a newer shape reads as none, and "not daily" would still remove it, so + * the warning keys off this rather than off `goal`. + */ + hasStoredGoal: boolean; + onDaily: (id: string, daily: boolean | undefined, clearsGoal?: boolean) => void; onGoal: (id: string, goal: Goal | undefined) => void; onToggleDay: (id: string, day: string) => void; onIgnore: (id: string) => void; @@ -246,10 +253,10 @@ export function EventDetail({ )} diff --git a/src/client/components/GoalControl.tsx b/src/client/components/GoalControl.tsx index 0552e6f..4cc16e1 100644 --- a/src/client/components/GoalControl.tsx +++ b/src/client/components/GoalControl.tsx @@ -1,3 +1,4 @@ +import { useEffect, useState } from "react"; import { clampRequired, type Goal } from "../../shared/goal.ts"; const WEEKDAYS = ["Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday", "Sunday"]; @@ -8,6 +9,44 @@ const MODES = [ { id: "event", label: "Across the event" }, ] as const; +type Mode = (typeof MODES)[number]["id"]; + +/** + * The goal a mode press asks for: a Goal, undefined to clear it, or null for + * "nothing to do". Pressing the mode already shown is null — including Off + * when no goal is *readable*, which may still be one stored by a newer + * version, and a press that looks like a no-op must not delete it. + */ +export function chooseGoal( + id: Mode, + goal: Goal | undefined, + defaultWeekStartsOn: number, + maxDays: number | null, +): Goal | undefined | null { + if (id === (goal?.period ?? "off")) return null; + if (id === "off") return undefined; + const required = goal?.required ?? 5; + if (id === "week") { + return { + period: "week", + measure: "days", + required: clampRequired("week", required, null), + weekStartsOn: goal?.period === "week" ? goal.weekStartsOn : defaultWeekStartsOn, + }; + } + return { period: "event", measure: "days", required: clampRequired("event", required, maxDays) }; +} + +/** A typed count, clamped; null while the field is empty or unreadable — mid-edit, not an answer. */ +export function parseDraft( + raw: string, + period: Goal["period"], + maxDays: number | null, +): number | null { + const n = Number.parseInt(raw, 10); + return Number.isFinite(n) ? clampRequired(period, n, maxDays) : null; +} + /** * How many check-ins the event actually wants. * @@ -32,30 +71,10 @@ export function GoalControl({ onChange: (goal: Goal | undefined) => void; }) { const mode = goal?.period ?? "off"; - const required = goal?.required ?? 5; - const choose = (id: (typeof MODES)[number]["id"]) => { - if (id === "off") return onChange(undefined); - if (id === "week") { - return onChange({ - period: "week", - measure: "days", - required: clampRequired("week", required, null), - weekStartsOn: goal?.period === "week" ? goal.weekStartsOn : defaultWeekStartsOn, - }); - } - onChange({ - period: "event", - measure: "days", - required: clampRequired("event", required, maxDays), - }); - }; - - const setRequired = (raw: string) => { - if (goal === undefined) return; - // An emptied field is mid-edit, not an answer; keep the goal as it was. - if (raw.trim() === "") return; - onChange({ ...goal, required: clampRequired(goal.period, Number.parseInt(raw, 10), maxDays) }); + const choose = (id: Mode) => { + const next = chooseGoal(id, goal, defaultWeekStartsOn, maxDays); + if (next !== null) onChange(next); }; return ( @@ -84,15 +103,7 @@ export function GoalControl({ {goal !== undefined && (
- setRequired(e.target.value)} - className="tnum w-14 rounded-lg border border-hairline bg-ground px-2 py-1 text-sm text-ink focus:border-faint focus:outline-none" - /> + {goal.period === "week" ? "days each week, weeks start" : "days across the event"} @@ -115,3 +126,40 @@ export function GoalControl({
); } + +/** + * The count, with its own draft text. Bound straight to `goal.required`, the + * field could never be emptied: an empty value changes no goal, so React put + * the old number straight back, and typing after a backspace appended to it. + * The draft holds what is typed; a readable value commits, and leaving the + * field shows the goal's count again. + */ +function CountField({ + goal, + maxDays, + onChange, +}: { + goal: Goal; + maxDays: number | null; + onChange: (goal: Goal) => void; +}) { + const [draft, setDraft] = useState(String(goal.required)); + useEffect(() => setDraft(String(goal.required)), [goal.required]); + + return ( + { + setDraft(e.target.value); + const n = parseDraft(e.target.value, goal.period, maxDays); + if (n !== null && n !== goal.required) onChange({ ...goal, required: n }); + }} + onBlur={() => setDraft(String(goal.required))} + className="tnum w-14 rounded-lg border border-hairline bg-ground px-2 py-1 text-sm text-ink focus:border-faint focus:outline-none" + /> + ); +} diff --git a/src/client/state/useProgress.ts b/src/client/state/useProgress.ts index d36f51d..e0a5c86 100644 --- a/src/client/state/useProgress.ts +++ b/src/client/state/useProgress.ts @@ -88,6 +88,19 @@ export function applyPatch( return { ...prev, [id]: merged }; } +/** + * A daily answer. Only "not daily" clears a goal: the two would contradict. + * "It repeats daily" leaves `goal` untouched, because the event it is offered + * on may hold a goal this build cannot read — one it shows as none — and an + * answer about dailiness is no licence to delete it. + */ +export function dailyPatch( + daily: boolean | undefined, + clearsGoal: boolean, +): Partial { + return clearsGoal ? { daily, goal: undefined } : { daily }; +} + /** * Setting a goal makes the event a daily; a stored `daily: false` would * contradict it, so that alone is cleared. Any other `daily` answer stays, so @@ -167,12 +180,13 @@ export function useProgress() { // the press after "done" cleared the status instead of undoing it. Status is // set outright now, by a control per state. - // "This isn't a daily event" must not leave a goal hidden in storage that - // contradicts it, so any daily answer clears the goal in the same write. The - // other direction — "it repeats daily" — only ever runs on an event with no - // goal, because a goal already makes it daily. + // `clearsGoal` is the caller saying "this isn't a daily event": that answer + // must not leave a contradicting goal hidden in storage. The stored override + // alone cannot say which way the reader answered — agreeing with detection + // stores undefined either way — so the caller says. const setDaily = useCallback( - (id: string, daily: boolean | undefined) => patch(id, { daily, goal: undefined }), + (id: string, daily: boolean | undefined, clearsGoal = false) => + patch(id, dailyPatch(daily, clearsGoal)), [patch], ); diff --git a/test/custom-ui.test.tsx b/test/custom-ui.test.tsx index 04d099d..7c2d6a6 100644 --- a/test/custom-ui.test.tsx +++ b/test/custom-ui.test.tsx @@ -784,6 +784,7 @@ describe("the derived-boundary note", () => { detectedDaily: false, dailyDays: [], goal: undefined, + hasStoredGoal: false, onDaily: noop, onGoal: noop, onToggleDay: noop, @@ -858,6 +859,7 @@ describe("the derived-boundary note", () => { status={status} daily goal={{ period: "event", measure: "days", required: 5 }} + hasStoredGoal dailyDays={["2026-08-20", "2026-08-21", "2026-08-22", "2026-08-23", "2026-08-24"]} row={{ event: PARSED, clock: clockFor(PARSED, detailProps.region, NOW) }} /> @@ -869,6 +871,21 @@ describe("the derived-boundary note", () => { expect(open).toContain("This isn't a daily event — removes the goal"); expect(sheet("done")).not.toContain("Mark event done"); }); + + test("'not daily' warns it removes a goal even when this build cannot read it", () => { + const html = renderToStaticMarkup( + metaFor(id, GAMES)}> + + , + ); + expect(html).toContain("removes the goal"); + }); }); describe("the cadence control", () => { diff --git a/test/progress.test.ts b/test/progress.test.ts index 4c88e82..c38fe92 100644 --- a/test/progress.test.ts +++ b/test/progress.test.ts @@ -1,7 +1,7 @@ import { describe, expect, test } from "bun:test"; import { buildExportData, parseImportData } from "../src/client/state/export.ts"; import { restorePrefsValue } from "../src/client/state/usePrefs.ts"; -import { applyPatch, goalPatch, mergeProgress } from "../src/client/state/useProgress.ts"; +import { applyPatch, dailyPatch, goalPatch, mergeProgress } from "../src/client/state/useProgress.ts"; import type { ProgressMap } from "../src/client/state/useProgress.ts"; /** @@ -221,3 +221,16 @@ describe("goals on progress", () => { expect(parseImportData(old)!.progress).toEqual({ e: { status: "done", at: at("10") } }); }); }); + +describe("a goal this build cannot read is never cleared by accident", () => { + // Final review: "It repeats daily" wiped a newer-shaped goal it could not + // see. Only saying the event is *not* daily clears a goal. + test("answering 'it repeats daily' leaves the goal alone", () => { + expect(dailyPatch(true, false)).toStrictEqual({ daily: true }); + expect(dailyPatch(undefined, false)).toStrictEqual({ daily: undefined }); + }); + + test("answering 'not daily' clears it in the same write", () => { + expect(dailyPatch(false, true)).toStrictEqual({ daily: false, goal: undefined }); + }); +}); diff --git a/test/views.test.tsx b/test/views.test.tsx index 2c23b70..b11746b 100644 --- a/test/views.test.tsx +++ b/test/views.test.tsx @@ -17,7 +17,7 @@ import { } from "../src/client/components/Dailies.tsx"; import type { GoalStatus } from "../src/shared/goal.ts"; import { Welcome } from "../src/client/components/Welcome.tsx"; -import { GoalControl } from "../src/client/components/GoalControl.tsx"; +import { chooseGoal, GoalControl, parseDraft } from "../src/client/components/GoalControl.tsx"; import { DailyChecklist } from "../src/client/components/DailyChecklist.tsx"; import { timelineLanes } from "../src/client/state/lanes.ts"; import { GameMetaProvider } from "../src/client/state/gameMeta.tsx"; @@ -1288,3 +1288,34 @@ describe("DailyChecklist with a goal", () => { expect(html).not.toContain("missed"); }); }); + +describe("GoalControl decisions", () => { + const weekly = { period: "week", measure: "days", required: 5, weekStartsOn: 1 } as const; + + test("pressing Off when no goal is readable changes nothing", () => { + // An unreadable stored goal reads as none, so Off already shows pressed; + // pressing it must not delete what is stored. + expect(chooseGoal("off", undefined, 1, 84)).toBeNull(); + expect(chooseGoal("off", weekly, 1, 84)).toBeUndefined(); + }); + + test("pressing the mode already chosen changes nothing", () => { + expect(chooseGoal("week", weekly, 1, 84)).toBeNull(); + }); + + test("switching modes keeps the count, clamped", () => { + expect(chooseGoal("event", weekly, 1, 3)).toEqual({ + period: "event", measure: "days", required: 3, + }); + expect(chooseGoal("week", undefined, 4, 84)).toEqual({ + period: "week", measure: "days", required: 5, weekStartsOn: 4, + }); + }); + + test("an emptied or unreadable field is mid-edit, not an answer", () => { + expect(parseDraft("", "week", null)).toBeNull(); + expect(parseDraft("abc", "week", null)).toBeNull(); + expect(parseDraft("3", "week", null)).toBe(3); + expect(parseDraft("53", "week", null)).toBe(7); + }); +});