fix(soon): stop pointing at events you've finished or ignored
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) <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
c3e9b9064d
commit
edc6f3e4bb
@@ -83,7 +83,7 @@ src/client/ React app, service worker, manifest
|
|||||||
state/ progress, daily log, ignores, prefs, sort — all localStorage
|
state/ progress, daily log, ignores, prefs, sort — all localStorage
|
||||||
scripts/ build-feed.ts, parse-fixture.ts (offline), refresh-sources.ts (fetches)
|
scripts/ build-feed.ts, parse-fixture.ts (offline), refresh-sources.ts (fetches)
|
||||||
serve.ts static server + /api/health
|
serve.ts static server + /api/health
|
||||||
test/ 301 tests
|
test/ 309 tests
|
||||||
fixtures/<game>/ raw HTML + .expected.json per source — pinned, kept forever
|
fixtures/<game>/ raw HTML + .expected.json per source — pinned, kept forever
|
||||||
snapshots/ current page per source, rewritten by refresh — see its README
|
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
|
- **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
|
claims. A source quietly moving a date must not erase a fortnight's streak that exists nowhere
|
||||||
else.
|
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
|
- **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`
|
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 —
|
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
|
- **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
|
`endingSoonestFirst`, so choosing one can never cost the reader the deadline order the product
|
||||||
exists for.
|
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.
|
||||||
|
|||||||
@@ -102,6 +102,7 @@ src/
|
|||||||
useDailyLog.ts which game-days are ticked off
|
useDailyLog.ts which game-days are ticked off
|
||||||
usePrefs.ts region, filters, onboarding flags
|
usePrefs.ts region, filters, onboarding flags
|
||||||
sort.ts deadline order, or what you're partway through
|
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
|
serve.ts static server + /api/health ✓ built
|
||||||
scripts/
|
scripts/
|
||||||
build-feed.ts fixtures → public/data/events.v1.json ✓ built
|
build-feed.ts fixtures → public/data/events.v1.json ✓ built
|
||||||
|
|||||||
+23
-7
@@ -16,6 +16,7 @@ import { useProgress } from "./state/useProgress.ts";
|
|||||||
import { useDailyLog, type DailyLogMap } from "./state/useDailyLog.ts";
|
import { useDailyLog, type DailyLogMap } from "./state/useDailyLog.ts";
|
||||||
import { usePrefs } from "./state/usePrefs.ts";
|
import { usePrefs } from "./state/usePrefs.ts";
|
||||||
import { compareRows, SORT_MODES, type Activity, type SortMode } from "./state/sort.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 { clockFor, DAY, formatRemaining } from "../shared/time.ts";
|
||||||
import { dailySummary, isDaily, resolveDaily } from "../shared/daily.ts";
|
import { dailySummary, isDaily, resolveDaily } from "../shared/daily.ts";
|
||||||
import type { GameId } from "../shared/schema.ts";
|
import type { GameId } from "../shared/schema.ts";
|
||||||
@@ -110,8 +111,10 @@ export function App() {
|
|||||||
return { doneToday: summary.doneToday, remaining: summary.remaining };
|
return { doneToday: summary.doneToday, remaining: summary.remaining };
|
||||||
};
|
};
|
||||||
|
|
||||||
|
const isIgnored = (id: string) => ignored.marks[id] !== undefined;
|
||||||
|
|
||||||
const toggleIgnored = (id: string, title: string) => {
|
const toggleIgnored = (id: string, title: string) => {
|
||||||
const wasIgnored = ignored.marks[id] !== undefined;
|
const wasIgnored = isIgnored(id);
|
||||||
ignored.toggle(id);
|
ignored.toggle(id);
|
||||||
setLastIgnored(wasIgnored ? null : { id, title });
|
setLastIgnored(wasIgnored ? null : { id, title });
|
||||||
};
|
};
|
||||||
@@ -153,7 +156,7 @@ export function App() {
|
|||||||
.filter((r) => !r.clock.ended)
|
.filter((r) => !r.clock.ended)
|
||||||
// Ignored events are gone from both views unless deliberately revealed
|
// Ignored events are gone from both views unless deliberately revealed
|
||||||
// — that is the whole point of ignoring one.
|
// — 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))
|
.filter((r) => prefs.showCompleted || !isDone(r.event.id))
|
||||||
// Sorting only ever groups: both modes fall back to soonest-ending
|
// Sorting only ever groups: both modes fall back to soonest-ending
|
||||||
// inside a group, so choosing one never costs the deadline order.
|
// 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 live = visible.filter((r) => r.clock.live);
|
||||||
const upcoming = visible.filter((r) => r.clock.upcoming);
|
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;
|
const openRow = allRows.find((r) => r.event.id === openId) ?? null;
|
||||||
|
|
||||||
if (state.status === "loading") {
|
if (state.status === "loading") {
|
||||||
@@ -267,7 +283,7 @@ export function App() {
|
|||||||
that expires tonight rather than next patch. */}
|
that expires tonight rather than next patch. */}
|
||||||
<Dailies
|
<Dailies
|
||||||
games={games.filter((g) => !prefs.hiddenGames.includes(g))}
|
games={games.filter((g) => !prefs.hiddenGames.includes(g))}
|
||||||
events={live.filter(repeatsDaily).map((r) => r.event)}
|
events={todo.filter(repeatsDaily).map((r) => r.event)}
|
||||||
region={prefs.region}
|
region={prefs.region}
|
||||||
now={now}
|
now={now}
|
||||||
daysFor={daily.daysFor}
|
daysFor={daily.daysFor}
|
||||||
@@ -302,7 +318,7 @@ export function App() {
|
|||||||
status={prog.progress[row.event.id]?.status}
|
status={prog.progress[row.event.id]?.status}
|
||||||
effort={prog.progress[row.event.id]?.effort}
|
effort={prog.progress[row.event.id]?.effort}
|
||||||
daily={dailyBadge(row)}
|
daily={dailyBadge(row)}
|
||||||
ignored={ignored.marks[row.event.id] !== undefined}
|
ignored={isIgnored(row.event.id)}
|
||||||
onRestore={(id) => ignored.toggle(id)}
|
onRestore={(id) => ignored.toggle(id)}
|
||||||
onOpen={setOpenId}
|
onOpen={setOpenId}
|
||||||
/>
|
/>
|
||||||
@@ -332,7 +348,7 @@ export function App() {
|
|||||||
status={prog.progress[row.event.id]?.status}
|
status={prog.progress[row.event.id]?.status}
|
||||||
effort={prog.progress[row.event.id]?.effort}
|
effort={prog.progress[row.event.id]?.effort}
|
||||||
daily={dailyBadge(row)}
|
daily={dailyBadge(row)}
|
||||||
ignored={ignored.marks[row.event.id] !== undefined}
|
ignored={isIgnored(row.event.id)}
|
||||||
onRestore={(id) => ignored.toggle(id)}
|
onRestore={(id) => ignored.toggle(id)}
|
||||||
onOpen={setOpenId}
|
onOpen={setOpenId}
|
||||||
/>
|
/>
|
||||||
@@ -393,7 +409,7 @@ export function App() {
|
|||||||
<EventDetail
|
<EventDetail
|
||||||
row={openRow}
|
row={openRow}
|
||||||
completed={isDone(openRow.event.id)}
|
completed={isDone(openRow.event.id)}
|
||||||
ignored={ignored.marks[openRow.event.id] !== undefined}
|
ignored={isIgnored(openRow.event.id)}
|
||||||
status={prog.progress[openRow.event.id]?.status}
|
status={prog.progress[openRow.event.id]?.status}
|
||||||
effort={prog.progress[openRow.event.id]?.effort}
|
effort={prog.progress[openRow.event.id]?.effort}
|
||||||
note={prog.progress[openRow.event.id]?.note ?? ""}
|
note={prog.progress[openRow.event.id]?.note ?? ""}
|
||||||
|
|||||||
@@ -30,7 +30,11 @@ export function Dailies({
|
|||||||
onToggleDay,
|
onToggleDay,
|
||||||
}: {
|
}: {
|
||||||
games: GameId[];
|
games: GameId[];
|
||||||
/** Live events that repeat daily — detected, or marked by the reader. */
|
/**
|
||||||
|
* Live events that repeat daily — detected, or marked by the reader — and
|
||||||
|
* that the reader has not already finished or ignored. An event they marked
|
||||||
|
* done has no line left to tick, and listing it is the app arguing with them.
|
||||||
|
*/
|
||||||
events: GachaEvent[];
|
events: GachaEvent[];
|
||||||
region: Region;
|
region: Region;
|
||||||
now: number;
|
now: number;
|
||||||
|
|||||||
@@ -10,14 +10,26 @@ import { Meter, URGENCY_COLOR } from "./Meter.tsx";
|
|||||||
* Deliberately not a stat grid. One number, because the reader has exactly one
|
* Deliberately not a stat grid. One number, because the reader has exactly one
|
||||||
* question on arrival.
|
* question on arrival.
|
||||||
*/
|
*/
|
||||||
export function NextUp({ row, onOpen }: { row: RowEvent | null; onOpen: (id: string) => 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) {
|
if (row === null) {
|
||||||
return (
|
return (
|
||||||
<section className="border-b border-hairline px-4 py-8">
|
<section className="border-b border-hairline px-4 py-8">
|
||||||
<p className="eyebrow">Nothing running</p>
|
<p className="eyebrow">Nothing running</p>
|
||||||
<p className="mt-2 max-w-sm text-sm text-muted">
|
<p className="mt-2 max-w-sm text-sm text-muted">
|
||||||
No live events in the games you have switched on. Turn a game back on
|
Nothing live and unfinished in the games you have switched on. Turn a
|
||||||
below, or check again after the next patch.
|
game back on below, or check again after the next patch.
|
||||||
</p>
|
</p>
|
||||||
</section>
|
</section>
|
||||||
);
|
);
|
||||||
|
|||||||
@@ -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<T extends Row>(
|
||||||
|
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<T extends Row>(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;
|
||||||
|
}
|
||||||
@@ -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();
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user