Public: anyone can read this; the crew can change it.
Design: a review verdict's body uses the design system, and review-body leaves §5d #3628
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/3619-design-a-review-verdict-s-body-uses-the → main 1 commit fork 1 thread · 1 unresolved
5 files
changed +81
−10
docs/DESIGN.md −1
| @@ -389,7 +389,6 @@ gone, so this list only shrinks. | ||
| 389 | 389 | - **`style/content.ts`, code view** — the source view and the syntax-highlight token classes `highlight-html.ts` builds as `tok-${kind}`: `.blob-view` `.ln` `.tok-attribute` `.tok-comment` `.tok-constant` `.tok-decorator` `.tok-deleted` `.tok-emphasis` `.tok-function` `.tok-heading` `.tok-inserted` `.tok-keyword` `.tok-link` `.tok-meta` `.tok-number` `.tok-operator` `.tok-plain` `.tok-property` `.tok-punctuation` `.tok-regex` `.tok-string` `.tok-tag` `.tok-type` `.tok-variable` |
| 390 | 390 | - **`style/content.ts`, prose (markdown)** — table cell alignment, built by `markdown.ts` as `ta-${align}`: `.ta-c` `.ta-r` |
| 391 | 391 | - **`style/content.ts`, diff viewer** — the diff table: its gutters, line kinds, file head and inline review comments: `.add` `.del` `.dh` `.diff` `.diff-body` `.diff-head` `.diff-note` `.diff-path` `.diff-stat` `.diff-stub` `.diff-summary` `.diff-table` `.dl` `.dl-review` `.dn` `.dn-comment` `.nonl` `.rc-body` `.rc-form` `.rc-head` `.rc-outdated` `.rc-resolved-line` `.wd` |
| 392 | - **`style/content.ts`, review verdicts** — the body under a verdict: `.review-body` | |
| 393 | 392 | - **`style/content.ts`, apps, runs, and the crew** — run pages, their logs and checks (a pull's look is §5's **look**): `.app-url` `.check-progress` `.check-steps` `.ln-forge` `.ln-pulse` `.ln-say` `.ln-tool` `.ln-you` `.log-tail-row` `.pinned-failures` `.pinned-slot` `.run-actions` `.run-jump` `.run-live` `.run-steer` `.run-summary` `.step-state` `.tests-bar` `.tests-words` `.txt` `.who` |
| 394 | 393 | - **`style/content.ts`, the code browser: explorer beside content** — the explorer column's own parts: `.code-main` `.code-split` `.ex-empty` `.ex-find` `.ex-head` `.ex-list` `.ex-name` `.ex-ref` `.ex-twist` `.explorer-toggle` |
| 395 | 394 | - **`style/content.ts`, the finder** — the finder's footnote: `.finder-note` |
src/web/studio/change.ts +4 −2
| @@ -176,8 +176,10 @@ function reviewEntry( | ||
| 176 | 176 | )}${when( |
| 177 | 177 | review.body !== "", |
| 178 | 178 | () => |
| 179 | html`<div class="prose review-body"> | |
| 180 | ${renderMarkdown(review.body)} | |
| 179 | html`<div class="card"> | |
| 180 | <div class="prose comment-body"> | |
| 181 | ${renderMarkdown(review.body)} | |
| 182 | </div> | |
| 181 | 183 | </div>`, |
| 182 | 184 | )}`, |
| 183 | 185 | }), |
src/web/style/components.ts +2
| @@ -747,6 +747,8 @@ a.row:hover .row-title { color: var(--accent); } | ||
| 747 | 747 | min-height: 32px; |
| 748 | 748 | } |
| 749 | 749 | .tl-item[data-kind="comment"] .card { margin: 0; } |
| 750 | .tl-item[data-kind="event"] > .card { flex-basis: 100%; margin: 4px 0 0; color: var(--fg); font-size: 14px; } | |
| 751 | .tl-item[data-kind="event"] > .card .comment-body { padding: 8px 12px; } | |
| 750 | 752 | |
| 751 | 753 | /* ---- comment card ---- */ |
| 752 | 754 | .comment .card-head { background: var(--muted); font-size: 13px; } |
src/web/style/content.ts −7
| @@ -777,13 +777,6 @@ form.inline { display: inline; margin-left: auto; } | ||
| 777 | 777 | keeps its words and loses its colour. */ |
| 778 | 778 | .verdict-stale { color: var(--muted-fg); background: none; } |
| 779 | 779 | .verdict-stale .ico { width: 12px; height: 12px; } |
| 780 | .review-body { | |
| 781 | flex-basis: 100%; margin-top: 4px; padding: 8px 12px; | |
| 782 | background: var(--card); border: 1px solid var(--border); | |
| 783 | border-radius: var(--radius); font-size: 14px; color: var(--fg); | |
| 784 | } | |
| 785 | .review-body p:first-child { margin-top: 0; } | |
| 786 | .review-body p:last-child { margin-bottom: 0; } | |
| 787 | 780 | .review-card .review-verdicts { border: 0; padding: 0; margin: 0; display: grid; gap: 10px; } |
| 788 | 781 | .review-verdicts .check .hint { |
| 789 | 782 | display: block; font-size: 12.5px; color: var(--muted-fg); margin-top: 1px; |
test/review-verdict-body.test.ts added +75
| @@ -0,0 +1,75 @@ | ||
| 1 | /** A review verdict's body is drawn from §5's components, not `.review-body` (#3619). */ | |
| 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, submitReview } 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 | ||
| 22 | beforeAll(async () => { | |
| 23 | dataDir = await mkdtemp(join(tmpdir(), "forge-verdict-body-")); | |
| 24 | app = createApp({ dataDir, port: 0 }); | |
| 25 | servers = await serve(app); | |
| 26 | const made = await createUser(app.db, { | |
| 27 | origin: "seed", | |
| 28 | trusted: true, | |
| 29 | username: "alice", | |
| 30 | email: "[email protected]", | |
| 31 | password: "a-good-passphrase", | |
| 32 | adminIfFirst: true, | |
| 33 | }); | |
| 34 | if (!made.ok) throw new Error(String(made.error)); | |
| 35 | const alice = made.value; | |
| 36 | cookie = `forge_session=${createSession(app.db, alice.id).token}`; | |
| 37 | const repo = await createRepo(app, alice, { name: "r", visibility: "public" }); | |
| 38 | if (!repo.ok) throw new Error(String(repo.error)); | |
| 39 | for (const [branch, file] of [[undefined, "forge.json"], ["topic", "t.txt"]] as const) { | |
| 40 | const done = await commitToBranch(app, alice, repo.value, { | |
| 41 | ...(branch === undefined ? {} : { branch }), | |
| 42 | message: "change", | |
| 43 | files: [{ path: file, content: file === "forge.json" ? "{}" : "x" }], | |
| 44 | }); | |
| 45 | if (!done.ok) throw new Error(JSON.stringify(done.error)); | |
| 46 | } | |
| 47 | const pull = await openPull(app, alice, repo.value, { | |
| 48 | title: "t", | |
| 49 | body: "", | |
| 50 | baseBranch: "main", | |
| 51 | headBranch: "topic", | |
| 52 | }); | |
| 53 | if (!pull.ok) throw new Error(JSON.stringify(pull.error)); | |
| 54 | number = pull.value.number; | |
| 55 | const review = await submitReview(app, alice, repo.value, pull.value, { | |
| 56 | state: "commented", | |
| 57 | body: "Looks **fine**.\n\n- one", | |
| 58 | }); | |
| 59 | if (!review.ok) throw new Error(JSON.stringify(review.error)); | |
| 60 | }); | |
| 61 | ||
| 62 | afterAll(async () => { | |
| 63 | await servers.stop(); | |
| 64 | app.close(); | |
| 65 | await rm(dataDir, { recursive: true, force: true }); | |
| 66 | }); | |
| 67 | ||
| 68 | test("a verdict's body renders its markdown inside a card, with no review-body class", async () => { | |
| 69 | const res = await fetch(`${servers.url}/alice/r/pulls/${number}`, { headers: { cookie } }); | |
| 70 | const page = await res.text(); | |
| 71 | expect(page).toContain("<strong>fine</strong>"); | |
| 72 | expect(page).toContain('class="prose comment-body"'); | |
| 73 | expect(page).not.toContain('class="prose review-body"'); | |
| 74 | expect(page).not.toMatch(/class="[^"]*\breview-body\b/); | |
| 75 | }); |
new above
offline
Scoping to
.tl-item[data-kind="event"] > .cardkeeps this away from comment cards. Check at 360px thatflex-basis: 100%still wraps under the verdict line, since the row must be a wrapping flex container.