diff --git a/web/scripts/camo-status.test.mjs b/web/scripts/camo-status.test.mjs new file mode 100644 index 0000000..6534008 --- /dev/null +++ b/web/scripts/camo-status.test.mjs @@ -0,0 +1,59 @@ +/** + * The camo editor says things in two places, and the way it goes wrong is + * always the same: something that is not a pattern writes to the line under + * the Pattern label. Signing in used to put "Picked up where you left off, + * saving to @you" there, above a grid of patterns it was not describing. + * + * The other half of the same rule is Save. With no account there is nothing to + * save to, and a sentence saying so under the pattern grid is not what a + * player who just pressed Save is looking at; the sign-in dialog is. + * + * Both checks are structural. There is nothing to catch at runtime: the wrong + * line is still a line, it renders, and nothing errors. + * + * Run with `npm test`. + */ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { readFile } from "node:fs/promises"; +import { join } from "node:path"; +import { fileURLToPath } from "node:url"; + +const web = fileURLToPath(new URL("../", import.meta.url)); +const source = await readFile(join(web, "src/camo/editor.ts"), "utf8"); + +/** The body of a top-level function in the editor, by its declaration. */ +function body(declaration) { + const start = source.indexOf(declaration); + assert.notEqual(start, -1, `${declaration} moved or was renamed`); + // Every nested block in this file closes at four spaces or deeper, so the + // first two-space brace is the function's own. + const end = source.indexOf("\n }", start); + assert.notEqual(end, -1, `${declaration} has no end`); + return { start, end, text: source.slice(start, end) }; +} + +test("only the pattern picker writes the pattern hint", () => { + const writes = [...source.matchAll(/patternHint\.textContent\s*=/g)]; + assert.equal( + writes.length, + 1, + "the pattern hint has more than one writer; a save, a sign-in or an " + + "import would leave it describing something else", + ); + const marker = body("function markPattern(): void {"); + const at = writes[0].index; + assert.ok( + at > marker.start && at < marker.end, + "the pattern hint is written from outside markPattern", + ); +}); + +test("save with no account opens the sign-in dialog", () => { + const save = body("function doSave("); + assert.match( + save.text, + /if \(!repoSession\) \{[^}]*collection\.signIn\(\)/, + "saving without an account should put the sign-in dialog up, not a message", + ); +}); diff --git a/web/src/camo/collection.ts b/web/src/camo/collection.ts index ab3a7bb..0160c71 100644 --- a/web/src/camo/collection.ts +++ b/web/src/camo/collection.ts @@ -62,6 +62,12 @@ export type Collection = { start(): Promise; /** The camo currently open in the editor, so Save knows what it is editing. */ setOpen(camo: StoredCamo | null): void; + /** + * Put the site's sign-in dialog up. The panel's own Sign in button is one + * way in; Save pressed with no account is the other, and both want the same + * dialog and the same stashing of the editor before it navigates. + */ + signIn(): void; }; export function camoCollection(hooks: CollectionHooks): Collection { @@ -415,6 +421,7 @@ export function camoCollection(hooks: CollectionHooks): Collection { openRkey = camo?.rkey ?? null; markOpen(); }, + signIn: showSignIn, }; } diff --git a/web/src/camo/editor.ts b/web/src/camo/editor.ts index 352c3cb..a5ff1be 100644 --- a/web/src/camo/editor.ts +++ b/web/src/camo/editor.ts @@ -252,8 +252,16 @@ export function camoEditor(): Node[] { const bgRow = el("div", { className: "camo-backgrounds" }); const bgCanvases = new Map(); + // Two lines, and which one a message goes to is not a detail. The hint sits + // under the Pattern label and describes the pattern on screen, so nothing + // but choosing a pattern is allowed to write to it — a save, a sign-in or a + // failed import left it describing something the grid below it disagreed + // with. Everything that just happened goes to the message line instead, + // under the camo and above the controls, where the buttons that caused it + // are. + const patternHint = el("p", { className: "small muted camo-pattern-hint" }); const status = el("p", { - className: "small muted camo-status", + className: "small muted camo-message", role: "status", }); @@ -367,6 +375,12 @@ export function camoEditor(): Node[] { btn.classList.toggle("active", active); btn.setAttribute("aria-pressed", String(active)); } + // The hint's only writer, and this is the only thing that changes it: the + // selection moved, so what is under the label follows it. Every path that + // changes the pattern already comes through here. + patternHint.textContent = state.imported + ? "An imported picture, so there is no pattern being generated." + : byId(state.generator).hint; } // Every pattern draws itself, in the palette and at the settings currently @@ -414,7 +428,6 @@ export function camoEditor(): Node[] { } markPattern(); renderShape(); - status.textContent = gen.hint; refresh(); }); patternButtons.set(gen.id, btn); @@ -498,7 +511,6 @@ export function camoEditor(): Node[] { state.imported = null; markPattern(); renderShape(); - status.textContent = gen.hint; refresh(); }); @@ -819,7 +831,10 @@ export function camoEditor(): Node[] { return; } if (!repoSession) { - status.textContent = "Sign in to save this camo to your own account."; + // Pressing Save is asking to save, so put the one thing in the way in + // front of them rather than a line of text about it. The dialog writes + // the camo down before the redirect, so it survives the round trip. + collection.signIn(); return; } const target = asNew ? null : editing; @@ -927,7 +942,6 @@ export function camoEditor(): Node[] { setPalette(rolled.palette); markPattern(); renderShape(); - status.textContent = byId(rolled.generator).hint; refresh(); } @@ -1035,12 +1049,15 @@ export function camoEditor(): Node[] { markPattern(); if (resumed) nameInput.value = resumed.name; - status.textContent = "Loading unit art…"; + const LOADING_ART = "Loading unit art…"; + status.textContent = LOADING_ART; Promise.all([loadSprites(), loadBackgrounds()]) .then(([units, tiles]) => { for (const [id, data] of units) sprites.set(id, data); for (const [id, data] of tiles) backgrounds.set(id, data); - status.textContent = byId(state.generator).hint; + // Take the loading line back down, unless the player has done something + // in the meantime and the line is already saying something newer. + if (status.textContent === LOADING_ART) status.textContent = ""; refresh(); }) .catch(() => { @@ -1094,13 +1111,18 @@ export function camoEditor(): Node[] { strip, ]), + // Full width under the camo and its units, above the controls: what the + // editor has to say about what just happened, next to the buttons that + // did it and clear of the pattern grid. + status, + el("div", { className: "camo-controls" }, [ el("div", { className: "camo-group" }, [ el("div", { className: "camo-group-head" }, [ el("span", { className: "camo-label", textContent: "Pattern" }), randomPattern, ]), - status, + patternHint, patternGrid, ]), // Colours above Shape rather than beside it: both are short, and diff --git a/web/src/styles.css b/web/src/styles.css index c42e119..40923c6 100644 --- a/web/src/styles.css +++ b/web/src/styles.css @@ -1914,15 +1914,25 @@ footer .debug { } } -/* Space kept for two lines whether or not the current pattern needs them, so - choosing one never nudges the grid underneath. */ -.camo-status { +/* The pattern hint. Space kept for two lines whether or not the current + pattern needs them, so choosing one never nudges the grid underneath. */ +.camo-pattern-hint { margin: 0; min-height: 2.7em; font-size: 0.8rem; line-height: 1.35; } +/* What the editor has to say — saving, importing, signing in. One line is + held open across the full width, so the controls under it do not jump every + time it speaks. */ +.camo-message { + margin: 0; + min-height: 1.35em; + font-size: 0.8rem; + line-height: 1.35; +} + .signin a { color: var(--accent); }