Skip to content

Commit dc152f2

Browse files
authored
Make the HTML report's tables, headings and copy consistent (#4817)
An audit of the report found the same concepts spelled several different ways across pages: numeric cells as .num/.cov-num/.number, cell tones as cell--tone-* vs cell-tone--, .responsive sometimes on the table instead of around it, listing-card titles ranging from h2 to h4, and data tables at three different cell sizes. Each divergence is invisible on its own page but makes the report read as five different tools. This standardises on one convention per concept: .number cells (with data-value so they sort numerically), cell-tone-- tones, a .responsive wrapper div that also scrolls wide tables on desktop, h3 for card titles and h2 for tab lead headings, and a shared .data-table class for the 0.95rem cell treatment. It also fixes a few real bugs the audit surfaced: the Screenshots tab id typo ("screeenshots") that broke #screenshots deep links, the wide-screen breakpoints that grew the content to 1800px and upscaled its fonts while the nav stayed at 1140px, screenshots glowing white in dark mode from hard-coded colors, and a stray global margin/padding reset living inside the tabs component. The Timings Summary Web Vitals medians were the only vitals in the report with no good/poor signal — they now carry the same status dots as the pages table. Co-authored-by: Claude Fable 5 noreply@anthropic.com
1 parent 242970b commit dc152f2

40 files changed

Lines changed: 255 additions & 220 deletions

