Public: anyone can read this; the crew can change it.
A pull's Files tab with a line comment has no sideways scroll at 390px or desktop, with looks on the pull #10097
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/10073-a-pull-s-files-tab-with-a-line-comment-h → main 1 commit fork
4 files
changed +44
−3
docs/design/areas/pull.md +1 −1
| @@ -1,6 +1,6 @@ | ||
| 1 | 1 | # Pull |
| 2 | 2 | |
| 3 | Every forge pull's preview carries desk and phone looks of this page as the owner, over the preview seed's `/preview-admin/sample/pulls/3` (#8865). | |
| 3 | Every forge pull's preview carries desk and phone looks of this page as the owner, over the preview seed's `/preview-admin/sample/pulls/3` (#8865). Its Files tab (`/files`) is pictured too, over a seeded line comment on `GREETING.md` (#10073); the comment is `.prose`, which wraps anywhere, and the diff table scrolls inside its own wrapper, so the page does not scroll sideways at 390 px or desk. No new class. | |
| 4 | 4 | |
| 5 | 5 | A pull is one change waiting on a person: read it, see its check, ship it or say what is off (#8773). |
| 6 | 6 |
src/spawn/preview-looks.ts +4
| @@ -27,6 +27,9 @@ export const PREVIEW_PULL = 3; | ||
| 27 | 27 | export const PREVIEW_REPO_HOME = `/${PREVIEW_ADMIN}/${PREVIEW_REPO}`; |
| 28 | 28 | export const PREVIEW_PULL_PAGE = `${PREVIEW_REPO_HOME}/pulls/${String(PREVIEW_PULL)}`; |
| 29 | 29 | |
| 30 | /** The pull's Files tab, whose diff holds a line comment that must not scroll sideways (#10073). */ | |
| 31 | export const PREVIEW_PULL_FILES = `${PREVIEW_PULL_PAGE}/files`; | |
| 32 | ||
| 30 | 33 | /** |
| 31 | 34 | * The seeded app: a second repository whose only app fact is its record, so |
| 32 | 35 | * its home is the app page (#9782). Nothing runs: the page says so. The seed |
| @@ -126,6 +129,7 @@ export const PREVIEW_OWNER_LOOKS: readonly OwnerLook[] = [ | ||
| 126 | 129 | PREVIEW_REPO_HOME, |
| 127 | 130 | PREVIEW_APP_PAGE, |
| 128 | 131 | PREVIEW_PULL_PAGE, |
| 132 | PREVIEW_PULL_FILES, | |
| 129 | 133 | PREVIEW_ITERATIONS_PAGE, |
| 130 | 134 | PREVIEW_HELD_PAGE, |
| 131 | 135 | PREVIEW_BETS_PAGE, |
src/spawn/preview-seed.ts +10 −1
| @@ -23,7 +23,7 @@ import { writeBoard } from "../actions/boards.ts"; | ||
| 23 | 23 | import { backBet } from "../actions/bets.ts"; |
| 24 | 24 | import { openIssue, setIssueState } from "../actions/issues.ts"; |
| 25 | 25 | import { pinBuild } from "../actions/builds.ts"; |
| 26 | import { mergePull, openPull } from "../actions/pulls.ts"; | |
| 26 | import { addReview, mergePull, openPull } from "../actions/pulls.ts"; | |
| 27 | 27 | import { commitToBranch, createRepo, forkRepo } from "../actions/repos.ts"; |
| 28 | 28 | import { reviewGate } from "../actions/gate_review.ts"; |
| 29 | 29 | import { linkIssue } from "../actions/dependencies.ts"; |
| @@ -254,6 +254,15 @@ async function seedWorld(app: App, admin: User): Promise<Result<void, string>> { | ||
| 254 | 254 | if (!pull.ok) return err(`the pull: ${String(pull.error)}`); |
| 255 | 255 | if (pull.value.number !== PREVIEW_PULL) return err(`the pull is #${String(pull.value.number)}, not #${String(PREVIEW_PULL)}`); |
| 256 | 256 | |
| 257 | // A review comment on a line of the pull's diff, so its Files tab draws one (#10073). | |
| 258 | const reviewed = await addReview(app, admin, repo.value, pull.value, { | |
| 259 | body: `Say hello to the whole forge, not only to the one who reads this line. ${"A long unbroken word follows: " + "x".repeat(80)}`, | |
| 260 | path: "GREETING.md", | |
| 261 | line: 1, | |
| 262 | side: "new", | |
| 263 | }); | |
| 264 | if (!reviewed.ok) return err(`the line comment: ${String(reviewed.error)}`); | |
| 265 | ||
| 257 | 266 | // Issues and pulls share one numbering, so the pull above keeps PREVIEW_PULL |
| 258 | 267 | // and the ones /next lists and the bets hold come after it. |
| 259 | 268 | const listed = [ |
test/pull-phone-fit.test.ts +29 −1
| @@ -14,7 +14,7 @@ import { serve, type Servers } from "../src/http/server.ts"; | ||
| 14 | 14 | import { addComment } from "../src/core/issues.ts"; |
| 15 | 15 | import { createSession } from "../src/core/sessions.ts"; |
| 16 | 16 | import { findChromium } from "../src/looks/capture.ts"; |
| 17 | import { PREVIEW_PULL_PAGE } from "../src/spawn/preview-looks.ts"; | |
| 17 | import { PREVIEW_OWNER_LOOKS, PREVIEW_PULL_FILES, PREVIEW_PULL_PAGE } from "../src/spawn/preview-looks.ts"; | |
| 18 | 18 | import { previewAdmin, seedPreview } from "../src/spawn/preview-seed.ts"; |
| 19 | 19 | import { base as elements, utilities as base } from "../src/web/style/base.ts"; |
| 20 | 20 | import { components } from "../src/web/style/components.ts"; |
| @@ -111,3 +111,31 @@ describe.skipIf(chromium === undefined)("a pull's conversation page, measured", | ||
| 111 | 111 | expect(m.scrollWidth).toBeLessThanOrEqual(m.viewportWidth); |
| 112 | 112 | }, 30_000); |
| 113 | 113 | }); |
| 114 | ||
| 115 | describe("a pull's Files tab with a line comment (#10073)", () => { | |
| 116 | test("the seeded diff carries the review comment", () => { | |
| 117 | expect(files).toContain("Say hello to the whole forge"); | |
| 118 | }); | |
| 119 | test("the preview takes looks of the Files tab at desk and phone", () => { | |
| 120 | const taken = PREVIEW_OWNER_LOOKS.filter((l) => l.page === PREVIEW_PULL_FILES).map((l) => l.kind).sort(); | |
| 121 | expect(taken).toEqual(["desktop", "phone"]); | |
| 122 | }); | |
| 123 | }); | |
| 124 | ||
| 125 | describe.skipIf(chromium === undefined)("a pull's Files tab with a line comment, measured", () => { | |
| 126 | const measure = `JSON.stringify({ scrollWidth: document.documentElement.scrollWidth, viewportWidth: document.documentElement.clientWidth, comment: document.body.textContent.includes("Say hello to the whole forge") })`; | |
| 127 | const look = async (width: number, height: number) => { | |
| 128 | const cookies = [{ name: "forge_session", value: createSession(app.db, previewAdmin(app)!.id).token }]; | |
| 129 | return JSON.parse(await evaluateLoaded(chromium!, `${servers.url}${PREVIEW_PULL_FILES}`, { width, height }, cookies, measure)) as { scrollWidth: number; viewportWidth: number; comment: boolean }; | |
| 130 | }; | |
| 131 | test("phone, 390x844: no sideways scroll", async () => { | |
| 132 | const m = await look(390, 844); | |
| 133 | expect(m.comment).toBe(true); | |
| 134 | expect(m.scrollWidth).toBeLessThanOrEqual(m.viewportWidth); | |
| 135 | }, 30_000); | |
| 136 | test("desk, 1280x800: no sideways scroll", async () => { | |
| 137 | const m = await look(1280, 800); | |
| 138 | expect(m.comment).toBe(true); | |
| 139 | expect(m.scrollWidth).toBeLessThanOrEqual(m.viewportWidth); | |
| 140 | }, 30_000); | |
| 141 | }); |