From edc6f3e4bb510b5d572f416369f62eb9d5971907 Mon Sep 17 00:00:00 2001 From: Lucas Winther Date: Sun, 16 Aug 2026 20:40:46 +0200 Subject: [PATCH] fix(soon): stop pointing at events you've finished or ignored MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two symptoms of one bug. The "next to expire" headline counted events the reader had marked done or ignored, and the dailies strip kept a tickable chip for a repeating event they had already finished. showCompleted and showIgnored decide what a reader can *look at*. The headline and the strip are *instructions*, so they answer a different question — what is still on your plate — and both now go through one `outstanding` lens. Also fixes a second bug in the same line: `next` took the head of the list, which under "doing first" sorting is whatever you're partway through, not the soonest deadline. It reads the minimum now. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 11 ++++- docs/ARCHITECTURE.md | 1 + src/client/App.tsx | 30 ++++++++++---- src/client/components/Dailies.tsx | 6 ++- src/client/components/NextUp.tsx | 18 ++++++-- src/client/state/lens.ts | 64 +++++++++++++++++++++++++++++ test/lens.test.ts | 68 +++++++++++++++++++++++++++++++ 7 files changed, 186 insertions(+), 12 deletions(-) create mode 100644 src/client/state/lens.ts create mode 100644 test/lens.test.ts diff --git a/CLAUDE.md b/CLAUDE.md index f9a5dcb..5daf8f4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -83,7 +83,7 @@ src/client/ React app, service worker, manifest state/ progress, daily log, ignores, prefs, sort — all localStorage scripts/ build-feed.ts, parse-fixture.ts (offline), refresh-sources.ts (fetches) serve.ts static server + /api/health -test/ 301 tests +test/ 309 tests fixtures// raw HTML + .expected.json per source — pinned, kept forever snapshots/ current page per source, rewritten by refresh — see its README ``` @@ -207,6 +207,9 @@ event's length. It adds **no schema field**, so the feed contract is untouched. - **A tick is never removed except by the reader**, including ticks outside the window the feed now claims. A source quietly moving a date must not erase a fortnight's streak that exists nowhere else. +- **A repeating event the reader marked done leaves the strip.** They have said there is nothing + left to do; keeping a tickable chip for it is the app arguing with them. Their logged days are + untouched, so unmarking it brings the chip and the streak straight back. - **Detection is a default, not a verdict.** The reader can mark any event as repeating, or unmark one detection got wrong (`progress.daily`, resolved by `resolveDaily`), and `prefs.detectDaily` switches the guessing off entirely. Store an override only when it *disagrees* with detection — @@ -229,3 +232,9 @@ event's length. It adds **no schema field**, so the feed contract is untouched. - **Sorting groups, it never reorders within a group.** Every mode falls back to `endingSoonestFirst`, so choosing one can never cost the reader the deadline order the product exists for. +- **Telling the reader to do something is not the same as showing it to them.** `showCompleted` and + `showIgnored` decide what they can *look at*; the "next to expire" headline and the dailies strip + are *instructions*, so both drop anything done or ignored regardless (`outstanding` in + `src/client/state/lens.ts`). Being pointed at a job you already finished is the bug either way. + For the same reason "next to expire" reads the minimum end date rather than the head of the list, + which under "doing first" is a different event entirely. diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 2e27e1d..27a2962 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -102,6 +102,7 @@ src/ useDailyLog.ts which game-days are ticked off usePrefs.ts region, filters, onboarding flags sort.ts deadline order, or what you're partway through + lens.ts who sees which rows — outstanding, next-to-expire serve.ts static server + /api/health ✓ built scripts/ build-feed.ts fixtures → public/data/events.v1.json ✓ built diff --git a/src/client/App.tsx b/src/client/App.tsx index 55afa24..5d01106 100644 --- a/src/client/App.tsx +++ b/src/client/App.tsx @@ -16,6 +16,7 @@ import { useProgress } from "./state/useProgress.ts"; import { useDailyLog, type DailyLogMap } from "./state/useDailyLog.ts"; import { usePrefs } from "./state/usePrefs.ts"; import { compareRows, SORT_MODES, type Activity, type SortMode } from "./state/sort.ts"; +import { firstToExpire, outstanding } from "./state/lens.ts"; import { clockFor, DAY, formatRemaining } from "../shared/time.ts"; import { dailySummary, isDaily, resolveDaily } from "../shared/daily.ts"; import type { GameId } from "../shared/schema.ts"; @@ -110,8 +111,10 @@ export function App() { return { doneToday: summary.doneToday, remaining: summary.remaining }; }; + const isIgnored = (id: string) => ignored.marks[id] !== undefined; + const toggleIgnored = (id: string, title: string) => { - const wasIgnored = ignored.marks[id] !== undefined; + const wasIgnored = isIgnored(id); ignored.toggle(id); setLastIgnored(wasIgnored ? null : { id, title }); }; @@ -153,7 +156,7 @@ export function App() { .filter((r) => !r.clock.ended) // Ignored events are gone from both views unless deliberately revealed // — that is the whole point of ignoring one. - .filter((r) => prefs.showIgnored || ignored.marks[r.event.id] === undefined) + .filter((r) => prefs.showIgnored || !isIgnored(r.event.id)) .filter((r) => prefs.showCompleted || !isDone(r.event.id)) // Sorting only ever groups: both modes fall back to soonest-ending // inside a group, so choosing one never costs the deadline order. @@ -172,7 +175,20 @@ export function App() { const live = visible.filter((r) => r.clock.live); const upcoming = visible.filter((r) => r.clock.upcoming); - const next = live.find((r) => r.clock.msRemaining !== null) ?? live[0] ?? null; + + /** + * What the page is telling the reader to *do*, as opposed to what it is + * letting them look at. + * + * The headline and the dailies strip are both instructions, so both drop + * events the reader has finished or ignored — being pointed at a job you + * already did is the bug whether the pointer is a countdown or a checkbox. + * `showCompleted` deliberately does not reach this: that preference says keep + * them on screen, not keep nagging me about them. + */ + const todo = outstanding(live, isDone, isIgnored); + const next = firstToExpire(todo); + const openRow = allRows.find((r) => r.event.id === openId) ?? null; if (state.status === "loading") { @@ -267,7 +283,7 @@ export function App() { that expires tonight rather than next patch. */} !prefs.hiddenGames.includes(g))} - events={live.filter(repeatsDaily).map((r) => r.event)} + events={todo.filter(repeatsDaily).map((r) => r.event)} region={prefs.region} now={now} daysFor={daily.daysFor} @@ -302,7 +318,7 @@ export function App() { status={prog.progress[row.event.id]?.status} effort={prog.progress[row.event.id]?.effort} daily={dailyBadge(row)} - ignored={ignored.marks[row.event.id] !== undefined} + ignored={isIgnored(row.event.id)} onRestore={(id) => ignored.toggle(id)} onOpen={setOpenId} /> @@ -332,7 +348,7 @@ export function App() { status={prog.progress[row.event.id]?.status} effort={prog.progress[row.event.id]?.effort} daily={dailyBadge(row)} - ignored={ignored.marks[row.event.id] !== undefined} + ignored={isIgnored(row.event.id)} onRestore={(id) => ignored.toggle(id)} onOpen={setOpenId} /> @@ -393,7 +409,7 @@ export function App() { void }) { +export function NextUp({ + row, + onOpen, +}: { + /** + * The soonest-expiring event the reader has neither finished nor ignored. + * A panel headed "next to expire" is a deadline they still have to meet, so + * an event they already ticked off does not belong in it however visible + * they have chosen to keep it elsewhere. + */ + row: RowEvent | null; + onOpen: (id: string) => void; +}) { if (row === null) { return (