AGENTS.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,9 @@ The report has accumulated several non-obvious behaviours that an agent should k
182182
- **CrUx field data exposes two shapes.** `crux/repackage.js` produces flat per-metric keys (`loadingExperience.ALL.LARGEST_CONTENTFUL_PAINT_MS.p75`, etc.) **and** keeps the raw API response under `loadingExperience.ALL.data.record.metrics.<snake_case>.percentiles.p75`. Each flat key only exists if that metric was in the CrUx response, so guard before reading. The summary template reads the raw `.data` path; the Web Vitals lab-vs-field line uses the flat keys.
183183
- **The Timings Summary table's markup leans on HTML parser error recovery.** In `url/summary/summaryBox.pug` the `getRow` mixin emits the min/median tds nested *inside* the metric-name `td` (the `each`/`if` blocks are indented under it) — the browser's tree builder hoists td-inside-td into siblings, so the rendered table is fine but the source order isn't the DOM order. Consequence: style its columns via classes / `td[data-title=…]` attribute selectors (scoped under `.timings-table`), never `nth-child`. When `iterations` is even the median is computed (average of the two middle runs) so it links to no run — that branch renders a plain cell and needs its own `data-title`/class.
184184
- **The Domains page ranks by a derived time footprint, and its inputs are in the summary-page locals.** `topDomains` (built in `lib/plugins/html/index.js` from the `domains.summary` message, capped at 200) carries per-domain `summarizeStats` objects (`median/mean/min/p90/max`, plus `sum` only on `transferSize`) — any phase can be missing entirely when every HAR timing was `-1`, so cells must guard and still emit `data-value="0"` (sortable.min.js reads `data-value` before text; a bare `—` in the first row would flip the whole column to alpha sorting). Rows are pre-sorted by `requestCount × totalTime.median` — a deliberate proxy for "which domain cost the most wall-clock", documented in the template. First/third-party marking prefers `options.firstParty` and falls back to hostnames of the tested pages via `headers.pages.pages` (the summary-page locals include `headers = this.summary`, so top-level pages CAN see the tested URLs — no extra plumbing needed). Two pug gotchas learned here: mixins compile as separate functions and cannot see `- const` helpers declared in `block content` (pass them as mixin arguments, like the existing `transferMax`), and browser globals/builtins (`URL`, `Set`, `RegExp`) DO resolve inside templates — pug's `with` wrapper falls back to `typeof X !== "undefined" ? X : undefined`, which is why `Math` already worked.
185-
- **The Google Web Vitals hero is one shared partial.** `lib/plugins/html/templates/url/metrics/index.pug` builds the `wvTiles` scoreboard and is `include`d by both `url/summary/index.pug` and `url/iteration/index.pug`. Edit it once; it renders on both the median-summary and per-run pages. The Web Vitals thresholds (`gwvLcpClass`, … and `wvStatusLabel`) are defined inline near the top of that file — reuse them rather than re-deriving cutoffs.
185+
- **The Google Web Vitals hero is one shared partial.** `lib/plugins/html/templates/url/metrics/index.pug` builds the `wvTiles` scoreboard and is `include`d by both `url/summary/index.pug` and `url/iteration/index.pug`. Edit it once; it renders on both the median-summary and per-run pages. The Web Vitals thresholds (`gwvLcpClass`, … and `wvStatusLabel`) are defined inline near the top of that file — reuse them rather than re-deriving cutoffs. BUT: those helpers are assigned without `const` inside `if browsertime`, and pug inlines every include of a page into one function scope. Declaring `const gwvLcpClass` in another include of the same page (e.g. `summaryBox.pug`, which renders BEFORE the metrics include executes) would make the later plain assignment throw "Assignment to constant variable" — that's why `summaryBox.pug` carries its own `sb*`-prefixed copies with a keep-in-sync comment, same as `pages.pug` (a separate compile unit) keeps `gwv*` copies.
186+
- **Report table conventions.** Numeric cells/headers use the foundation `td.number`/`th.number` (right-aligned; give sortable ones `data-value` so sortable.min.js sorts numerically — `+numberCell` in `_tableMixins.pug` does). `.responsive` is a WRAPPER DIV around a table, never a class on the table itself: the foundation gives it `overflow-x: auto` on desktop and Chris Coyier row-stacking below 800px. Data-heavy listing tables (pages, timings, assets, toplist, detailed, domains) share the 0.95rem cell treatment — new ones should add the `.data-table` class (foundations/tables.scss) instead of re-declaring the size. Cell tone classes are spelled `cell-tone--warning` / `cell-tone--error` (toplist/domains/assets all agree now). `.listing-card-title` is `h3` everywhere (its size comes from the class, not the element); tab lead headings ("Waterfall", "Coverage", "PageXray", "CPU") are `h2`.
187+
- **`.loader` looks dead but isn't.** `components/loader.scss` styles the spinner `fetchHAR.pug` shows while the waterfall fetches the HAR (`_waterfallRender.pug` removes the class when loaded). Don't delete it on an "unused CSS" sweep. Also: `summaryBox.pug` contains a non-breaking space in `if min || min === 0` (valid JS whitespace, invisible in editors) — exact-match tooling that types a normal space won't find the line.
186188
- **The start page is a scorecard ("every page is a dot") with a boxes fallback.** `templates/index.pug` reads the site-wide summary stats through the locals that `htmlBuilder` merges in from `dataCollector.getSummary('index')` — they surface as top-level `browsertime`/`pagexray`/`coach`/`axe`/`sustainable` locals, each `{ summary: … }`, and any of them can be missing (pug resolves unknown identifiers to `locals.*`, so plain truthiness guards work). CrUx data is deliberately NOT on the start page — it is per-URL/origin field data and belongs on the page reports. The per-page dots and the Page-by-page list come from the `scorecardPages` local computed in `htmlBuilder.js`. In scorecard mode summary boxes are deliberately NOT rendered (no letter grades either — they collide with WebPageTest/GTmetrix); only when there is no browsertime summary or no gradeable subject does the template fall back to the classic hero+boxes grid. The subject value column shows median + worst (the stat objects carry `min/median/p90/max`) because the extreme page dot IS that value; the Coach performance score is a higher-is-better subject (`hib` flag) — grading inverts, its track zones are mirrored and worst is the MIN. Note the FCP summary stat lives at `summary.paintTiming['first-contentful-paint']` (top level, not under `timings.`) and needs bracket access — `objectPath.get` splits on dots only.
187189
- **Summary boxes are no longer user-configurable.** `--html.summaryBoxes` / `--html.summaryBoxesThresholds` were removed in the major after the scorecard redesign made them fallback-only. The box set and thresholds now come exclusively from `defaultConfig.js` and `setup/summaryBoxesDefaultLimits.js`; `setup/summaryBoxes.js` imports both directly instead of reading `options.html`. Note WHY it can't read `options.html`: the html plugin's `open()` does a SHALLOW `Object.assign({}, defaultConfig, options)` and yargs always materialises `options.html`, so `defaultConfig.html.*` never survives the merge — defaults that must always apply have to be imported at the point of use. The text plugin's terminal summary shares `summaryBoxesSetup` and therefore the same defaults.
188190
- **Division after `]` breaks pug attribute expressions.** Inside a tag attribute, `style='width:' + m[1] / mmax + '%'` makes pug's character-parser treat `/` as a regex start ("no closing bracket found"). Compute the value in a `- const …` line first and interpolate the variable. Also remember mixins compile to separate functions: they see locals but NOT consts/functions defined in `block content`, so pass everything a mixin needs as arguments.

