Compare commits

..
Author SHA1 Message Date
TudorandClaude Fable 5 e4565e9f15 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
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
2026-07-14 23:24:39 +01:00
3 changed files with 22 additions and 22 deletions
+3 -20
View File
@@ -5,13 +5,6 @@ on:
branches:
- main
# Cancel superseded runs: pushing a new commit to a PR (or an empty
# re-trigger) aborts the previous still-running checks instead of running
# a second full matrix alongside them.
concurrency:
group: pr-checks-${{ gitea.event.pull_request.number }}
cancel-in-progress: true
env:
REGISTRY: privaterepo.sitaru.org
BACKEND_IMAGE_NAME: ${{ gitea.repository }}-backend
@@ -30,22 +23,12 @@ jobs:
uses: actions/setup-node@v4
with:
node-version: 22
# Cache the resolved node_modules (452 MB / 460 packages) keyed on the
# lockfile. On a hit — the common case, since deps change rarely — the
# whole `npm ci` step is skipped, not just its download phase. The key
# pins OS + node major so we never restore incompatible native binaries.
- name: Cache node_modules
id: node-modules-cache
uses: actions/cache@v4
with:
path: nextjs-app/node_modules
key: nextjs-node-modules-${{ runner.os }}-node22-${{ hashFiles('nextjs-app/package-lock.json') }}
cache: npm
cache-dependency-path: nextjs-app/package-lock.json
- name: Install dependencies
if: steps.node-modules-cache.outputs.cache-hit != 'true'
working-directory: nextjs-app
run: npm ci --prefer-offline --no-audit --no-fund
run: npm ci
- name: Typecheck
working-directory: nextjs-app
+6 -1
View File
@@ -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 });
@@ -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 {