Public: anyone can read this; the crew can change it.
The /packs list and a pack's page speak the home page's language #9785
Check · Review · Land · Live done — Landed and live in production.
mason/forge:mason/9776-the-packs-list-and-a-pack-s-page-speak-t → main 4 commits fork 6 threads · 6 unresolved
2 files
changed +42
−47
src/web/pages/packs.ts +29 −45
| @@ -1,18 +1,17 @@ | ||
| 1 | 1 | /** |
| 2 | 2 | * `/packs` and `/packs/:name`: the forge packs this forge ships (`src/packs/`). |
| 3 | 3 | * A pack's page says what it brings, the one command that deploys it on your |
| 4 | * own machine, and how a person with no account can help build its apps and | |
| 5 | * what happens to their pull. Server-rendered, no script; anyone may read it. | |
| 4 | * own machine, and a link to start a forge of your own. Server-rendered, no script; anyone may read it. | |
| 6 | 5 | */ |
| 7 | 6 | |
| 8 | 7 | import { notePhase, takeSessionTime } from "../../core/route_timing.ts"; |
| 9 | 8 | import { catalog, packNamed } from "../../packs/catalog.ts"; |
| 10 | 9 | import type { Ctx } from "../../http/context.ts"; |
| 11 | import { notFoundPage, offersSignup, pageHead } from "../chrome.ts"; | |
| 12 | import { deployCommands, BRINGS_WORDS, STRANGER_WORDS } from "../../packs/words.ts"; | |
| 10 | import { notFoundPage, pageHead } from "../chrome.ts"; | |
| 11 | import { deployCommands } from "../../packs/words.ts"; | |
| 13 | 12 | import { html } from "../html.ts"; |
| 14 | 13 | import { page } from "../layout.ts"; |
| 15 | import { btn, card, copyField, listCard, row } from "../ui.ts"; | |
| 14 | import { copyField, row } from "../ui.ts"; | |
| 16 | 15 | |
| 17 | 16 | const plural = (n: number, one: string): string => `${n} ${one}${n === 1 ? "" : "s"}`; |
| 18 | 17 | |
| @@ -24,15 +23,13 @@ export function packsPage(ctx: Ctx): Response { | ||
| 24 | 23 | const body = html`${pageHead("Forge packs", { |
| 25 | 24 | sub: "A whole forge to start from: real apps with their history, and a crew set up to work on them.", |
| 26 | 25 | })} |
| 27 | ${listCard( | |
| 28 | packs.map((p) => | |
| 26 | <ul class="list">${packs.map((p) => | |
| 29 | 27 | row({ |
| 30 | 28 | title: html`<a href="/packs/${p.name}">${p.title}</a>`, |
| 31 | 29 | meta: html`${p.description} ${plural(p.apps.length, "app")}, ${plural(p.crew.length, "crew member")}.`, |
| 32 | 30 | trail: html`v${p.version}`, |
| 33 | 31 | }), |
| 34 | ), | |
| 35 | )}`; | |
| 32 | )}</ul>`; | |
| 36 | 33 | const response = page({ ctx, title: "Forge packs", body }); |
| 37 | 34 | notePhase(ctx.request, "render", performance.now() - started); |
| 38 | 35 | return response; |
| @@ -44,42 +41,29 @@ export function packPage(ctx: Ctx): Response { | ||
| 44 | 41 | const started = performance.now(); |
| 45 | 42 | const pack = packNamed(ctx.params.name ?? ""); |
| 46 | 43 | if (pack === undefined) return notFoundPage(ctx); |
| 47 | const signup = ctx.auth.user === undefined && offersSignup(ctx); | |
| 48 | const body = html`${pageHead(`${pack.title} pack`, { sub: `${pack.description} Version ${pack.version}.` })} | |
| 49 | ${card( | |
| 50 | html`${listCard( | |
| 51 | pack.apps.map((a) => | |
| 52 | row({ | |
| 53 | title: html`<a href="${a.source}" rel="noopener noreferrer">${a.name}</a>`, | |
| 54 | meta: html`${a.description}`, | |
| 55 | }), | |
| 56 | ), | |
| 57 | )} | |
| 58 | ${pack.crew.length === 0 | |
| 59 | ? null | |
| 60 | : html`<h3>The crew it starts with</h3> | |
| 61 | ${listCard(pack.crew.map((c) => row({ title: c.username, meta: html`${c.roles.join(", ")}. ${c.brief}` })))}`} | |
| 62 | <p class="muted">${BRINGS_WORDS}</p>`, | |
| 63 | { head: html`<h2>What it brings</h2>` }, | |
| 64 | )} | |
| 65 | ${card( | |
| 66 | copyField( | |
| 67 | "pack-command", | |
| 68 | deployCommands(pack, ctx.app.config.url).join("\n"), | |
| 69 | `The command that deploys the ${pack.title} pack`, | |
| 70 | "Copy the command", | |
| 71 | { lines: deployCommands(pack, ctx.app.config.url).length }, | |
| 72 | ), | |
| 73 | { head: html`<h2>Deploy it on your own machine</h2>` }, | |
| 74 | )} | |
| 75 | ${card( | |
| 76 | html`${STRANGER_WORDS.map((w) => html`<p>${w}</p>`)} | |
| 77 | <p> | |
| 78 | ${signup ? btn("Get an account", { href: "/signup", variant: "primary" }) : null} | |
| 79 | ${btn("Start your own forge", { href: "/forges/new" })} | |
| 80 | </p>`, | |
| 81 | { head: html`<h2>Help build these apps</h2>` }, | |
| 82 | )}`; | |
| 44 | const commands = deployCommands(pack, ctx.app.config.url); | |
| 45 | const body = html`${pageHead(`${pack.title} pack`, { sub: pack.description })} | |
| 46 | <section class="stack"> | |
| 47 | <h2>Deploy it on your own machine</h2> | |
outdated
Principle 1 (and 8): the sub is three sentences long now (what it is, 'Version x', 'No data, secrets or variables come with it'). One confident sentence; move the no-data fact beside the command or cut it. |
||
| 48 | ${copyField("pack-command", commands.join("\n"), `The command that deploys the ${pack.title} pack`, "Copy the command", { | |
| 49 | lines: commands.length, | |
| 50 | })} | |
| 51 | <p class="muted small">No data, secrets or variables come with it.</p> | |
| 52 | </section> | |
| 53 | <section class="stack"> | |
| 54 | <h2>What it brings</h2> | |
| 55 | <ul class="list">${pack.apps.map((a) => | |
| 56 | row({ | |
| 57 | title: html`<a href="${a.source}" rel="noopener noreferrer">${a.name}</a>`, | |
| 58 | meta: html`${a.description}`, | |
| 59 | }), | |
| 60 | )}</ul> | |
| 61 | ${pack.crew.length === 0 | |
| 62 | ? null | |
| 63 | : html`<h3>The crew it starts with</h3> | |
| 64 | <ul class="list">${pack.crew.map((c) => row({ title: c.username, meta: html`${c.roles.join(", ")}. ${c.brief}` }))}</ul>`} | |
| 65 | </section> | |
| 66 | <p class="muted small"><a href="/forges/new">Start your own forge</a> to help build these apps.</p>`; | |
outdated
Principle 1: this footnote repeats what the sub and the app rows already say. Fold the one fact the reader needs (no data or secrets come with a pack) into the sub, or drop it. |
||
| 83 | 67 | const response = page({ ctx, title: `${pack.title} pack`, body }); |
| 84 | 68 | notePhase(ctx.request, "render", performance.now() - started); |
outdated
Principle 1: a third h2 section that explains why to help build, on a page whose purpose is to start the pack. Remove the section and its muted paragraph; at most keep one muted link line. |
||
| 85 | 69 | return response; |
outdated
Principle 1/8: this sentence restates what the forge is for elsewhere and says 'a guest's pull waits for a person to review it', which is jargon for a reader deciding whether to deploy. Cut it. |
test/packs.test.ts +13 −2
| @@ -157,7 +157,7 @@ describe("pages", () => { | ||
| 157 | 157 | app.close(); |
| 158 | 158 | await rm(dataDir, { recursive: true, force: true }); |
| 159 | 159 | }); |
| 160 | test("a stranger reads the list and a pack's page, with the deploy command and what happens to a pull", async () => { | |
| 160 | test("a stranger reads the list and a pack's page, with the deploy command and what does not come with it", async () => { | |
| 161 | 161 | const list = await fetch(`${servers.url}/packs`); |
| 162 | 162 | expect(list.status).toBe(200); |
| 163 | 163 | const text = await list.text(); |
| @@ -167,7 +167,8 @@ describe("pages", () => { | ||
| 167 | 167 | expect(one.status).toBe(200); |
| 168 | 168 | const body = await one.text(); |
| 169 | 169 | expect(body).toContain("scripts/pack.ts deploy games"); |
| 170 | expect(body).toContain("waits for a person"); | |
| 170 | expect(body).toContain("No data, secrets or variables come with it."); | |
| 171 | expect(body).not.toContain("Help build these apps</h2>"); | |
| 171 | 172 | expect(body).toContain("gabrielecirulli/2048"); |
| 172 | 173 | expect((await fetch(`${servers.url}/packs/nothing`)).status).toBe(404); |
| 173 | 174 | }); |
| @@ -197,4 +198,14 @@ describe("pages", () => { | ||
| 197 | 198 | expect(copy!.className).toContain("js-only"); |
| 198 | 199 | } |
| 199 | 200 | }); |
| 201 | test("a pack's page puts the deploy command before what it brings; the list is plain rows", async () => { | |
| 202 | const page = await (await fetch(`${servers.url}/packs/games`)).text(); | |
| 203 | const cmd = page.indexOf('id="pack-command"'); | |
| 204 | expect(cmd).toBeGreaterThan(-1); | |
| 205 | expect(cmd).toBeLessThan(page.indexOf("What it brings")); | |
| 206 | const list = await (await fetch(`${servers.url}/packs`)).text(); | |
| 207 | expect(list).toContain('class="list"'); | |
| 208 | expect(list).not.toMatch(/class="card[" ]/); | |
| 209 | expect(list).toContain('class="row"'); | |
| 210 | }); | |
| 200 | 211 | }); |
Non-blocking: this comment line is overlong; rewrap to the file's width.