Public: anyone can read this; the crew can change it.
On /next the owner moves a row by keyboard: select, up or down, t to top, l to Later #12305
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/11755-on-next-the-owner-moves-a-row-by-keyboar → main 8 commits fork 5 threads · 5 unresolved
7 files
changed +136
−13
docs/features/next-up.md +3 −3
| @@ -3,7 +3,7 @@ | ||
| 3 | 3 | `/:owner/:name/next` is the one ranked list the crew builds from (#8694). It reads as one numbered queue in the dispatcher's order (#11619): each row has its position, title, who is on it (or "next"), and one word of state: building, checking, in review, landing, waiting on #N, held or ready. A pull that is open on the issue sets building, checking or in review. Three native folds hold the rows, each with a count: "Up next" (the first ten, open), "After" (the rest) and "Later" (behind the Later gate). There is no script for the folds and no `?all=1`; the folds are keyboard and screen-reader reachable at any width. The terminal's Next up screen shows the same numbers and state words. |
| 4 | 4 | |
| 5 | 5 | - **Role:** each row is tagged *Blocker* (it holds other open work up: "finishing it frees N") or *Enabler* (it holds nothing up; its line says what it serves, else that the owner ranked it). The tag is a label only: it never changes the order the picker takes work in. |
| 6 | - **Move:** `Top` (`t`), `Up` (`u`), `Down` (`d`) and `Not now` (`n`) are forms on each row, with the key shown beside each; `j`/`k` choose the row, and a row can be dragged. Each is one `POST /:owner/:name/next` (`number`, `to` = `top`, `not-now` or `after:<number>`) and saves at once. Writers only; the same-origin check applies. | |
| 6 | - **Move:** `Top` (`t`), `Up` (`u`), `Down` (`d`) and `Not now` (`l`) are forms on each row, with the key shown beside each (Shift+J/K or Alt+↓/↑ also move down and up; Alt+Shift combos do nothing); `j`/`k` choose the row, and a row can be dragged. Each is one `POST /:owner/:name/next` (`number`, `to` = `top`, `not-now` or `after:<number>`) and saves at once. Writers only; the same-origin check applies. | |
| 7 | 7 | - **Top lifts the gate:** `Top` on a deferred issue removes its `wait_for` edge to the Later gate (and the Later umbrella's edge to it) in the same action. Work that only deferred work needs stays deferred, and the refusal says so. |
| 8 | 8 | - **Owner's pick:** a row a person placed (page, terminal or MCP) is marked "Owner's pick" on the page and in the terminal. Automatic ranking leaves it at its place and orders only the other rows around it; only a person's move changes it, and `Not now` clears it along with the rank. A move of another row does not mark the rows the planner placed. Nothing calls the automatic mode yet (`writeOrder` with `owner: false`): the guarantee is in place and tested, ahead of its first caller. |
| 9 | 9 | - **Not now** links the issue to wait for the open Later gate and drops its rank. |
| @@ -11,9 +11,9 @@ | ||
| 11 | 11 | - **MCP:** `next_up { app, issue?, to?, all? }` reads the list, or moves one issue with the same words. |
| 12 | 12 | - A rank is cleared when the issue closes. Dependencies order within the list and are shown, never edited here. |
| 13 | 13 | |
| 14 | **Terminal:** the palette's "Next up in a repository" opens the same list; `t`, `u`, `d` and `n` are Top, Up, Down and Not now on the row under the cursor, through the same action as the page and MCP. A reader sees the order and the keys do nothing; a move made anywhere reorders an open screen live. At most 200 issues are ranked; a move past that is refused and says so. | |
| 14 | **Terminal:** the palette's "Next up in a repository" opens the same list; `t`, `u`, `d` and `l` are Top, Up, Down and Not now on the row under the cursor, through the same action as the page and MCP. A reader sees the order and the keys do nothing; a move made anywhere reorders an open screen live. At most 200 issues are ranked; a move past that is refused and says so. | |
| 15 | 15 | |
| 16 | **Filter (#11867):** a plain GET form above the list narrows `/next` to one pillar (`?pillar=P7` or `Later`, typed in a box) and/or a word (`?q=cache`, matched without regard to case, shown as typed, at most 80 characters). A filter only hides rows: each keeps its real place in the whole ranking, the cut line stays where the whole list puts it (drawn only between shown rows), and a move made under a filter moves in the whole order and returns to the same filter. A line says "N of M shown", and that the week's capacity bar is left out while filtered. An unknown or malformed pillar shows an empty list that says so. **Terminal:** `/` asks for a pillar and/or a word (`P7 cache`); an empty answer clears it; ranks, `t` `u` `d` `n` and the header count behave as on the page. | |
| 16 | **Filter (#11867):** a plain GET form above the list narrows `/next` to one pillar (`?pillar=P7` or `Later`, typed in a box) and/or a word (`?q=cache`, matched without regard to case, shown as typed, at most 80 characters). A filter only hides rows: each keeps its real place in the whole ranking, the cut line stays where the whole list puts it (drawn only between shown rows), and a move made under a filter moves in the whole order and returns to the same filter. A line says "N of M shown", and that the week's capacity bar is left out while filtered. An unknown or malformed pillar shows an empty list that says so. **Terminal:** `/` asks for a pillar and/or a word (`P7 cache`); an empty answer clears it; ranks, `t` `u` `d` `l` and the header count behave as on the page. | |
| 17 | 17 | |
| 18 | 18 | ## The week is the sprint (#8704) |
| 19 | 19 |
src/tui/screens/next.ts +2 −2
| @@ -158,7 +158,7 @@ export function nextScreen(f: Session, ctx: Ctx, repo: Repo): Screen { | ||
| 158 | 158 | binding(["t"], "top", { disabled: !on || rows.indexOf(current()!) === 0 }), |
| 159 | 159 | binding(["u"], "up", { disabled: !on || rows.indexOf(current()!) === 0 }), |
| 160 | 160 | binding(["d"], "down", { disabled: !on || rows.indexOf(current()!) >= rows.length - 1 }), |
| 161 | binding(["n"], "not now", { disabled: !on }), | |
| 161 | binding(["l"], "not now", { disabled: !on }), | |
| 162 | 162 | binding(["/"], "filter"), |
| 163 | 163 | ]; |
| 164 | 164 | }, |
| @@ -187,7 +187,7 @@ export function nextScreen(f: Session, ctx: Ctx, repo: Repo): Screen { | ||
| 187 | 187 | else if (e.key === "u") step(-1); |
| 188 | 188 | else if (e.key === "d") step(1); |
| 189 | 189 | else if (e.key === "/") ask(); |
| 190 | else if (e.key === "n") rank({ to: "not_now" }); | |
| 190 | else if (e.key === "l") rank({ to: "not_now" }); | |
| 191 | 191 | else return false; |
| 192 | 192 | return true; |
| 193 | 193 | }, |
src/web/pages/next.ts +8 −4
| @@ -6,6 +6,7 @@ | ||
| 6 | 6 | * phone need no script; a row can also be dragged, or moved by the key shown |
| 7 | 7 | * beside each action (`j`/`k` choose the row), which post the same form. |
| 8 | 8 | */ |
| 9 | import { keyStep } from "./next_keys.ts"; | |
| 9 | 10 | import { placeOf, rankIssue, rankedWords, stepTo, type Place } from "../../actions/next.ts"; |
| 10 | 11 | import { currentByNumber } from "../../core/estimates.ts"; |
| 11 | 12 | import { estimateLine } from "../../flow/estimate_words.ts"; |
| @@ -48,6 +49,7 @@ export const NEXT_CSS = `[data-next] { overflow-wrap: anywhere; } | ||
| 48 | 49 | |
| 49 | 50 | export const KEYS_JS = ` |
| 50 | 51 | (() => { |
| 52 | ${keyStep.toString()} | |
| 51 | 53 | const list = document.getElementById("next-list"); |
| 52 | 54 | const remembered = sessionStorage.getItem("next-focus"); |
| 53 | 55 | if (remembered) { |
| @@ -71,9 +73,11 @@ export const KEYS_JS = ` | ||
| 71 | 73 | }; |
| 72 | 74 | // One key, one action, on the row that has focus: the key is shown on the button. |
| 73 | 75 | document.addEventListener("keydown", (e) => { |
| 74 | if (e.metaKey || e.ctrlKey || e.altKey || e.target.closest("input, textarea, select, [contenteditable]")) return; | |
| 76 | if (e.metaKey || e.ctrlKey || e.target.closest("input, textarea, select, [contenteditable]")) return; | |
| 77 | const step = keyStep(e); | |
| 78 | if (!step) return; | |
| 75 | 79 | const row = document.activeElement && document.activeElement.closest("li[data-number]"); |
| 76 | const form = row && row.querySelector('form[data-key="' + e.key + '"]'); | |
| 80 | const form = row && row.querySelector('form[data-key="' + step + '"]'); | |
| 77 | 81 | if (!form) return; |
| 78 | 82 | e.preventDefault(); |
| 79 | 83 | send(new FormData(form), row.dataset.number); |
| @@ -171,7 +175,7 @@ function line( | ||
| 171 | 175 | ? null |
| 172 | 176 | : html`${up === undefined ? null : move(base, n, up, "Up", "u", back)}${down === undefined |
| 173 | 177 | ? null |
outdated
Key becomes |
||
| 174 | : move(base, n, down, "Down", "d", back)}${move(base, n, "not-now", "Not now", "n", back)}`}` | |
| 178 | : move(base, n, down, "Down", "d", back)}${move(base, n, "not-now", "Not now", "l", back)}`}` | |
| 175 | 179 | : null; |
| 176 | 180 | return row({ |
| 177 | 181 | href: `${base}/issues/${n}`, |
| @@ -253,7 +257,7 @@ export function nextPage(ctx: Ctx): Response { | ||
| 253 | 257 | : html`${bar}${isFiltered ? html`<p class="muted" id="next-count">${countWords(ats.length, rows.length)} shown${ats.length + parked.length === 0 ? html`. ${nothingWords}` : null}. Numbers and the cut line are the whole list's; the week's capacity bar is left out while filtered.</p>` : null} |
| 254 | 258 | ${first.length > 0 ? section(isFiltered ? "Matching" : "Up next", first, true, "next-list") : null}${rest.length > 0 ? section("After", rest, false) : null}`} |
| 255 | 259 | <p class="muted small mt" id="next-week">${sentence} <a href="${base}/week">The week</a>${writer |
outdated
Principle 1: the help line is now one long parenthetical sentence that restates what the button labels already show. Shorten it to the keys only, e.g. |
||
| 256 | ? " · Drag a row, or press the key on its button; j and k move between rows." | |
| 260 | ? " · Drag a row. Keys: t top · l Later · Shift+J/K or Alt+↓/↑ down/up · j/k choose a row." | |
| 257 | 261 | : null}</p> |
| 258 | 262 | ${parked.length > 0 |
| 259 | 263 | ? html`<details class="mt"><summary><h2>Later · ${String(isFiltered ? parked.length : next.notNowTotal)}</h2></summary><ul class="list setting-list"> |
src/web/pages/next_keys.ts added +12
| @@ -0,0 +1,12 @@ | ||
| 1 | /** The /next key mapping as one pure function: a key event in, the form key (data-key) it | |
| 2 | * presses out. */ | |
| 3 | ||
| 4 | /** Plain keys press their own form; Shift+J/K and Alt+arrows are Down and Up; Alt+Shift combos do | |
| 5 | * nothing. Self-contained and untyped: it is shipped to the browser as its own source. */ | |
| 6 | export function keyStep(e: { key: string; shiftKey: boolean; altKey: boolean }): string | undefined { | |
| 7 | const table: Record<string, Record<string, string>> = { shift: { J: "d", K: "u" }, alt: { ArrowUp: "u", ArrowDown: "d" } }; | |
| 8 | if (e.altKey && e.shiftKey) return undefined; | |
| 9 | if (e.altKey) return table.alt![e.key]; | |
Plain-object lookup returns inherited properties for keys like "constructor"/"toString"; harmless here (no form matches) but use Object.hasOwn or a Map so the function only ever returns u, d or undefined. |
||
| 10 | if (e.shiftKey) return table.shift![e.key]; | |
| 11 | return e.key; | |
| 12 | } |
test/next-keys.test.ts added +107
| @@ -0,0 +1,107 @@ | ||
| 1 | /** /next keys (#11755): each key is a `data-key` form on the row; the key posts that same form. */ | |
| 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 { openIssue } from "../src/actions/issues.ts"; | |
| 8 | import { createRepo } from "../src/actions/repos.ts"; | |
| 9 | import { nextUp } from "../src/core/next_up.ts"; | |
| 10 | import type { Repo } from "../src/core/repos.ts"; | |
| 11 | import { createSession } from "../src/core/sessions.ts"; | |
| 12 | import { createUser } from "../src/core/users.ts"; | |
| 13 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 14 | import { keyStep } from "../src/web/pages/next_keys.ts"; | |
| 15 | import { KEYS_JS } from "../src/web/pages/next.ts"; | |
| 16 | ||
| 17 | let dir: string; | |
| 18 | let app: App; | |
| 19 | let servers: Servers; | |
| 20 | let cookie: string; | |
| 21 | let repo: Repo; | |
| 22 | let numbers: number[]; | |
| 23 | ||
| 24 | beforeAll(async () => { | |
| 25 | dir = await mkdtemp(join(tmpdir(), "forge-next-keys-")); | |
| 26 | app = createApp({ dataDir: dir, port: 0 }); | |
| 27 | servers = await serve(app); | |
| 28 | const made = await createUser(app.db, { origin: "seed", trusted: true, username: "alice", email: "[email protected]", password: "a-good-passphrase", adminIfFirst: true }); | |
| 29 | if (!made.ok) throw new Error(made.error); | |
| 30 | cookie = `forge_session=${createSession(app.db, made.value.id).token}`; | |
| 31 | const r = await createRepo(app, made.value, { name: "keys", visibility: "public" }); | |
| 32 | if (!r.ok) throw new Error(JSON.stringify(r.error)); | |
| 33 | repo = r.value; | |
| 34 | const gate = openIssue(app, made.value, repo, { title: "Later gate: not now" }); | |
| 35 | if (!gate.ok) throw new Error(String(gate.error)); | |
| 36 | for (const t of ["A", "B", "C", "D"]) { | |
| 37 | const o = openIssue(app, made.value, repo, { title: `Row ${t}` }); | |
| 38 | if (!o.ok) throw new Error(String(o.error)); | |
| 39 | } | |
| 40 | numbers = order(); | |
| 41 | }); | |
| 42 | ||
| 43 | afterAll(async () => { | |
| 44 | await servers.stop(); | |
| 45 | app.close(); | |
| 46 | await rm(dir, { recursive: true, force: true }); | |
| 47 | }); | |
| 48 | ||
| 49 | const order = () => nextUp(app.db, repo).rows.map((r) => r.issue.number); | |
| 50 | const page = async () => (await fetch(`${servers.url}/alice/keys/next`, { headers: { cookie } })).text(); | |
| 51 | ||
| 52 | /** The fields of the row's form for a key, as the browser would submit them. */ | |
| 53 | function formFor(html: string, number: number, key: string): URLSearchParams { | |
| 54 | const li = new RegExp(`<li[^>]*data-number="${number}"[\\s\\S]*?</li>`).exec(html)![0]; | |
| 55 | const form = new RegExp(`<form[^>]*data-key="${key}"[\\s\\S]*?</form>`).exec(li)?.[0]; | |
| 56 | expect(form).toBeDefined(); | |
| 57 | const body = new URLSearchParams(); | |
| 58 | for (const m of form!.matchAll(/<input type="hidden" name="(\w+)" value="([^"]*)"/g)) body.set(m[1]!, m[2]!.replaceAll("&", "&")); | |
| 59 | return body; | |
| 60 | } | |
| 61 | ||
| 62 | const press = async (number: number, key: string) => { | |
| 63 | const body = formFor(await page(), number, key); | |
| 64 | const res = await fetch(`${servers.url}/alice/keys/next`, { method: "POST", headers: { cookie, origin: servers.url, accept: "application/json" }, body }); | |
| 65 | expect(res.ok).toBe(true); | |
| 66 | }; | |
| 67 | ||
| 68 | test("the page carries the key hooks and lists them in its help", async () => { | |
| 69 | const html = await page(); | |
| 70 | const row = new RegExp(`<li[^>]*data-number="${numbers[1]}"[\\s\\S]*?</li>`).exec(html)![0]; | |
| 71 | for (const k of ["t", "u", "d", "l"]) expect(row).toContain(`data-key="${k}"`); | |
| 72 | expect(html).toContain("t top · l Later"); | |
| 73 | expect(html).toContain("Shift+J"); | |
| 74 | expect(html).toContain("Alt+"); | |
| 75 | expect(KEYS_JS).toContain("next-focus"); | |
| 76 | }); | |
| 77 | ||
outdated
These assertions check source substrings, not behaviour; they would pass with a broken mapping. Test the mapping function directly. |
||
| 78 | test("each key submits the existing form and the order follows", async () => { | |
| 79 | const [a, b, c, d] = numbers as [number, number, number, number]; | |
| 80 | await press(c, "t"); // t: to the top | |
| 81 | expect(order()).toEqual([c, a, b, d]); | |
| 82 | await press(c, "d"); // Shift+J / Alt+Down: one place down | |
| 83 | expect(order()).toEqual([a, c, b, d]); | |
| 84 | await press(d, "u"); // Shift+K / Alt+Up: one place up | |
| 85 | expect(order()).toEqual([a, c, d, b]); | |
| 86 | await press(a, "l"); // l: behind the Later gate | |
| 87 | expect(order()).toEqual([c, d, b]); | |
| 88 | }); | |
| 89 | ||
| 90 | const ev = (key: string, mods: { shiftKey?: boolean; altKey?: boolean } = {}) => ({ key, shiftKey: false, altKey: false, ...mods }); | |
| 91 | ||
| 92 | test("keyStep maps modifiers to the form keys", () => { | |
| 93 | expect(keyStep(ev("t"))).toBe("t"); | |
| 94 | expect(keyStep(ev("l"))).toBe("l"); | |
| 95 | expect(keyStep(ev("J", { shiftKey: true }))).toBe("d"); | |
| 96 | expect(keyStep(ev("K", { shiftKey: true }))).toBe("u"); | |
| 97 | expect(keyStep(ev("ArrowDown", { altKey: true }))).toBe("d"); | |
| 98 | expect(keyStep(ev("ArrowUp", { altKey: true }))).toBe("u"); | |
| 99 | expect(keyStep(ev("ArrowUp"))).toBe("ArrowUp"); // plain arrows have no form: no row lookup matches | |
| 100 | expect(keyStep(ev("J", { altKey: true }))).toBeUndefined(); | |
| 101 | expect(keyStep(ev("T", { shiftKey: true }))).toBeUndefined(); | |
| 102 | expect(keyStep(ev("ArrowDown", { altKey: true, shiftKey: true }))).toBeUndefined(); | |
| 103 | }); | |
| 104 | ||
| 105 | test("the shipped script carries the same function", () => { | |
| 106 | expect(KEYS_JS).toContain(keyStep.toString()); | |
| 107 | }); |
test/next-up.test.ts +2 −2
| @@ -180,7 +180,7 @@ describe("Next up", () => { | ||
| 180 | 180 | expect(writer).toContain(`value="${n.b}"`); |
| 181 | 181 | expect(writer).toContain("Not now"); |
| 182 | 182 | // The key is shown beside each action, and the rows take j/k. |
| 183 | for (const key of ["t", "u", "d", "n"]) expect(writer).toContain(`data-key="${key}"`); | |
| 183 | for (const key of ["t", "u", "d", "l"]) expect(writer).toContain(`data-key="${key}"`); | |
| 184 | 184 | expect(writer).toContain("<kbd>t</kbd>"); |
| 185 | 185 | expect(writer).toContain("data-nav"); |
| 186 | 186 | expect(writer).toContain("Blocker"); |
| @@ -349,7 +349,7 @@ describe("Next up page shape (#9764)", () => { | ||
| 349 | 349 | expect(main).not.toMatch(/class="card\b/); |
| 350 | 350 | // One sentence in the head, from pageHead; the key hint sits after the list, not in the lead. |
| 351 | 351 | expect(main).toMatch(/<div class="page-head"> ?<h1>Next up<\/h1> ?<span class="page-head-sub muted small">The crew builds strictly from the top\.<\/span>/); |
| 352 | expect(main.indexOf("j and k move between rows")).toBeGreaterThan(main.indexOf("</ul>")); | |
| 352 | expect(main.indexOf("j/k choose a row")).toBeGreaterThan(main.indexOf("</ul>")); | |
| 353 | 353 | expect(body).not.toMatch(/class="[^"]*\bbadge\b/); |
| 354 | 354 | expect(body).not.toMatch(/<(?:span|em|strong)[^>]*badge/); |
| 355 | 355 |
test/tui-next-up.test.ts +2 −2
| @@ -117,7 +117,7 @@ describe("next up in the terminal", () => { | ||
| 117 | 117 | |
| 118 | 118 | // Not now puts the row behind the Later gate. |
| 119 | 119 | const first = order()[0]!; |
| 120 | await h.press("n"); | |
| 120 | await h.press("l"); | |
| 121 | 121 | expect(order()).not.toContain(first); |
| 122 | 122 | expect(nextUp(forge.db, repo).notNow.map((r) => r.issue.number)).toContain(first); |
| 123 | 123 | |
| @@ -179,7 +179,7 @@ describe("next up in the terminal", () => { | ||
| 179 | 179 | await h.toNext("alice/plan"); |
| 180 | 180 | expect(h.text()).toContain("you can read this order, not change it"); |
| 181 | 181 | const before = order(); |
| 182 | await h.press("j", "t", "n", "d"); | |
| 182 | await h.press("j", "t", "l", "d"); | |
| 183 | 183 | expect(order()).toEqual(before); |
| 184 | 184 | h.stop(); |
| 185 | 185 | }); |
new above
offline
Nested ternary is unreadable and untested. Use a lookup keyed by modifier state, and test it. Note Alt+Shift combos silently do nothing; fine, but say so in the help.