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

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

scout/forge:scout/3619-design-a-review-verdict-s-body-uses-the → main 1 commit fork 1 thread · 1 unresolved

  • Merged. scout/3619-design-a-review-verdict-s-body-uses-the was merged into main as 2997565.

Review

Sign in to review this change.
5 files changed +81 −10
docs/DESIGN.md −1
@@ -389,7 +389,6 @@ gone, so this list only shrinks.
389389 - **`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`
390390 - **`style/content.ts`, prose (markdown)** — table cell alignment, built by `markdown.ts` as `ta-${align}`: `.ta-c` `.ta-r`
391391 - **`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`
393392 - **`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`
394393 - **`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`
395394 - **`style/content.ts`, the finder** — the finder's footnote: `.finder-note`
src/web/studio/change.ts +4 −2
@@ -176,8 +176,10 @@ function reviewEntry(
176176 )}${when(
177177 review.body !== "",
178178 () =>
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>
181183 </div>`,
182184 )}`,
183185 }),
src/web/style/components.ts +2
@@ -747,6 +747,8 @@ a.row:hover .row-title { color: var(--accent); }
747747 min-height: 32px;
748748 }
749749 .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; }

Scoping to .tl-item[data-kind="event"] > .card keeps this away from comment cards. Check at 360px that flex-basis: 100% still wraps under the verdict line, since the row must be a wrapping flex container.

751 .tl-item[data-kind="event"] > .card .comment-body { padding: 8px 12px; }
750752
751753 /* ---- comment card ---- */
752754 .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; }
777777 keeps its words and loses its colour. */
778778 .verdict-stale { color: var(--muted-fg); background: none; }
779779 .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; }
787780 .review-card .review-verdicts { border: 0; padding: 0; margin: 0; display: grid; gap: 10px; }
788781 .review-verdicts .check .hint {
789782 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 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs