Public: anyone can read this; the crew can change it.
Revert "The check summary's vitals table names a phone look that scrolls sideways and how many screens tall each look is" #9475
Check · Review · Land · Live — It conflicts with its base.
revert-9389 → main 1 commit
9 files
changed +20
−118
docs/features/vitals-screens.md deleted −8
| @@ -1,8 +0,0 @@ | ||
| 1 | # Screens and sideways scroll in the check's vitals (#9320) | |
| 2 | ||
| 3 | A check's per-look vitals say how many screens tall each look is (`scrollHeight / innerHeight`, one decimal; empty when the look recorded no size) and name any phone look that scrolls sideways (`scrollWidth > innerWidth`). | |
| 4 | ||
| 5 | - Run page: a right-aligned **Screens** column in the vitals table and a muted line under it, `phone /x scrolls sideways`. | |
| 6 | - Terminal: the checks line carries the same, `vitals home: LCP …, 2.4 screens; phone: …, 1.1 screens · phone /x scrolls sideways` (`vitalsLine`, `src/checks/progress.ts`). | |
| 7 | ||
| 8 | Report-only: neither can turn a check red. Skipped looks show no screens. |
src/checks/looks.ts +3 −16
| @@ -10,7 +10,7 @@ | ||
| 10 | 10 | |
| 11 | 11 | import { existsSync, mkdirSync, readFileSync, renameSync, rmSync, statSync, writeFileSync } from "node:fs"; |
| 12 | 12 | import { join } from "node:path"; |
| 13 | import type { LookVitals, PageSize } from "./summary.ts"; | |
| 13 | import type { LookVitals } from "./summary.ts"; | |
| 14 | 14 | import { runsRoot } from "../runs/log.ts"; |
| 15 | 15 | |
| 16 | 16 | export const looksPath = (dataDir: string, runId: string): string => |
| @@ -36,17 +36,6 @@ const MAX_WORDS = 80; | ||
| 36 | 36 | const words = (v: unknown): string | null => |
| 37 | 37 | typeof v === "string" && v.trim() !== "" ? v.trim().slice(0, MAX_WORDS) : null; |
| 38 | 38 | |
| 39 | const whole = (v: unknown): v is number => | |
| 40 | typeof v === "number" && Number.isInteger(v) && v >= 0 && v <= 1e6; | |
| 41 | ||
| 42 | function parseSize(v: unknown): PageSize | undefined { | |
| 43 | if (!v || typeof v !== "object") return undefined; | |
| 44 | const { scrollWidth, scrollHeight, innerWidth, innerHeight } = v as Record<string, unknown>; | |
| 45 | return whole(scrollWidth) && whole(scrollHeight) && whole(innerWidth) && whole(innerHeight) | |
| 46 | ? { scrollWidth, scrollHeight, innerWidth, innerHeight } | |
| 47 | : undefined; | |
| 48 | } | |
| 49 | ||
| 50 | 39 | /** Plain data in: the parsed file's value, narrowed to the looks it validly holds. */ |
| 51 | 40 | export function parseLooks(raw: unknown): LookVitals[] { |
| 52 | 41 | if (!Array.isArray(raw)) return []; |
| @@ -58,10 +47,8 @@ export function parseLooks(raw: unknown): LookVitals[] { | ||
| 58 | 47 | if (look === null) continue; |
| 59 | 48 | const skipped = words(rec.skipped); |
| 60 | 49 | if (skipped !== null) out.push({ look, skipped }); |
| 61 | else if (rec.entries && typeof rec.entries === "object" && !Array.isArray(rec.entries)) { | |
| 62 | const size = parseSize(rec.size); | |
| 63 | out.push({ look, entries: rec.entries, ...(size === undefined ? {} : { size }) }); | |
| 64 | } | |
| 50 | else if (rec.entries && typeof rec.entries === "object" && !Array.isArray(rec.entries)) | |
| 51 | out.push({ look, entries: rec.entries }); | |
| 65 | 52 | } |
| 66 | 53 | return out; |
| 67 | 54 | } |
src/checks/progress.ts +7 −10
| @@ -9,7 +9,7 @@ | ||
| 9 | 9 | |
| 10 | 10 | import type { CheckSummary } from "../core/runs.ts"; |
| 11 | 11 | import { readLooks } from "./looks.ts"; |
| 12 | import { checkSteps, sidewaysLine, stepsWords, vitalsRows, type LookVitals } from "./summary.ts"; | |
| 12 | import { checkSteps, stepsWords, vitalsRows, type LookVitals } from "./summary.ts"; | |
| 13 | 13 | import { parseTestsRow, testsRowLine } from "./tests-row.ts"; |
| 14 | 14 | import { readRunLog } from "../runs/log.ts"; |
| 15 | 15 | |
| @@ -115,13 +115,10 @@ export function stepsLine( | ||
| 115 | 115 | |
| 116 | 116 | /** `vitals home: LCP 3100 ms needs-improvement, CLS 0.3 poor, TBT 100 ms good; mobile: skipped: no browser` */ |
| 117 | 117 | export function vitalsLine(looks: readonly LookVitals[]): string { |
| 118 | const rows = vitalsRows(looks); | |
| 119 | const groups = rows.map((g) => { | |
| 120 | if (g.skipped !== "") return `${g.look}: skipped: ${g.skipped}`; | |
| 121 | const vitals = g.rows.map((r) => `${r.vital} ${r.value}${r.grade === "n/a" ? "" : ` ${r.grade}`}`); | |
| 122 | return `${g.look}: ${vitals.join(", ")}${g.screens === "" ? "" : `, ${g.screens} screens`}`; | |
| 123 | }); | |
| 124 | if (groups.length === 0) return ""; | |
| 125 | const sideways = sidewaysLine(rows); | |
| 126 | return `vitals ${groups.join("; ")}${sideways === "" ? "" : ` · ${sideways}`}`; | |
| 118 | const groups = vitalsRows(looks).map((g) => | |
| 119 | g.skipped !== "" | |
| 120 | ? `${g.look}: skipped: ${g.skipped}` | |
| 121 | : `${g.look}: ${g.rows.map((r) => `${r.vital} ${r.value}${r.grade === "n/a" ? "" : ` ${r.grade}`}`).join(", ")}`, | |
| 122 | ); | |
| 123 | return groups.length === 0 ? "" : `vitals ${groups.join("; ")}`; | |
| 127 | 124 | } |
src/checks/summary.ts +3 −23
| @@ -13,8 +13,6 @@ import { fullShard, OUT_OF_ROOM, TEMP_FULL } from "./disk_full.ts"; | ||
| 13 | 13 | import { oneLine } from "./words.ts"; |
| 14 | 14 | import { clip } from "../text/clip.ts"; |
| 15 | 15 | import { webvitals, type Entries } from "../release/webvitals.ts"; |
| 16 | import type { PageSize } from "../looks/capture.ts"; | |
| 17 | export type { PageSize }; | |
| 18 | 16 | |
| 19 | 17 | /** |
| 20 | 18 | * The first line of the block a timed full gate ends its log with |
| @@ -222,7 +220,7 @@ export function stepsWords(steps: readonly CheckStep[]): string { | ||
| 222 | 220 | |
| 223 | 221 | /** One look's recorded entries, or why it has none; plain data from the caller. */ |
| 224 | 222 | export type LookVitals = |
| 225 | | { readonly look: string; readonly entries: Entries; readonly size?: PageSize } | |
| 223 | | { readonly look: string; readonly entries: Entries } | |
| 226 | 224 | | { readonly look: string; readonly skipped: string }; |
| 227 | 225 | |
| 228 | 226 | /** One cell row of a look's table: words only, escaped where they are rendered. */ |
| @@ -239,20 +237,6 @@ export interface LookRows { | ||
| 239 | 237 | /** Why the look has no numbers (`no browser`); empty when it has rows. */ |
| 240 | 238 | readonly skipped: string; |
| 241 | 239 | readonly rows: readonly VitalsRow[]; |
| 242 | /** Screens tall (scrollHeight / innerHeight, one decimal); empty without a size. */ | |
| 243 | readonly screens: string; | |
| 244 | /** A phone look whose page is wider than its window; false without a size. */ | |
| 245 | readonly sideways: boolean; | |
| 246 | } | |
| 247 | ||
| 248 | const isPhone = (look: string): boolean => look === "phone" || look.startsWith("phone "); | |
| 249 | ||
| 250 | /** `phone /x scrolls sideways` for each phone look wider than its window; empty when none. */ | |
| 251 | export function sidewaysLine(groups: readonly LookRows[]): string { | |
| 252 | return groups | |
| 253 | .filter((g) => g.sideways) | |
| 254 | .map((g) => `${g.look} scrolls sideways`) | |
| 255 | .join("; "); | |
| 256 | 240 | } |
| 257 | 241 | |
| 258 | 242 | const VITAL_ROWS = [ |
| @@ -269,7 +253,7 @@ const shown = (n: number, unit: string): string => `${n}${unit === "" ? "" : ` $ | ||
| 269 | 253 | */ |
| 270 | 254 | export function vitalsRows(looks: readonly LookVitals[]): readonly LookRows[] { |
| 271 | 255 | return looks.map((l) => { |
| 272 | if ("skipped" in l) return { look: l.look, skipped: l.skipped, rows: [], screens: "", sideways: false }; | |
| 256 | if ("skipped" in l) return { look: l.look, skipped: l.skipped, rows: [] }; | |
| 273 | 257 | const v = webvitals(l.entries); |
| 274 | 258 | const rows = VITAL_ROWS.map(([vital, key]): VitalsRow => { |
| 275 | 259 | const x = v[key]; |
| @@ -280,10 +264,6 @@ export function vitalsRows(looks: readonly LookVitals[]): readonly LookRows[] { | ||
| 280 | 264 | grade: x.grade ?? "n/a", |
| 281 | 265 | }; |
| 282 | 266 | }); |
| 283 | const z = l.size; | |
| 284 | const tall = z === undefined || z.innerHeight <= 0 ? null : z.scrollHeight / z.innerHeight; | |
| 285 | const screens = tall === null ? "" : (Math.round(tall * 10) / 10).toFixed(1); | |
| 286 | const sideways = z !== undefined && isPhone(l.look) && z.scrollWidth > z.innerWidth; | |
| 287 | return { look: l.look, skipped: "", rows, screens, sideways }; | |
| 267 | return { look: l.look, skipped: "", rows }; | |
| 288 | 268 | }); |
| 289 | 269 | } |
src/looks/service.ts +1 −4
| @@ -432,10 +432,7 @@ export function startLooks(app: App, options: LooksOptions): LooksHandle { | ||
| 432 | 432 | // The thumb is the desktop page again, as a stranger: it adds no row. |
| 433 | 433 | if (kind !== "thumb" && theme === "light") { |
| 434 | 434 | const look = page === "/" ? kind : `${kind} ${page}`; |
| 435 | if (shot.ok) { | |
| 436 | const size = shot.value.size === undefined ? {} : { size: shot.value.size }; | |
| 437 | vitals.push({ look, entries: shot.value.entries, ...size }); | |
| 438 | } | |
| 435 | if (shot.ok) vitals.push({ look, entries: shot.value.entries }); | |
| 439 | 436 | else if (isNoBrowser(shot.error)) |
| 440 | 437 | vitals.push({ look, skipped: "no browser" }); |
| 441 | 438 | } |
src/web/studio/vitals.ts +5 −6
| @@ -4,25 +4,24 @@ | ||
| 4 | 4 | * look recorded anything. |
| 5 | 5 | */ |
| 6 | 6 | |
| 7 | import { sidewaysLine, vitalsRows, type LookVitals } from "../../checks/summary.ts"; | |
| 7 | import { vitalsRows, type LookVitals } from "../../checks/summary.ts"; | |
| 8 | 8 | import { html, type Html } from "../html.ts"; |
| 9 | 9 | |
| 10 | 10 | export function vitalsTable(looks: readonly LookVitals[]): Html { |
| 11 | 11 | const groups = vitalsRows(looks); |
| 12 | 12 | if (groups.length === 0) return html``; |
| 13 | const sideways = sidewaysLine(groups); | |
| 14 | 13 | return html`<div class="table-scroll" tabindex="0" role="region" aria-label="Web vitals per look"> |
| 15 | 14 | <table class="table" data-dense> |
| 16 | <thead><tr><th>Look</th><th>Vital</th><th>Value</th><th>Thresholds</th><th>Grade</th><th class="num">Screens</th></tr></thead> | |
| 15 | <thead><tr><th>Look</th><th>Vital</th><th>Value</th><th>Thresholds</th><th>Grade</th></tr></thead> | |
| 17 | 16 | <tbody> |
| 18 | 17 | ${groups.map((g) => |
| 19 | 18 | g.skipped !== "" |
| 20 | ? html`<tr><th scope="row">${g.look}</th><td colspan="5">skipped: ${g.skipped}</td></tr>` | |
| 19 | ? html`<tr><th scope="row">${g.look}</th><td colspan="4">skipped: ${g.skipped}</td></tr>` | |
| 21 | 20 | : g.rows.map( |
| 22 | (r, i) => html`<tr>${i === 0 ? html`<th scope="row" rowspan="${g.rows.length}">${g.look}</th>` : ""}<td>${r.vital}</td><td>${r.value}</td><td>${r.thresholds}</td><td>${r.grade}</td>${i === 0 ? html`<td class="num" rowspan="${g.rows.length}">${g.screens}</td>` : ""}</tr>`, | |
| 21 | (r, i) => html`<tr>${i === 0 ? html`<th scope="row" rowspan="${g.rows.length}">${g.look}</th>` : ""}<td>${r.vital}</td><td>${r.value}</td><td>${r.thresholds}</td><td>${r.grade}</td></tr>`, | |
| 23 | 22 | ), |
| 24 | 23 | )} |
| 25 | 24 | </tbody> |
| 26 | 25 | </table> |
| 27 | </div>${sideways === "" ? "" : html`<p class="muted small" data-sideways>${sideways}</p>`}`; | |
| 26 | </div>`; | |
| 28 | 27 | } |
test/check-vitals-size.test.ts deleted −45
| @@ -1,45 +0,0 @@ | ||
| 1 | /** A look's recorded size: kept by the parser, shown as screens, and named when a phone look scrolls sideways (#9320). */ | |
| 2 | ||
| 3 | import { expect, test } from "bun:test"; | |
| 4 | import { parseLooks } from "../src/checks/looks.ts"; | |
| 5 | import { sidewaysLine, vitalsRows } from "../src/checks/summary.ts"; | |
| 6 | import { vitalsLine } from "../src/checks/progress.ts"; | |
| 7 | import { vitalsTable } from "../src/web/studio/vitals.ts"; | |
| 8 | ||
| 9 | const size = (scrollWidth: number, scrollHeight: number) => ({ scrollWidth, scrollHeight, innerWidth: 390, innerHeight: 800 }); | |
| 10 | ||
| 11 | test("a valid size survives parsing; a broken one is dropped", () => { | |
| 12 | const [a, b] = parseLooks([ | |
| 13 | { look: "phone", entries: {}, size: size(390, 1000) }, | |
| 14 | { look: "phone /x", entries: {}, size: { scrollWidth: "wide" } }, | |
| 15 | ]); | |
| 16 | expect(a).toMatchObject({ size: size(390, 1000) }); | |
| 17 | expect(b).toEqual({ look: "phone /x", entries: {} }); | |
| 18 | }); | |
| 19 | ||
| 20 | test("screens is scrollHeight over innerHeight to one decimal", () => { | |
| 21 | const g = vitalsRows([{ look: "phone", entries: {}, size: size(390, 2000) }, { look: "desktop", entries: {} }]); | |
| 22 | expect(g[0]!.screens).toBe("2.5"); | |
| 23 | expect(g[1]!.screens).toBe(""); | |
| 24 | expect(vitalsTable([{ look: "phone", entries: {}, size: size(390, 1000) }]).toString()).toContain(">1.3<"); | |
| 25 | }); | |
| 26 | ||
| 27 | test("the sideways line names a wide phone look and only that", () => { | |
| 28 | const wide = { look: "phone /preview-admin/sample/held", entries: {}, size: size(500, 800) }; | |
| 29 | expect(sidewaysLine(vitalsRows([wide]))).toBe("phone /preview-admin/sample/held scrolls sideways"); | |
| 30 | expect(vitalsTable([wide]).toString()).toContain("phone /preview-admin/sample/held scrolls sideways"); | |
| 31 | const fits = { look: "phone", entries: {}, size: size(390, 800) }; | |
| 32 | const desktop = { look: "desktop", entries: {}, size: size(2000, 800) }; | |
| 33 | const none = { look: "phone /y", entries: {} }; | |
| 34 | expect(sidewaysLine(vitalsRows([fits, desktop, none]))).toBe(""); | |
| 35 | expect(vitalsTable([fits, desktop, none]).toString()).not.toContain("sideways"); | |
| 36 | }); | |
| 37 | ||
| 38 | test("the terminal line carries screens and the sideways note", () => { | |
| 39 | const wide = { look: "phone /x", entries: {}, size: size(500, 1600) }; | |
| 40 | const line = vitalsLine([wide, { look: "desktop", skipped: "no browser" }]); | |
| 41 | expect(line).toContain("phone /x:"); | |
| 42 | expect(line).toContain("2.0 screens"); | |
| 43 | expect(line).toContain("desktop: skipped: no browser · phone /x scrolls sideways"); | |
| 44 | expect(vitalsLine([{ look: "phone", entries: {} }])).not.toContain("screens"); | |
| 45 | }); |
test/check-vitals.test.ts +1 −1
| @@ -32,7 +32,7 @@ describe("vitalsRows", () => { | ||
| 32 | 32 | |
| 33 | 33 | test("a look without a browser carries its reason and no rows", () => { |
| 34 | 34 | expect(vitalsRows([{ look: "mobile", skipped: "no browser" }])).toEqual([ |
| 35 | { look: "mobile", skipped: "no browser", rows: [], screens: "", sideways: false }, | |
| 35 | { look: "mobile", skipped: "no browser", rows: [] }, | |
| 36 | 36 | ]); |
| 37 | 37 | }); |
| 38 | 38 | }); |
test/looks.test.ts −5
| @@ -92,9 +92,6 @@ describe("the look", () => { | ||
| 92 | 92 | writeFileSync(input.out, input.width === 1280 ? DESKTOP_PNG : PNG); |
| 93 | 93 | // What the page recorded: a slow paint for the phone, a shift for the desktop. |
| 94 | 94 | return ok({ |
| 95 | ...(input.width === 390 | |
| 96 | ? { size: { scrollWidth: 390, scrollHeight: 1600, innerWidth: 390, innerHeight: 800 } } | |
| 97 | : {}), | |
| 98 | 95 | entries: |
| 99 | 96 | input.width === 390 |
| 100 | 97 | ? { lcp: [{ startTime: 3100, size: 10 }] } |
| @@ -297,8 +294,6 @@ describe("the look", () => { | ||
| 297 | 294 | expect(recorded).toHaveLength(2); |
| 298 | 295 | expect(recorded.map((l) => l.look).sort()).toEqual(["desktop", "phone"]); |
| 299 | 296 | const graded = vitalsRows(recorded); |
| 300 | // The phone's recorded size is stored beside its entries: two screens tall. | |
| 301 | expect(graded.find((g) => g.look === "phone")!.screens).toBe("2.0"); | |
| 302 | 297 | expect(graded.find((g) => g.look === "phone")!.rows[0]).toMatchObject({ |
| 303 | 298 | vital: "LCP", |
| 304 | 299 | value: "3100 ms", |