trevato/forge

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

More
Forks1g fMapg aRuns15g rEventsFlows

The next-up page fits a 390 px phone: no sideways scroll, long names wrap #9366

Merged scoutcrew opened this

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

ConversationFiles
Merged

scout/forge:scout/9324-the-next-up-page-fits-a-390-px-phone-no → main 4 commits fork

  • Merged. scout/9324-the-next-up-page-fits-a-390-px-phone-no was merged into main as 8d484d0.

Review

Sign in to review this change.
4 files changed +122 −1
docs/design/areas/next-up.md +1
@@ -2,3 +2,4 @@
22
33 - `.next-week`: a row dimmed (opacity only, no layout change) while it is dragged below the cut line, so it reads as next week. Added and removed by the page's script during a drag; absent at rest.
44 - Dragging is mouse-only (HTML5 drag does not fire on touch); on a phone a row moves by its Top/Up/Down buttons. A pointer path is a later change.
5 - Fit at 390 px (#9324): no new class. `[data-next]` (an attribute on the page's wrapper) breaks long words anywhere; a row asks for 14rem so its Top/Up/Down/Not now buttons wrap under the text instead of squeezing the title, and the buttons wrap in `.row-after`.
src/spawn/preview-looks.ts +4
@@ -57,6 +57,9 @@ export const PREVIEW_ISSUES_PAGE = `${PREVIEW_REPO_HOME}/issues`;
5757 /** The pulls list, whose long titles must wrap at a 390 px phone (#9346). */
5858 export const PREVIEW_PULLS_PAGE = `${PREVIEW_REPO_HOME}/pulls`;
5959
60 /** The next-up page, whose long titles and action buttons must fit a 390 px phone (#9324). */
61 export const PREVIEW_NEXT_PAGE = `${PREVIEW_REPO_HOME}/next`;
62
6063 /** The commits list, whose long messages must wrap at a 390 px phone (#9348). */
6164 export const PREVIEW_COMMITS_PAGE = `${PREVIEW_REPO_HOME}/commits`;
6265
@@ -96,6 +99,7 @@ export const PREVIEW_OWNER_LOOKS: readonly OwnerLook[] = [
9699 PREVIEW_MILESTONES_PAGE,
97100 PREVIEW_WAVES_PAGE,
98101 PREVIEW_PULLS_PAGE,
102 PREVIEW_NEXT_PAGE,
99103 PREVIEW_COMMITS_PAGE,
100104 PREVIEW_ISSUES_PAGE,
101105 ].flatMap((page) => (["desktop", "phone"] as const).map((kind) => ({ page, kind })));
src/web/pages/next.ts +15 −1
@@ -20,6 +20,17 @@ import { html, type Html } from "../html.ts";
2020 import { flashOrJson, page } from "../layout.ts";
2121 import { badge, btn, card, kbd, row } from "../ui.ts";
2222
23 /**
24 * The owner's issue titles are their own words, so inside `[data-next]` a long
25 * word breaks anywhere, and a row's actions drop under its text (the row asks
26 * for 14rem, not 0) in place of squeezing the title to a sliver at 390 px.
27 * No new class.
28 */
29 export const NEXT_CSS = `[data-next] { overflow-wrap: anywhere; }
30 [data-next] .list > li.has-after > .row { flex: 1 1 14rem; }
31 [data-next] .row-after { display: flex; flex-wrap: wrap; gap: 6px; max-width: 100%; padding: 0 14px 8px; }
32 [data-next] .cut-line { overflow-wrap: anywhere; }`;
33
2334 export const KEYS_JS = `
2435 (() => {
2536 const list = document.getElementById("next-list");
@@ -187,6 +198,7 @@ export function nextPage(ctx: Ctx): Response {
187198 return out;
188199 };
189200 const body = html`${repoHeader(ctx, repo, "issues")}
201 <div data-next>
190202 <div class="page-head mt">
191203 <h1>Next up</h1>
192204 <p class="muted">
@@ -230,13 +242,15 @@ export function nextPage(ctx: Ctx): Response {
230242 { pad: false },
231243 )}
232244 </section>`
233 : null}`;
245 : null}
246 </div>`;
234247 const response = page({
235248 ctx,
236249 title: `Next up · ${repo.name}`,
237250 crumbs: crumbsFor(repo),
238251 palette: repoPalette(ctx, repo),
239252 body,
253 styleCss: NEXT_CSS,
240254 ...(writer ? { scriptJs: KEYS_JS } : {}),
241255 });
242256 notePhase(ctx.request, "render", performance.now() - rendering);
test/next-fit.test.ts added +102
@@ -0,0 +1,102 @@
1 /**
2 * The next-up page fits a 390 px phone (#9324): seeded long content (an
3 * unbroken 300-character word, a long title, many rows) ships no fixed width
4 * over 390 px, the long-text container wraps anywhere, and a row's actions
5 * wrap under its text in place of squeezing it.
6 */
7 import { afterAll, beforeAll, describe, expect, test } from "bun:test";
8 import { mkdtemp, rm } from "node:fs/promises";
9 import { tmpdir } from "node:os";
10 import { join } from "node:path";
11 import { createRepo } from "../src/actions/repos.ts";
12 import { openIssue } from "../src/actions/issues.ts";
13 import { createApp, type App } from "../src/app.ts";
14 import { createSession } from "../src/core/sessions.ts";
15 import { createUser } from "../src/core/users.ts";
16 import { serve, type Servers } from "../src/http/server.ts";
17 import { PREVIEW_NEXT_PAGE, PREVIEW_OWNER_LOOKS } from "../src/spawn/preview-looks.ts";
18 import { utilities as base } from "../src/web/style/base.ts";
19 import { components } from "../src/web/style/components.ts";
20 import { content } from "../src/web/style/content.ts";
21 import { findChromium } from "../src/looks/capture.ts";
22 import { chromiumRuns, evaluateLoaded } from "./helpers/chrome.ts";
23
24 const WORD = "N".repeat(250);
25 let dir: string;
26 let app: App;
27 let servers: Servers;
28 let page: string;
29 let cookie: string;
30
31 const must = <T>(r: { ok: true; value: T } | { ok: false; error: unknown }): T => {
32 if (!r.ok) throw new Error(JSON.stringify(r.error));
33 return r.value;
34 };
35
36 beforeAll(async () => {
37 dir = await mkdtemp(join(tmpdir(), "forge-next-fit-"));
38 app = createApp({ dataDir: dir, port: 0 });
39 servers = await serve(app);
40 const u = must(await createUser(app.db, { origin: "seed", trusted: true, username: "alice", email: "[email protected]", password: "a-good-passphrase", adminIfFirst: true }));
41 const repo = must(await createRepo(app, u, { name: "fit", visibility: "public" }));
42 for (let i = 0; i < 14; i++) {
43 const title = i === 0 ? WORD : i === 1 ? "A very long title ".repeat(12).trim() : `Work item ${i}`;
44 must(openIssue(app, u, repo, { title, body: "" }));
45 }
46 cookie = `forge_session=${createSession(app.db, u.id).token}`;
47 page = await (await fetch(`${servers.url}/alice/fit/next?all=1`, { headers: { cookie } })).text();
48 }, 60_000);
49 afterAll(async () => {
50 servers.stop();
51 app.close();
52 await rm(dir, { recursive: true, force: true });
53 });
54
55 describe("the next-up page at 390 px", () => {
56 test("the long content is on the page, in many rows, each with actions", () => {
57 expect(page).toContain(WORD);
58 expect(page).toContain("A very long title");
59 expect(page.match(/<li[^>]*data-number=/g)?.length ?? 0).toBeGreaterThanOrEqual(12);
60 expect(page).toContain("row-after");
61 });
62
63 test("no inline width and no shipped rule fixes a width over 390 px", () => {
64 expect(page).not.toMatch(/style="[^"]*(?:^|[;\s])(?:min-|max-)?width\s*:/i);
65 for (const m of page.matchAll(/\swidth=["']?(\d+)/gi)) expect(Number(m[1])).toBeLessThanOrEqual(390);
66 const over: string[] = [];
67 for (const css of [base, components, content, ...(page.match(/<style[^>]*>[\s\S]*?<\/style>/g) ?? [])]) {
68 for (const m of css.matchAll(/(?<![-\w(])(?:min-)?width\s*:\s*(-?[\d.]+)(px|rem|em|ch)\b/g)) {
69 if (Number(m[1]) * (m[2] === "px" ? 1 : m[2] === "ch" ? 8 : 16) > 390) over.push(m[0]);
70 }
71 }
72 expect(over).toEqual([]);
73 });
74
75 test("the container wraps anywhere, and a row's actions wrap under its text", () => {
76 expect(page).toContain("data-next");
77 const css = (page.match(/<style[^>]*>[\s\S]*?<\/style>/g) ?? []).join("\n");
78 expect(css).toMatch(/\[data-next\]\s*\{[^}]*overflow-wrap:\s*(anywhere|break-word)/);
79 expect(css).toMatch(/\[data-next\] \.list > li\.has-after > \.row\s*\{[^}]*flex:\s*1 1 14rem/);
80 expect(css).toMatch(/\[data-next\] \.row-after\s*\{[^}]*flex-wrap:\s*wrap/);
81 });
82 });
83
84 test("the owner's phone look takes the next-up page", () => {
85 expect(PREVIEW_NEXT_PAGE).toBe("/preview-admin/sample/next");
86 expect(PREVIEW_OWNER_LOOKS).toContainEqual({ page: PREVIEW_NEXT_PAGE, kind: "phone" });
87 });
88
89 const found = findChromium();
90 const chromium = found !== undefined && chromiumRuns(found) ? found : undefined;
91 if (chromium === undefined) console.log("SKIPPING: no chromium that starts here, so the next-up phone look is not measured");
92
93 describe.skipIf(chromium === undefined)("measured at 390x844", () => {
94 test("no sideways scroll, and no row taller than the screen", async () => {
95 const measure = `(() => JSON.stringify({ sw: document.documentElement.scrollWidth, vw: document.documentElement.clientWidth,
96 tall: [...document.querySelectorAll("#next-list li[data-number]")].filter((li) => li.getBoundingClientRect().height > 844).length }))()`;
97 const m = JSON.parse(await evaluateLoaded(chromium!, `${servers.url}/alice/fit/next?all=1`, { width: 390, height: 844 }, [{ name: "forge_session", value: cookie.split("=")[1]! }], measure));
98 console.log(`next look 390x844: ${JSON.stringify(m)}`);
99 expect(m.sw).toBeLessThanOrEqual(m.vw);
100 expect(m.tall).toBe(0);
101 }, 30_000);
102 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs