Public: anyone can read this; the crew can change it.

A review's words on a change's timeline read as plain prose under its verdict, not a boxed card #9991

Merged opened this

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

Merged

mason/forge:mason/9987-a-review-s-words-on-a-change-s-timeline → main 3 commits fork 2 threads · 2 unresolved

  • Merged. mason/9987-a-review-s-words-on-a-change-s-timeline was merged into main as 5b4e643.

Review

Sign in to review this change.
3 files changed +70 −5
src/web/studio/change.ts +2 −4
@@ -205,10 +205,8 @@ function reviewEntry(
205205 )}${when(
206206 review.body !== "",
207207 () =>
208 html`<div class="card">
209 <div class="prose comment-body">
210 ${renderMarkdown(review.body)}
211 </div>
208 html`<div class="prose comment-body">

The card is gone, as asked. It only reads as plain prose once the padding fix in components.ts lands.

209 ${renderMarkdown(review.body)}
212210 </div>`,
213211 )}`,
214212 }),
src/web/style/components.ts +1 −1
@@ -896,7 +896,7 @@ details[open] > summary.row > .row-lead { transform: rotate(90deg); }
896896 .tl-earlier { margin: 0; padding-left: 36px; color: var(--muted-fg); font-size: 13px; }
897897 .tl-item[data-kind="comment"] { padding-bottom: 16px; }
898898 .tl-item[data-kind="event"] > .card { flex-basis: 100%; margin: 4px 0 0; color: var(--fg); font-size: 14px; }
899 .tl-item[data-kind="event"] > .card .comment-body { padding: 8px 12px; }
899 .tl-item[data-kind="event"] > .comment-body { flex-basis: 100%; min-width: 0; margin: 4px 0 0; padding: 0; color: var(--fg); font-size: 14px; }
outdated

Principle 3 (rhythm, not boxes): this rule never resets padding, and .prose { padding: 16px 20px } (content.ts:478) still applies, so the text is pushed 20px in and 16px down from the verdict line at 360px. Add padding: 0, as .comment-row .comment-body does.

900900
901901 /* ---- comment card ---- */
902902 .comment .card-head { background: var(--muted); font-size: 13px; }
test/review-language.test.ts added +67
@@ -0,0 +1,67 @@
1 /** A review's words on a change's timeline sit under the verdict as plain prose, not a boxed card. */
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 { openPull } from "../src/actions/pulls.ts";
7 import { commitToBranch, createRepo } from "../src/actions/repos.ts";
8 import { createApp, type App } from "../src/app.ts";
9 import { submitReview } from "../src/core/reviews.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 import { components as componentsCss } from "../src/web/style/components.ts";
14
15 quietLogs();
16
17 let dataDir = "";
18 let app: App, servers: Servers;
19 let pullNo = 0;
20
21 beforeAll(async () => {
22 dataDir = await mkdtemp(join(tmpdir(), "forge-review-lang-"));
23 app = createApp({ dataDir, port: 0 });
24 const made = await createUser(app.db, {
25 origin: "seed", trusted: true, username: "alice", email: "[email protected]",
26 password: "a-good-passphrase", adminIfFirst: true,
27 });
28 if (!made.ok) throw new Error(made.error);
29 const r = await createRepo(app, made.value, { name: "garden", visibility: "public" });
30 if (!r.ok) throw new Error(String(r.error));
31 await commitToBranch(app, made.value, r.value, { message: "seed", files: [{ path: "README.md", content: "# g\n" }] });
32 const c = await commitToBranch(app, made.value, r.value, { branch: "one", base: "main", message: "one", files: [{ path: "one.md", content: "x\n" }] });
33 if (!c.ok) throw new Error(JSON.stringify(c.error));
34 const o = await openPull(app, made.value, r.value, { title: "One", body: "", baseBranch: "main", headBranch: "one" });
35 if (!o.ok) throw new Error(String(o.error));
36 pullNo = o.value.number;
37 const bob = await createUser(app.db, {
38 origin: "seed", trusted: true, username: "bob", email: "[email protected]", password: "a-good-passphrase",
39 });
40 if (!bob.ok) throw new Error(bob.error);
41 const rv = (state: "approved" | "commented", body: string) => {
42 const x = submitReview(app.db, { issueId: o.value.id, authorId: bob.value.id, state, body, commitOid: c.value.commitOid });
43 if (!x.ok) throw new Error(x.error);
44 };
45 rv("approved", "**ok** <script>x</script>");
46 rv("commented", "");
47 servers = await serve(app);
48 }, 30_000);
49
50 afterAll(async () => {
51 servers.stop();
52 app.close();
53 await rm(dataDir, { recursive: true, force: true });
54 });
55
56 test("a review's body is prose under the verdict, escaped, outside any card", async () => {
57 // the padding of .prose must not apply on the timeline
58 expect(componentsCss).toMatch(/data-kind="event"\] > \.comment-body \{[^}]*padding: 0/);
59 const page = await (await fetch(`${servers.url}/alice/garden/pulls/${pullNo}`)).text();
60 const mine = page.indexOf("<strong>ok</strong>");
61 expect(mine).toBeGreaterThan(0);
62 const item = page.slice(page.lastIndexOf('data-kind="event"', mine), mine);
63 expect(item).toContain('<div class="prose comment-body">');
64 expect(item).not.toContain('class="card');
65 expect(page).not.toContain("<script>x</script>");
66 expect(page).toContain("&lt;script&gt;");
67 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs