Fixes the squashed Explore-trends chart. Validated on staging against the mockups.
What was wrong
ComparisonChart runs Chart.js with maintainAspectRatio: false, so the canvas fills its container's height — and its own CSS comment states the container "must have a fixed height." TrendsExplorer gave .chartBox a min-height: 320px, and min-height doesn't resolve the chart wrapper's height: 100%. So Chart.js fell back to its ~150px default: the canvas rendered at 1287×150 (an 8.6:1 sliver) inside a 320px box that was mostly empty — nothing like the mockup's ~3:1 chart.
Fix
Give .chartBox a definite height (420px desktop; 360px mobile, where the school-chip legend sits above the canvas).
Validated live on staging by patching the height and forcing a resize: the canvas went from 1287×150 to 1287×392 (desktop) / 284 (mobile) — proper proportions matching the mockup, with the school lines, the dashed England average reference line, the 2019/20–2021/22 gap rendered as broken lines with the honest "DfE didn't publish school-level figures for 2021/22" note, and the year-by-year table all reading correctly.
Regression guard
An e2e assertion checks the trends canvas is taller than 220px, so the squash can't come back silently.
Note (separate issue, not this PR)
While validating I confirmed the refresh-blank bug is live on staging — on a fresh load the sections below the school bar don't render until the school set changes. That's fixed by PR #37 (refresh + dead-fetch), still pending merge. Merging #37 will make the whole compare page render on first load; this PR fixes how the chart looks once it's there.
Fixes the squashed Explore-trends chart. Validated on staging against the mockups.
## What was wrong
`ComparisonChart` runs Chart.js with `maintainAspectRatio: false`, so the canvas fills its container's height — and its own CSS comment states the container "must have a fixed height." `TrendsExplorer` gave `.chartBox` a **`min-height: 320px`**, and `min-height` doesn't resolve the chart wrapper's `height: 100%`. So Chart.js fell back to its ~150px default: the canvas rendered at **1287×150** (an 8.6:1 sliver) inside a 320px box that was mostly empty — nothing like the mockup's ~3:1 chart.
## Fix
Give `.chartBox` a definite `height` (420px desktop; 360px mobile, where the school-chip legend sits above the canvas).
Validated live on staging by patching the height and forcing a resize: the canvas went from **1287×150** to **1287×392** (desktop) / **284** (mobile) — proper proportions matching the mockup, with the school lines, the dashed **England average** reference line, the **2019/20–2021/22 gap** rendered as broken lines with the honest "DfE didn't publish school-level figures for 2021/22" note, and the year-by-year table all reading correctly.
## Regression guard
An e2e assertion checks the trends canvas is taller than 220px, so the squash can't come back silently.
## Note (separate issue, not this PR)
While validating I confirmed the **refresh-blank bug is live on staging** — on a fresh load the sections below the school bar don't render until the school set changes. That's fixed by PR #37 (refresh + dead-fetch), still pending merge. Merging #37 will make the whole compare page render on first load; this PR fixes how the chart looks once it's there.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_0146VHeLAWjDVE2B5uU67jCB
ComparisonChart runs Chart.js with maintainAspectRatio:false, so it sizes
to its container's height — which must be definite. TrendsExplorer gave
.chartBox a min-height, which doesn't resolve the chart wrapper's
height:100%, so Chart.js fell back to its ~150px default: a squashed
8.6:1 sliver that didn't match the mockups. Set a definite height (420px
desktop, 360px mobile where the chips row sits above the canvas).
Verified on staging by patching the live height: canvas went from
1287x150 to 1287x392 (desktop) / 284 (mobile) — proper ~3:1 proportions
matching the mockup, with the England dashed line, COVID/2021-22 gap and
table all reading correctly.
An e2e guard asserts the trends canvas is taller than 220px so the
squash can't regress.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0146VHeLAWjDVE2B5uU67jCB
This PR fixes a previously squashed trends chart by replacing a min-height with a definite height on .chartBox (420px desktop, 360px mobile), and adds an e2e assertion that the chart canvas renders above 220px tall to catch regressions. The change is small, well-scoped, and low-risk with no backend, deploy, or data-handling impact.
✅ No issues found.
## 🤖 AI Code Review (Claude Code)
This PR fixes a previously squashed trends chart by replacing a `min-height` with a definite `height` on `.chartBox` (420px desktop, 360px mobile), and adds an e2e assertion that the chart canvas renders above 220px tall to catch regressions. The change is small, well-scoped, and low-risk with no backend, deploy, or data-handling impact.
✅ No issues found.
tudor
merged commit b89fa47ec5 into main2026-07-14 22:51:04 +00:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Fixes the squashed Explore-trends chart. Validated on staging against the mockups.
What was wrong
ComparisonChartruns Chart.js withmaintainAspectRatio: false, so the canvas fills its container's height — and its own CSS comment states the container "must have a fixed height."TrendsExplorergave.chartBoxamin-height: 320px, andmin-heightdoesn't resolve the chart wrapper'sheight: 100%. So Chart.js fell back to its ~150px default: the canvas rendered at 1287×150 (an 8.6:1 sliver) inside a 320px box that was mostly empty — nothing like the mockup's ~3:1 chart.Fix
Give
.chartBoxa definiteheight(420px desktop; 360px mobile, where the school-chip legend sits above the canvas).Validated live on staging by patching the height and forcing a resize: the canvas went from 1287×150 to 1287×392 (desktop) / 284 (mobile) — proper proportions matching the mockup, with the school lines, the dashed England average reference line, the 2019/20–2021/22 gap rendered as broken lines with the honest "DfE didn't publish school-level figures for 2021/22" note, and the year-by-year table all reading correctly.
Regression guard
An e2e assertion checks the trends canvas is taller than 220px, so the squash can't come back silently.
Note (separate issue, not this PR)
While validating I confirmed the refresh-blank bug is live on staging — on a fresh load the sections below the school bar don't render until the school set changes. That's fixed by PR #37 (refresh + dead-fetch), still pending merge. Merging #37 will make the whole compare page render on first load; this PR fixes how the chart looks once it's there.
🤖 Generated with Claude Code
https://claude.ai/code/session_0146VHeLAWjDVE2B5uU67jCB
🤖 AI Code Review (Claude Code)
This PR fixes a previously squashed trends chart by replacing a
min-heightwith a definiteheighton.chartBox(420px desktop, 360px mobile), and adds an e2e assertion that the chart canvas renders above 220px tall to catch regressions. The change is small, well-scoped, and low-risk with no backend, deploy, or data-handling impact.✅ No issues found.