lib/plugins/html/assets/css/index-dark.min.css

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/plugins/html/assets/css/index-light.min.css

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/plugins/html/src/sass/dark/components/assets.scss

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,14 +42,14 @@
4242
}
4343

4444
table#assets {
45-
td.cell--tone-warning,
46-
td.cell--tone-warning .cell-bar-text {
45+
td.cell-tone--warning,
46+
td.cell-tone--warning .cell-bar-text {
4747
color: $color--warning;
4848
font-weight: $font--weight--semibold;
4949
}
5050

51-
td.cell--tone-error,
52-
td.cell--tone-error .cell-bar-text {
51+
td.cell-tone--error,
52+
td.cell-tone--error .cell-bar-text {
5353
color: $color--error;
5454
font-weight: $font--weight--semibold;
5555
}

lib/plugins/html/src/sass/dark/components/coverage.scss

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@
4646
vertical-align: middle;
4747
}
4848

49-
.cov-num {
49+
.number {
5050
text-align: right;
5151
font-variant-numeric: tabular-nums;
5252
white-space: nowrap;

lib/plugins/html/src/sass/dark/components/metricsCards.scss

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ $loaf--style-layout: #a78bfa;
211211
vertical-align: top;
212212
}
213213

214-
td.num {
214+
td.number {
215215
font-variant-numeric: tabular-nums;
216216
white-space: nowrap;
217217
}

lib/plugins/html/src/sass/dark/components/navigation.scss

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@
6161
}
6262
.navgrid {
6363
width: 100%;
64-
max-width: 1140px;
64+
max-width: 1240px;
6565
min-width: 755px;
6666
margin: 0 auto;
6767
overflow: hidden;

lib/plugins/html/src/sass/dark/components/screenshot.scss

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
.screenshot {
77
padding: 4px;
8-
background-color: #fff;
9-
border: 1px solid #ddd;
8+
background-color: $color--surface-card;
9+
border: 1px solid $color--border;
1010
border-radius: 4px;
1111
}

lib/plugins/html/src/sass/dark/components/tabs.scss

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,6 @@ Thanks Oliver Knoblich
33
http://codepen.io/oknoblich/pen/tfjFl
44
*/
55

6-
*,
7-
*:before,
8-
*:after {
9-
margin: 0;
10-
padding: 0;
11-
box-sizing: border-box;
12-
}
13-
146
section {
157
display: block;
168
padding: 24px 0 0;

lib/plugins/html/src/sass/dark/components/timingsSummary.scss

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,3 +63,11 @@
6363
}
6464
}
6565
}
66+
67+
// The Web Vitals medians carry the same status dot as the pages table
68+
// (.pages-dot, components/pages). The dot sits before the value so the
69+
// right-aligned digits keep lining up with the rows that have no dot.
70+
.timings-table .pages-dot {
71+
display: inline-block;
72+
margin-right: 6px;
73+
}

0 commit comments

Comments
 (0)