From 60918da483bff9d908ad3935c66c2f61a33604b4 Mon Sep 17 00:00:00 2001 From: Tudor Date: Fri, 17 Jul 2026 22:54:17 +0100 Subject: [PATCH 1/2] fix(compare): give the trend chart its own definite canvas height so the gap note can't squash it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The chart note (and the 2-row mobile chip legend) lived inside the fixed 360px/420px chartBox and competed with the canvas for it, so a longer caption — e.g. the new KS4 gap note — shrank the mobile canvas to ~178px and tripped the e2e >220px guard. The canvas now owns a definite height (280px mobile / 380px desktop) with chips above and the note flowing below at natural size, so neither can eat the plot. Verified on staging: patched canvas renders 280px vs the squashed 178px. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0146VHeLAWjDVE2B5uU67jCB --- .../components/ComparisonChart.module.css | 19 +++++++++++++------ .../compare/TrendsExplorer.module.css | 18 +++++------------- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/nextjs-app/components/ComparisonChart.module.css b/nextjs-app/components/ComparisonChart.module.css index d847f1d..43acaf0 100644 --- a/nextjs-app/components/ComparisonChart.module.css +++ b/nextjs-app/components/ComparisonChart.module.css @@ -1,16 +1,23 @@ -/* Chart wrapper: chips (mobile) above, canvas filling the rest of the - parent .chartContainer, whose fixed height drives Chart.js sizing via - maintainAspectRatio: false. */ +/* Chart wrapper: chips (mobile) above, then the canvas, then the gap note. + The canvas has its OWN definite height (Chart.js needs one for + maintainAspectRatio: false); the chips and the note flow at their natural + size around it rather than competing with it for a fixed outer height — + so a longer note (e.g. the KS4 gap caption) or a two-row chip legend can + never squash the chart. */ .wrapper { display: flex; flex-direction: column; - height: 100%; } .canvasBox { position: relative; - flex: 1 1 auto; - min-height: 0; + height: 380px; +} + +@media (max-width: 640px) { + .canvasBox { + height: 280px; + } } /* School chips: mobile-only legend + tap-to-focus control. Desktop keeps diff --git a/nextjs-app/components/compare/TrendsExplorer.module.css b/nextjs-app/components/compare/TrendsExplorer.module.css index 7c74367..e5e007b 100644 --- a/nextjs-app/components/compare/TrendsExplorer.module.css +++ b/nextjs-app/components/compare/TrendsExplorer.module.css @@ -60,18 +60,10 @@ 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. */ +/* ComparisonChart owns its own canvas height now (a definite px value per + breakpoint), with the mobile chip legend above and the gap note below it + flowing at natural size. This box therefore only needs to not constrain + that height — no fixed height, or the note would again eat the plot. */ .chartBox { - 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; - } + min-height: 0; } From e5f7f4c959f024c073472122d858333a0f24866c Mon Sep 17 00:00:00 2001 From: Tudor Date: Fri, 17 Jul 2026 23:02:12 +0100 Subject: [PATCH 2/2] fix(compare): repair the mobile add-school modal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 'add school' modal passed no title to Modal, so its header held only the close button — shoved to the far left by justify-content:space-between in an otherwise-empty bar — while the real title was rendered separately inside the content. Pass the title to Modal so the header reads title (left) + close (right), and drop the duplicate in-content heading. Also: the mobile full-width result button targeted a dead .addButton selector (the button never had that class), so it rendered inconsistently — give the button a real module class and full width on mobile, tighten the stacked cards, and give the bottom sheet a stable min-height so its empty state isn't a tiny stub. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0146VHeLAWjDVE2B5uU67jCB --- nextjs-app/components/Modal.module.css | 4 +++ .../components/SchoolSearchModal.module.css | 28 ++++++++----------- nextjs-app/components/SchoolSearchModal.tsx | 8 ++---- 3 files changed, 19 insertions(+), 21 deletions(-) diff --git a/nextjs-app/components/Modal.module.css b/nextjs-app/components/Modal.module.css index d1ecf4c..eab03ac 100644 --- a/nextjs-app/components/Modal.module.css +++ b/nextjs-app/components/Modal.module.css @@ -123,6 +123,10 @@ .modal { width: 100%; max-width: 100%; + /* Bottom sheet: open at a stable, comfortable height so the empty + "start typing" state isn't a tiny stub and the sheet doesn't jump as + results load (the results list scrolls within). */ + min-height: 55vh; max-height: 95vh; border-bottom-left-radius: 0; border-bottom-right-radius: 0; diff --git a/nextjs-app/components/SchoolSearchModal.module.css b/nextjs-app/components/SchoolSearchModal.module.css index c00e8c3..640dcea 100644 --- a/nextjs-app/components/SchoolSearchModal.module.css +++ b/nextjs-app/components/SchoolSearchModal.module.css @@ -2,14 +2,6 @@ padding: 1.5rem; } -.title { - font-size: 1.5rem; - font-weight: 700; - color: var(--text-primary, #1a1612); - margin-bottom: 1.5rem; - font-family: var(--font-playfair), 'Playfair Display', serif; -} - .warning { background: var(--accent-gold-bg); border: 1px solid var(--accent-gold, #c9a227); @@ -119,12 +111,16 @@ font-size: 1rem; font-weight: 600; color: var(--text-primary, #1a1612); - margin-bottom: 0.5rem; + margin-bottom: 0.25rem; white-space: nowrap; overflow: hidden; text-overflow: ellipsis; } +.resultButton { + flex: 0 0 auto; +} + .schoolMeta { display: flex; gap: 1rem; @@ -159,21 +155,21 @@ padding: 1rem; } - .title { - font-size: 1.25rem; - } - + /* Compact stacked card: name + meta, then a full-width action so the tap + target is obvious and the card doesn't waste vertical space. */ .resultItem { flex-direction: column; align-items: stretch; + gap: 0.625rem; + padding: 0.875rem; } - .addButton { + .resultButton { width: 100%; } .schoolMeta { - flex-direction: column; - gap: 0.25rem; + flex-wrap: wrap; + gap: 0.25rem 1rem; } } diff --git a/nextjs-app/components/SchoolSearchModal.tsx b/nextjs-app/components/SchoolSearchModal.tsx index 798c2af..e7e6d42 100644 --- a/nextjs-app/components/SchoolSearchModal.tsx +++ b/nextjs-app/components/SchoolSearchModal.tsx @@ -81,10 +81,8 @@ export function SchoolSearchModal({ isOpen, onClose }: SchoolSearchModalProps) { }; return ( - +
-

Add School to Comparison

- {!canAddMore && (
Maximum 5 schools can be compared. Remove a school to add another. @@ -129,9 +127,9 @@ export function SchoolSearchModal({ isOpen, onClose }: SchoolSearchModalProps) {