refactor(timeline): move the unstarted-events switch into settings
It shipped as a pill in the board's own header, beside the stacking and scale controls. That is the wrong company: those two reshape what is already on the board, which is why they are reached for while reading it, and this one decides what is on it at all — the same question "Show events I've finished" and "Show events I'm ignoring" answer, from the panel where both of those live. It is the only one of the three scoped to a single view, so the row says so instead of reading as a promise about the whole app. The board loses the count the pill carried; with nothing left running it still says how many are waiting and names the setting, and the page header has said "N live · N upcoming" all along. Controls had no tests at all, so the three view filters get some: what each checkbox is bound to is invisible in a diff and obvious only to the reader it happens to. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
440144fdc1
commit
a66803384a
+2
-2
@@ -549,9 +549,9 @@ export function App() {
|
||||
group={prefs.timelineGroup}
|
||||
onGroup={(timelineGroup) => update({ timelineGroup })}
|
||||
// The board holds these back itself rather than being handed a
|
||||
// shorter list, so its own control can say how many are waiting.
|
||||
// shorter list, so it can say how many are waiting when there is
|
||||
// nothing else left to draw. The switch is in settings.
|
||||
showUpcoming={prefs.timelineUpcoming}
|
||||
onShowUpcoming={(timelineUpcoming) => update({ timelineUpcoming })}
|
||||
onOpen={setOpenId}
|
||||
isDone={isDone}
|
||||
/>
|
||||
|
||||
@@ -134,6 +134,32 @@ export function Controls({
|
||||
Show events I've finished
|
||||
</label>
|
||||
|
||||
{/* Sits with the other two "what am I allowed to look at" rows
|
||||
because that is the question it answers — but unlike them it
|
||||
is answered for the board only, and a row that did not say so
|
||||
would read as a promise about the whole app. The checklist
|
||||
keeps its own "Not started yet" section either way, which is
|
||||
why switching this off costs a reader nothing they cannot
|
||||
still go and read. */}
|
||||
<label className="flex cursor-pointer select-none items-start gap-2 text-xs text-muted">
|
||||
<input
|
||||
type="checkbox"
|
||||
checked={prefs.timelineUpcoming}
|
||||
onChange={(e) =>
|
||||
onUpdate({ timelineUpcoming: e.target.checked })
|
||||
}
|
||||
className="mt-px size-4 accent-[var(--color-near)]"
|
||||
/>
|
||||
<span>
|
||||
Show events that haven't started
|
||||
<span className="mt-0.5 block max-w-xs leading-relaxed text-faint">
|
||||
On the timeline, which draws its span from what it plots —
|
||||
so this stretches the board weeks past today. The checklist
|
||||
lists them under "Not started yet" regardless.
|
||||
</span>
|
||||
</span>
|
||||
</label>
|
||||
|
||||
{/* Detection reads the source's wording and is wrong in both
|
||||
directions, so it ships off and says so. Off leaves only the
|
||||
events the reader marked, and discards nothing — every mark and
|
||||
|
||||
@@ -98,7 +98,6 @@ export function Timeline({
|
||||
group,
|
||||
onGroup,
|
||||
showUpcoming,
|
||||
onShowUpcoming,
|
||||
onOpen,
|
||||
isDone,
|
||||
}: {
|
||||
@@ -116,11 +115,15 @@ export function Timeline({
|
||||
* Off by default (`prefs.timelineUpcoming`). The board is asked "how does the
|
||||
* time I am in lay out?", and every lane has a next patch queued behind it —
|
||||
* plotting those unasked stretches the window weeks past today and squeezes
|
||||
* the running bars the reader came for. The count is on the control, so what
|
||||
* is held back is stated rather than merely absent.
|
||||
* the running bars the reader came for.
|
||||
*
|
||||
* Read-only here: the switch lives in settings with the other two answers to
|
||||
* "what am I allowed to look at" (`Controls`), not in the board's own header
|
||||
* beside the stacking and scale controls. Those two reshape what is already
|
||||
* on the board, which is why they are reached for while reading it; this one
|
||||
* decides what is on it at all.
|
||||
*/
|
||||
showUpcoming: boolean;
|
||||
onShowUpcoming: (showUpcoming: boolean) => void;
|
||||
onOpen: (id: string) => void;
|
||||
/**
|
||||
* Asked rather than derived from the progress store: an entry exists there
|
||||
@@ -213,14 +216,7 @@ export function Timeline({
|
||||
floating over the chart: pinned inside, it would sit on top of the
|
||||
calendar and cover the very dates it sends you back to. */}
|
||||
<div className="flex flex-wrap items-center justify-between gap-x-3 gap-y-2 border-b border-hairline px-4 py-2.5">
|
||||
<div className="flex items-center gap-2">
|
||||
<StackControl value={group} onChange={onGroup} />
|
||||
<UpcomingControl
|
||||
on={showUpcoming}
|
||||
waiting={waiting.length}
|
||||
onChange={onShowUpcoming}
|
||||
/>
|
||||
</div>
|
||||
<StackControl value={group} onChange={onGroup} />
|
||||
|
||||
<div className="flex items-center gap-3">
|
||||
<div role="group" aria-label="Scale" className="flex items-center gap-1">
|
||||
@@ -259,7 +255,8 @@ export function Timeline({
|
||||
{waiting.length === 1
|
||||
? "One event has not started yet"
|
||||
: `${waiting.length} events have not started yet`}{" "}
|
||||
— switch “Not started” on to see when they begin.
|
||||
— switch on “Show events that haven't started” below to see when they
|
||||
begin.
|
||||
</p>
|
||||
) : (
|
||||
<div
|
||||
@@ -541,50 +538,6 @@ function StackControl({
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether the board also plots what has not started yet.
|
||||
*
|
||||
* A switch rather than a third stacking pill, and drawn as one — it does not
|
||||
* reshape the board, it changes what is on it. The count is the point: a board
|
||||
* that quietly withheld nine events would be indistinguishable from a quiet
|
||||
* fortnight, and "nothing scheduled" is exactly the wrong thing for this app to
|
||||
* imply by accident.
|
||||
*
|
||||
* Absent when there is nothing waiting and it is switched off: a toggle that
|
||||
* cannot change anything invites a tap that does nothing.
|
||||
*/
|
||||
function UpcomingControl({
|
||||
on,
|
||||
waiting,
|
||||
onChange,
|
||||
}: {
|
||||
on: boolean;
|
||||
waiting: number;
|
||||
onChange: (on: boolean) => void;
|
||||
}) {
|
||||
if (waiting === 0 && !on) return null;
|
||||
return (
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => onChange(!on)}
|
||||
aria-pressed={on}
|
||||
title={
|
||||
on
|
||||
? "Stop plotting events that have not started yet"
|
||||
: "Also plot events that have not started yet"
|
||||
}
|
||||
className={`flex items-center gap-1.5 rounded-full border px-2.5 py-1 text-[0.6875rem] font-medium transition-colors ${
|
||||
on
|
||||
? "border-ink/60 text-ink"
|
||||
: "border-hairline text-faint hover:text-muted"
|
||||
}`}
|
||||
>
|
||||
Not started
|
||||
<span className="tnum text-[0.625rem] text-faint">{waiting}</span>
|
||||
</button>
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* One step of the scale control.
|
||||
*
|
||||
|
||||
@@ -0,0 +1,101 @@
|
||||
import { describe, expect, test } from "bun:test";
|
||||
import { renderToStaticMarkup } from "react-dom/server";
|
||||
import { Controls } from "../src/client/components/Controls.tsx";
|
||||
import { GameMetaProvider } from "../src/client/state/gameMeta.tsx";
|
||||
import { metaFor } from "../src/shared/games.ts";
|
||||
import type { Prefs } from "../src/client/state/usePrefs.ts";
|
||||
|
||||
/**
|
||||
* The settings panel's view filters.
|
||||
*
|
||||
* Three rows answer the same question — *what am I allowed to look at?* — and
|
||||
* they are the only place two of them can be reached from, so what they are
|
||||
* bound to is worth pinning. A checkbox wired to the wrong preference is
|
||||
* invisible in a diff and obvious only to the reader it happens to.
|
||||
*/
|
||||
|
||||
const PREFS: Prefs = {
|
||||
region: "europe",
|
||||
hiddenGames: [],
|
||||
focusGame: null,
|
||||
sort: "ending",
|
||||
view: "soon",
|
||||
timelineDayWidth: 32,
|
||||
timelineGroup: "game",
|
||||
timelineUpcoming: false,
|
||||
detectDaily: false,
|
||||
showCompleted: true,
|
||||
showIgnored: false,
|
||||
theme: "dark",
|
||||
regionConfirmed: true,
|
||||
onboarded: true,
|
||||
};
|
||||
|
||||
function render(prefs: Prefs, ignoredCount = 0): string {
|
||||
return renderToStaticMarkup(
|
||||
<GameMetaProvider value={(id) => metaFor(id, {})}>
|
||||
<Controls
|
||||
games={["genshin", "hsr"]}
|
||||
prefs={prefs}
|
||||
onToggleGame={() => {}}
|
||||
onUpdate={() => {}}
|
||||
ignoredCount={ignoredCount}
|
||||
onExport={() => {}}
|
||||
onImport={() => {}}
|
||||
own={{
|
||||
games: {},
|
||||
events: {},
|
||||
lanes: ["genshin", "hsr"],
|
||||
onAddGame: () => {},
|
||||
onEditGame: () => {},
|
||||
onRemoveGame: () => ({ removed: true, blockedBy: 0 }),
|
||||
onAddEvent: () => {},
|
||||
}}
|
||||
/>
|
||||
</GameMetaProvider>,
|
||||
);
|
||||
}
|
||||
|
||||
/** The nth checkbox's `checked` attribute, in document order. */
|
||||
function checkboxes(html: string): boolean[] {
|
||||
return [...html.matchAll(/<input type="checkbox"[^>]*>/g)].map((m) =>
|
||||
m[0].includes('checked=""'),
|
||||
);
|
||||
}
|
||||
|
||||
describe("Controls: what am I allowed to look at", () => {
|
||||
test("the unstarted-events switch is here, in the reader's words", () => {
|
||||
// It used to be a pill in the board's own header, next to the stacking and
|
||||
// scale controls. Those two reshape what is already on the board; this one
|
||||
// decides what is on it at all, which is the question the two rows beside
|
||||
// it answer.
|
||||
const html = render(PREFS);
|
||||
expect(html).toContain("Show events that haven't started");
|
||||
expect(html).toContain("Show events I've finished");
|
||||
});
|
||||
|
||||
test("it says the board is what it applies to", () => {
|
||||
// Sitting between two app-wide filters, a row that named no scope would
|
||||
// read as a promise about the whole app — and the checklist lists these
|
||||
// whatever this says.
|
||||
const html = render(PREFS);
|
||||
expect(html).toContain("On the timeline");
|
||||
expect(html).toContain("Not started yet");
|
||||
});
|
||||
|
||||
test("it reads its own preference and not a neighbour's", () => {
|
||||
// Both neighbours are on and this one is off, so a checkbox bound to the
|
||||
// wrong key shows up as the wrong count of ticks.
|
||||
const off = checkboxes(render(PREFS));
|
||||
const on = checkboxes(render({ ...PREFS, timelineUpcoming: true }));
|
||||
expect(off.filter(Boolean)).toHaveLength(1);
|
||||
expect(on.filter(Boolean)).toHaveLength(2);
|
||||
});
|
||||
|
||||
test("the ignored row appears only once something is ignored", () => {
|
||||
// Nothing to restore means nothing to offer — the row would be a filter
|
||||
// over an empty set.
|
||||
expect(render(PREFS)).not.toContain("I'm ignoring");
|
||||
expect(render(PREFS, 3)).toContain("Show the 3 events I'm");
|
||||
});
|
||||
});
|
||||
+12
-22
@@ -261,7 +261,6 @@ describe("Timeline stacking", () => {
|
||||
group={group}
|
||||
onGroup={() => {}}
|
||||
showUpcoming={false}
|
||||
onShowUpcoming={() => {}}
|
||||
onOpen={() => {}}
|
||||
isDone={() => false}
|
||||
/>,
|
||||
@@ -316,22 +315,19 @@ describe("Timeline: events that have not started", () => {
|
||||
group="ending"
|
||||
onGroup={() => {}}
|
||||
showUpcoming={showUpcoming}
|
||||
onShowUpcoming={() => {}}
|
||||
onOpen={() => {}}
|
||||
isDone={() => false}
|
||||
/>,
|
||||
);
|
||||
|
||||
test("the board holds them back by default and says how many", () => {
|
||||
test("the board holds them back by default", () => {
|
||||
// Off is the default because the board answers "how does the time I am in
|
||||
// lay out?" — but an absence nobody mentioned is indistinguishable from a
|
||||
// quiet fortnight, which is the wrong thing for this app to imply.
|
||||
// lay out?", and it draws its span from what it plots — so a next patch on
|
||||
// every lane pushes the running bars the reader came for off to the left.
|
||||
const html = board(false);
|
||||
expect(html).toContain("Closing Ceremony");
|
||||
expect(html).not.toContain("Frost Parade");
|
||||
expect(html).not.toContain("Long Way Round");
|
||||
expect(html).toContain("Not started");
|
||||
expect(html).toContain(">3<");
|
||||
});
|
||||
|
||||
test("switching it on plots them", () => {
|
||||
@@ -340,26 +336,20 @@ describe("Timeline: events that have not started", () => {
|
||||
expect(html).toContain("Long Way Round");
|
||||
});
|
||||
|
||||
test("the control says which way it is set", () => {
|
||||
expect(board(false)).toContain('aria-pressed="false"');
|
||||
expect(board(true)).toContain('aria-pressed="true"');
|
||||
});
|
||||
|
||||
test("nothing waiting and switched off means no control at all", () => {
|
||||
// A toggle that cannot change anything invites a tap that does nothing.
|
||||
expect(board(false, [row("Closing Ceremony", "genshin", 100)])).not.toContain(
|
||||
"Not started",
|
||||
);
|
||||
});
|
||||
|
||||
test("a board with only future events says so rather than reading empty", () => {
|
||||
// Otherwise the reader is looking at "nothing to plot" while three events
|
||||
// are scheduled, and the reason is a control they did not notice.
|
||||
// are scheduled, and an absence nobody mentioned is indistinguishable from
|
||||
// a quiet fortnight. It names the setting, since the switch is not on the
|
||||
// board any more.
|
||||
const html = board(false, rows.slice(1));
|
||||
expect(html).toContain("Nothing is running right now");
|
||||
expect(html).toContain("3 events have not started yet");
|
||||
// The control is still there to act on what the sentence just told them.
|
||||
expect(html).toContain("Not started");
|
||||
expect(html).toContain("Show events that haven't started");
|
||||
});
|
||||
|
||||
test("one waiting event is counted in words, not as \"1 events\"", () => {
|
||||
const html = board(false, [rows[1]!]);
|
||||
expect(html).toContain("One event has not started yet");
|
||||
});
|
||||
|
||||
test("each clump of starts is marked in words", () => {
|
||||
|
||||
Reference in New Issue
Block a user