fix(compare): give the trends chart a real height (was squashed to ~150px)
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 9m37s
PR Checks / Backend Smoke (pull_request) Successful in 7s
PR Checks / Build Backend (no push) (pull_request) Successful in 16s
PR Checks / Build Frontend (no push) (pull_request) Successful in 45s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 10s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 17s
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 9m37s
PR Checks / Backend Smoke (pull_request) Successful in 7s
PR Checks / Build Backend (no push) (pull_request) Successful in 16s
PR Checks / Build Frontend (no push) (pull_request) Successful in 45s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 10s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 17s
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 commit is contained in:
@@ -213,7 +213,12 @@ test('compare chart on mobile shows school chips with tap-to-focus', async ({ pa
|
|||||||
expect(bodyOverflowsX).toBe(false);
|
expect(bodyOverflowsX).toBe(false);
|
||||||
|
|
||||||
// The trends chart still renders (inside the Explore trends section)…
|
// 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.
|
// …with the mobile chart legend chips and tap-to-focus behaviour intact.
|
||||||
const chipGroup = page.getByRole('group', { name: /highlight a school/i });
|
const chipGroup = page.getByRole('group', { name: /highlight a school/i });
|
||||||
|
|||||||
@@ -60,8 +60,20 @@
|
|||||||
margin: 0 0 1rem;
|
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 {
|
.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 {
|
.tableWrapper {
|
||||||
|
|||||||
Reference in New Issue
Block a user