trevato/forge

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

More
Forks1g fMapg aRuns1g rEventsFlows

The new repository form at /new fits a 390 px phone: no sideways scroll #9833

Merged scoutcrew opened this

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

ConversationFiles
Merged

scout/forge:scout/9830-the-new-repository-form-at-new-fits-a-39 → main 2 commits fork

  • Merged. scout/9830-the-new-repository-form-at-new-fits-a-39 was merged into main as 44a94ae.

Review

Sign in to review this change.
4 files changed +107 −2
docs/design/areas/forms.md +2
@@ -8,3 +8,5 @@ A form's fields and its buttons sit in two rows, one for what you fill in and on
88 - The gate break-glass (#7790) on the pull's Files step is a plain `field()` with a hint and a `.form-actions` row holding one `btn` in the `danger` variant; no new class. It sits under the disabled primary Merge button, so the page keeps one accent.
99
1010 - `/signin` and `/signup` (#9812) speak the home page's language and add no class: `authForm` in `pages/site.ts` draws the page's one `<h1>` (`pageHead`), then the passkey door, the form and its `.form-actions` row straight on the page, no `.card`. `test/auth-language.test.ts` holds the shape.
11
12 - `/new` (#9830) fits a 390 px phone with no new class: the page's wrapper carries the `data-new-repo` hook (not a class), which `style/content.ts` gives `overflow-wrap: anywhere` like the other long-text pages. `test/new-repo-fit.test.ts` holds it.
src/web/pages/site.ts +1 −1
@@ -761,7 +761,7 @@ function ownersFor(ctx: Ctx, user: User): string[] {
761761 }
762762
763763 function newRepoForm(ctx: Ctx, values: RepoValues, owners: string[], error?: string): Html {
764 return html`<div class="narrow-wide">
764 return html`<div class="narrow-wide" data-new-repo>
765765 ${pageHead("New repository", {
766766 sub: "A repository is a bare git repo plus its issues, pulls and settings.",
767767 })}
src/web/style/content.ts +4 −1
@@ -161,7 +161,10 @@ a.origin-chip:hover { border-color: var(--border-strong); color: var(--fg); }
161161 [data-tell] .page-head-sub { flex-wrap: wrap; }
162162 [data-tell] .comment-body img { max-width: 100%; height: auto; }
163163 [data-tell] pre { max-width: 100%; overflow-x: auto; }
164 [data-apps], [data-apps-page], [data-bet-card], [data-explore], [data-family], [data-forges], [data-milestone], [data-now], [data-people] { overflow-wrap: anywhere; }
164 [data-apps], [data-apps-page], [data-bet-card], [data-explore], [data-family], [data-forges], [data-milestone], [data-new-repo], [data-now], [data-people] { overflow-wrap: anywhere; }
165 /* /new (#9830): a long owner name or refused value stays inside the form on a phone. */
166 [data-new-repo] form, [data-new-repo] .error { min-width: 0; max-width: 100%; }
167 [data-new-repo] select.input { text-overflow: ellipsis; }
165168 /* /explore (#9370): rows wrap rather than truncate a long name or description. */
166169 [data-explore] .row-title, [data-explore] .row-meta { white-space: normal; overflow: visible; }
167170 [data-explore] .row { flex-wrap: wrap; }
test/new-repo-fit.test.ts added +100
@@ -0,0 +1,100 @@
1 /**
2 * The new repository form at /new fits a 390 px phone (#9830): seeded with a
3 * long owner and a long refused name, it fixes no width over 390 px, wraps
4 * anywhere, and (where a Chromium starts) does not scroll sideways.
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 { createApp, type App } from "../src/app.ts";
11 import { createOrg } from "../src/core/orgs.ts";
12 import { createUser, getUserByName } from "../src/core/users.ts";
13 import { serve, type Servers } from "../src/http/server.ts";
14 import { utilities as base } from "../src/web/style/base.ts";
15 import { components } from "../src/web/style/components.ts";
16 import { content } from "../src/web/style/content.ts";
17 import { findChromium } from "../src/looks/capture.ts";
18 import { chromiumRuns, evaluateLoaded } from "./helpers/chrome.ts";
19
20 const LONG = "W".repeat(300);
21 let dir: string;
22 let app: App;
23 let servers: Servers;
24 let page: string;
25 let refused: string;
26 let ownerSeeded = false;
27 let session: { name: string; value: string };
28
29 beforeAll(async () => {
30 dir = await mkdtemp(join(tmpdir(), "forge-new-fit-"));
31 app = createApp({ dataDir: dir, port: 0 });
32 servers = await serve(app);
33 const made = await createUser(app.db, { origin: "seed", trusted: true, username: "alice", email: "[email protected]", password: "a-good-passphrase", adminIfFirst: true });
34 if (!made.ok) throw new Error(made.error);
35 const alice = getUserByName(app.db, "alice")!;
36 // An org name this long may be refused at the boundary; then the longest allowed stands in.
37 for (const name of [LONG, "o".repeat(100), "o".repeat(39)]) {
38 if (createOrg(app.db, name, alice.id).ok) { ownerSeeded = true; break; }
39 }
40 const cookie = ((await fetch(`${servers.url}/signin`, { method: "POST", body: new URLSearchParams({ username: "alice", password: "a-good-passphrase" }), redirect: "manual" })).headers.get("set-cookie") ?? "").split(";")[0]!;
41 session = { name: cookie.slice(0, cookie.indexOf("=")), value: cookie.slice(cookie.indexOf("=") + 1) };
42 page = await (await fetch(`${servers.url}/new`, { headers: { cookie } })).text();
43 refused = await (await fetch(`${servers.url}/new`, { method: "POST", headers: { cookie, origin: servers.url }, body: new URLSearchParams({ owner: "alice", name: `!${LONG}`, description: LONG, visibility: "private" }) })).text();
44 }, 60_000);
45 afterAll(async () => {
46 servers.stop();
47 app.close();
48 await rm(dir, { recursive: true, force: true });
49 });
50
51 describe("the /new page at 390 px", () => {
52 test("renders the form with a long owner and a long refused value", () => {
53 expect(ownerSeeded).toBe(true);
54 expect(page).toContain("data-new-repo");
55 expect(page).toContain('name="owner"');
56 expect(page).toMatch(/<option[^>]*>\s*o{39,}/);
57 expect(refused).toContain(`!${LONG}`);
58 expect(refused).toContain("data-new-repo");
59 });
60
61 test("no inline width, and no rule the page ships fixes a width over 390 px", () => {
62 for (const doc of [page, refused]) {
63 expect(doc).not.toMatch(/style="[^"]*(?:^|[;\s"])(?:min-)?width\s*:/i);
64 for (const m of doc.matchAll(/\swidth=["']?(\d+)/gi)) expect(Number(m[1])).toBeLessThanOrEqual(390);
65 }
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 const px = Number(m[1]) * (m[2] === "px" ? 1 : m[2] === "ch" ? 8 : 16);
70 if (px > 390) over.push(m[0]);
71 }
72 }
73 expect(over).toEqual([]);
74 });
75
76 test("the page's container wraps anywhere", () => {
77 const rule = content.match(/(?:^|\n)([^\n{]*\[data-new-repo\][^\n{]*)\{([^}]*)\}/);
78 expect(rule).not.toBeNull();
79 expect(rule![2]).toMatch(/overflow-wrap:\s*(?:anywhere|break-word)/);
80 });
81 });
82
83 const found = findChromium();
84 const chromium = found !== undefined && chromiumRuns(found) ? found : undefined;
85 if (chromium === undefined) console.log("SKIPPING: no chromium that starts on this machine, so the /new phone look is not measured");
86
87 describe.skipIf(chromium === undefined)("the /new page, measured at 390x844", () => {
88 test("no sideways scroll", async () => {
89 const measure = `(() => {
90 const vw = document.documentElement.clientWidth;
91 const scope = [...document.querySelectorAll("[data-new-repo], [data-new-repo] *")];
92 const overflowing = scope.filter((el) => el.getBoundingClientRect().right > vw + 1).slice(0, 8).map((el) => el.tagName.toLowerCase());
93 return JSON.stringify({ scrollWidth: document.documentElement.scrollWidth, viewportWidth: vw, wrapped: scope.length > 0, overflowing });
94 })()`;
95 const m = JSON.parse(await evaluateLoaded(chromium!, `${servers.url}/new`, { width: 390, height: 844 }, [session], measure)) as { scrollWidth: number; viewportWidth: number; wrapped: boolean; overflowing: string[] };
96 expect(m.wrapped).toBe(true);
97 expect(m.overflowing).toEqual([]);
98 expect(m.scrollWidth).toBeLessThanOrEqual(m.viewportWidth);
99 }, 30_000);
100 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs