refactor: give the region and the theme a settings line each
They were one group called "Reading", grouped on "both are how do I read this?" — which summarised as "Europe · Dark", two unrelated answers joined by a dot, under a name for neither of them, in a panel whose whole premise is that a closed line answers its own group. The region is not a reading preference. Region-scoped ends, a date printed without a time, and every daily streak are all cut on that server's clock, so it is the one control in this panel that can make a countdown wrong — and it was filed behind a word for the theme. Split, each line answers one question, and the region group says what it governs when opened. The eyebrow above each pill row goes with it, since the group summary now asks the question the eyebrow was repeating; aria-label keeps the accessible name, which is what stops six unlabelled buttons in a row from being all a screen reader hears. The summary assertions are looked up by group name rather than by index, so splitting a group no longer moves every assertion below it onto a neighbour. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
35b3ab8e89
commit
f3f5524ffc
@@ -924,16 +924,23 @@ to an open page). Four things hold it up:
|
||||
because touch fires no drag events at all: the arrows are the mechanism and the drag is the pointer
|
||||
fast path, and being ordinary buttons is what makes the whole thing reachable by keyboard and
|
||||
screen reader without a second implementation of the same interaction.
|
||||
- **The settings panel is groups that state their own answer, and they ship closed.** Five
|
||||
`<details>` groups — games, reading, what you see, your own, your progress — each with its current
|
||||
state on the summary line (`Europe · Dark`, `17 of 18 on · A–Z`, `plus finished, not started`). It
|
||||
was one open block in two columns, which is readable at four games and not at eighteen, where the
|
||||
game list is eighteen rows of four controls sitting above the checkbox somebody came to tick. The
|
||||
state line is the half that makes collapsing honest: without it every question about how the app is
|
||||
set up costs a click, so the closed panel has to be a report and not a menu. Two rules follow.
|
||||
**Do not default a group open** to make something inside it findable — the empty states name the
|
||||
group as well as the switch, which is what makes that unnecessary. And the summaries are **derived
|
||||
from `prefs` at render**, never stored, so they cannot drift from the controls they describe.
|
||||
- **The settings panel is groups that state their own answer, and they ship closed.** Six
|
||||
`<details>` groups — games, server region, appearance, what you see, your own, your progress — each
|
||||
with its current state on the summary line (`Europe`, `17 of 18 on · A–Z`, `plus finished, not
|
||||
started`). It was one open block in two columns, which is readable at four games and not at
|
||||
eighteen, where the game list is eighteen rows of four controls sitting above the checkbox somebody
|
||||
came to tick. The state line is the half that makes collapsing honest: without it every question
|
||||
about how the app is set up costs a click, so the closed panel has to be a report and not a menu.
|
||||
Three rules follow. **Do not default a group open** to make something inside it findable — the empty
|
||||
states name the group as well as the switch, which is what makes that unnecessary. The summaries are
|
||||
**derived from `prefs` at render**, never stored, so they cannot drift from the controls they
|
||||
describe. And **one group answers one question**, which is what the sixth group is: the region and
|
||||
the theme were a group called `Reading`, grouped on "both are how do I read this?" and summarising
|
||||
as `Europe · Dark` — two unrelated answers joined by a dot, under a name for neither of them, in a
|
||||
panel whose whole premise is that a closed line answers *its* group. The region is not a reading
|
||||
preference: region-scoped ends and every daily reset are cut on that server's clock (§ Domain
|
||||
rules), so it is the one control here that can make a countdown wrong, and it is now a line of its
|
||||
own rather than filed behind a word for the theme.
|
||||
Native `<details>` for the reason the reorder arrows are ordinary buttons: keyboard and screen
|
||||
reader reach it without a second implementation. `summary` is not an `a`, `button` or `[tabindex]`,
|
||||
so it needs its own `:focus-visible` rule in `styles.css` — the shared one does not reach it.
|
||||
|
||||
+18
-12
@@ -203,19 +203,25 @@ but de-emphasized; a filter toggles them out entirely.
|
||||
Filter by game (multi-select, persisted) and by event type. Hiding a game hides it from both views.
|
||||
Preferences persist in `localStorage`.
|
||||
|
||||
The panel holding them is **five collapsed groups, each stating its own answer on the summary line**
|
||||
— games, reading, what you see, your own games and events, your progress. It was one open block of
|
||||
everything, laid out in two columns on a wide screen, which read fine at four games and stopped
|
||||
working at eighteen: the game list alone is eighteen rows of four controls, and it sat above the
|
||||
checkbox a reader had come down here to tick. Collapsing on its own would only trade that friction
|
||||
for another, though — a group that shows nothing but its name turns *what is my region set to?* into
|
||||
a click — so each summary answers its group's question, and the closed panel is a five-line report
|
||||
of how the app is configured. Opening one is for changing an answer, not for reading it.
|
||||
The panel holding them is **six collapsed groups, each stating its own answer on the summary line**
|
||||
— games, server region, appearance, what you see, your own games and events, your progress. It was
|
||||
one open block of everything, laid out in two columns on a wide screen, which read fine at four games
|
||||
and stopped working at eighteen: the game list alone is eighteen rows of four controls, and it sat
|
||||
above the checkbox a reader had come down here to tick. Collapsing on its own would only trade that
|
||||
friction for another, though — a group that shows nothing but its name turns *what is my region set
|
||||
to?* into a click — so each summary answers its group's question, and the closed panel is a six-line
|
||||
report of how the app is configured. Opening one is for changing an answer, not for reading it.
|
||||
|
||||
Two things follow. **The groups ship closed**, including the one holding the filters that other copy
|
||||
points at: the empty states name both the switch and the group it is in, which is more findable than
|
||||
a checkbox in a wall was. And **the summary states are derived, never stored** — they are a reading
|
||||
of `prefs`, so they cannot disagree with the controls inside.
|
||||
Three things follow. **The groups ship closed**, including the one holding the filters that other
|
||||
copy points at: the empty states name both the switch and the group it is in, which is more findable
|
||||
than a checkbox in a wall was. **The summary states are derived, never stored** — they are a reading
|
||||
of `prefs`, so they cannot disagree with the controls inside. And **a group answers one question**.
|
||||
The region and the theme began as one group called *Reading*, on the argument that both are "how do I
|
||||
read this?" — which summarised as `Europe · Dark`, two unrelated answers joined by a dot, under a
|
||||
name for neither. The region is not a reading preference: it is which server the reader's account is
|
||||
on, and region-scoped ends, undated deadlines and every daily streak are read off that server's
|
||||
clock, so it is the one control in this panel that can make a countdown wrong. It gets its own line,
|
||||
and says as much when opened.
|
||||
|
||||
**The panel uses the page's width**, like every other block on it. The rows were held to a narrower
|
||||
measure on the argument that a name and its state stop reading as one line across a wide screen, but
|
||||
|
||||
@@ -48,7 +48,7 @@ const SPLITS: Array<{ id: boolean; label: string; hint: string }> = [
|
||||
/**
|
||||
* The settings panel.
|
||||
*
|
||||
* Five groups, each collapsed until asked for, and each stating its own answer
|
||||
* Six groups, each collapsed until asked for, and each stating its own answer
|
||||
* on the summary line. It used to be one open block of everything in two
|
||||
* columns, which was readable at four games and is not at eighteen: the game
|
||||
* list alone is eighteen rows of four controls, and it sat above the pills and
|
||||
@@ -57,7 +57,7 @@ const SPLITS: Array<{ id: boolean; label: string; hint: string }> = [
|
||||
*
|
||||
* Collapsing it is only half an answer, though — a closed group that says
|
||||
* nothing turns "what is my region set to?" into a click. So every summary
|
||||
* carries its group's current state, which makes the closed panel a five-line
|
||||
* carries its group's current state, which makes the closed panel a six-line
|
||||
* report of how the app is configured, and makes opening one a deliberate act
|
||||
* rather than the price of reading it.
|
||||
*
|
||||
@@ -130,30 +130,42 @@ export function Controls({
|
||||
/>
|
||||
</Group>
|
||||
|
||||
{/* Region and appearance together: both are "how do I read this?", and
|
||||
neither changes what the page knows. Switching either is instant and
|
||||
costs nothing — no reload, and nothing marked, typed or ticked is
|
||||
touched. */}
|
||||
<Group
|
||||
name="Reading"
|
||||
state={`${REGION_LABEL[prefs.region]} · ${
|
||||
THEMES.find((t) => t.id === prefs.theme)?.label ?? prefs.theme
|
||||
}`}
|
||||
>
|
||||
<div className="flex flex-wrap gap-x-8 gap-y-4">
|
||||
<PillGroup
|
||||
label="Server region"
|
||||
options={REGIONS}
|
||||
value={prefs.region}
|
||||
onChange={(region) => onUpdate({ region, regionConfirmed: true })}
|
||||
/>
|
||||
<PillGroup
|
||||
label="Appearance"
|
||||
options={THEMES}
|
||||
value={prefs.theme}
|
||||
onChange={(theme) => onUpdate({ theme })}
|
||||
/>
|
||||
</div>
|
||||
{/* Two groups, where this was one called "Reading" — a name that
|
||||
described neither of them. It was grouped on "both are how do I read
|
||||
this?", which is true of the theme and not of the region: the region
|
||||
is a fact about the reader's account, and it is the one setting here
|
||||
that can make a countdown wrong, because region-scoped ends and every
|
||||
daily reset are read off that server's clock. So it summarised as
|
||||
"Europe · Dark", two unrelated answers joined by a dot, in a panel
|
||||
whose whole premise is that a closed group states *its* answer. Split,
|
||||
each line answers one question and the consequential one is no longer
|
||||
filed behind a word for the other. */}
|
||||
<Group name="Server region" state={REGION_LABEL[prefs.region]}>
|
||||
<PillGroup
|
||||
label="Server region"
|
||||
labelHidden
|
||||
options={REGIONS}
|
||||
value={prefs.region}
|
||||
onChange={(region) => onUpdate({ region, regionConfirmed: true })}
|
||||
/>
|
||||
<p className="mt-2.5 max-w-md text-xs leading-relaxed text-faint">
|
||||
Which server your account plays on. Events that end per region end on
|
||||
its clock, a date printed without a time is read as that server's
|
||||
daily reset, and every streak is counted in its days — so this is the
|
||||
setting to get right before trusting a countdown.
|
||||
</p>
|
||||
</Group>
|
||||
|
||||
{/* Genuinely only how the page looks, and instant: no reload, and
|
||||
nothing marked, typed or ticked is touched. */}
|
||||
<Group name="Appearance" state={themeLabel(prefs.theme)}>
|
||||
<PillGroup
|
||||
label="Appearance"
|
||||
labelHidden
|
||||
options={THEMES}
|
||||
value={prefs.theme}
|
||||
onChange={(theme) => onUpdate({ theme })}
|
||||
/>
|
||||
</Group>
|
||||
|
||||
<Group
|
||||
@@ -268,6 +280,11 @@ export function Controls({
|
||||
);
|
||||
}
|
||||
|
||||
/** The theme in the reader's words, falling back to the stored value. */
|
||||
function themeLabel(theme: ThemeChoice): string {
|
||||
return THEMES.find((t) => t.id === theme)?.label ?? theme;
|
||||
}
|
||||
|
||||
/** "12 of 18 on · your order" — what the games group is set to, without opening it. */
|
||||
function gamesState(total: number, shown: number, ordered: boolean): string {
|
||||
if (total === 0) return "none yet";
|
||||
@@ -360,6 +377,7 @@ function PillGroup<T extends string | boolean>({
|
||||
value,
|
||||
onChange,
|
||||
small = false,
|
||||
labelHidden = false,
|
||||
}: {
|
||||
label: string;
|
||||
options: ReadonlyArray<{ id: T; label: string }>;
|
||||
@@ -367,11 +385,21 @@ function PillGroup<T extends string | boolean>({
|
||||
onChange: (id: T) => void;
|
||||
/** Subordinate to the control above it, rather than a question of its own. */
|
||||
small?: boolean;
|
||||
/**
|
||||
* For the group whose own summary already asks the question. Drops the
|
||||
* eyebrow and nothing else — `aria-label` still names the row, so the
|
||||
* accessible name survives the visual one going away.
|
||||
*/
|
||||
labelHidden?: boolean;
|
||||
}) {
|
||||
return (
|
||||
<div>
|
||||
<p className="eyebrow">{label}</p>
|
||||
<div role="group" aria-label={label} className="mt-2 flex flex-wrap gap-1.5">
|
||||
{!labelHidden && <p className="eyebrow">{label}</p>}
|
||||
<div
|
||||
role="group"
|
||||
aria-label={label}
|
||||
className={`flex flex-wrap gap-1.5 ${labelHidden ? "" : "mt-2"}`}
|
||||
>
|
||||
{options.map((option) => {
|
||||
const on = option.id === value;
|
||||
return (
|
||||
|
||||
+50
-10
@@ -152,41 +152,81 @@ describe("Controls: what a closed group says", () => {
|
||||
(m[1] ?? "").replace(/<[^>]+>/g, " ").replace(/\s+/g, " ").trim(),
|
||||
);
|
||||
|
||||
/**
|
||||
* One group's line, found by its name rather than its position.
|
||||
*
|
||||
* These used to be indexed, which meant splitting one group in two moved
|
||||
* every assertion below it onto a neighbour — and a test that fails because
|
||||
* the panel gained a group says nothing about the line it was written for.
|
||||
*/
|
||||
const line = (html: string, name: string): string =>
|
||||
summaries(html).find((l) => l.startsWith(`${name} `)) ?? "";
|
||||
|
||||
test("every group is named, and states where it stands", () => {
|
||||
const lines = summaries(render(PREFS));
|
||||
expect(lines).toHaveLength(5);
|
||||
expect(lines).toHaveLength(6);
|
||||
expect(lines[0]).toBe("Games 2 of 2 on · A–Z");
|
||||
expect(lines[1]).toBe("Reading Europe · Dark");
|
||||
expect(lines[3]).toBe("Your own games and events none yet");
|
||||
expect(lines[5]).toBe("Your progress Backup to a file");
|
||||
expect(line(render(PREFS), "Your own games and events")).toBe(
|
||||
"Your own games and events none yet",
|
||||
);
|
||||
});
|
||||
|
||||
test("the region and the theme are two questions, not one line", () => {
|
||||
// They were one group called "Reading", summarising as "Europe · Dark" —
|
||||
// two unrelated answers joined by a dot, under a name that described
|
||||
// neither. The region is the one setting here that can make a countdown
|
||||
// wrong, so it gets its own line rather than sharing a word for the theme.
|
||||
const lines = summaries(render(PREFS));
|
||||
expect(lines).not.toContain("Reading Europe · Dark");
|
||||
expect(line(render(PREFS), "Server region")).toBe("Server region Europe");
|
||||
expect(line(render(PREFS), "Appearance")).toBe("Appearance Dark");
|
||||
expect(line(render({ ...PREFS, theme: "system" }), "Appearance")).toBe(
|
||||
"Appearance System",
|
||||
);
|
||||
});
|
||||
|
||||
test("the games line counts what is on, and whose order it is in", () => {
|
||||
const some = render({ ...PREFS, hiddenGames: ["hsr"], gameOrder: ["hsr", "genshin"] });
|
||||
expect(summaries(some)[0]).toBe("Games 1 of 2 on · your order");
|
||||
expect(line(some, "Games")).toBe("Games 1 of 2 on · your order");
|
||||
});
|
||||
|
||||
test("the visibility line names the additions, not the switches", () => {
|
||||
// The app's answer is what expires next; each of these puts something else
|
||||
// alongside it, so that is how the line reads.
|
||||
expect(summaries(render({ ...PREFS, showCompleted: false }))[2]).toBe(
|
||||
expect(line(render({ ...PREFS, showCompleted: false }), "What you see")).toBe(
|
||||
"What you see live deadlines only",
|
||||
);
|
||||
expect(
|
||||
summaries(render({ ...PREFS, showUpcoming: true }))[2],
|
||||
).toBe("What you see plus finished, not started");
|
||||
expect(line(render({ ...PREFS, showUpcoming: true }), "What you see")).toBe(
|
||||
"What you see plus finished, not started",
|
||||
);
|
||||
});
|
||||
|
||||
test("ignored counts in the line only once it is actually revealed", () => {
|
||||
// `showIgnored` with nothing ignored is a filter over an empty set, and the
|
||||
// row itself is not even offered — so the summary must not claim it either.
|
||||
expect(summaries(render({ ...PREFS, showIgnored: true }, 0))[2]).toBe(
|
||||
expect(line(render({ ...PREFS, showIgnored: true }, 0), "What you see")).toBe(
|
||||
"What you see plus finished",
|
||||
);
|
||||
expect(summaries(render({ ...PREFS, showIgnored: true }, 3))[2]).toBe(
|
||||
expect(line(render({ ...PREFS, showIgnored: true }, 3), "What you see")).toBe(
|
||||
"What you see plus finished, ignored",
|
||||
);
|
||||
});
|
||||
|
||||
test("dropping a repeated eyebrow does not drop the accessible name", () => {
|
||||
// The region and theme pills sit in groups whose summaries already ask the
|
||||
// question, so the eyebrow above them would only say it twice and is gone.
|
||||
// `aria-label` is the half that has to survive that: without it a screen
|
||||
// reader reads six unlabelled buttons in a row with nothing saying which
|
||||
// question either half answers.
|
||||
const html = render(PREFS);
|
||||
expect(html).toContain('aria-label="Server region"');
|
||||
expect(html).toContain('aria-label="Appearance"');
|
||||
// Twice and no more: the summary a reader sees, and the label a screen
|
||||
// reader hears. A third copy is the eyebrow coming back.
|
||||
expect([...html.matchAll(/Server region/g)]).toHaveLength(2);
|
||||
});
|
||||
|
||||
test("the panel has a heading of its own", () => {
|
||||
// It used to begin with an unannounced wall of controls, which reads as more
|
||||
// of the page rather than as the place settings live.
|
||||
|
||||
Reference in New Issue
Block a user