Public: anyone can read this; the crew can change it.
The header's Talk button shows when a conversation waits on your yes #10644
Check · Review · Land · Live done — Landed and live in production.
mason/forge:mason/10327-the-header-s-talk-button-shows-when-a-co → main 6 commits fork 5 threads · 5 unresolved
9 files
changed +97
−9
docs/design/areas/talk-count.md added +5
| @@ -0,0 +1,5 @@ | ||
| 1 | # Talk count | |
| 2 | ||
| 3 | The header's Talk button carries `.talk-btn` (the button, positioned) and `.talk-count` (a plain | |
| 4 | count in its corner, no fill, no radius) when the viewer has confirms waiting. It is absolutely | |
| 5 | positioned inside the button, so it adds no width and nothing shifts whether it shows or not. |
src/actions/conversation_confirms.ts +6 −1
| @@ -14,7 +14,7 @@ | ||
| 14 | 14 | |
| 15 | 15 | import { actingVia } from "../core/acting_via.ts"; |
| 16 | 16 | import { appendTurn, conversationOfVia, MAX_TURN_CHARS } from "../core/conversations.ts"; |
| 17 | import { answerHeld, HELD_TTL_MS, waitingFor, type YesRefusal } from "../core/fence.ts"; | |
| 17 | import { answerHeld, countWaiting, HELD_TTL_MS, waitingFor, type YesRefusal } from "../core/fence.ts"; | |
| 18 | 18 | import type { User } from "../core/users.ts"; |
| 19 | 19 | import { argumentsShown, confirmSentence, keptOf, undoOf } from "../crew/conversation_confirm.ts"; |
| 20 | 20 | import type { Ctx } from "../http/context.ts"; |
| @@ -43,6 +43,11 @@ export function waitingConfirms(ctx: Pick<Ctx, "app">, user: User, now: number = | ||
| 43 | 43 | })); |
| 44 | 44 | } |
| 45 | 45 | |
| 46 | /** How many confirms wait on the person's yes, for the header's Talk button. */ | |
| 47 | export function waitingCount(ctx: Pick<Ctx, "app">, user: User, now: number = Date.now()): number { | |
| 48 | return countWaiting(ctx.app.db, user.id, now); | |
| 49 | } | |
| 50 | ||
| 46 | 51 | export const CONFIRM_ID = /^[0-9a-z]{1,40}$/; |
| 47 | 52 | |
| 48 | 53 | /** Why a yes did nothing, in words, with the status the API answers. */ |
src/core/fence.ts +13
| @@ -421,6 +421,19 @@ export function pastTime(db: Database, userId: string, ids: readonly string[]): | ||
| 421 | 421 | /** Why a yes is refused: no such confirm of this person's, already answered, or past its time. */ |
| 422 | 422 | export type YesRefusal = "not_found" | "answered" | "expired"; |
| 423 | 423 | |
| 424 | /** | |
| 425 | * How many confirms wait on the person's yes: one indexed count | |
| 426 | * (`conversation_held_waiting`), no sweep, so a page can ask it on every | |
| 427 | * render. Those past their time do not count, swept or not. | |
| 428 | */ | |
| 429 | export function countWaiting(db: Database, userId: string, now: number = Date.now()): number { | |
| 430 | return db | |
| 431 | .query<{ n: number }, [string, string]>( | |
| 432 | "SELECT COUNT(*) AS n FROM conversation_held WHERE user_id = ?1 AND answered_at IS NULL AND created_at > ?2", | |
| 433 | ) | |
| 434 | .get(userId, new Date(now - HELD_TTL_MS).toISOString())!.n; | |
| 435 | } | |
| 436 | ||
| 424 | 437 | /** A person's confirms still waiting for their yes, oldest first; those past their time are swept first. */ |
| 425 | 438 | export function waitingFor(db: Database, userId: string, now: number = Date.now()): Held[] { |
| 426 | 439 | sweepHeld(db, now); |
src/web/layout.ts +11 −3
| @@ -7,6 +7,7 @@ | ||
| 7 | 7 | * `/search`), and every dialog (a link to a real page). |
| 8 | 8 | */ |
| 9 | 9 | |
| 10 | import { waitingCount } from "../actions/conversation_confirms.ts"; | |
| 10 | 11 | import { goLevel, goPaletteRows } from "./go_pages.ts"; |
| 11 | 12 | import { NEW_ISSUE_KEY } from "./script_keys.ts"; |
| 12 | 13 | import { newIssueHref } from "./issue_from.ts"; |
| @@ -622,6 +623,15 @@ export function signInHref(ctx: Ctx): string { | ||
| 622 | 623 | return token === undefined ? "/signin" : edgeLoginUrl(ctx.url.pathname + ctx.url.search); |
| 623 | 624 | } |
| 624 | 625 | |
| 626 | /** The Talk button; the count of confirms waiting on the viewer's yes is asked only here, where the button renders. */ | |
| 627 | function talkButton(ctx: Ctx, user: User, about?: string): Html { | |
| 628 | const waiting = waitingCount(ctx, user); | |
| 629 | const label = waiting > 0 ? `Talk to the forge, ${waiting} waiting` : "Talk to the forge"; | |
| 630 | return html`<a class="btn btn-icon talk-btn" href="${talkHref(about)}" aria-label="${label}" data-shortcut="g t" ${hintAttr("g t", "Talk to the forge")} | |
| 631 | >${icon("comment")}${waiting > 0 ? html`<span class="talk-count" aria-hidden="true">${waiting > 9 ? "9+" : String(waiting)}</span>` : null}</a | |
| 632 | >`; | |
| 633 | } | |
| 634 | ||
| 625 | 635 | /** The skip link and the topbar: the wordmark, the crumbs, the tools. */ |
| 626 | 636 | function topbar( |
| 627 | 637 | ctx: Ctx, |
| @@ -653,9 +663,7 @@ function topbar( | ||
| 653 | 663 | <span class="topbar-tools"> |
| 654 | 664 | ${tools ?? html`${searchBar()} |
| 655 | 665 | ${user |
| 656 | ? html`<a class="btn btn-icon" href="${talkHref(about)}" aria-label="Talk to the forge" data-shortcut="g t" ${hintAttr("g t", "Talk to the forge")} | |
| 657 | >${icon("comment")}</a | |
| 658 | ><a class="small" href="/asks" data-shortcut="g q" ${hintAttr("g q")}>Asks</a | |
| 666 | ? html`${talkButton(ctx, user, about)}<a class="small" href="/asks" data-shortcut="g q" ${hintAttr("g q")}>Asks</a | |
| 659 | 667 | >${newMenu(ctx)}${userMenu( |
| 660 | 668 | ctx, |
| 661 | 669 | user, |
src/web/style/components.ts +2
| @@ -65,6 +65,8 @@ export const components = /* css */ ` | ||
| 65 | 65 | .btn-icon { padding: 0; width: 36px; min-width: 36px; } |
outdated
These rules were inserted between the "An icon button is a square..." comment and the .btn-icon rules it describes. Move them after the .btn-icon group. |
||
| 66 | 66 | .btn-icon[data-size="sm"] { width: 28px; min-width: 28px; } |
outdated
Principle 5: --accent is for interaction alone; a filled accent pill is a status badge, not an interaction, and it is a boxed shape (principle 6). Use --fg/--muted text with no fill and no radius, or a plain number beside the icon. Also hand-rolled sizes (16px, -4px, 12.5px) are not tokens. |
||
| 67 | 67 | .btn-icon[data-size="lg"] { width: 44px; min-width: 44px; } |
| 68 | .talk-btn { position: relative; } | |
| 69 | .talk-count { position: absolute; top: 0; right: 2px; font-size: 12.5px; line-height: 1; font-weight: 600; color: var(--fg); pointer-events: none; } | |
| 68 | 70 | /* Touch targets stay thumb-sized on a phone; padding does it, so text stays centered. */ |
| 69 | 71 | @media (pointer: coarse) { |
| 70 | 72 | .btn { min-height: 44px; } |
test/capacity-timing.test.ts +1 −1
| @@ -84,7 +84,7 @@ test("render does not include the time already named as errors", async () => { | ||
| 84 | 84 | // counted, not timed (#7824), so a regression — an N+1 over the settings or |
| 85 | 85 | // the change history, a lost cache — fails every run, not a lucky one. |
| 86 | 86 | // 59 warm today (one more with the week.use setting), 60 once a setting is away from its default; the rest is margin. |
| 87 | const WARM_STATEMENTS = 68; | |
| 87 | const WARM_STATEMENTS = 69; // +1: the header Talk count (#10327) | |
| 88 | 88 | |
| 89 | 89 | test("a warm capacity page costs a bounded number of statements, however many changes it lists", async () => { |
| 90 | 90 | const url = `${servers.url}/settings/capacity`; |
test/pages-budget.test.ts +1 −1
| @@ -34,7 +34,7 @@ const DEFAULT = { statements: 60, git: 8 }; | ||
| 34 | 34 | const HEAVIER: Record<string, { statements?: number; git?: number }> = { |
| 35 | 35 | "/:owner/:name/issues/:number": { statements: 70 }, |
| 36 | 36 | "/:owner/:name/next": { statements: 64 }, |
| 37 | "/:owner/:name/pulls/:number": { statements: 108 }, | |
| 37 | "/:owner/:name/pulls/:number": { statements: 109 }, // +1: the header Talk count (#10327) | |
| 38 | 38 | "/board": { statements: 64 }, |
| 39 | 39 | "/settings/capacity": { statements: 71 }, |
| 40 | 40 | }; |
test/reports-page-shape.test.ts +1 −1
| @@ -36,7 +36,7 @@ afterAll(async () => { | ||
| 36 | 36 | }); |
| 37 | 37 | |
| 38 | 38 | /** Statements the warm reports page ran as cards (902f53c7), measured with one report waiting. */ |
| 39 | const BUDGET = 34; | |
| 39 | const BUDGET = 35; // +1: the header Talk count (#10327) | |
| 40 | 40 | const main = (page: string) => page.slice(page.indexOf("<main"), page.indexOf("</main>")); |
| 41 | 41 | |
| 42 | 42 | test("the queue is a heading over rows, escaped, with its forms and no card", async () => { |
test/talk-page.test.ts +57 −2
| @@ -11,14 +11,14 @@ import { tmpdir } from "node:os"; | ||
| 11 | 11 | import { join } from "node:path"; |
| 12 | 12 | import { createApp, type App } from "../src/app.ts"; |
| 13 | 13 | import { commitToBranch, createRepo } from "../src/actions/repos.ts"; |
| 14 | import { sayNo } from "../src/actions/conversation_confirms.ts"; | |
| 14 | import { sayNo, sayYes } from "../src/actions/conversation_confirms.ts"; | |
| 15 | 15 | import { openIssue } from "../src/actions/issues.ts"; |
| 16 | 16 | import { openPull } from "../src/actions/pulls.ts"; |
| 17 | 17 | import { clock, STREAMS_MAX } from "../src/web/pages/talk.ts"; |
| 18 | 18 | import { serveTalk } from "../src/tui/ssh-talk.ts"; |
| 19 | 19 | import { aboutLine, parseAbout } from "../src/core/talk_about.ts"; |
| 20 | 20 | import { appendTurn, listTurns, openConversation } from "../src/core/conversations.ts"; |
| 21 | import { hold, waitingFor } from "../src/core/fence.ts"; | |
| 21 | import { HELD_TTL_MS, hold, waitingFor } from "../src/core/fence.ts"; | |
| 22 | 22 | import { getRepoById } from "../src/core/repos.ts"; |
| 23 | 23 | import { createSession } from "../src/core/sessions.ts"; |
| 24 | 24 | import { createUser, type User } from "../src/core/users.ts"; |
| @@ -803,3 +803,58 @@ describe("the header's Talk link carries the subject the page rendered", () => { | ||
| 803 | 803 | expect(ta).toEqual(["/talk", "/talk"]); |
| 804 | 804 | }); |
| 805 | 805 | }); |
| 806 | ||
| 807 | describe("the header's Talk button counts confirms waiting", () => { | |
| 808 | const talk = async (u?: User) => /<a class="btn btn-icon talk-btn"[^>]*>.*?<\/a>/s.exec(await (await get("/", u)).text())?.[0] ?? ""; | |
| 809 | ||
| 810 | test("a waiting confirm counts for its person only, and a confirm answered clears it", async () => { | |
| 811 | const fay = (await createUser(app.db, { origin: "seed", trusted: true, username: "fay", email: "[email protected]", password: "a-good-passphrase" }) as { value: User }).value; | |
| 812 | expect(await talk(fay)).not.toContain("talk-count"); | |
| 813 | expect(await talk(fay)).toContain('aria-label="Talk to the forge"'); | |
| 814 | const id = hold(app.db, "conversation:talk", fay.id, "mint_invite", {})!; | |
| 815 | hold(app.db, "conversation:talk", fay.id, "mint_invite", {}); | |
| 816 | const mine = await talk(fay); | |
| 817 | expect(mine).toContain('aria-label="Talk to the forge, 2 waiting"'); | |
| 818 | expect(mine).toContain('<span class="talk-count" aria-hidden="true">2</span>'); | |
| 819 | expect(await talk(bob)).not.toContain("2 waiting"); | |
Tests: 'a confirmed one clears it' is only tested through sayNo. Add a yes path, a hold past HELD_TTL_MS that does not count, and eleven holds showing '9+'; the 9+ branch and the expiry filter in countWaiting would survive deletion today. |
||
| 820 | expect(sayNo({ app }, fay, id).ok).toBe(true); | |
| 821 | expect(await talk(fay)).toContain("Talk to the forge, 1 waiting"); | |
| 822 | expect(await (await get("/")).text()).not.toContain("talk-count"); | |
| 823 | }); | |
| 824 | test("a confirm said yes to clears it, and the box says it was done", async () => { | |
| 825 | const gil = (await createUser(app.db, { origin: "seed", trusted: true, username: "gil", email: "[email protected]", password: "a-good-passphrase" }) as { value: User }).value; | |
| 826 | promote(app.db, gil); | |
| 827 | const made = await createRepo(app, gil, { name: "yes-count" }); | |
| 828 | if (!made.ok) throw new Error(String(made.error)); | |
| 829 | const issue = openIssue(app, gil, made.value, { title: "close me", body: "x" }); | |
| 830 | if (!issue.ok) throw new Error(JSON.stringify(issue.error)); | |
| 831 | const id = hold(app.db, "conversation:talk", gil.id, "close", { repo: "gil/yes-count", number: issue.value.number })!; | |
| 832 | expect(await talk(gil)).toContain("Talk to the forge, 1 waiting"); | |
| 833 | const yes = await sayYes({ app } as never, gil, id); | |
| 834 | expect(yes.ok && yes.done).toBe(true); | |
| 835 | const after = await talk(gil); | |
| 836 | expect(after).not.toContain("talk-count"); | |
| 837 | expect(after).toContain('aria-label="Talk to the forge"'); | |
| 838 | }); | |
| 839 | ||
| 840 | test("a confirm past its time does not count, swept or not", async () => { | |
| 841 | const hal = (await createUser(app.db, { origin: "seed", trusted: true, username: "hal", email: "[email protected]", password: "a-good-passphrase" }) as { value: User }).value; | |
| 842 | hold(app.db, "conversation:talk", hal.id, "mint_invite", {}, Date.now() - HELD_TTL_MS - 1000); | |
| 843 | const left = app.db.query<{ n: number }, [string]>("SELECT COUNT(*) AS n FROM conversation_held WHERE user_id = ?1 AND answered_at IS NULL").get(hal.id)!.n; | |
| 844 | expect(left).toBe(1); // still unswept: the count's own filter is what hides it | |
| 845 | expect(await talk(hal)).not.toContain("talk-count"); | |
| 846 | hold(app.db, "conversation:talk", hal.id, "mint_invite", {}); | |
| 847 | expect(await talk(hal)).toContain("Talk to the forge, 1 waiting"); | |
| 848 | }); | |
| 849 | ||
| 850 | test("more than nine waiting shows 9+, and the label says the exact number", async () => { | |
| 851 | const ida = (await createUser(app.db, { origin: "seed", trusted: true, username: "ida", email: "[email protected]", password: "a-good-passphrase" }) as { value: User }).value; | |
| 852 | for (let i = 0; i < 9; i++) hold(app.db, "conversation:talk", ida.id, "mint_invite", {}); | |
| 853 | expect(await talk(ida)).toContain('<span class="talk-count" aria-hidden="true">9</span>'); | |
| 854 | hold(app.db, "conversation:talk", ida.id, "mint_invite", {}); | |
| 855 | hold(app.db, "conversation:talk", ida.id, "mint_invite", {}); | |
| 856 | const many = await talk(ida); | |
| 857 | expect(many).toContain('<span class="talk-count" aria-hidden="true">9+</span>'); | |
| 858 | expect(many).toContain('aria-label="Talk to the forge, 11 waiting"'); | |
| 859 | }); | |
| 860 | }); |
Clean: the 'skip link and the topbar' doc comment now heads talkButton's comment, so topbar is uncommented and talkButton has two stacked docblocks. Put talkButton (with its own comment) above this comment, directly before topbar.