Public: anyone can read this; the crew can change it.
The machines list is keyboard-first: j/k move between rows, enter opens one #10370
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/10357-the-machines-list-is-keyboard-first-j-k → main 6 commits fork 3 threads · 3 unresolved
3 files
changed +78
−11
src/web/pages/machines.ts +15 −9
| @@ -61,20 +61,26 @@ export async function machinesSettingsPage(ctx: Ctx): Promise<Response> { | ||
| 61 | 61 | return timePhase(ctx.request, "render", () => render(ctx, repo, config.machines, sessions)); |
| 62 | 62 | } |
| 63 | 63 | |
| 64 | /** One data-nav link per machine, to its shell; none when there are no machines. */ | |
| 65 | export function machineRows(repo: Repo, list: readonly Machine[]) { | |
| 66 | return html`${list.map((m) => | |
| 67 | row({ | |
| 68 | lead: icon("terminal"), | |
| 69 | title: m.name, | |
| 70 | meta: html`Tools: ${m.tools.length === 0 ? "none" : m.tools.join(", ")} · Reach: ${m.reach.length === 0 ? "the forge only" : "the forge and its apps"}`, | |
| 71 | href: shellPath(repo, m.name), | |
| 72 | nav: true, | |
| 73 | data: { machine: m.name }, | |
| 74 | }), | |
| 75 | )}`; | |
| 76 | } | |
| 77 | ||
| 64 | 78 | function render(ctx: Ctx, repo: Repo, list: readonly Machine[], sessions: readonly SessionRecord[]): Response { |
| 65 | 79 | const machines = html`<section class="settings-block"> |
| 66 | 80 | <h2>Machines</h2> |
| 67 | 81 | <p class="muted">A machine is where the crew shell runs; each writer gets a directory and home of their own on it.</p> |
| 68 | 82 | <ul class="list setting-list" id="machines"> |
| 69 | ${list.map((m) => | |
| 70 | row({ | |
| 71 | lead: icon("terminal"), | |
| 72 | title: m.name, | |
| 73 | meta: html`Tools: ${m.tools.length === 0 ? "none" : m.tools.join(", ")} · Reach: ${m.reach.length === 0 ? "the forge only" : "the forge and its apps"}`, | |
| 74 | trail: btn("Open shell", { href: shellPath(repo, m.name), icon: "terminal", size: "sm" }), | |
| 75 | data: { machine: m.name }, | |
| 76 | }), | |
| 77 | )} | |
| 83 | ${machineRows(repo, list)} | |
| 78 | 84 | </ul> |
| 79 | 85 | </section>`; |
| 80 | 86 | // The mark is not colour alone: a running shell carries the word in a badge, |
test/keys-machines-list.test.ts added +62
| @@ -0,0 +1,62 @@ | ||
| 1 | /** The machines list joins the shell's j/k row traversal (#10357): one data-nav per machine, linking to its shell. */ | |
| 2 | import { afterAll, beforeAll, expect, test } from "bun:test"; | |
| 3 | import { mkdtempSync } 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 { createRepo } from "../src/core/repos.ts"; | |
| 8 | import { createSession } from "../src/core/sessions.ts"; | |
| 9 | import { createUser } from "../src/core/users.ts"; | |
| 10 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 11 | import type { Repo } from "../src/core/repos.ts"; | |
| 12 | import { machineRows } from "../src/web/pages/machines.ts"; | |
| 13 | import { quietLogs } from "./helpers/globals.ts"; | |
| 14 | ||
| 15 | quietLogs(); | |
| 16 | ||
| 17 | let app: App; | |
| 18 | let servers: Servers; | |
| 19 | const cookies: Record<string, string> = {}; | |
| 20 | ||
| 21 | async function person(name: string): Promise<string> { | |
| 22 | const made = await createUser(app.db, { username: name, email: `${name}@x.test`, password: "correct horse battery", origin: "seed", trusted: true }); | |
| 23 | if (!made.ok) throw new Error(String(made.error)); | |
| 24 | cookies[name] = `forge_session=${createSession(app.db, made.value.id).token}`; | |
| 25 | return made.value.id; | |
| 26 | } | |
| 27 | ||
| 28 | beforeAll(async () => { | |
| 29 | app = createApp({ dataDir: mkdtempSync(join(tmpdir(), "keys-machines-")), port: 0 }); | |
| 30 | const alice = await person("alice"); | |
| 31 | await person("bob"); | |
| 32 | const made = await createRepo(app.db, app.reposRoot, { ownerId: alice, name: "lab", visibility: "public" }); | |
| 33 | if (!made.ok) throw new Error(String(made.error)); | |
| 34 | servers = await serve(app); | |
| 35 | }); | |
| 36 | ||
| 37 | afterAll(() => { | |
| 38 | servers.stop(); | |
| 39 | app.close(); | |
| 40 | }); | |
| 41 | ||
| 42 | const get = (who: string) => | |
| 43 | fetch(`${servers.url}/alice/lab/settings/machines`, { headers: { cookie: cookies[who]! } }); | |
| 44 | ||
| 45 | test("each machine row is one data-nav link to its own shell, with no nested link", async () => { | |
| 46 | const page = await (await get("alice")).text(); | |
| 47 | const at = page.indexOf('id="machines"'); | |
| 48 | const text = page.slice(at, page.indexOf("</ul>", at)); | |
| 49 | const rows = [...text.matchAll(/<a href="([^"]+)"\s+class="row"[^>]*data-nav tabindex="-1"/g)].map((m) => m[1]); | |
| 50 | expect(rows.length).toBeGreaterThan(0); | |
| 51 | expect(text.match(/data-nav/g)!.length).toBe(rows.length); | |
| 52 | expect(rows).toContain("/alice/lab/machines/standard/shell"); | |
| 53 | // The row is the only link; no second button says the same thing. | |
| 54 | const inner = text.match(/<a href="[^"]+"\s+class="row"[^>]*data-nav[\s\S]*?<\/a>/g)!; | |
| 55 | for (const a of inner) expect(a.match(/<a /g)!.length).toBe(1); | |
| 56 | expect(text).not.toContain("Open shell"); | |
| 57 | }); | |
outdated
This tests the 403 page, not an empty list. Box 2 wants no data-nav on an empty page; assert that against a rendering with zero machines. |
||
| 58 | ||
| 59 | test("a list of no machines draws no data-nav", () => { | |
| 60 | const repo = { ownerName: "alice", name: "lab" } as Repo; | |
| 61 | expect(String(machineRows(repo, []))).not.toContain("data-nav"); | |
| 62 | }); |
test/machine-shell-page.test.ts +1 −2
| @@ -78,12 +78,11 @@ test("a reader is refused the page; a stranger to a private repository and an un | ||
| 78 | 78 | expect((await get("/alice/lab/machines/nope/shell", "alice")).status).toBe(404); |
| 79 | 79 | }); |
| 80 | 80 | |
| 81 | test("the machines list offers Open shell, and its own CSP admits no WebAssembly", async () => { | |
| 81 | test("the machines list links each machine to its shell, and its own CSP admits no WebAssembly", async () => { | |
| 82 | 82 | const res = await get("/alice/lab/settings/machines", "alice"); |
| 83 | 83 | expect(res.status).toBe(200); |
| 84 | 84 | const body = await res.text(); |
| 85 | 85 | expect(body).toContain('href="/alice/lab/machines/standard/shell"'); |
| 86 | expect(body).toContain("Open shell"); | |
| 87 | 86 | expect(res.headers.get("content-security-policy")).not.toContain("wasm-unsafe-eval"); |
| 88 | 87 | }); |
| 89 | 88 |
Principle 1: the row already opens the shell, so this button says the same thing twice and adds a block to the page. Drop it, or justify keeping it.