Nothing running

- No live events in the games you have switched on. Turn a game back on - below, or check again after the next patch. + Nothing live and unfinished in the games you have switched on. Turn a + game back on below, or check again after the next patch.

); diff --git a/src/client/state/lens.ts b/src/client/state/lens.ts new file mode 100644 index 0000000..f691569 --- /dev/null +++ b/src/client/state/lens.ts @@ -0,0 +1,64 @@ +import type { GameId } from "../../shared/schema.ts"; + +/** + * Which rows each part of the page gets to see. + * + * These decisions used to sit inline in `App`, where they were untestable and + * quietly inconsistent with each other — the "next to expire" headline counted + * events the reader had finished or ignored, and the dailies strip listed a + * repeating event they had already marked done. They are the same question + * asked twice, so they are one function asked twice, and pure so a test can + * pin them down. + */ + +/** The shape every lens here needs. Structural so this module stays cheap. */ +interface Row { + event: { id: string; game: GameId }; + clock: { msRemaining: number | null }; +} + +/** + * Rows the reader still has something to do with. + * + * "Done" and "ignored" mean different things everywhere else in the app — + * a done event stays visible and counted, an ignored one disappears — but to + * anything answering *what is still on your plate?* they are the same answer: + * not this one. The headline and the dailies strip are both that question. + * + * Note this is deliberately not the same as the main list's filters, which + * honour `showCompleted` / `showIgnored`. Those preferences control what the + * reader can *look at*; this controls what the app *tells them to do*, and + * being reminded of a job you already finished is the bug either way. + */ +export function outstanding( + rows: readonly T[], + isDone: (id: string) => boolean, + isIgnored: (id: string) => boolean, +): T[] { + return rows.filter((r) => !isDone(r.event.id) && !isIgnored(r.event.id)); +} + +/** + * The single row closest to expiring. + * + * Reads the minimum rather than taking the first row, because the list it is + * given is sorted by whatever mode the reader chose — under "doing first" the + * head of the list is what they are partway through, which is not what a panel + * headed "next to expire" is claiming to show. + * + * An event with no announced end can only ever be the answer when nothing else + * is running: it is real, but it is not a deadline. + */ +export function firstToExpire(rows: readonly T[]): T | null { + let best: T | null = null; + let bestMs = Infinity; + for (const row of rows) { + const ms = row.clock.msRemaining; + if (ms === null) continue; + if (ms < bestMs) { + best = row; + bestMs = ms; + } + } + return best ?? rows[0] ?? null; +} diff --git a/test/lens.test.ts b/test/lens.test.ts new file mode 100644 index 0000000..de8e9f2 --- /dev/null +++ b/test/lens.test.ts @@ -0,0 +1,68 @@ +import { describe, expect, test } from "bun:test"; +import { firstToExpire, outstanding } from "../src/client/state/lens.ts"; +import type { GameId } from "../src/shared/schema.ts"; + +const row = (id: string, game: GameId, msRemaining: number | null) => ({ + event: { id, game }, + clock: { msRemaining }, +}); + +const none = () => false; + +describe("outstanding", () => { + const rows = [ + row("a", "genshin", 1000), + row("b", "hsr", 2000), + row("c", "zzz", 3000), + ]; + + test("drops what the reader has finished", () => { + // The headline and the dailies strip both tell the reader what to do, and + // pointing at a job they already ticked off is the app arguing with them. + expect(outstanding(rows, (id) => id === "b", none).map((r) => r.event.id)).toEqual( + ["a", "c"], + ); + }); + + test("drops what they have ignored", () => { + expect(outstanding(rows, none, (id) => id === "a").map((r) => r.event.id)).toEqual( + ["b", "c"], + ); + }); + + test("done and ignored at once is still just gone", () => { + expect(outstanding(rows, (id) => id === "a", (id) => id === "a")).toHaveLength(2); + }); + + test("nothing outstanding is an empty list, not a null", () => { + expect(outstanding(rows, () => true, none)).toEqual([]); + }); +}); + +describe("firstToExpire", () => { + test("takes the soonest, not the first row", () => { + // The list arrives sorted by whatever mode the reader chose. Under "doing + // first" its head is what they are partway through, which is not what a + // panel headed "next to expire" claims to be showing. + const rows = [ + row("mid-run", "genshin", 9 * 86_400_000), + row("tonight", "hsr", 3 * 3_600_000), + ]; + expect(firstToExpire(rows)?.event.id).toBe("tonight"); + }); + + test("an unannounced end is never the deadline while a real one exists", () => { + const rows = [row("unknown", "zzz", null), row("real", "wuwa", 5000)]; + expect(firstToExpire(rows)?.event.id).toBe("real"); + }); + + test("falls back to an unknown end when it is all there is", () => { + // It is still a live event and still worth showing; it is just not a + // countdown. Showing nothing would be worse. + expect(firstToExpire([row("unknown", "zzz", null)])?.event.id).toBe("unknown"); + }); + + test("no rows is null rather than a crash", () => { + expect(firstToExpire([])).toBeNull(); + }); +});