trevato/forge

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

More
Forks1g fMapg aRuns17g rEventsFlows

A pull's page fits a 390 px phone: its title, branch names and conversation wrap instead of scrolling sideways #9356

Merged masoncrew opened this

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

ConversationFiles
Merged

mason/forge:mason/9327-a-pull-s-page-fits-a-390-px-phone-its-ti → main 8 commits fork

  • Merged. mason/9327-a-pull-s-page-fits-a-390-px-phone-its-ti was merged into main as 77fa477.

Review

Sign in to review this change.
5 files changed +122 −3
docs/design/areas/pull.md +4
@@ -9,3 +9,7 @@ At rest: the state is a dot (`.dot`, `role=img`, one `aria-label`, no `title`),
99 Rare actions (Hold, Take it back) sit in a `<details id="more" class="mt mb">`, the same disclosure Hold always used. Their palette rows link to `?more=1#hold` / `?more=1#take-back`: the server renders the details open for `?more`, so the target is visible with no script. Every other action is a palette row too (`pullActions`, "This pull request"). No new class.
1010
1111 The terminal's pull screen (`src/tui/screens/pull.ts`) draws its own rows and shares no markup with this page; it needs no change.
12
13 On a 390 px phone the page does not scroll sideways (#9327): the title (`.page-head h1`) and every `.mono` (branch names) wrap with `overflow-wrap: anywhere`, comments already do through `.prose`. The diff scrolls inside its own box. No new class; `test/pull-phone-fit.test.ts` holds it.
14
15 `.split` (content.ts) has one `minmax(0, 1fr)` column below 960 px, so a long unbroken line in a comment cannot widen the page. It is global to every `.split` page; the `min-width: 960px` and `data-rail="wide"` rules come after it in the same file and still set their two columns, so desk layouts are unchanged.
src/web/style/base.ts +1
@@ -57,6 +57,7 @@ body {
5757 a { color: inherit; text-decoration: none; transition: color 120ms var(--ease); }
5858 a:hover { color: var(--accent); }
5959 code, pre, .mono { font-family: var(--mono); font-size: 12.5px; }
60 .mono { overflow-wrap: anywhere; }
6061
6162 h1 { font-size: 20px; font-weight: 620; letter-spacing: -0.02em; line-height: 1.25; text-wrap: balance; }
6263 h2 { font-size: 16px; font-weight: 600; letter-spacing: -0.01em; line-height: 1.3; text-wrap: balance; }
src/web/style/components.ts +2 −2
@@ -814,7 +814,7 @@ details[open] > summary.row > .row-lead { transform: rotate(90deg); }
814814 display: flex; align-items: center; gap: 10px; flex-wrap: wrap;
815815 margin: 20px 0 14px;
816816 }
817 .page-head h1 { min-width: 0; }
817 .page-head h1 { min-width: 0; overflow-wrap: anywhere; }
818818 .page-head > .page-head-sub { flex-basis: 100%; margin-top: -4px; }
819819
820820 /* ---- breadcrumbs ---- */
@@ -847,7 +847,7 @@ details[open] > summary.row > .row-lead { transform: rotate(90deg); }
847847 content: ""; position: absolute; left: 13px; top: 8px; bottom: 8px;
848848 width: 2px; background: var(--border);
849849 }
850 .tl-item { position: relative; padding-left: 36px; }
850 .tl-item { position: relative; padding-left: 36px; min-width: 0; }
851851 .tl-item .tl-dot {
852852 position: absolute; left: 4px; top: 6px;
853853 display: inline-flex; align-items: center; justify-content: center;
src/web/style/content.ts +2 −1
@@ -217,6 +217,7 @@ form.inline { display: inline; margin-left: auto; }
217217 than the phone rather than letting its own ellipsis do the work. A zero
218218 min-width is what makes truncation inside a grid actually truncate. */
219219 .split > * { min-width: 0; }
220 .split { grid-template-columns: minmax(0, 1fr); }
220221
221222 /* The generic two-column page: content, then a sidebar that sticks. */
222223 .split { display: grid; gap: 16px; margin-top: 16px; align-items: start; }
@@ -330,7 +331,7 @@ form.inline { display: inline; margin-left: auto; }
330331 .imgview-pane.is-actual .imgview-img { max-width: none; cursor: zoom-out; }
331332
332333 /* ---- prose (markdown) ---- */
333 .prose { padding: 16px 20px; font-size: 15px; line-height: 1.65; overflow-wrap: break-word; }
334 .prose { padding: 16px 20px; font-size: 15px; line-height: 1.65; overflow-wrap: anywhere; }
334335 .prose > * { max-width: var(--measure); }
335336 .prose h1, .prose h2, .prose h3, .prose h4 { margin: 24px 0 8px; }
336337 .prose h1 { font-size: 20px; }
test/pull-phone-fit.test.ts added +113
@@ -0,0 +1,113 @@
1 /**
2 * A pull's page fits a 390 px phone (#9327): the preview seed's pull, given a
3 * long title, a 200-character branch name and a comment holding a 300-character
4 * unbroken word, ships no fixed width over 390 px, wraps its long text, and
5 * (where a Chromium starts) does not scroll sideways.
6 */
7 import { afterAll, beforeAll, describe, expect, test } from "bun:test";
8 import { createHmac } from "node:crypto";
9 import { mkdtemp, rm } from "node:fs/promises";
10 import { tmpdir } from "node:os";
11 import { join } from "node:path";
12 import { createApp, type App } from "../src/app.ts";
13 import { serve, type Servers } from "../src/http/server.ts";
14 import { addComment } from "../src/core/issues.ts";
15 import { createSession } from "../src/core/sessions.ts";
16 import { findChromium } from "../src/looks/capture.ts";
17 import { PREVIEW_PULL_PAGE } from "../src/spawn/preview-looks.ts";
18 import { previewAdmin, seedPreview } from "../src/spawn/preview-seed.ts";
19 import { base as elements, utilities as base } from "../src/web/style/base.ts";
20 import { components } from "../src/web/style/components.ts";
21 import { content } from "../src/web/style/content.ts";
22 import { chromiumRuns, evaluateLoaded } from "./helpers/chrome.ts";
23
24 const KEY = "pull-phone-fit-key";
25 const WORD = "w".repeat(300);
26 const BRANCH = `feature/${"b".repeat(192)}`;
27 const TITLE = `Make every long and winding pull title wrap ${"t".repeat(120)} and many words after it`;
28 let dir: string;
29 let app: App;
30 let servers: Servers;
31 let page: string;
32 let files: string;
33
34 beforeAll(async () => {
35 dir = await mkdtemp(join(tmpdir(), "forge-pull-fit-"));
36 app = createApp({ dataDir: dir, port: 0, previewIdentityKey: KEY });
37 const seeded = await seedPreview(app, { FORGE_APP_ENV: "preview" });
38 if (!seeded.ok) throw new Error(seeded.error);
39 const admin = previewAdmin(app)!;
40 const number = Number(PREVIEW_PULL_PAGE.split("/").pop());
41 const issue = app.db.query("SELECT id FROM issue WHERE number = ?1 AND id IN (SELECT issue_id FROM pull)").get(number) as { id: string };
42 app.db.query("UPDATE issue SET title = ?1 WHERE id = ?2").run(TITLE, issue.id);
43 app.db.query("UPDATE pull SET head_branch = ?1 WHERE issue_id = ?2").run(BRANCH, issue.id);
44 for (let i = 0; i < 12; i++) addComment(app.db, { issueId: issue.id, authorId: admin.id, body: `Row ${String(i)} ${WORD} https://example.com/${WORD}` });
45 servers = await serve(app);
46 const said = `u1|trevato|admin|${Date.now() + 60_000}`;
47 const headers = { "x-forge-identity": said, "x-forge-signature": createHmac("sha256", KEY).update(said).digest("base64url") };
48 page = await (await fetch(`${servers.url}${PREVIEW_PULL_PAGE}`, { headers })).text();
49 files = await (await fetch(`${servers.url}${PREVIEW_PULL_PAGE}/files`, { headers })).text();
50 }, 30_000);
51 afterAll(async () => {
52 servers.stop();
53 app.close();
54 await rm(dir, { recursive: true, force: true });
55 });
56
57 /** Every `width`/`min-width` in `css` that fixes more than 390 px, as written. */
58 const widthsOver390 = (css: string): string[] =>
59 [...css.matchAll(/(?<![-\w(])(?:min-)?width\s*:\s*(-?[\d.]+)(px|rem|em|ch)\b/g)]
60 .filter((m) => Number(m[1]) * (m[2] === "px" ? 1 : m[2] === "ch" ? 8 : 16) > 390)
61 .map((m) => m[0]);
62
63 describe("a pull's conversation page at 390 px", () => {
64 test("the seeded long content is on the page", () => {
65 expect(page).toContain(TITLE);
66 expect(page).toContain(WORD);
67 // The branch is drawn where the pull says where it came from: the Files step.
68 expect(files).toContain(BRANCH);
69 });
70
71 test("no inline width and no rule the page ships fixes a width over 390 px", () => {
72 for (const m of page.matchAll(/\swidth=["']?(\d+)/gi)) expect(Number(m[1])).toBeLessThanOrEqual(390);
73 expect(page).not.toMatch(/style="[^"]*(?:^|[;\s])min-width\s*:/i);
74 const sheets = [base, components, content, ...[page, files].flatMap((html) => html.match(/<style[^>]*>[\s\S]*?<\/style>/g) ?? [])];
75 const inline = [...page.matchAll(/style="([^"]*)"/g)].map((m) => m[1]!);
76 expect([...inline, ...sheets].flatMap(widthsOver390)).toEqual([]);
77 });
78
79 test("the long-text containers wrap anywhere", () => {
80 expect(components).toMatch(/\.page-head h1[^{]*\{[^}]*overflow-wrap:\s*(anywhere|break-word)/);
81 expect(elements).toMatch(/\.mono[^{]*\{[^}]*overflow-wrap:\s*(anywhere|break-word)/);
82 expect(content).toMatch(/\.prose\s*\{[^}]*overflow-wrap:\s*(anywhere|break-word)/);
83 });
84 });
85
86 const found = findChromium();
87 const chromium = found !== undefined && chromiumRuns(found) ? found : undefined;
88 if (chromium === undefined) console.log("SKIPPING: no chromium that starts here, so the pull's phone look is not measured");
89
90 describe.skipIf(chromium === undefined)("a pull's conversation page, measured", () => {
91 const measure = `(() => {
92 const vw = document.documentElement.clientWidth;
93 const clipped = (el) => { for (let p = el.parentElement; p && p !== document.body; p = p.parentElement) { const o = getComputedStyle(p).overflowX; if (o !== "visible") return true; } return false; };
94 const overflowing = [...document.querySelectorAll(".page-head, .page-head *, .split, .split *")].filter((el) => el.getBoundingClientRect().right > vw + 1 && !clipped(el) && getComputedStyle(el).clipPath === "none" && getComputedStyle(el).position !== "fixed").slice(0, 8).map((el) => el.tagName.toLowerCase() + "." + String(el.className));
95 return JSON.stringify({ scrollWidth: document.documentElement.scrollWidth, viewportWidth: vw, overflowing });
96 })()`;
97 // The layout's own chrome (tabs, off-canvas menus) is web-phone.test.ts's to hold; the page's content is measured here.
98 const look = async (width: number, height: number) => {
99 const cookies = [{ name: "forge_session", value: createSession(app.db, previewAdmin(app)!.id).token }];
100 const m = JSON.parse(await evaluateLoaded(chromium!, `${servers.url}${PREVIEW_PULL_PAGE}`, { width, height }, cookies, measure)) as { scrollWidth: number; viewportWidth: number; overflowing: string[] };
101 console.log(`pull look ${width}x${height}: ${JSON.stringify(m)}`);
102 return m;
103 };
104 test("phone, 390x844: no sideways scroll", async () => {
105 const m = await look(390, 844);
106 expect(m.overflowing).toEqual([]);
107 expect(m.scrollWidth).toBeLessThanOrEqual(m.viewportWidth);
108 }, 30_000);
109 test("desk, 1280x800: no sideways scroll", async () => {
110 const m = await look(1280, 800);
111 expect(m.scrollWidth).toBeLessThanOrEqual(m.viewportWidth);
112 }, 30_000);
113 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs