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

The new repository form at /new speaks the home page's language #9822

Merged opened this

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

Merged

scout/forge:scout/9813-the-new-repository-form-at-new-speaks-th → main 2 commits fork 2 threads · 2 unresolved

  • Merged. scout/9813-the-new-repository-form-at-new-speaks-th was merged into main as e9bfd5c.

Review

Sign in to review this change.
2 files changed +69 −6
src/web/pages/site.ts +3 −6
@@ -761,13 +761,12 @@ function ownersFor(ctx: Ctx, user: User): string[] {
761761 }
762762
763763 function newRepoForm(ctx: Ctx, values: RepoValues, owners: string[], error?: string): Html {
764
765764 return html`<div class="narrow-wide">
766765 ${pageHead("New repository", {
767766 sub: "A repository is a bare git repo plus its issues, pulls and settings.",
768767 })}

Principle 1/8 hold: one sentence, plain error. The error paragraph is not linked to the name input, so a screen reader hears it as an alert but the field is not marked invalid. Add an id and aria-describedby.

769 ${card(
770 html`<form method="post" action="/new">
768 <form method="post" action="/new">

The field blocks below are still indented for the removed card. Dedent them one level.

769 ${error ? html`<p class="error" role="alert">${error}</p>` : null}
771770 ${selectField({
772771 label: "Owner",
773772 name: "owner",
@@ -785,7 +784,6 @@ function newRepoForm(ctx: Ctx, values: RepoValues, owners: string[], error?: str
785784 autofocus: true,
786785 pattern: "[A-Za-z0-9][A-Za-z0-9._-]*",
787786 hint: "Letters, digits, dots, dashes and underscores.",
788 ...(error ? { error } : {}),
789787 })}
790788 ${formField({
791789 label: "Description",
@@ -814,8 +812,7 @@ function newRepoForm(ctx: Ctx, values: RepoValues, owners: string[], error?: str
814812 ${btn("Create repository", { variant: "primary" })}
815813 ${btn("Cancel", { href: "/", variant: "ghost" })}
816814 </div>
817 </form>`,
818 )}
815 </form>
819816 </div>`;
820817 }
821818
test/new-repo-form.test.ts added +66
@@ -0,0 +1,66 @@
1 /** /new reads like the new issue form: one heading, no card, one error line. */
2 import { afterAll, beforeAll, expect, test } from "bun:test";
3 import { mkdtempSync, rmSync } from "node:fs";
4 import { tmpdir } from "node:os";
5 import { join } from "node:path";
6 import { createApp, type App } from "../src/app.ts";
7 import { createUser } from "../src/core/users.ts";
8 import { serve, type Servers } from "../src/http/server.ts";
9 import { proveAll } from "./helpers/verified.ts";
10 import { quietLogs } from "./helpers/globals.ts";
11
12 quietLogs();
13
14 let root = "";
15 let app: App;
16 let servers: Servers;
17 let cookie = "";
18
19 beforeAll(async () => {
20 root = mkdtempSync(join(tmpdir(), "forge-new-repo-form-"));
21 app = createApp({ dataDir: root, port: 0, forgeRepo: "alice/forge" });
22 const u = await createUser(app.db, {
23 origin: "seed",
24 trusted: true,
25 username: "alice",
26 email: "[email protected]",
27 password: "a-good-passphrase",
28 adminIfFirst: true,
29 });
30 if (!u.ok) throw new Error(u.error);
31 proveAll(app);
32 servers = await serve(app);
33 const signin = await fetch(`${servers.url}/signin`, {
34 method: "POST",
35 headers: { origin: servers.url },
36 body: new URLSearchParams({ username: "alice", password: "a-good-passphrase" }),
37 redirect: "manual",
38 });
39 cookie = (signin.headers.get("set-cookie") ?? "").split(";")[0]!;
40 });
41 afterAll(() => {
42 servers.stop();
43 app.close();
44 rmSync(root, { recursive: true, force: true });
45 });
46
47 test("/new has a heading and a name field and no card", async () => {
48 const html = await (await fetch(`${servers.url}/new`, { headers: { cookie } })).text();
49 expect(html).toContain("New repository");
50 expect(html).toContain('name="name"');
51 expect(html).not.toMatch(/class="[^"]*\bcard\b/);
52 });
53
54 test("a refused name shows its error once, escaped", async () => {
55 const res = await fetch(`${servers.url}/new`, {
56 method: "POST",
57 headers: { origin: servers.url, cookie },
58 body: new URLSearchParams({ owner: "alice", name: "<b>bad", description: "<i>x", visibility: "private" }),
59 });
60 expect(res.status).toBe(422);
61 const html = await res.text();
62 expect(html.match(/class="error"/g)?.length).toBe(1);
63 expect(html).not.toContain("<b>bad");
64 expect(html).toContain("&lt;b&gt;bad");
65 expect(html).not.toMatch(/class="[^"]*\bcard\b/);
66 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs