Public: anyone can read this; the crew can change it.
The pulls list fits a 390 px phone: no sideways scroll, long titles and branch names wrap #9354
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/9346-the-pulls-list-fits-a-390-px-phone-no-si → main 3 commits fork 2 threads · 2 unresolved
4 files
changed +106
−2
docs/design/areas/lists.md +7
| @@ -18,3 +18,10 @@ trails a palette row. | ||
| 18 | 18 | The proofs under a forge on `/forges` (#7959) are `row()` `children`, one per public address; the row's trail badge says "reachable" or "not reachable", so the state is a word and not only a colour. No new class. |
| 19 | 19 | |
| 20 | 20 | `/pipeline` (#7703) is a `.list` of `row()`s, one per step, each folding (`fold`) to the parts inside it; no new class. The pull page's path line marks a finished path with a `badge("done")`, so the state is a word. |
| 21 | ||
| 22 | ## The pulls list at 390 px (#9346) | |
| 23 | ||
| 24 | `/:owner/:name/pulls` (`changesList`) is a `listCard` of `row()`s. A pull's title is the writer's words, so inside | |
| 25 | `[data-pulls]` (an attribute, not a class) the row's title and meta wrap and break long words anywhere, in place of the | |
| 26 | one-line ellipsis. No fixed width; the page does not scroll sideways. No new class. The owner's phone look of | |
| 27 | `/preview-admin/sample/pulls` is attached to the pull from its preview. |
src/spawn/preview-looks.ts +4
| @@ -47,6 +47,9 @@ export const PREVIEW_BETS_PAGE = `${PREVIEW_REPO_HOME}/bets`; | ||
| 47 | 47 | /** The waves page, whose long headlines must wrap at a 390 px phone (#9325). */ |
| 48 | 48 | export const PREVIEW_WAVES_PAGE = `${PREVIEW_REPO_HOME}/waves`; |
| 49 | 49 | |
| 50 | /** The pulls list, whose long titles must wrap at a 390 px phone (#9346). */ | |
| 51 | export const PREVIEW_PULLS_PAGE = `${PREVIEW_REPO_HOME}/pulls`; | |
| 52 | ||
| 50 | 53 | /** The Later gate's proposal on the sample (#9212): what the gate holds, grouped by theme. */ |
| 51 | 54 | export const PREVIEW_HELD_PAGE = `${PREVIEW_REPO_HOME}/held`; |
| 52 | 55 | /** |
| @@ -81,6 +84,7 @@ export const PREVIEW_OWNER_LOOKS: readonly OwnerLook[] = [ | ||
| 81 | 84 | PREVIEW_HELD_PAGE, |
| 82 | 85 | PREVIEW_BETS_PAGE, |
| 83 | 86 | PREVIEW_WAVES_PAGE, |
| 87 | PREVIEW_PULLS_PAGE, | |
| 84 | 88 | ].flatMap((page) => (["desktop", "phone"] as const).map((kind) => ({ page, kind }))); |
| 85 | 89 | |
| 86 | 90 | /** |
src/web/studio/lists.ts +11 −2
| @@ -560,6 +560,15 @@ function stepAfter(step: PathStep | null | undefined): { after?: Html } { | ||
| 560 | 560 | return step == null ? {} : { after: stepLink(step) }; |
| 561 | 561 | } |
| 562 | 562 | |
| 563 | /** | |
| 564 | * A pull's title and branch are the writer's words, so inside `[data-pulls]` | |
| 565 | * a row wraps them (a long word breaks anywhere) in place of the list's | |
| 566 | * one-line ellipsis, and the page never scrolls sideways at 390 px. No new class. | |
| 567 | */ | |
| 568 | export const PULLS_CSS = `[data-pulls] { overflow-wrap: anywhere; } | |
| 569 | [data-pulls] .row-title, [data-pulls] .row-meta { white-space: normal; overflow: visible; text-overflow: clip; } | |
| 570 | [data-pulls] .row { align-items: flex-start; }`; | |
| 571 | ||
| 563 | 572 | /** |
| 564 | 573 | * GET /:owner/:name/pulls: every pull request, for every |
| 565 | 574 | * repository and every viewer — one row per pull in its studio word, who |
| @@ -636,7 +645,7 @@ export async function changesList(ctx: Ctx): Promise<Response> { | ||
| 636 | 645 | const listBase = ctx.url.pathname; |
| 637 | 646 | |
| 638 | 647 | const body = html`${repoHeader(ctx, repo, "pulls")} |
| 639 | <div class="mt"> | |
| 648 | <div class="mt" data-pulls> | |
| 640 | 649 | ${wordFilterBar( |
| 641 | 650 | (word) => facetHref(listBase, f, { word }), |
| 642 | 651 | f.word, |
| @@ -659,7 +668,7 @@ export async function changesList(ctx: Ctx): Promise<Response> { | ||
| 659 | 668 | crumbs: crumbsFor(repo), |
| 660 | 669 | palette: repoPalette(ctx, repo), |
| 661 | 670 | body, |
| 662 | styleCss: labelCss([...repoLabels, ...[...labels.values()].flat()]), | |
| 671 | styleCss: `${PULLS_CSS}\n${labelCss([...repoLabels, ...[...labels.values()].flat()])}`, | |
| 663 | 672 | scriptJs: liveClientJs, |
| 664 | 673 | }); |
| 665 | 674 | lap("render"); |
test/pulls-fit.test.ts added +84
| @@ -0,0 +1,84 @@ | ||
| 1 | /** | |
| 2 | * The pulls list fits a 390 px phone (#9346): seeded long content (an unbroken | |
| 3 | * 250-character word, the longest a title may be, plus a long title and many rows) | |
| 4 | * ships no fixed width over 390 px, and the long-text container wraps anywhere. | |
| 5 | */ | |
| 6 | import { afterAll, beforeAll, describe, expect, test } from "bun:test"; | |
| 7 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 8 | import { tmpdir } from "node:os"; | |
| 9 | import { join } from "node:path"; | |
| 10 | import { commitToBranch, createRepo } from "../src/actions/repos.ts"; | |
| 11 | import { openPull } from "../src/actions/pulls.ts"; | |
| 12 | import { createApp, type App } from "../src/app.ts"; | |
| 13 | import { createSession } from "../src/core/sessions.ts"; | |
| 14 | import { createUser } from "../src/core/users.ts"; | |
| 15 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 16 | import { PREVIEW_OWNER_LOOKS, PREVIEW_PULLS_PAGE } from "../src/spawn/preview-looks.ts"; | |
| 17 | import { utilities as base } from "../src/web/style/base.ts"; | |
| 18 | import { components } from "../src/web/style/components.ts"; | |
| 19 | import { content } from "../src/web/style/content.ts"; | |
| 20 | ||
| 21 | const WORD = "P".repeat(250); | |
| 22 | let dir: string; | |
| 23 | let app: App; | |
| 24 | let servers: Servers; | |
| 25 | let page: string; | |
| 26 | ||
| 27 | const must = <T>(r: { ok: true; value: T } | { ok: false; error: unknown }): T => { | |
The branch name is seeded but the row never prints it, so no assertion covers branch wrapping; drop the seed or assert it once the row shows it. |
||
| 28 | if (!r.ok) throw new Error(JSON.stringify(r.error)); | |
| 29 | return r.value; | |
| 30 | }; | |
| 31 | ||
| 32 | beforeAll(async () => { | |
| 33 | dir = await mkdtemp(join(tmpdir(), "forge-pulls-fit-")); | |
| 34 | app = createApp({ dataDir: dir, port: 0 }); | |
| 35 | servers = await serve(app); | |
| 36 | const u = must(await createUser(app.db, { origin: "seed", trusted: true, username: "alice", email: "[email protected]", password: "a-good-passphrase", adminIfFirst: true })); | |
| 37 | const repo = must(await createRepo(app, u, { name: "fit", visibility: "public" })); | |
| 38 | must(await commitToBranch(app, u, repo, { message: "init", files: [{ path: "README.md", content: "x" }] })); | |
| 39 | for (let i = 0; i < 12; i++) { | |
| 40 | const branch = i === 0 ? `topic-${"b".repeat(100)}` : `topic-${i}`; | |
| 41 | must(await commitToBranch(app, u, repo, { branch, message: "c", files: [{ path: `f${i}.txt`, content: "x" }] })); | |
| 42 | const title = i === 0 ? WORD : i === 1 ? "A very long title ".repeat(12).trim() : `Pull number ${i}`; | |
| 43 | must(await openPull(app, u, repo, { title, body: "", baseBranch: "main", headBranch: branch })); | |
| 44 | } | |
| 45 | const cookie = `forge_session=${createSession(app.db, u.id).token}; forge_mode=dev`; | |
| 46 | page = await (await fetch(`${servers.url}/alice/fit/pulls`, { headers: { cookie } })).text(); | |
| 47 | }, 60_000); | |
| 48 | afterAll(async () => { | |
| 49 | servers.stop(); | |
| 50 | app.close(); | |
| 51 | await rm(dir, { recursive: true, force: true }); | |
| 52 | }); | |
| 53 | ||
| 54 | describe("the pulls list at 390 px", () => { | |
| 55 | test("the long content is on the page, in many rows", () => { | |
| 56 | expect(page).toContain(WORD); | |
| 57 | expect(page).toContain("A very long title"); | |
| 58 | expect(page.match(/class="row[ "]/g)?.length ?? 0).toBeGreaterThanOrEqual(12); | |
| 59 | }); | |
| 60 | ||
| 61 | test("no inline width and no shipped rule fixes a width over 390 px", () => { | |
| 62 | expect(page).not.toMatch(/style="[^"]*(?:^|[;\s])(?:min-|max-)?width\s*:/i); | |
| 63 | for (const m of page.matchAll(/\swidth=["']?(\d+)/gi)) expect(Number(m[1])).toBeLessThanOrEqual(390); | |
| 64 | const over: string[] = []; | |
| 65 | for (const css of [base, components, content, ...(page.match(/<style[^>]*>[\s\S]*?<\/style>/g) ?? [])]) { | |
| 66 | for (const m of css.matchAll(/(?<![-\w(])(?:min-)?width\s*:\s*(-?[\d.]+)(px|rem|em|ch)\b/g)) { | |
| 67 | if (Number(m[1]) * (m[2] === "px" ? 1 : m[2] === "ch" ? 8 : 16) > 390) over.push(m[0]); | |
| 68 | } | |
| 69 | } | |
| 70 | expect(over).toEqual([]); | |
| 71 | }); | |
| 72 | ||
| 73 | test("the list's container wraps anywhere, and its rows may wrap", () => { | |
| 74 | expect(page).toContain("data-pulls"); | |
| 75 | const css = (page.match(/<style[^>]*>[\s\S]*?<\/style>/g) ?? []).join("\n"); | |
| 76 | expect(css).toMatch(/\[data-pulls\]\s*\{[^}]*overflow-wrap:\s*(anywhere|break-word)/); | |
| 77 | expect(css).toMatch(/\[data-pulls\] \.row-title[^{]*\{[^}]*white-space:\s*normal/); | |
| 78 | }); | |
| 79 | }); | |
| 80 | ||
| 81 | test("the owner's phone look takes the pulls list", () => { | |
| 82 | expect(PREVIEW_PULLS_PAGE).toBe("/preview-admin/sample/pulls"); | |
| 83 | expect(PREVIEW_OWNER_LOOKS).toContainEqual({ page: PREVIEW_PULLS_PAGE, kind: "phone" }); | |
| 84 | }); |
new above
offline
Garbled sentence: "a 250-char (the longest a title may be) unbroken word". Reword it.