Public: anyone can read this; the crew can change it.
/settings shows each setting as one line: its name, its value and its control, with the reason one tap deeper #11033
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/10999-settings-shows-each-setting-as-one-line → main 6 commits fork 4 threads · 4 unresolved
8 files
changed +193
−92
docs/design/areas/settings.md +11
| @@ -20,6 +20,17 @@ the text wrap and gives the control a place that is always visible. Use `.row-ac | ||
| 20 | 20 | `src/web/style/components.ts`. |
| 21 | 21 | - `.record-value` — a DNS record's value on an app's domain row (`.setting-list` row meta): monospace, wraps instead of ellipsizing, and `user-select: all` so one click selects the whole value to copy. |
| 22 | 22 | |
| 23 | ## A setting as one line (#10999) | |
| 24 | ||
| 25 | A repository's Branch and gates form (`settingsPage()` in `src/web/pages/settings.ts`) shows each setting as one line: | |
| 26 | its name, its control (the checkbox, select or number is the value) and a mark that opens its reason. Built by | |
| 27 | `settingLine()` and `toggleLine()` in `src/web/setting_line.ts`; styled in `src/web/style/components.ts`. | |
| 28 | ||
| 29 | - `.setting-line` — the `.field` row: a wrapping flex line of label, control and mark, at desktop and 390 px alike. | |
| 30 | - `.why` — the `<details>` holding a setting's reason. Its summary is the help mark drawn as a small bordered round | |
| 31 | button (28px, 44px on touch), named "Why" for a screen reader and a tooltip; open, the reason takes the full width | |
| 32 | under the row. | |
| 33 | ||
| 23 | 34 | ## Labels |
| 24 | 35 | |
| 25 | 36 | Each label is a `row()` in a `.setting-list`: its chip is the title, `N open` the meta, and a `.row-fields` form (name, |
docs/design/attention-budget.json +4 −4
| @@ -233,13 +233,13 @@ | ||
| 233 | 233 | }, |
| 234 | 234 | "crew": { |
| 235 | 235 | "390": { |
| 236 | "words": 940, | |
| 237 | "blocks": 87, | |
| 236 | "words": 739, | |
| 237 | "blocks": 85, | |
| 238 | 238 | "colours": 4 |
| 239 | 239 | }, |
| 240 | 240 | "1280": { |
| 241 | "words": 940, | |
| 242 | "blocks": 87, | |
| 241 | "words": 739, | |
| 242 | "blocks": 85, | |
| 243 | 243 | "colours": 4 |
| 244 | 244 | } |
| 245 | 245 | } |
src/web/pages/settings.ts +24 −86
| @@ -91,7 +91,8 @@ import { settingsLead, teamJs } from "../studio/crew.ts"; | ||
| 91 | 91 | import { pageTitle } from "../studio/nouns.ts"; |
| 92 | 92 | import { appHref } from "./apps.ts"; |
| 93 | 93 | import { field, readForm, type Form } from "../form.ts"; |
| 94 | import { html, when, type Html } from "../html.ts"; | |
| 94 | import { html, raw, when, type Html } from "../html.ts"; | |
| 95 | import { settingLine, toggleLine } from "../setting_line.ts"; | |
| 95 | 96 | import { NOT_CHANGED } from "../edit_words.ts"; |
| 96 | 97 | import { STOP_APP } from "../danger_words.ts"; |
| 97 | 98 | import { icon } from "../icons.ts"; |
| @@ -279,101 +280,38 @@ export async function settingsPage( | ||
| 279 | 280 | // sends them back unchanged, as that one does the gates below. |
| 280 | 281 | const body = html`<section class="settings-block"> |
| 281 | 282 | <h2>Branch and gates</h2> |
| 282 | <p class="muted"> | |
| 283 | Where the repository opens, and what must hold before a change lands on it. | |
| 284 | </p> | |
| 285 | 283 | <form method="post" action="${base}/settings"> |
| 286 | 284 | <input type="hidden" name="card" value="gates" /> |
| 287 | ${selectField({ | |
| 288 | label: "Default branch", | |
| 289 | name: "defaultBranch", | |
| 290 | hint: "Where the repository opens, and what a bare clone checks out.", | |
| 291 | options: branches.map((b) => ({ | |
| 292 | value: b, | |
| 293 | label: b, | |
| 294 | selected: b === repo.defaultBranch, | |
| 295 | })), | |
| 296 | })} | |
| 297 | <div class="field"> | |
| 298 | <label class="check"> | |
| 299 | <input | |
| 300 | type="checkbox" | |
| 301 | name="protect" | |
| 302 | ${repo.protectDefault ? html`checked` : null} | |
| 303 | /> | |
| 304 | <span | |
| 305 | >Protect the default branch — no force-push, no deletion, and only | |
| 306 | its admins push it; everyone else lands through a pull. Enforced | |
| 307 | by the pre-receive hook, so it holds for every client.</span | |
| 308 | > | |
| 309 | </label> | |
| 310 | </div> | |
| 311 | <div class="field"> | |
| 312 | <label class="check"> | |
| 313 | <input | |
| 314 | type="checkbox" | |
| 315 | name="requireReview" | |
| 316 | ${repo.requireReview ? html`checked` : null} | |
| 317 | /> | |
| 318 | <span | |
| 319 | >Require an approving review before merging — of the commit that is | |
| 320 | head <em>now</em>, so a push after an approval dismisses it. Refused | |
| 321 | in the merge itself, not just hidden on the button.</span | |
| 322 | > | |
| 323 | </label> | |
| 324 | </div> | |
| 325 | <div class="field"> | |
| 326 | <label class="check"> | |
| 327 | <input | |
| 328 | type="checkbox" | |
| 329 | name="requireChecks" | |
| 330 | ${repo.requireChecks ? html`checked` : null} | |
| 331 | /> | |
| 332 | <span | |
| 333 | >Require a passing deploy before merging — the check is the pull | |
| 334 | request's preview coming up at the commit that is head <em>now</em>. | |
| 335 | Read live inside the merge, like the review.</span | |
| 336 | > | |
| 337 | </label> | |
| 338 | </div> | |
| 339 | <div class="field"> | |
| 340 | <input type="hidden" name="crewOutsidersShown" value="1" /> | |
| 341 | <label class="check"> | |
| 342 | <input | |
| 343 | type="checkbox" | |
| 344 | name="crewOutsiders" | |
| 345 | ${repo.crewOutsiders ? html`checked` : null} | |
| 346 | /> | |
| 347 | <span | |
| 348 | >Outsiders may trigger the crew — anyone signed in who can read | |
| 349 | this repository starts and steers runs by commenting, on your | |
| 350 | budget. Off, only people who can write here direct the crew; | |
| 351 | anyone else's words stay on the thread. Turning it on or off is | |
| 352 | recorded in the activity.</span | |
| 353 | > | |
| 354 | </label> | |
| 355 | </div> | |
| 356 | <div class="field"> | |
| 357 | <input type="hidden" name="polishShareShown" value="1" /> | |
| 358 | <label for="polish-share">Polish share</label> | |
| 285 | ${settingLine("defaultBranch", html`<label for="f-defaultBranch">Default branch</label> | |
| 286 | <select class="input" id="f-defaultBranch" name="defaultBranch" aria-describedby="why-defaultBranch"> | |
| 287 | ${branches.map( | |
| 288 | (b) => | |
| 289 | html`<option value="${b}" ${b === repo.defaultBranch ? raw("selected") : null}>${b}</option>`, | |
| 290 | )} | |
| 291 | </select>`, "Where the repository opens, and what a bare clone checks out.")} | |
| 292 | ${toggleLine("protect", "Protect the default branch", repo.protectDefault, | |
| 293 | "No force-push, no deletion, and only its admins push it; everyone else lands through a pull. Enforced by the pre-receive hook, so it holds for every client.")} | |
| 294 | ${toggleLine("requireReview", "Require an approving review", repo.requireReview, | |
| 295 | html`Of the commit that is head <em>now</em>, so a push after an approval dismisses it. Refused in the merge itself, not just hidden on the button.`)} | |
| 296 | ${toggleLine("requireChecks", "Require a passing deploy", repo.requireChecks, | |
| 297 | html`The check is the pull request's preview coming up at the commit that is head <em>now</em>. Read live inside the merge, like the review.`)} | |
| 298 | ${toggleLine("crewOutsiders", "Outsiders may trigger the crew", repo.crewOutsiders, | |
| 299 | "Anyone signed in who can read this repository starts and steers runs by commenting, on your budget. Off, only people who can write here direct the crew; anyone else's words stay on the thread. Turning it on or off is recorded in the activity.", | |
| 300 | "crewOutsidersShown")} | |
| 301 | ${settingLine("polishShare", html`<input type="hidden" name="polishShareShown" value="1" /> | |
| 302 | <label for="polish-share">Polish share, percent (0 to ${String(POLISH_SHARE_MAX)})</label> | |
| 359 | 303 | <input |
| 360 | class="input mono" | |
| 304 | class="input input-num mono" | |
| 361 | 305 | id="polish-share" |
| 362 | type="number" | |
| 363 | 306 | name="polishShare" |
| 307 | type="number" | |
| 364 | 308 | min="0" |
| 365 | 309 | max="${String(POLISH_SHARE_MAX)}" |
| 366 | 310 | step="1" |
| 367 | 311 | value="${String(repo.polishShare)}" |
| 368 | 312 | inputmode="numeric" |
| 369 | /> | |
| 370 | <p class="hint"> | |
| 371 | Percent of the week's agent spend that polish may take, 0 to ${String(POLISH_SHARE_MAX)} | |
| 372 | (starts at ${String(POLISH_SHARE_DEFAULT)}) — so small fixes get steady attention without crowding out the plan. | |
| 373 | Over it, the crew stops picking issues labelled polish or in the small-fixes lane until the week catches up; the | |
| 374 | <a href="${base}/week">week</a> shows where it stands. | |
| 375 | </p> | |
| 376 | </div> | |
| 313 | aria-describedby="why-polishShare" | |
| 314 | />`, html`Percent of the week's agent spend that polish may take, 0 to ${String(POLISH_SHARE_MAX)} (starts at ${String(POLISH_SHARE_DEFAULT)}). Over it, the crew stops picking issues labelled polish or in the small-fixes lane until the week catches up; the <a href="${base}/week">week</a> shows where it stands.`)} | |
| 377 | 315 | ${cellMemory === undefined |
| 378 | 316 | ? null |
| 379 | 317 | : html`<div class="field"><p class="hint" data-check-memory>${ |
src/web/setting_line.ts added +28
| @@ -0,0 +1,28 @@ | ||
| 1 | /** | |
| 2 | * A setting as one line (#10999): its name, its control (which shows its | |
| 3 | * value) and a mark that opens its reason, all on one flex row | |
| 4 | * (`.setting-line`, docs/design/areas/settings.md). The reason sits one tap | |
| 5 | * deeper in a <details>; the mark is drawn as a small bordered button, so it | |
| 6 | * reads as a control without a word that would cost every row a text block. | |
| 7 | */ | |
| 8 | import { html, raw, type Html } from "./html.ts"; | |
| 9 | import { icon } from "./icons.ts"; | |
| 10 | ||
| 11 | /** The reason behind a setting, behind its mark. `id` ties it to the control via aria-describedby. */ | |
| 12 | export function why(id: string, reason: string | Html): Html { | |
| 13 | return html`<details class="hint why" id="why-${id}"><summary aria-label="Why" title="Why">${icon("help")}</summary><p>${reason}</p></details>`; | |
| 14 | } | |
| 15 | ||
| 16 | /** A setting line around any control: the caller passes the label and control markup. */ | |
| 17 | export function settingLine(name: string, body: Html, reason: string | Html): Html { | |
| 18 | return html`<div class="field setting-line" data-setting="${name}">${body}${why(name, reason)}</div>`; | |
| 19 | } | |
| 20 | ||
| 21 | /** A yes/no setting: the checkbox is its value. `shown` names the hidden field saying the box was on the form. */ | |
| 22 | export function toggleLine(name: string, label: string, on: boolean, reason: string | Html, shown?: string): Html { | |
| 23 | return settingLine( | |
| 24 | name, | |
| 25 | html`${shown ? html`<input type="hidden" name="${shown}" value="1" />` : null}<label class="check"><input type="checkbox" name="${name}" aria-describedby="why-${name}" ${on ? raw("checked") : null} /><span>${label}</span></label>`, | |
| 26 | reason, | |
| 27 | ); | |
| 28 | } |
src/web/style/components.ts +19
| @@ -131,6 +131,25 @@ select.input { | ||
| 131 | 131 | display: block; font-size: 13px; font-weight: 550; margin: 0 0 6px; color: var(--fg); |
| 132 | 132 | } |
| 133 | 133 | .field .hint { display: block; font-size: 12.5px; color: var(--muted-fg); margin-top: 6px; overflow-wrap: anywhere; } |
| 134 | /* A setting as one line (#10999): name, control, and the mark that opens its | |
| 135 | reason. The mark is a small bordered button; open, the reason takes a line | |
| 136 | of its own under the row. */ | |
| 137 | .setting-line { display: flex; flex-wrap: wrap; align-items: center; gap: 6px 10px; } | |
| 138 | .setting-line > label:not(.check) { margin: 0; } | |
| 139 | .setting-line > select.input { width: auto; max-width: 100%; flex: 0 1 14rem; } | |
| 140 | .setting-line > .check { flex: 0 1 auto; } | |
| 141 | .setting-line > details.why { margin: 0; } | |
| 142 | .setting-line > details.why[open] { flex-basis: 100%; } | |
| 143 | .why > summary { | |
| 144 | list-style: none; cursor: pointer; display: inline-grid; place-items: center; | |
| 145 | width: 28px; height: 28px; border: 1px solid var(--border); border-radius: 999px; | |
| 146 | background: var(--card); color: var(--muted-fg); | |
| 147 | } | |
| 148 | .why > summary::-webkit-details-marker { display: none; } | |
| 149 | .why > summary:hover, .why[open] > summary { border-color: var(--border-strong); color: var(--fg); } | |
| 150 | .why > summary:focus-visible { outline: 2px solid var(--accent); outline-offset: 2px; } | |
| 151 | .why > p { margin: 6px 0 0; font-size: 12.5px; color: var(--muted-fg); overflow-wrap: anywhere; } | |
| 152 | @media (pointer: coarse) { .why > summary { width: 44px; height: 44px; } } | |
| 134 | 153 | .field .error { display: block; font-size: 12.5px; color: var(--danger); margin-top: 6px; font-weight: 500; overflow-wrap: anywhere; } |
| 135 | 154 | .field[data-invalid] > label { color: var(--danger); } |
| 136 | 155 | /* A group of checks is one field: the legend reads as a label, no frame. */ |
test/proposal-token-settings.test.ts +1 −1
| @@ -157,7 +157,7 @@ describe("proposal tokens on the settings page", () => { | ||
| 157 | 157 | expect(mintRes.status).toBe(403); |
| 158 | 158 | const { secret } = await mint("keep"); |
| 159 | 159 | expect(secret).not.toBe(""); |
| 160 | const id = /proposal-tokens\/([^/"%]+)\/delete/.exec(decodeURIComponent(await settings(owner)))?.[1]; | |
| 160 | const id = /proposal-tokens\/([^/"]+)\/delete/.exec(await settings(owner))?.[1]; | |
| 161 | 161 | expect((await post(stranger, `/ada/lib/settings/proposal-tokens/${id}/delete`)).status).toBe(403); |
| 162 | 162 | expect(await settings(owner)).toContain("keep"); |
| 163 | 163 | }); |
test/settings-lines.test.ts added +105
| @@ -0,0 +1,105 @@ | ||
| 1 | /** | |
| 2 | * #10999: each setting on a repository's General page is one row — a label, | |
| 3 | * its current value and its control — with its reason in a <details> on that | |
| 4 | * row. Every setting the page had is still a row posting to the same route, | |
| 5 | * and posting that form back to the route changes exactly those settings. | |
| 6 | * Scope: the General page's Branch and gates form; the cloud and secrets | |
| 7 | * sections are lists of rows already (`.setting-list`) and are not touched. | |
| 8 | */ | |
| 9 | import { afterAll, beforeAll, expect, test } from "bun:test"; | |
| 10 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 11 | import { tmpdir } from "node:os"; | |
| 12 | import { join } from "node:path"; | |
| 13 | import { createApp, type App } from "../src/app.ts"; | |
| 14 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 15 | import { adminCookie } from "./helpers/admin.ts"; | |
| 16 | ||
| 17 | let dir = ""; | |
| 18 | let app: App; | |
| 19 | let servers: Servers; | |
| 20 | let cookie = ""; | |
| 21 | ||
| 22 | beforeAll(async () => { | |
| 23 | dir = await mkdtemp(join(tmpdir(), "forge-lines-")); | |
| 24 | app = createApp({ dataDir: dir, port: 0 }); | |
| 25 | servers = await serve(app); | |
| 26 | cookie = await adminCookie(app, { username: "olive", email: "[email protected]", password: "settings-password" }); | |
| 27 | await fetch(`${servers.url}/new`, { | |
| 28 | method: "POST", | |
| 29 | headers: { cookie }, | |
| 30 | body: new URLSearchParams({ name: "proj", visibility: "private", description: "x" }), | |
| 31 | redirect: "manual", | |
| 32 | }); | |
| 33 | }); | |
| 34 | ||
| 35 | afterAll(async () => { | |
| 36 | servers.stop(); | |
| 37 | app.close(); | |
| 38 | await rm(dir, { recursive: true, force: true }); | |
| 39 | }); | |
| 40 | ||
| 41 | const KEYS = ["defaultBranch", "protect", "requireReview", "requireChecks", "crewOutsiders", "polishShare"]; | |
| 42 | ||
| 43 | test("every setting key is one row inside the form that posts to /settings, reason in a details", async () => { | |
| 44 | const html = await ( | |
| 45 | await fetch(`${servers.url}/olive/proj/settings`, { headers: { cookie, accept: "text/html" } }) | |
| 46 | ).text(); | |
| 47 | const form = gatesForm(html); | |
| 48 | expect(form).not.toBe(""); | |
| 49 | for (const key of KEYS) { | |
| 50 | const row = form.match(new RegExp(`<div class="field setting-line" data-setting="${key}">([\\s\\S]*?)</details>`))?.[1]; | |
| 51 | expect(row, key).toBeDefined(); | |
| 52 | expect(row, key).toContain(`name="${key}"`); | |
| 53 | // Its own reason, on its own row, named for a screen reader and tied to the control. | |
| 54 | expect(row, key).toContain('<details class="hint why"'); | |
| 55 | expect(row, key).toContain('aria-label="Why"'); | |
| 56 | expect(row, key).toContain(`aria-describedby="why-${key}"`); | |
| 57 | expect(row, key).toContain(`id="why-${key}"`); | |
| 58 | // No stale on/off word: the checkbox is the value. | |
| 59 | expect(row, key).not.toContain("<strong>"); | |
| 60 | } | |
| 61 | expect(form.match(/<details/g)?.length).toBe(KEYS.length); | |
| 62 | expect(form).not.toContain('<p class="hint"'); | |
| 63 | }); | |
| 64 | ||
| 65 | /** The names the form posts beside `card=gates` (matched by gatesForm): on main before #10999 they were exactly these. */ | |
| 66 | const POSTED = ["defaultBranch", "protect", "requireReview", "requireChecks", "crewOutsidersShown", "crewOutsiders", "polishShareShown", "polishShare"]; | |
| 67 | ||
| 68 | const page = async (): Promise<string> => | |
| 69 | (await fetch(`${servers.url}/olive/proj/settings`, { headers: { cookie, accept: "text/html" } })).text(); | |
| 70 | ||
| 71 | const post = (fields: Record<string, string>) => | |
| 72 | fetch(`${servers.url}/olive/proj/settings`, { | |
| 73 | method: "POST", | |
| 74 | headers: { cookie, origin: servers.url }, | |
| 75 | body: new URLSearchParams(fields), | |
| 76 | redirect: "manual", | |
| 77 | }); | |
| 78 | ||
| 79 | const checked = (form: string, key: string): boolean => | |
| 80 | new RegExp(`name="${key}" [^>]*\\bchecked\\b`).test(form); | |
| 81 | ||
| 82 | test("the form posts the same names to the same route, and each one lands", async () => { | |
| 83 | const before = gatesForm(await page()); | |
| 84 | expect([...before.matchAll(/\bname="([^"]+)"/g)].map((m) => m[1]).sort()).toEqual([...POSTED].sort()); | |
| 85 | const branch = /<option value="([^"]+)" selected/.exec(before)?.[1] ?? ""; | |
| 86 | expect(branch).not.toBe(""); | |
| 87 | ||
| 88 | const on = { card: "gates", defaultBranch: branch, protect: "on", requireReview: "on", requireChecks: "on", crewOutsidersShown: "1", crewOutsiders: "on", polishShareShown: "1", polishShare: "7" }; | |
| 89 | expect((await post(on)).status).toBe(303); | |
| 90 | let form = gatesForm(await page()); | |
| 91 | for (const key of ["protect", "requireReview", "requireChecks", "crewOutsiders"]) expect(checked(form, key), key).toBe(true); | |
| 92 | expect(form).toMatch(/name="polishShare"[^>]*value="7"/); | |
| 93 | ||
| 94 | const off = { card: "gates", defaultBranch: branch, crewOutsidersShown: "1", polishShareShown: "1", polishShare: "0" }; | |
| 95 | expect((await post(off)).status).toBe(303); | |
| 96 | form = gatesForm(await page()); | |
| 97 | for (const key of ["protect", "requireReview", "requireChecks", "crewOutsiders"]) expect(checked(form, key), key).toBe(false); | |
| 98 | expect(form).toMatch(/name="polishShare"[^>]*value="0"/); | |
| 99 | }); | |
| 100 | ||
| 101 | function gatesForm(html: string): string { | |
| 102 | return ( | |
| 103 | html.match(/<form method="post" action="\/olive\/proj\/settings">\s*<input type="hidden" name="card" value="gates" \/>([\s\S]*?)<\/form>/)?.[1] ?? "" | |
| 104 | ); | |
| 105 | } |
test/web-workflows.test.ts +1 −1
| @@ -750,7 +750,7 @@ describe("verdicts, threads, and the gate they open", () => { | ||
| 750 | 750 | test("require-review is a checkbox in general settings, and it sticks", async () => { |
| 751 | 751 | const settings = await bodyOf("ada", "/acme/lib/settings"); |
| 752 | 752 | expect(settings).toContain('name="requireReview"'); |
| 753 | expect(words(settings)).toContain("Require an approving review before merging"); | |
| 753 | expect(words(settings)).toContain("Require an approving review"); | |
| 754 | 754 | |
| 755 | 755 | const saved = await web("ada", "/acme/lib/settings", { |
| 756 | 756 | form: { |
Principle 3: this keeps the mark on its own line below the control. Make the row one line (name, control, mark) so box 1 holds at 390 px.