diff --git a/docs/decisions/054-the-app-signs-its-own-updates.md b/docs/decisions/054-the-app-signs-its-own-updates.md index f66dd28..0824ddd 100644 --- a/docs/decisions/054-the-app-signs-its-own-updates.md +++ b/docs/decisions/054-the-app-signs-its-own-updates.md @@ -136,3 +136,8 @@ responsibility; this is that responsibility finally built, in the only shape ad- where nothing is wrong, which today is exactly one thing. It wears the accent rather than a colour of its own, because there is no `--color-info` in any of the forty-three palettes and adding one would be forty-three generated files changed to tint one strip. + +> Superseded by ADR 055. The announcement moved out of the banner and into the title bar, and the +> tone had no callers left, so `info` and `InfoIcon` are gone. Everything else in this record — +> the manifest, the key, the eligibility guards, the swap script, and "never without a press" — +> stands unchanged. diff --git a/docs/decisions/055-the-update-lives-in-the-title-bar.md b/docs/decisions/055-the-update-lives-in-the-title-bar.md new file mode 100644 index 0000000..6c72bd9 --- /dev/null +++ b/docs/decisions/055-the-update-lives-in-the-title-bar.md @@ -0,0 +1,100 @@ +# 055: The update lives in the title bar + +Status: Accepted + +## Decision + +A newer preman is announced by a chip in the title bar, not by a banner across the window. + +`updateChip()` in `packages/desktop/src/renderer/model/update.ts` replaces `updateBanner()`. It +answers for three phases and `null` for the other five. `App.tsx` draws the answer as one +chrome-tier control at the trailing end of the title bar, immediately before the Settings gear: + +| Phase | Chip | Press | +| ------------- | ------------------------- | ------------------ | +| `available` | `Update 0.4.2` | downloads | +| `downloading` | `Downloading 47%` | nothing to press | +| `ready` | `Restart to update 0.4.2` | quits and installs | + +`idle`, `checking`, `current`, `unsupported` and `failed` draw nothing at all. The full sentence — +the one the banner used to carry, `updateHeadline()` — is the chip's tooltip, and the Settings +pane's Updates section is unchanged and still holds Skip, Check now, and every phase in words. + +`Banner`'s `info` tone and `InfoIcon` are deleted. Both existed for this one caller. + +## Rationale + +**A bar is the shape this app uses for a problem, and this is not one.** `Banner` is +`ui/Banner.tsx`, it is a sibling of the whole resizable workspace, and `design-system.md` lists it +under "Saying something went wrong". ADR 054 knew this and answered it by adding a third tone that +means "nothing is wrong" — which is a tone arguing with its own component. The argument was visible +in the pixels: the strip was the same width as the window and the same shape as a host crash, so a +release announcement read with the urgency of an engine that had died. + +**It cost a permanent row for a once-a-release fact.** The banner is `shrink-0` above the panel +group, so every pixel it takes is a pixel the editor and the console do not get, for as long as the +user does not press it — and "not now" is the honest answer to an update most of the time. A +40-pixel strip is a tax on postponing. A chip is not. + +**Chrome is where a fact about the whole window already goes.** The title bar's stated job in +`App.tsx` is the workspace picker: a fact about the window rather than about the request. Which +build this is, and that there is a newer one, is the same kind of fact. The chip borrows the +picker's geometry exactly — `h-control`, `rounded-sm`, `px-1.5`, `text-xs` — so the row reads as one +strip with two things in it rather than as a strip with a notification bolted on. + +**The accent stops being an exception.** ADR 054 had to argue that the banner was the one place the +accent appears without being the thing you came to press. In the title bar it _is_ the thing you +came to press, and it is the only accent in the row, so `design-system.md`'s rule holds as written +instead of carrying a carve-out. + +**`downloading` becomes showable, which is the one thing the move adds.** The banner deliberately +refused that phase: the user pressed the button that started the download, and a strip across the +window reporting their own press back at them is noise. A chip has the opposite problem — it _is_ +the button they pressed, and a control that disappears on the press and returns two minutes later as +a different control is a control that looks broken. So the chip stays and reports itself in place, +with the percentage where the version was. There is still no `Progress`: `design-system.md` reserves +that for a wait that is long, opaque, and has a denominator that cannot be revised, and a download +with no `content-length` fails the third. When there is no denominator the chip says the version +instead of inventing a percentage. + +**The phase with nothing to press is a ``, not a disabled ` + + )} + + )} + + ); +} + /** * The tab strip and the environment picker, sharing one row. * @@ -1444,38 +1525,6 @@ function HostBanner(): React.JSX.Element { ); } -/** - * The one interruption the updater is allowed. - * - * `available` and `ready` only, and `tone="info"` because neither is a problem: nothing is wrong - * with the app the user is looking at. A failed check gets no bar at all — a laptop that could not - * reach GitHub is not worth a strip across the window, and the Settings pane says so for whoever - * goes looking. Decision 054. - */ -function UpdateBanner(): React.JSX.Element { - const status = useUpdateStore(selectStatus); - const banner = updateBanner(status); - return ( - - {banner === null ? null : ( - - - - )} - - ); -} - function DegradedBanner(): React.JSX.Element { const degraded = useSessionStore((state) => state.degraded); const setDegraded = useSessionStore((state) => state.setDegraded); diff --git a/packages/desktop/src/renderer/model/update.ts b/packages/desktop/src/renderer/model/update.ts index 39fae16..97dee68 100644 --- a/packages/desktop/src/renderer/model/update.ts +++ b/packages/desktop/src/renderer/model/update.ts @@ -2,7 +2,7 @@ * The updater's phase, in words. * * Pure and no React, like everything else in `model/`: the window has two readers of the same - * union — the Settings section and the strip above the workspace — and a sentence written twice is + * union — the Settings section and the chip in the title bar — and a sentence written twice is * a sentence that eventually says two different things about one state. * * Nothing here decides. The phase is main's; this only chooses which words go with it. @@ -38,7 +38,7 @@ export const LOCAL_NETWORK_CAVEAT = const BYTES_IN_MB = 1024 * 1024; const MB_DECIMALS = 0; -const NO_BANNER = null; +const NO_CHIP = null; const NOTHING_RECEIVED = 0; const WHOLE = 100; @@ -82,28 +82,54 @@ export function updateHeadline(status: UpdateStatus): string { } } -export interface UpdateBanner { - readonly message: string; - /** The version, monospace and beside the message — an id, not a second sentence. */ +export interface UpdateChip { + /** The verb, in a title bar's worth of room: what the press does, or what is already happening. */ + readonly label: string; + /** + * Monospace beside the label: the shortest true thing. The version while there is a decision to + * make about it, and how far the download has got once the decision is made — an id, never a + * second sentence. + */ readonly detail: string; - /** Whether the one action is Install rather than Download. */ - readonly ready: boolean; + /** The whole sentence, for the tooltip. `updateHeadline` already writes it; this does not. */ + readonly title: string; + /** What the press does, or `null` while a download is in flight and there is nothing to press. */ + readonly action: "download" | "install" | null; } /** - * The strip above the workspace, or nothing. + * The chip in the title bar, or nothing. * - * Only `available` and `ready` interrupt. Not `failed`: a background check that could not reach - * GitHub is not worth a bar across someone's window, and the Settings section says so for whoever - * goes looking. Not `downloading` either — the user pressed the button that started it and the - * pane they pressed it in is where the progress belongs. + * Three phases earn a place in the window's chrome and five do not. `idle`, `checking`, `current` + * and `unsupported` are answers to a question nobody asked from here — the Settings section is + * where someone who wants them goes looking. `failed` is not here either: a laptop that could not + * reach GitHub has nothing for the user to do about it, and a permanent mark in the chrome saying + * so is worse than a bar that at least went away. + * + * `downloading` _is_ here, which is the one thing the move changes rather than relocates. A bar + * across the window could not show it — the user pressed the button that started it and a strip + * reporting their own press back at them is noise — but the chip is the button they pressed, and a + * control that vanishes on the press and reappears two minutes later as a different control is a + * control that looks broken. It reports itself in place instead. */ -export function updateBanner(status: UpdateStatus): UpdateBanner | null { +export function updateChip(status: UpdateStatus): UpdateChip | null { if (status.phase === "available") { - return { message: "A new version of preman is available.", detail: status.version, ready: false }; + return { label: "Update", detail: status.version, title: updateHeadline(status), action: "download" }; + } + if (status.phase === "downloading") { + const percent = downloadPercent(status); + return { + label: "Downloading", + // The version until a denominator exists, because a response with no `content-length` has no + // percentage to state and an invented one jumps backwards. + detail: percent === null ? status.version : `${percent}%`, + title: updateHeadline(status), + action: null, + }; } if (status.phase === "ready") { - return { message: "The update is ready. preman will restart to finish.", detail: status.version, ready: true }; + // Not "Restart": the one press in this app that quits it says what it quits for. + return { label: "Restart to update", detail: status.version, title: updateHeadline(status), action: "install" }; } - return NO_BANNER; + return NO_CHIP; } diff --git a/packages/desktop/src/renderer/ui/Banner.tsx b/packages/desktop/src/renderer/ui/Banner.tsx index 87d60fc..195daf3 100644 --- a/packages/desktop/src/renderer/ui/Banner.tsx +++ b/packages/desktop/src/renderer/ui/Banner.tsx @@ -13,39 +13,33 @@ */ import { useCallback, useEffect, useRef, useState, type ReactElement, type ReactNode } from "react"; -import { CheckIcon, CopyIcon, InfoIcon, WarningIcon, type Icon } from "@preman/desktop/renderer/ui/icons.js"; +import { CheckIcon, CopyIcon, WarningIcon } from "@preman/desktop/renderer/ui/icons.js"; import { IconButton } from "@preman/desktop/renderer/ui/Controls.js"; import { cn } from "@preman/desktop/renderer/ui/cn.js"; import { m } from "@preman/desktop/renderer/ui/motion.js"; /** - * The three tones that mean "read me". `ok` and `neutral` still do not warrant a bar. + * The two tones that mean "read me". `ok` and `neutral` still do not warrant a bar. * - * `info` is the late arrival and it is deliberately narrow: it exists for a fact the user has to be - * told and can act on, where nothing is wrong. Today that is exactly one thing — a newer preman is - * available, and then that it is ready to install. It wears the accent rather than a colour of its - * own because there is no `--color-info` in any of the forty-three palettes and adding one would be - * forty-three generated files changed to tint one strip. See `docs/design-system.md`. + * There were three for a while. `info` existed for a fact the user had to be told and could act on + * where nothing was wrong, and there was exactly one of those — a newer preman is available — so + * when decision 055 moved that into the title bar the tone had no callers left. It is gone rather + * than kept warm: a tone nothing wears is a tone the next reader has to decide about, and a bar is + * the shape this app uses for a problem. Good news that is not a problem is not a bar. + * + * Both remaining tones therefore wear the same glyph, which is why there is no map from tone to + * icon any more. Reintroduce one the day a tone arrives that is not a warning. */ -export type BannerTone = "danger" | "warn" | "info"; +export type BannerTone = "danger" | "warn"; const SURFACE_CLASS: Record = { danger: "border-danger/40 bg-danger/10", warn: "border-warn/40 bg-warn/10", - info: "border-accent/40 bg-accent/10", }; const ICON_CLASS: Record = { danger: "text-danger", warn: "text-warn", - info: "text-accent", -}; - -/** A warning triangle over good news says the wrong thing, so the glyph moves with the tone. */ -const ICON_BY_TONE: Record = { - danger: WarningIcon, - warn: WarningIcon, - info: InfoIcon, }; const NO_DETAILS: readonly string[] = []; @@ -112,7 +106,6 @@ export function Banner({ /** The one thing to do about it - Retry, Dismiss - to the right of the copy button. */ readonly children?: ReactNode; }): ReactElement { - const Glyph = ICON_BY_TONE[tone]; return ( /* `role="alert"` stays on the animated element. On a wrapper it would change what the screen * reader announces, and the height is deliberately not animated: the banner's own height is @@ -123,7 +116,7 @@ export function Banner({ {...BANNER_MOTION} className={cn("flex shrink-0 items-start gap-2 border-b px-gutter py-1.5", SURFACE_CLASS[tone])} > - +
{message} diff --git a/packages/desktop/src/renderer/ui/icons.ts b/packages/desktop/src/renderer/ui/icons.ts index dc4bd4e..634e2cd 100644 --- a/packages/desktop/src/renderer/ui/icons.ts +++ b/packages/desktop/src/renderer/ui/icons.ts @@ -11,6 +11,12 @@ */ export { ArrowClockwise as RefreshIcon, + // A newer preman, in the title bar's chip. Not `ImportIcon`: the tray below is already the + // sidebar's "Import from cURL", and one glyph in one window cannot mean both "read this file" + // and "there is a new version". Up rather than down because what the chip announces is a version + // to move to, which is the same arrow an app store draws; the download it starts is a step on the + // way and says so in words. + ArrowCircleUp as UpdateIcon, // Which way a frame went. Every event in a server-sent stream came down, so today this is the // only direction there is - it is here because the alternative, an unmarked row, reads as a log // line rather than as something that arrived, and because a stream that can also send (a @@ -47,10 +53,6 @@ export { Funnel as FilterIcon, GitBranch as BranchIcon, Globe as EnvironmentIcon, - // The banner's third face. A warning triangle over "version 1.4.0 is available" says the wrong - // thing twice - it is not a warning, and the app is not in trouble - so the one tone that is - // news rather than a problem gets the one glyph that means exactly that. - Info as InfoIcon, Key as AuthIcon, // A chain link, for the thing that is literally a symlink. The noun is the same on both sides // of the screen, which is what makes "the acquiring-core link is missing" a sentence about a diff --git a/test/renderer/settings.test.ts b/test/renderer/settings.test.ts index 2e8e416..5aa1152 100644 --- a/test/renderer/settings.test.ts +++ b/test/renderer/settings.test.ts @@ -21,13 +21,15 @@ import { describe, expect, it } from "vitest"; import { INELIGIBILITY_REASON, LOCAL_NETWORK_CAVEAT, - updateBanner, + updateChip, updateHeadline, } from "@preman/desktop/renderer/model/update.js"; const DESKTOP_DIR = join(dirname(fileURLToPath(import.meta.url)), "../../packages/desktop/src"); const SETTINGS = readFileSync(join(DESKTOP_DIR, "renderer/panes/SettingsPane.tsx"), "utf8"); const BRIDGE = readFileSync(join(DESKTOP_DIR, "preload/bridge.ts"), "utf8"); +const APP = readFileSync(join(DESKTOP_DIR, "renderer/App.tsx"), "utf8"); +const BANNER = readFileSync(join(DESKTOP_DIR, "renderer/ui/Banner.tsx"), "utf8"); const BLOCK_COMMENT = /\/\*[\s\S]*?\*\//g; const LINE_COMMENT = /\/\/.*$/gm; @@ -36,6 +38,8 @@ const NOTHING = ""; /** The section's body, from its `function` line to the first close at column zero. */ const DIAGNOSTICS_SECTION = /function DiagnosticsSection\(\)[\s\S]*?\n\}\n/; const UPDATES_SECTION = /function UpdatesSection\(\)[\s\S]*?\n\}\n/; +/** The title bar's chip, from its `function` line to the first close at column zero. */ +const UPDATE_CHIP = /function UpdateChip\(\)[\s\S]*?\n\}\n/; /** The tab list the pane is split by. */ const SETTINGS_TABS = /const SETTINGS_TABS = \[([^\]]*)\] as const/; /** Every `"quoted"` string in a matched fragment. */ @@ -123,9 +127,9 @@ describe("the Settings pane's Diagnostics section", () => { * The Updates section, read two ways. * * What the section *says* is not in the `.tsx` at all — it is `model/update.ts`, which is pure and - * importable, so those three cases are behaviour rather than text. What is left for the source - * reading is the two structural promises the model cannot make: that the caveat is drawn only when - * an update is staged, and that nothing here installs anything without a press. + * importable, so those cases are behaviour rather than text. What is left for the source reading is + * the two structural promises the model cannot make: that the caveat is drawn only when an update + * is staged, and that nothing here installs anything without a press. */ describe("the Settings pane's Updates section", () => { it("givenAnAvailableUpdate_whenTheUpdatesSectionRenders_thenTheVersionAndActionAreShown", () => { @@ -170,13 +174,86 @@ describe("the Settings pane's Updates section", () => { expect(body).not.toContain("Banner"); expect(LOCAL_NETWORK_CAVEAT).toContain("Local Network"); }); +}); + +/** + * The chip in the title bar, read the same two ways. + * + * What it says is `model/update.ts` and is behaviour. What is left for the source reading is the + * two structural promises the model cannot make: that the phase with nothing to press is not a + * disabled button, and that the updater no longer owns a bar across the window. + */ +describe("the title bar's update chip", () => { + it("givenAPhaseWithNothingToDo_whenTheTitleBarRenders_thenNoChipIsDrawn", () => { + // Chrome is permanent, which is exactly why `failed` is not in it: a laptop that could not + // reach GitHub gives the user nothing to do, and a standing mark saying so is worse than the + // bar it replaced, because the bar at least went away. The Settings section still says it. + expect(updateChip({ phase: "failed", message: "nope", details: [] })).toBeNull(); + expect(updateChip({ phase: "idle" })).toBeNull(); + expect(updateChip({ phase: "checking" })).toBeNull(); + expect(updateChip({ phase: "current" })).toBeNull(); + expect(updateChip({ phase: "unsupported", reason: "translocated" })).toBeNull(); + }); + + it("givenAnUpdateToActOn_whenTheChipRenders_thenOnePressDoesTheOneThing", () => { + const available = updateChip({ + phase: "available", + version: "1.4.0", + notesUrl: "https://example.invalid", + sizeBytes: 134_217_728, + }); + + expect(available?.action).toBe("download"); + expect(available?.detail).toBe("1.4.0"); + // The tooltip is the sentence the bar used to be, so nothing the move dropped is unsaid. + expect(available?.title).toContain("128 MB"); + expect(updateChip({ phase: "ready", version: "1.4.0" })?.action).toBe("install"); + }); + + it("givenADownloadInFlight_whenTheChipRenders_thenItReportsItselfAndOffersNoPress", () => { + // The one phase the move adds rather than relocates: a bar could not report a press back at + // the presser, but the chip *is* the control that was pressed and must not vanish mid-download. + const half = updateChip({ phase: "downloading", version: "1.4.0", receivedBytes: 1, totalBytes: 2 }); + expect(half?.action).toBeNull(); + expect(half?.detail).toBe("50%"); + + // No `content-length`, no denominator, and no invented percentage that could jump backwards. + const unknown = updateChip({ phase: "downloading", version: "1.4.0", receivedBytes: 1, totalBytes: 0 }); + expect(unknown?.detail).toBe("1.4.0"); + }); + + it("givenTheChipInAPhaseItCannotActIn_whenItRenders_thenItIsNotADisabledButton", () => { + const found = UPDATE_CHIP.exec(code(APP)); + expect(found).not.toBeNull(); + const body = found?.[0] ?? NOTHING; + + // A disabled `