Public: anyone can read this; the crew can change it.
small: j/k, enter, esc and `?` on the lists that still lack them, one checkbox each #10744
Check · Review · Land · Live done — Landed and live in production.
mason/forge:mason/10736-small-j-k-enter-esc-and-on-the-lists-tha → main 3 commits fork 2 threads · 2 unresolved
7 files
changed +265
−11
src/web/pages/audit.ts +2 −2
| @@ -530,7 +530,7 @@ function crossingsSection(crossings: readonly Crossing[]): Html { | ||
| 530 | 530 | } |
| 531 | 531 | |
| 532 | 532 | /** Contract commits, newest first, each with what it did to the cells. */ |
| 533 | function contractChangesSection(changes: readonly ContractChange[]): Html { | |
| 533 | export function contractChangesSection(changes: readonly ContractChange[]): Html { | |
| 534 | 534 | return part( |
| 535 | 535 | html`<ul class="list"> |
| 536 | 536 | ${changes.map((c) => { |
| @@ -559,7 +559,7 @@ function contractChangesSection(changes: readonly ContractChange[]): Html { | ||
| 559 | 559 | ? "success" |
| 560 | 560 | : "default"; |
| 561 | 561 | return html`<li> |
| 562 | <div class="row"> | |
| 562 | <div class="row" data-nav tabindex="-1"> | |
| 563 | 563 | <span class="row-lead">${icon("commit")}</span> |
| 564 | 564 | <span class="row-main"> |
| 565 | 565 | <span class="row-title">${name(c.repo)}: ${c.subject}</span> |
src/web/pages/conflicts.ts +13 −9
| @@ -145,6 +145,18 @@ const CSS = ` | ||
| 145 | 145 | .rs-text { width: 100%; min-height: 3.5em; font-family: monospace; } |
| 146 | 146 | `; |
| 147 | 147 | |
| 148 | /** The other conflicted files as links the shell's j/k walk; nothing when there is one file. */ | |
| 149 | export function fileSwitcher(here: string, files: readonly string[], file: string): Html { | |
| 150 | if (files.length < 2) return html``; | |
| 151 | return html`<p class="small"> | |
| 152 | ${files.map((f) => | |
| 153 | f === file | |
| 154 | ? html`<strong class="mono">${f}</strong> ` | |
| 155 | : html`<a class="mono" data-nav href="${here}?file=${encodeURIComponent(f)}">${f}</a> `, | |
| 156 | )} | |
| 157 | </p>`; | |
| 158 | } | |
| 159 | ||
| 148 | 160 | export async function conflictsPage(ctx: Ctx): Promise<Response> { |
| 149 | 161 | const req = ctx.request; |
| 150 | 162 | notePhase(req, "session", takeSessionTime(req)); |
| @@ -231,15 +243,7 @@ export async function conflictsPage(ctx: Ctx): Promise<Response> { | ||
| 231 | 243 | <strong id="rs-count">${done} of ${total} resolved</strong> |
| 232 | 244 | · nothing is saved until you finish |
| 233 | 245 | </p> |
| 234 | ${files.length > 1 | |
| 235 | ? html`<p class="small"> | |
| 236 | ${files.map((f) => | |
| 237 | f === file | |
| 238 | ? html`<strong class="mono">${f}</strong> ` | |
| 239 | : html`<a class="mono" href="${here}?file=${encodeURIComponent(f)}">${f}</a> `, | |
| 240 | )} | |
| 241 | </p>` | |
| 242 | : ""} | |
| 246 | ${fileSwitcher(here, files, file)} | |
| 243 | 247 | ${crew} |
| 244 | 248 | ${total === 0 |
| 245 | 249 | ? html`<p class="mt">This file has no text to choose between. It may be deleted on |
test/helpers/shell-page.ts added +88
| @@ -0,0 +1,88 @@ | ||
| 1 | /** A page with the shell script (`shellJs`) running over given markup in happy-dom; `store` seeds the tab's sessionStorage. */ | |
| 2 | import { shellJs } from "../../src/web/client.ts"; | |
| 3 | ||
| 4 | export async function shellPage(path: string, body: string, store: Record<string, string> = {}) { | |
| 5 | const { Window } = await import("happy-dom"); | |
| 6 | const win = new Window({ url: `http://forge.test${path}` }); | |
| 7 | for (const [k, v] of Object.entries(store)) win.sessionStorage.setItem(k, v); | |
| 8 | win.document.body.innerHTML = body; | |
| 9 | (win.HTMLElement.prototype as any).scrollIntoView ??= () => {}; | |
| 10 | const assigned: string[] = []; | |
| 11 | const loc = new Proxy(win.location, { | |
| 12 | get: (t, k) => (k === "assign" ? (u: string) => void assigned.push(u) : (t as any)[k]), | |
| 13 | }); | |
| 14 | new Function("document", "window", "location", "sessionStorage", "CSS", "fetch", "EventSource", shellJs)( | |
| 15 | win.document, win, loc, win.sessionStorage, { escape: (x: string) => x.replace(/([^\w-])/g, "\\$1") }, | |
| 16 | async () => ({ ok: false, status: 500, text: async () => "", headers: { get: () => null } }), undefined, | |
| 17 | ); | |
| 18 | const doc = win.document as any; | |
| 19 | const key = (k: string, target: unknown = doc) => { | |
| 20 | const e = new win.KeyboardEvent("keydown", { key: k, bubbles: true, cancelable: true }); | |
| 21 | (target as any).dispatchEvent(e); | |
| 22 | return e; | |
| 23 | }; | |
| 24 | /** The hrefs the Enter key clicks while the click is cancelled. */ | |
| 25 | const clicks: string[] = []; | |
| 26 | doc.addEventListener("click", (e: any) => { clicks.push(e.target.getAttribute("href")); e.preventDefault(); }, true); | |
| 27 | /** The href of the link inside (or being) the focused data-nav row, or null when focus is not on a row. */ | |
| 28 | const focusedRow = () => { | |
| 29 | const row = doc.activeElement?.closest?.("[data-nav]"); | |
| 30 | return row ? (row.matches("a") ? row : row.querySelector("a"))?.getAttribute("href") ?? null : null; | |
| 31 | }; | |
| 32 | return { win, doc, key, assigned, clicks, focusedRow }; | |
| 33 | } | |
| 34 | ||
| 35 | /** What a list owes the shell: j/k walk `hrefs`, Enter opens the focused one, no key fires in a field, and Esc on the opened page returns. */ | |
| 36 | export async function expectListKeys(opts: { path: string; html: string; hrefs: string[]; opened: { path: string; body: string } | null }) { | |
| 37 | const { expect } = await import("bun:test"); | |
| 38 | const p = await shellPage(opts.path, opts.html + "<input id=f>"); | |
| 39 | const rows = [...p.doc.querySelectorAll("[data-nav]")]; | |
| 40 | expect(rows.length).toBe(opts.hrefs.length); | |
| 41 | p.key("j"); | |
| 42 | expect(p.focusedRow()).toBe(opts.hrefs[0]!); | |
| 43 | expect(p.doc.activeElement).toBe(rows[0]); // the row itself carries the focus marker | |
| 44 | p.key("j"); | |
| 45 | expect(p.focusedRow()).toBe(opts.hrefs[1] ?? opts.hrefs[0]!); | |
| 46 | p.key("k"); | |
| 47 | expect(p.focusedRow()).toBe(opts.hrefs[0]!); | |
| 48 | p.key("Enter"); | |
| 49 | expect(p.clicks).toEqual([opts.hrefs[0]!]); | |
| 50 | const stored = JSON.parse(p.win.sessionStorage.getItem("nav-from")!); | |
| 51 | expect(stored).toMatchObject({ list: opts.path, href: opts.hrefs[0]! }); | |
| 52 | ||
| 53 | // In a field no key moves, opens or leaves. | |
| 54 | const field = p.doc.getElementById("f") as any; | |
| 55 | field.focus(); | |
| 56 | const before = p.clicks.length; | |
| 57 | p.key("j", field); p.key("Enter", field); | |
| 58 | expect(p.doc.activeElement).toBe(field); | |
| 59 | expect(p.clicks.length).toBe(before); | |
| 60 | ||
| 61 | // Esc on the page that was opened goes back to this list. | |
| 62 | // `opened: null` is a list whose links lead to a page that is itself a list: nowhere to return to. | |
| 63 | if (!opts.opened) { | |
| 64 | const same = await shellPage(opts.path, opts.html, { "nav-from": JSON.stringify(stored) }); | |
| 65 | same.key("Escape"); | |
| 66 | expect(same.assigned).toEqual([]); | |
| 67 | return; | |
| 68 | } | |
| 69 | const back = await shellPage(opts.opened.path, opts.opened.body, { "nav-from": JSON.stringify(stored) }); | |
| 70 | const e = back.key("Escape"); | |
| 71 | expect(back.assigned).toEqual([opts.path]); | |
| 72 | expect(e.defaultPrevented).toBe(true); | |
| 73 | } | |
| 74 | ||
| 75 | /** The part of a rendered page worth driving: its <main>, scripts removed. */ | |
| 76 | export function mainOf(page: string): string { | |
| 77 | const at = page.indexOf("<main"); | |
| 78 | const end = page.lastIndexOf("</main>"); | |
| 79 | return (at >= 0 && end > at ? page.slice(at, end + 7) : page).replace(/<script[\s\S]*?<\/script>/g, ""); | |
| 80 | } | |
| 81 | ||
| 82 | /** The hrefs of the links the shell would walk, read from parsed markup rather than attribute order. */ | |
| 83 | export async function navHrefs(markup: string): Promise<string[]> { | |
| 84 | const { Window } = await import("happy-dom"); | |
| 85 | const doc = new Window({ url: "http://forge.test/" }).document as any; | |
| 86 | doc.body.innerHTML = markup; | |
| 87 | return [...doc.querySelectorAll("[data-nav]")].map((r: any) => (r.matches("a") ? r : r.querySelector("a"))?.getAttribute("href")); | |
| 88 | } |
test/keys-audit.test.ts added +31
| @@ -0,0 +1,31 @@ | ||
| 1 | /** The audit's contract-change rows join the shell's j/k traversal (#10736): one focusable row each, Enter opening its commit; none when empty. */ | |
| 2 | import { expect, test } from "bun:test"; | |
| 3 | import type { ContractChange } from "../src/contract/changes.ts"; | |
| 4 | import type { Repo } from "../src/core/repos.ts"; | |
| 5 | import { contractChangesSection } from "../src/web/pages/audit.ts"; | |
| 6 | import { expectListKeys, navHrefs, shellPage } from "./helpers/shell-page.ts"; | |
| 7 | ||
| 8 | const change = (n: number): ContractChange => ({ | |
| 9 | repo: { ownerName: "alice", name: "notes" } as Repo, | |
| 10 | oid: String(n).repeat(40), date: "2026-03-05T10:00:00Z", author: "a", subject: `Change ${n}`, | |
| 11 | loosened: [], tightened: [], unsound: false, removed: false, | |
| 12 | }); | |
| 13 | const commit = (n: number) => `/alice/notes/commit/${String(n).repeat(40)}`; | |
| 14 | ||
outdated
Box 5: this only counts |
||
| 15 | test("each contract change is a focusable row; j/k focus it, Enter opens its commit, Esc comes back, fields keep their keys", async () => { | |
| 16 | const markup = String(contractChangesSection([change(1), change(2)])); | |
| 17 | const hrefs = await navHrefs(markup); | |
| 18 | expect(hrefs).toEqual([commit(1), commit(2)]); | |
| 19 | await expectListKeys({ path: "/audit", html: markup, hrefs, opened: { path: commit(1), body: "<main>commit</main>" } }); | |
| 20 | }); | |
| 21 | ||
| 22 | test("a focused row is the row itself, reachable by script only (tabindex -1)", async () => { | |
| 23 | const p = await shellPage("/audit", String(contractChangesSection([change(1)]))); | |
| 24 | p.key("j"); | |
| 25 | expect(p.doc.activeElement.getAttribute("tabindex")).toBe("-1"); | |
| 26 | expect(p.doc.activeElement.hasAttribute("data-nav")).toBe(true); | |
| 27 | }); | |
| 28 | ||
| 29 | test("no changes, no data-nav row", () => { | |
| 30 | expect(String(contractChangesSection([]))).not.toContain("data-nav"); | |
| 31 | }); |
test/keys-conflicts.test.ts added +28
| @@ -0,0 +1,28 @@ | ||
| 1 | /** The resolve page's other-file links join the shell's j/k traversal (#10736): one data-nav per other file, none for one file. */ | |
| 2 | import { expect, test } from "bun:test"; | |
| 3 | import { fileSwitcher } from "../src/web/pages/conflicts.ts"; | |
| 4 | import { expectListKeys, navHrefs, shellPage } from "./helpers/shell-page.ts"; | |
| 5 | ||
| 6 | const here = "/a/b/pulls/1/resolve"; | |
| 7 | ||
| 8 | test("every file but the current one is a data-nav link; j/k focus it and Enter opens it", async () => { | |
| 9 | const markup = String(fileSwitcher(here, ["x.txt", "y z.txt", "w.txt"], "x.txt")); | |
| 10 | const hrefs = await navHrefs(markup); | |
| 11 | expect(hrefs).toEqual([`${here}?file=y%20z.txt`, `${here}?file=w.txt`]); | |
| 12 | expect(markup).toContain("<strong"); | |
| 13 | await expectListKeys({ path: here, html: markup, hrefs, opened: null }); | |
| 14 | }); | |
| 15 | ||
| 16 | test("a single conflicted file draws no data-nav", () => { | |
| 17 | expect(String(fileSwitcher(here, ["x.txt"], "x.txt"))).not.toContain("data-nav"); | |
| 18 | }); | |
| 19 | ||
| 20 | test("Esc on the resolve page, which now has data-nav, neither leaves nor throws; a pull page without rows still does", async () => { | |
| 21 | const stale = JSON.stringify({ list: "/a/b/pulls", to: "/a/b/pulls/1", href: "/a/b/pulls/1" }); | |
| 22 | const resolve = await shellPage(here, String(fileSwitcher(here, ["x", "y"], "x")), { "nav-from": stale }); | |
| 23 | resolve.key("Escape"); | |
| 24 | expect(resolve.assigned).toEqual([]); // it was never the opened page, so there is nowhere to leave to | |
| 25 | const pull = await shellPage("/a/b/pulls/1", "<main>pull</main>", { "nav-from": stale }); | |
| 26 | pull.key("Escape"); | |
| 27 | expect(pull.assigned).toEqual(["/a/b/pulls"]); | |
| 28 | }); |
test/keys-issues-list.test.ts added +50
| @@ -0,0 +1,50 @@ | ||
| 1 | /** The issues list joins the shell's j/k row traversal (#10736): one data-nav per issue, none on the empty page. */ | |
| 2 | import { afterAll, beforeAll, expect, test } from "bun:test"; | |
| 3 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 4 | import { tmpdir } from "node:os"; | |
| 5 | import { join } from "node:path"; | |
| 6 | import { createApp, type App } from "../src/app.ts"; | |
| 7 | import { createRepo } from "../src/actions/repos.ts"; | |
| 8 | import { createIssue } from "../src/core/issues.ts"; | |
| 9 | import { forgeHttp, type ForgeHttp } from "../src/http/server.ts"; | |
| 10 | import { seedAdmin } from "./helpers/admin.ts"; | |
| 11 | import { quietLogs } from "./helpers/globals.ts"; | |
| 12 | import { expectListKeys, mainOf, navHrefs } from "./helpers/shell-page.ts"; | |
| 13 | ||
| 14 | quietLogs(); | |
| 15 | let dataDir = ""; | |
| 16 | let app: App; | |
| 17 | let http: ForgeHttp; | |
| 18 | ||
| 19 | beforeAll(async () => { | |
| 20 | dataDir = await mkdtemp(join(tmpdir(), "forge-keys-issues-")); | |
| 21 | app = createApp({ dataDir, port: 0 }); | |
| 22 | http = forgeHttp(app); | |
| 23 | const admin = await seedAdmin(app, { username: "alice", email: "[email protected]", password: "correct-horse-battery" }); | |
| 24 | if (!admin.ok) throw new Error(admin.error); | |
| 25 | const full = await createRepo(app, admin.value, { name: "full", visibility: "public", description: "" }); | |
| 26 | const bare = await createRepo(app, admin.value, { name: "bare", visibility: "public", description: "" }); | |
| 27 | if (!full.ok || !bare.ok) throw new Error("repo"); | |
| 28 | for (const title of ["First", "Second", "Third"]) { | |
| 29 | const made = createIssue(app.db, { repoId: full.value.id, authorId: admin.value.id, title, kind: "issue" }); | |
| 30 | if (!made.ok) throw new Error(String(made.error)); | |
| 31 | } | |
| 32 | }); | |
| 33 | ||
| 34 | afterAll(async () => { | |
| 35 | app.close(); | |
| 36 | await rm(dataDir, { recursive: true, force: true }); | |
| 37 | }); | |
| 38 | ||
| 39 | const get = async (path: string) => (await http.handle(new Request(`http://forge.test${path}`))).text(); | |
| 40 | ||
| 41 | test("each issue is one data-nav link; j/k focus it, Enter opens it, Esc comes back, fields keep their keys", async () => { | |
| 42 | const markup = mainOf(await get("/alice/full/issues")); | |
| 43 | const hrefs = await navHrefs(markup); | |
| 44 | expect(hrefs).toEqual(["/alice/full/issues/3", "/alice/full/issues/2", "/alice/full/issues/1"]); | |
| 45 | await expectListKeys({ path: "/alice/full/issues", html: markup, hrefs, opened: { path: hrefs[0]!, body: "<main>thread</main>" } }); | |
| 46 | }); | |
| 47 | ||
| 48 | test("the empty list draws no data-nav", async () => { | |
| 49 | expect(await get("/alice/bare/issues")).not.toContain("data-nav"); | |
| 50 | }); |
test/keys-runs-list.test.ts added +53
| @@ -0,0 +1,53 @@ | ||
| 1 | /** The runs list joins the shell's j/k row traversal (#10736): one data-nav per run, none on the empty page. */ | |
| 2 | import { afterAll, beforeAll, expect, test } from "bun:test"; | |
| 3 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 4 | import { tmpdir } from "node:os"; | |
| 5 | import { join } from "node:path"; | |
| 6 | import { createApp, type App } from "../src/app.ts"; | |
| 7 | import { createRepo } from "../src/actions/repos.ts"; | |
| 8 | import { claimRun, createRun, finishRun } from "../src/core/runs.ts"; | |
| 9 | import { forgeHttp, type ForgeHttp } from "../src/http/server.ts"; | |
| 10 | import { seedAdmin } from "./helpers/admin.ts"; | |
| 11 | import { quietLogs } from "./helpers/globals.ts"; | |
| 12 | import { expectListKeys, mainOf, navHrefs } from "./helpers/shell-page.ts"; | |
| 13 | ||
| 14 | quietLogs(); | |
| 15 | let dataDir = ""; | |
| 16 | let app: App; | |
| 17 | let http: ForgeHttp; | |
| 18 | ||
| 19 | beforeAll(async () => { | |
| 20 | dataDir = await mkdtemp(join(tmpdir(), "forge-keys-runs-")); | |
| 21 | app = createApp({ dataDir, port: 0 }); | |
| 22 | http = forgeHttp(app); | |
| 23 | const admin = await seedAdmin(app, { username: "alice", email: "[email protected]", password: "correct-horse-battery" }); | |
| 24 | if (!admin.ok) throw new Error(admin.error); | |
| 25 | const full = await createRepo(app, admin.value, { name: "full", visibility: "public", description: "" }); | |
| 26 | const bare = await createRepo(app, admin.value, { name: "bare", visibility: "public", description: "" }); | |
| 27 | if (!full.ok || !bare.ok) throw new Error("repo"); | |
| 28 | for (let i = 0; i < 3; i++) { | |
| 29 | const id = createRun(app.db, { repoId: full.value.id, kind: "check" }).id; | |
| 30 | claimRun(app.db, id); | |
| 31 | finishRun(app.db, id, { state: "ok", summary: "fine" }); | |
| 32 | } | |
| 33 | }); | |
| 34 | ||
| 35 | afterAll(async () => { | |
| 36 | app.close(); | |
| 37 | await rm(dataDir, { recursive: true, force: true }); | |
| 38 | }); | |
| 39 | ||
| 40 | const get = async (path: string) => (await http.handle(new Request(`http://forge.test${path}`))).text(); | |
| 41 | ||
| 42 | test("each run is one data-nav link; j/k focus it, Enter opens it, Esc comes back, fields keep their keys", async () => { | |
| 43 | const markup = mainOf(await get("/alice/full/runs")); | |
| 44 | const hrefs = await navHrefs(markup); | |
| 45 | expect(hrefs.length).toBe(3); | |
| 46 | expect(new Set(hrefs).size).toBe(3); | |
| 47 | for (const href of hrefs) expect(href).toMatch(/^\/alice\/full\/runs\/\w+/); | |
| 48 | await expectListKeys({ path: "/alice/full/runs", html: markup, hrefs, opened: { path: hrefs[0]!, body: "<main>run</main>" } }); | |
| 49 | }); | |
| 50 | ||
| 51 | test("the empty list draws no data-nav", async () => { | |
| 52 | expect(await get("/alice/bare/runs")).not.toContain("data-nav"); | |
| 53 | }); |
A row with
tabindex=-1must show a visible focus ring. Show it in the screenshot, and confirm enter finds the commit link inside the row.