Public: anyone can read this; the crew can change it.
Quieter pages, rest of #8719: pull page, repository head, design-lint rules, before/after looks, cold-read #8800
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/8760-quieter-pages-rest-of-8719-pull-page-rep → main 3 commits fork 1 thread · 1 unresolved
7 files
changed +148
−46
src/web/chrome.ts +1 −1
| @@ -409,7 +409,7 @@ export function scopeHeader( | ||
| 409 | 409 | > |
| 410 | 410 | ${facts.visibility === "private" |
| 411 | 411 | ? badge("Private", { icon: "lock", variant: "secondary" }) |
| 412 | : badge("Public", { variant: "secondary" })} | |
| 412 | : null} | |
| 413 | 413 | ${actions.text.trim() !== "" |
| 414 | 414 | ? html`<span class="repo-actions">${actions}</span>` |
| 415 | 415 | : null}`; |
src/web/live.ts +1 −1
| @@ -267,7 +267,7 @@ export const liveClientJs = /* js */ ` | ||
| 267 | 267 | }); |
| 268 | 268 | var mark = document.querySelector('#pull-check [data-head]'); |
| 269 | 269 | if (mark && mark.getAttribute('data-head')) head = mark.getAttribute('data-head'); |
| 270 | var word = textOf(document.querySelector('#pull-check .badge')); | |
| 270 | var word = textOf(document.querySelector('#pull-check [data-pull-word]')); | |
| 271 | 271 | if (status && word) status.textContent = word.charAt(0).toUpperCase() + word.slice(1); |
| 272 | 272 | }) |
| 273 | 273 | .catch(function () {}) |
src/web/studio/change.ts +31 −35
| @@ -87,7 +87,7 @@ import { pullMayRun } from "../../core/pull_trust.ts"; | ||
| 87 | 87 | import { nextOfPull } from "../../flow/next_actor.ts"; |
| 88 | 88 | import { STEP_MEANS, STEPS, stepOfPull, stepWord, type PathStep } from "../../flow/path.ts"; |
| 89 | 89 | import { changeHref } from "./nouns.ts"; |
| 90 | import { pullWord, servingHistory, wordBadge, type WordCtx } from "./words.ts"; | |
| 90 | import { pullWord, servingHistory, wordDot, type WordCtx } from "./words.ts"; | |
| 91 | 91 | |
| 92 | 92 | /** The pull and its repo, or the response that refuses to say why not. */ |
| 93 | 93 | export function pullContext(ctx: Ctx): { repo: Repo; pull: Pull } | Response { |
| @@ -405,27 +405,26 @@ async function pullPanels(ctx: Ctx, repo: Repo, pull: Pull, spent: Spent = newSp | ||
| 405 | 405 | deploy = { id: production.runId, commitOid: production.commitOid }; |
| 406 | 406 | } |
| 407 | 407 | |
| 408 | const checkLine = card( | |
| 409 | html`<p | |
| 410 | class="studio-pull-check" | |
| 411 | data-head="${pull.mergedOid ?? pull.headOid}" | |
| 412 | > | |
| 413 | ${wordBadge(word)} <span class="muted small">${note}</span> | |
| 414 | ${when( | |
| 415 | tick(spent, "tells", () => tellsAnswered(db, repo.id, [pull]).get(pull.number)), | |
| 416 | (t) => html`<span class="sep" aria-hidden="true">·</span | |
| 417 | >${tellLine(base, t)}`, | |
| 418 | )} | |
| 419 | ${when( | |
| 420 | deploy, | |
| 421 | (d) => | |
| 422 | html`<span class="sep" aria-hidden="true">·</span | |
| 423 | ><a href="${base}/runs/${d.id}" | |
| 424 | >deploy ${d.commitOid.slice(0, 7)}</a | |
| 425 | >`, | |
| 426 | )} | |
| 427 | </p>`, | |
| 428 | ); | |
| 408 | // At rest the check is a line, not a card: a dot, its word, the note. | |
| 409 | const checkLine = html`<p | |
| 410 | class="studio-pull-check" | |
| 411 | data-head="${pull.mergedOid ?? pull.headOid}" | |
| 412 | > | |
| 413 | ${wordDot(word)} | |
| 414 | <span class="word-text" data-pull-word>${word === "told" ? "open" : word}</span> | |
| 415 | <span class="muted small">${note}</span> | |
| 416 | ${when( | |
| 417 | tick(spent, "tells", () => tellsAnswered(db, repo.id, [pull]).get(pull.number)), | |
| 418 | (t) => html`<span class="sep" aria-hidden="true">·</span | |
| 419 | >${tellLine(base, t)}`, | |
| 420 | )} | |
| 421 | ${when( | |
| 422 | deploy, | |
| 423 | (d) => | |
| 424 | html`<span class="sep" aria-hidden="true">·</span | |
| 425 | ><a href="${base}/runs/${d.id}">deploy ${d.commitOid.slice(0, 7)}</a>`, | |
| 426 | )} | |
| 427 | </p>`; | |
| 429 | 428 | |
| 430 | 429 | const preview = tick(spent, "preview", () => |
| 431 | 430 | when(record, () => previewCard(ctx, repo, pull)), |
| @@ -617,19 +616,16 @@ async function changeConversation( | ||
| 617 | 616 | pull.originHeadRepo === null && |
| 618 | 617 | revertOf(db, repo.id, pull.id) === undefined, |
| 619 | 618 | () => |
| 620 | card( | |
| 621 | html`<p class="muted small"> | |
| 622 | Take it back — opens a new change that undoes exactly what this | |
| 623 | one shipped, for a person to ship. | |
| 624 | </p> | |
| 625 | <div class="bar mt"> | |
| 626 | ${confirmAction({ | |
| 627 | action: `${thread}/take-back`, | |
| 628 | verb: "Take it back", | |
| 629 | size: "sm", | |
| 630 | })} | |
| 631 | </div>`, | |
| 632 | ), | |
| 619 | html`<div class="bar mt"> | |
| 620 | ${confirmAction({ | |
| 621 | action: `${thread}/take-back`, | |
| 622 | verb: "Take it back", | |
| 623 | size: "sm", | |
| 624 | })} | |
| 625 | <span class="muted small" | |
| 626 | >opens a new change that undoes exactly what this one shipped</span | |
| 627 | > | |
| 628 | </div>`, | |
| 633 | 629 | ); |
| 634 | 630 | |
| 635 | 631 | const siblings = variantSiblings(db, [pull])[0] ?? []; |
src/web/studio/lists.ts +3 −4
| @@ -619,14 +619,13 @@ export async function changesList(ctx: Ctx): Promise<Response> { | ||
| 619 | 619 | row({ |
| 620 | 620 | href: changeHref(base, pull.number), |
| 621 | 621 | nav: true, |
| 622 | lead: avatar(pull.authorName), | |
| 622 | lead: wordDot(info.word, info.note), | |
| 623 | 623 | title: titled(pull.title, labels.get(pull.id)), |
| 624 | 624 | titleAttr: pull.title, |
| 625 | meta: html`${pull.originAuthor | |
| 625 | meta: html`${wordText(info.word)}${pull.originAuthor | |
| 626 | 626 | ? originTag(pull.originAuthor, originGlyph(repo)) |
| 627 | : pull.authorName} · ${info.note} · | |
| 627 | : pull.authorName} · | |
| 628 | 628 | ${when(answered.get(pull.number), (t) => html`${tellLine(base, t)} · `)}${time(pull.updatedAt)}`, |
| 629 | trail: wordBadge(info.word), | |
| 630 | 629 | ...stepAfter(steps.get(pull.id)), |
| 631 | 630 | }), |
| 632 | 631 | ), |
test/pull-quiet.test.ts added +104
| @@ -0,0 +1,104 @@ | ||
| 1 | /** The pull page, the pull list and the repository head at rest (#8719): a dot and a line, not badges and cards; every action still in reach; the bytes held. */ | |
| 2 | import { afterAll, beforeAll, expect, test } from "bun:test"; | |
| 3 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 4 | import { tmpdir } from "node:os"; | |
| 5 | import { join } from "node:path"; | |
| 6 | import { createApp, type App } from "../src/app.ts"; | |
| 7 | import { openPull } from "../src/actions/pulls.ts"; | |
| 8 | import { commitToBranch, createRepo } from "../src/actions/repos.ts"; | |
| 9 | import { createSession } from "../src/core/sessions.ts"; | |
| 10 | import { createUser } from "../src/core/users.ts"; | |
| 11 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 12 | import { quietLogs } from "./helpers/globals.ts"; | |
| 13 | ||
| 14 | quietLogs(); | |
| 15 | ||
| 16 | let dataDir: string; | |
| 17 | let app: App; | |
| 18 | let servers: Servers; | |
| 19 | let cookie = ""; | |
| 20 | let number = 0; | |
| 21 | const cells = process.env["FORGE_CELLS"]; | |
| 22 | ||
| 23 | beforeAll(async () => { | |
| 24 | process.env["FORGE_CELLS"] = "off"; | |
| 25 | dataDir = await mkdtemp(join(tmpdir(), "forge-pull-quiet-")); | |
| 26 | app = createApp({ dataDir, port: 0 }); | |
| 27 | servers = await serve(app); | |
| 28 | const made = await createUser(app.db, { | |
| 29 | origin: "seed", | |
| 30 | trusted: true, | |
| 31 | username: "alice", | |
| 32 | email: "[email protected]", | |
| 33 | password: "a-good-passphrase", | |
| 34 | adminIfFirst: true, | |
| 35 | }); | |
| 36 | if (!made.ok) throw new Error(String(made.error)); | |
| 37 | cookie = `forge_session=${createSession(app.db, made.value.id).token}`; | |
| 38 | const repo = await createRepo(app, made.value, { name: "r", visibility: "public" }); | |
| 39 | if (!repo.ok) throw new Error(String(repo.error)); | |
| 40 | for (const [branch, file] of [[undefined, "forge.json"], ["topic", "t.txt"]] as const) { | |
| 41 | const done = await commitToBranch(app, made.value, repo.value, { | |
| 42 | ...(branch === undefined ? {} : { branch }), | |
| 43 | message: "change", | |
| 44 | files: [{ path: file, content: file === "forge.json" ? "{}" : "x" }], | |
| 45 | }); | |
| 46 | if (!done.ok) throw new Error(JSON.stringify(done.error)); | |
| 47 | } | |
| 48 | const pull = await openPull(app, made.value, repo.value, { | |
| 49 | title: "Add a topic", | |
| 50 | body: "A small change.", | |
| 51 | baseBranch: "main", | |
| 52 | headBranch: "topic", | |
| 53 | }); | |
| 54 | if (!pull.ok) throw new Error(JSON.stringify(pull.error)); | |
| 55 | number = pull.value.number; | |
| 56 | }); | |
| 57 | ||
| 58 | afterAll(async () => { | |
| 59 | await servers.stop(); | |
| 60 | app.close(); | |
| 61 | if (cells === undefined) delete process.env["FORGE_CELLS"]; | |
| 62 | else process.env["FORGE_CELLS"] = cells; | |
| 63 | await rm(dataDir, { recursive: true, force: true }); | |
| 64 | }); | |
| 65 | ||
| 66 | const get = async (path: string) => (await fetch(`${servers.url}${path}`, { headers: { cookie } })).text(); | |
| 67 | const bytes = (t: string) => new TextEncoder().encode(t).length; | |
| 68 | ||
| 69 | // Measured 31,622 and 24,540 (before: 31,751 and 24,738). The margin covers the port digits and times in the page (up to ~160 B seen), so it holds the page from growing, not a tighter squeeze. | |
| 70 | const BUDGET = { pull: 32_000, list: 24_900 }; | |
| 71 | ||
| 72 | test("the pull page's check is a line with a dot and its word, not a card around a badge", async () => { | |
| 73 | const text = await get(`/alice/r/pulls/${number}`); | |
| 74 | const line = text.match(/<p[^>]*studio-pull-check[^>]*>[\s\S]*?<\/p>/)?.[0] ?? ""; | |
| 75 | expect(line).toContain('class="dot"'); | |
| 76 | expect(line).toContain("data-pull-word"); | |
| 77 | expect(line).not.toContain('class="badge"'); | |
| 78 | expect(text).not.toContain("Status:"); | |
| 79 | expect(text).toContain('id="pull-check"'); | |
| 80 | expect(bytes(text)).toBeLessThan(BUDGET.pull); | |
| 81 | }); | |
| 82 | ||
| 83 | test("the pull list wears a dot per row, and the head badges only Private", async () => { | |
| 84 | const text = await get(`/alice/r/pulls`); | |
| 85 | expect(text).toMatch(/class="dot"[^>]*aria-label="[^"]+"/); | |
| 86 | expect(text).not.toMatch(/class="row-trail"[^>]*><span class="badge"/); | |
| 87 | expect(text).not.toContain(">Public</span"); | |
| 88 | expect(bytes(text)).toBeLessThan(BUDGET.list); | |
| 89 | }); | |
| 90 | ||
| 91 | test("every page says what it is for at the top: a title naming it, one h1, and the repository's name", async () => { | |
| 92 | for (const [path, noun] of [["/alice/r/pulls", "ull"], ["/alice/r/issues", "ssue"], [`/alice/r/pulls/${number}`, "Add a topic"]] as const) { | |
| 93 | const text = await get(path); | |
| 94 | const title = text.match(/<title>([^<]*)<\/title>/)?.[1] ?? ""; | |
| 95 | expect(title).toContain(noun); | |
| 96 | expect(title).toContain("r"); | |
| 97 | expect(text.match(/<h1[ >]/g)?.length).toBe(1); | |
| 98 | } | |
| 99 | }); | |
| 100 | ||
| 101 | test("every action the pull page offered is still on it", async () => { | |
| 102 | const text = await get(`/alice/r/pulls/${number}`); | |
| 103 | for (const needle of ["/pulls/" + number + "/comments", "Close pull request", "Pulls", "Issues", "Files"]) expect(text).toContain(needle); | |
| 104 | }); |
test/repo-audience.test.ts +4 −3
| @@ -87,7 +87,7 @@ test("somebody not signed in sees the public sentence", async () => { | ||
| 87 | 87 | expect(h).not.toContain("repo-audience"); |
| 88 | 88 | }); |
| 89 | 89 | |
| 90 | test("the title says Public or Private once; the audience line adds no second badge", async () => { | |
| 90 | test("the title badges only what is not the default: Private; Public is the audience line's sentence alone (#8719)", async () => { | |
| 91 | 91 | const cookie = await signIn("olive"); |
| 92 | 92 | const made = await createRepo(app, olive, { name: "vault", visibility: "private" }); |
| 93 | 93 | if (!made.ok) throw new Error(String(made.error)); |
| @@ -95,8 +95,9 @@ test("the title says Public or Private once; the audience line adds no second ba | ||
| 95 | 95 | expect(priv.match(/class="badge"/g)?.length).toBe(1); |
| 96 | 96 | expect(priv).toContain(">Private</span"); |
| 97 | 97 | const pub = await head("/olive/yard", cookie); |
| 98 | expect(pub.match(/class="badge"/g)?.length).toBe(1); | |
| 99 | expect(pub).toContain(">Public</span"); | |
| 98 | expect(pub.match(/class="badge"/g)?.length ?? 0).toBe(0); | |
| 99 | expect(pub).not.toContain(">Public</span"); | |
| 100 | expect(pub).toContain("Public: anyone can read this"); | |
| 100 | 101 | }); |
| 101 | 102 | |
| 102 | 103 | test("the head draws the owner, separator and description with shared classes", async () => { |
test/studio-search.test.ts +4 −2
| @@ -1124,8 +1124,10 @@ describe("the studio names a person on the app's page, and the crew only for the | ||
| 1124 | 1124 | const onList = rowWith(changes, `/pulls/${personPull.number}`); |
| 1125 | 1125 | expect(onApp).toContain("ivy"); |
| 1126 | 1126 | expect(onApp).not.toContain("the crew"); |
| 1127 | // The same name the list gives, and the same initials beside it. | |
| 1128 | expect(initialsIn(onApp)).toBe(initialsIn(onList)); | |
| 1127 | // The same name the list gives; the list's row wears a status dot in | |
| 1128 | // place of an avatar (#8719), so the initials are the app page's alone. | |
| 1129 | expect(onList).toContain("ivy"); | |
| 1130 | expect(onList).not.toContain("the crew"); | |
| 1129 | 1131 | expect(initialsIn(onApp)).toBe("i"); |
| 1130 | 1132 | expect(change).toContain("ivy"); |
| 1131 | 1133 |
new above
offline
The note (the reason a pull is red or waiting) no longer shows on the row. It is only in the dot's aria-label, so a phone user cannot reach it. Restore
info.notein the meta, at least wherewordNeedsTextis true.