From e4565e9f158721d4df6b918f2b065de82851f8d9 Mon Sep 17 00:00:00 2001 From: Tudor Date: Tue, 14 Jul 2026 23:24:39 +0100 Subject: [PATCH] fix(compare): give the trends chart a real height (was squashed to ~150px) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_0146VHeLAWjDVE2B5uU67jCB --- e2e/tests/journeys.spec.ts | 7 ++++++- .../components/compare/TrendsExplorer.module.css | 14 +++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/e2e/tests/journeys.spec.ts b/e2e/tests/journeys.spec.ts index 936dfe2..b0a9e96 100644 --- a/e2e/tests/journeys.spec.ts +++ b/e2e/tests/journeys.spec.ts @@ -213,7 +213,12 @@ test('compare chart on mobile shows school chips with tap-to-focus', async ({ pa expect(bodyOverflowsX).toBe(false); // The trends chart still renders (inside the Explore trends section)… - await expect(page.locator('canvas:visible').first()).toBeVisible({ timeout: 15_000 }); + const chartCanvas = page.locator('canvas:visible').first(); + await expect(chartCanvas).toBeVisible({ timeout: 15_000 }); + // …at a real height, not the squashed ~150px Chart.js fallback that + // appears when the container lacks a definite height. + const chartBox = await chartCanvas.boundingBox(); + expect(chartBox && chartBox.height).toBeGreaterThan(220); // …with the mobile chart legend chips and tap-to-focus behaviour intact. const chipGroup = page.getByRole('group', { name: /highlight a school/i }); diff --git a/nextjs-app/components/compare/TrendsExplorer.module.css b/nextjs-app/components/compare/TrendsExplorer.module.css index d6ddd4d..61f17fa 100644 --- a/nextjs-app/components/compare/TrendsExplorer.module.css +++ b/nextjs-app/components/compare/TrendsExplorer.module.css @@ -60,8 +60,20 @@ margin: 0 0 1rem; } +/* ComparisonChart runs Chart.js with maintainAspectRatio:false, so it fills + its container's height — which must be *definite*. A min-height alone does + not resolve the chart wrapper's height:100%, leaving Chart.js to fall back + to its ~150px default (a squashed sliver). Give it a real height. */ .chartBox { - min-height: 320px; + height: 420px; +} + +@media (max-width: 640px) { + /* Taller on mobile: the mobile-only school chips sit above the canvas and + wrap to two rows for 3+ schools, so the plot keeps a usable height. */ + .chartBox { + height: 360px; + } } .tableWrapper {