Compare commits

..
Author SHA1 Message Date
TudorandClaude Fable 5 993a6133cf fix(compare): give the trend chart its own definite canvas height so the gap note can't squash it
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0146VHeLAWjDVE2B5uU67jCB
2026-07-17 22:54:17 +01:00
10 changed files with 35 additions and 197 deletions
+1 -4
View File
@@ -67,12 +67,9 @@ jobs:
echo "Promoting $FULL_SHA (images tagged $SHORT_SHA)" echo "Promoting $FULL_SHA (images tagged $SHORT_SHA)"
- name: Verify the staging E2E gate passed for this commit - name: Verify the staging E2E gate passed for this commit
# Use the built-in Actions token (GITHUB_TOKEN is the documented name;
# it carries repository read scope), NOT REGISTRY_TOKEN — the registry
# token has no repo scope, so the commit-status API returns 403.
run: | run: |
STATUS_JSON=$(curl -fsS \ STATUS_JSON=$(curl -fsS \
-H "Authorization: token ${{ secrets.GITHUB_TOKEN }}" \ -H "Authorization: token ${{ secrets.REGISTRY_TOKEN }}" \
"https://${REGISTRY}/api/v1/repos/${{ gitea.repository }}/commits/${{ steps.resolve.outputs.full }}/status") "https://${REGISTRY}/api/v1/repos/${{ gitea.repository }}/commits/${{ steps.resolve.outputs.full }}/status")
echo "$STATUS_JSON" | python3 -c " echo "$STATUS_JSON" | python3 -c "
import json, sys import json, sys
@@ -1,65 +0,0 @@
/**
* Secondary academics: every headline number carries its England anchor and a
* verdict chip (expert sign-off SF1 — the grade-5 and EBacc rows previously
* rendered as bare numbers, breaking the "anchored against England" promise).
*/
import { render, screen, within } from '@testing-library/react';
import { CompareAcademics } from '@/components/compare/CompareAcademics';
import type { ComparisonData, NationalAverages, School } from '@/lib/types';
function school(urn: number, name: string): School {
return { urn, school_name: name, attainment_8_score: 58.7 } as School;
}
function data(urn: number): ComparisonData {
return {
school_info: school(urn, 'Test High'),
yearly_data: [
{
year: 202425,
attainment_8_score: 58.7,
english_maths_strong_pass_pct: 30,
ebacc_entry_pct: 10,
},
] as ComparisonData['yearly_data'],
ofsted: null,
census: null,
admissions: null,
admissions_history: [],
deprivation: null,
};
}
const NATIONAL: NationalAverages = {
year: 202425,
primary: {},
secondary: {
attainment_8_score: 46.0,
english_maths_strong_pass_pct: 45.4,
ebacc_entry_pct: 40.5,
},
by_year: [],
};
test('grade-5 and EBacc rows show the England anchor and a Below chip when under it', () => {
const s = school(137086, 'Bishop Stopford School');
render(
<CompareAcademics
schools={[s]}
data={{ '137086': data(137086) }}
nationalAverages={NATIONAL}
isSecondary
/>,
);
// The official anchors appear (45.4% and 40.5%), not just the school numbers.
expect(screen.getByText(/England average 45%/)).toBeInTheDocument();
expect(screen.getByText(/England average 41%/)).toBeInTheDocument();
// 30% grade-5 and 10% EBacc are both well below their anchors → Below chips.
// Attainment 8 (58.7 vs 46.0) is above → at least one "Above" chip too.
expect(screen.getAllByText(/Below England average/).length).toBeGreaterThanOrEqual(2);
expect(screen.getAllByText(/Above England average/).length).toBeGreaterThanOrEqual(1);
});
+3 -8
View File
@@ -183,15 +183,10 @@ body {
} }
/* Secondary: teal outline — supporting actions (+ Compare) */ /* Secondary: teal outline — supporting actions (+ Compare) */
/* NOTE: a duplicate `.btn` block further down this file sets `border: none`,
which wins over the base `.btn`'s `1px solid transparent`. The outline
variants below therefore declare the full `border` shorthand explicitly so
they don't depend on the base border-width — otherwise `border-color` alone
has no width and the outline never renders (buttons read as plain text). */
.btn-secondary { .btn-secondary {
background: transparent; background: transparent;
color: var(--accent-teal); color: var(--accent-teal);
border: 1px solid var(--accent-teal); border-color: var(--accent-teal);
} }
.btn-secondary:hover:not(:disabled) { .btn-secondary:hover:not(:disabled) {
background: var(--accent-teal-bg); background: var(--accent-teal-bg);
@@ -201,7 +196,7 @@ body {
.btn-tertiary { .btn-tertiary {
background: var(--bg-secondary); background: var(--bg-secondary);
color: var(--text-secondary); color: var(--text-secondary);
border: 1px solid var(--border-color); border-color: var(--border-color);
} }
.btn-tertiary:hover:not(:disabled) { .btn-tertiary:hover:not(:disabled) {
background: var(--border-color); background: var(--border-color);
@@ -212,7 +207,7 @@ body {
.btn-active { .btn-active {
background: var(--accent-teal-bg); background: var(--accent-teal-bg);
color: var(--accent-teal); color: var(--accent-teal);
border: 1px solid var(--accent-teal); border-color: var(--accent-teal);
} }
.btn-active:hover:not(:disabled) { .btn-active:hover:not(:disabled) {
background: transparent; background: transparent;
+1 -7
View File
@@ -123,13 +123,7 @@
.modal { .modal {
width: 100%; width: 100%;
max-width: 100%; max-width: 100%;
/* Bottom sheet sized against the overlay (which tracks the visual max-height: 95vh;
viewport), NOT vh: when the keyboard is open the overlay is short, so
max-height:100% keeps the whole sheet — input and results — above the
keyboard. min-height gives a comfortable default without a tiny stub,
but is capped at 100% so it never exceeds the visible area. */
min-height: min(55vh, 100%);
max-height: 100%;
border-bottom-left-radius: 0; border-bottom-left-radius: 0;
border-bottom-right-radius: 0; border-bottom-right-radius: 0;
animation: slideUp 0.3s ease; animation: slideUp 0.3s ease;
+2 -30
View File
@@ -5,7 +5,7 @@
'use client'; 'use client';
import { useEffect, useCallback, useRef } from 'react'; import { useEffect, useCallback } from 'react';
import { createPortal } from 'react-dom'; import { createPortal } from 'react-dom';
import styles from './Modal.module.css'; import styles from './Modal.module.css';
@@ -18,8 +18,6 @@ interface ModalProps {
} }
export function Modal({ isOpen, onClose, children, title, size = 'medium' }: ModalProps) { export function Modal({ isOpen, onClose, children, title, size = 'medium' }: ModalProps) {
const overlayRef = useRef<HTMLDivElement>(null);
const handleEscape = useCallback((e: KeyboardEvent) => { const handleEscape = useCallback((e: KeyboardEvent) => {
if (e.key === 'Escape') { if (e.key === 'Escape') {
onClose(); onClose();
@@ -41,32 +39,6 @@ export function Modal({ isOpen, onClose, children, title, size = 'medium' }: Mod
}; };
}, [isOpen, handleEscape]); }, [isOpen, handleEscape]);
// Pin the overlay to the VISUAL viewport, not the layout viewport. On mobile
// the on-screen keyboard shrinks the visual viewport but not the layout one,
// so a `position: fixed; inset: 0` overlay keeps full height — leaving the
// bottom-anchored sheet (and the dim backdrop's lower half) hidden behind
// the keyboard. Tracking visualViewport.height/offsetTop keeps the whole
// overlay — backdrop and sheet — inside the visible area, above the keyboard.
useEffect(() => {
if (!isOpen) return;
const vv = typeof window !== 'undefined' ? window.visualViewport : null;
const el = overlayRef.current;
if (!vv || !el) return;
const sync = () => {
el.style.top = `${vv.offsetTop}px`;
el.style.height = `${vv.height}px`;
el.style.bottom = 'auto';
};
sync();
vv.addEventListener('resize', sync);
vv.addEventListener('scroll', sync);
return () => {
vv.removeEventListener('resize', sync);
vv.removeEventListener('scroll', sync);
};
}, [isOpen]);
if (!isOpen || typeof window === 'undefined') return null; if (!isOpen || typeof window === 'undefined') return null;
const handleOverlayClick = (e: React.MouseEvent) => { const handleOverlayClick = (e: React.MouseEvent) => {
@@ -76,7 +48,7 @@ export function Modal({ isOpen, onClose, children, title, size = 'medium' }: Mod
}; };
return createPortal( return createPortal(
<div ref={overlayRef} className={styles.overlay} onClick={handleOverlayClick}> <div className={styles.overlay} onClick={handleOverlayClick}>
<div className={`${styles.modal} ${styles[size]}`}> <div className={`${styles.modal} ${styles[size]}`}>
<div className={styles.header}> <div className={styles.header}>
{title && <h2 className={styles.title}>{title}</h2>} {title && <h2 className={styles.title}>{title}</h2>}
@@ -2,6 +2,14 @@
padding: 1.5rem; 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 { .warning {
background: var(--accent-gold-bg); background: var(--accent-gold-bg);
border: 1px solid var(--accent-gold, #c9a227); border: 1px solid var(--accent-gold, #c9a227);
@@ -111,16 +119,12 @@
font-size: 1rem; font-size: 1rem;
font-weight: 600; font-weight: 600;
color: var(--text-primary, #1a1612); color: var(--text-primary, #1a1612);
margin-bottom: 0.25rem; margin-bottom: 0.5rem;
white-space: nowrap; white-space: nowrap;
overflow: hidden; overflow: hidden;
text-overflow: ellipsis; text-overflow: ellipsis;
} }
.resultButton {
flex: 0 0 auto;
}
.schoolMeta { .schoolMeta {
display: flex; display: flex;
gap: 1rem; gap: 1rem;
@@ -155,30 +159,21 @@
padding: 1rem; padding: 1rem;
} }
/* One scroll container on mobile: the modal content itself scrolls, so the .title {
results list must not add its own inner scroll (double scrollbars, and font-size: 1.25rem;
the input would be trapped above a short 400px window when the keyboard
shrinks the sheet). */
.results {
max-height: none;
overflow: visible;
} }
/* 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 { .resultItem {
flex-direction: column; flex-direction: column;
align-items: stretch; align-items: stretch;
gap: 0.625rem;
padding: 0.875rem;
} }
.resultButton { .addButton {
width: 100%; width: 100%;
} }
.schoolMeta { .schoolMeta {
flex-wrap: wrap; flex-direction: column;
gap: 0.25rem 1rem; gap: 0.25rem;
} }
} }
+5 -3
View File
@@ -81,8 +81,10 @@ export function SchoolSearchModal({ isOpen, onClose }: SchoolSearchModalProps) {
}; };
return ( return (
<Modal isOpen={isOpen} onClose={handleClose} title="Add School to Comparison"> <Modal isOpen={isOpen} onClose={handleClose}>
<div className={styles.modalContent}> <div className={styles.modalContent}>
<h2 className={styles.title}>Add School to Comparison</h2>
{!canAddMore && ( {!canAddMore && (
<div className={styles.warning}> <div className={styles.warning}>
Maximum 5 schools can be compared. Remove a school to add another. Maximum 5 schools can be compared. Remove a school to add another.
@@ -127,9 +129,9 @@ export function SchoolSearchModal({ isOpen, onClose }: SchoolSearchModalProps) {
<button <button
onClick={() => handleAddSchool(school)} onClick={() => handleAddSchool(school)}
disabled={alreadySelected || !canAddMore} disabled={alreadySelected || !canAddMore}
className={`${styles.resultButton} ${ className={
alreadySelected ? "btn btn-active" : "btn btn-secondary" alreadySelected ? "btn btn-active" : "btn btn-secondary"
}`} }
> >
{alreadySelected ? "✓ Comparing" : "+ Compare"} {alreadySelected ? "✓ Comparing" : "+ Compare"}
</button> </button>
@@ -162,21 +162,6 @@ export function CompareAcademics({
const grade5 = latestValues(data, urns, 'english_maths_strong_pass_pct'); const grade5 = latestValues(data, urns, 'english_maths_strong_pass_pct');
const ebacc = latestValues(data, urns, 'ebacc_entry_pct'); const ebacc = latestValues(data, urns, 'ebacc_entry_pct');
const att8Anchor = nationalAverages?.secondary?.attainment_8_score; const att8Anchor = nationalAverages?.secondary?.attainment_8_score;
const grade5Anchor = nationalAverages?.secondary?.english_maths_strong_pass_pct;
const ebaccAnchor = nationalAverages?.secondary?.ebacc_entry_pct;
// Every headline number gets its England anchor + verdict chip, so the
// "anchored against the England average" promise holds for the grade-5
// and EBacc rows too, not just Attainment 8.
const anchorChip = (value: number | null, anchor: number | null | undefined, tol: number) => {
if (value == null || anchor == null) return null;
const v = verdict(value, anchor, tol);
return (
<Chip tone={v === 'above' ? 'good' : v === 'below' ? 'warn' : 'neutral'}>
{v === 'above' ? 'Above' : v === 'below' ? 'Below' : 'Close to'} England average
</Chip>
);
};
return ( return (
<Section <Section
@@ -189,8 +174,7 @@ export function CompareAcademics({
<Cell key={school.urn} school={school} index={i}> <Cell key={school.urn} school={school} index={i}>
{att8[i] != null ? ( {att8[i] != null ? (
<> <>
<span className={s.big}>{(att8[i] as number).toFixed(1)}</span>{' '} <span className={s.big}>{(att8[i] as number).toFixed(1)}</span>
{anchorChip(att8[i], att8Anchor, 2)}
{att8Anchor != null && ( {att8Anchor != null && (
<span className={s.small}>England average {att8Anchor.toFixed(1)}</span> <span className={s.small}>England average {att8Anchor.toFixed(1)}</span>
)} )}
@@ -232,38 +216,14 @@ export function CompareAcademics({
</RowLabel> </RowLabel>
{schools.map((school, i) => ( {schools.map((school, i) => (
<Cell key={school.urn} school={school} index={i}> <Cell key={school.urn} school={school} index={i}>
{grade5[i] != null ? ( {grade5[i] != null ? `${Math.round(grade5[i] as number)}%` : <span className={s.small}>No data</span>}
<>
<span className={s.big} style={{ fontSize: '1.1rem' }}>
{Math.round(grade5[i] as number)}%
</span>{' '}
{anchorChip(grade5[i], grade5Anchor, 3)}
{grade5Anchor != null && (
<span className={s.small}>England average {Math.round(grade5Anchor)}%</span>
)}
</>
) : (
<span className={s.small}>No data</span>
)}
</Cell> </Cell>
))} ))}
<RowLabel tip="% entering the English Baccalaureate subject combination.">EBacc entry</RowLabel> <RowLabel tip="% entering the English Baccalaureate subject combination.">EBacc entry</RowLabel>
{schools.map((school, i) => ( {schools.map((school, i) => (
<Cell key={school.urn} school={school} index={i}> <Cell key={school.urn} school={school} index={i}>
{ebacc[i] != null ? ( {ebacc[i] != null ? `${Math.round(ebacc[i] as number)}%` : <span className={s.small}>No data</span>}
<>
<span className={s.big} style={{ fontSize: '1.1rem' }}>
{Math.round(ebacc[i] as number)}%
</span>{' '}
{anchorChip(ebacc[i], ebaccAnchor, 3)}
{ebaccAnchor != null && (
<span className={s.small}>England average {Math.round(ebaccAnchor)}%</span>
)}
</>
) : (
<span className={s.small}>No data</span>
)}
</Cell> </Cell>
))} ))}
</SectionGrid> </SectionGrid>
@@ -239,17 +239,8 @@ export function CompareOfsted({
`https://reports.ofsted.gov.uk/provider/21/${school.urn}`; `https://reports.ofsted.gov.uk/provider/21/${school.urn}`;
return ( return (
<Cell key={school.urn} school={school} index={i}> <Cell key={school.urn} school={school} index={i}>
{/* Short visible label to save space on mobile (the coloured <a className={s.link} href={url} target="_blank" rel="noopener noreferrer">
school name already leads the row); the full name stays in {school.school_name}&apos;s Ofsted page
aria-label so screen readers can tell the links apart. */}
<a
className={s.link}
href={url}
target="_blank"
rel="noopener noreferrer"
aria-label={`${school.school_name}'s Ofsted page`}
>
Ofsted page
</a> </a>
</Cell> </Cell>
); );
@@ -72,18 +72,15 @@
align-items: center; align-items: center;
gap: 0.4rem; gap: 0.4rem;
flex-basis: 100%; flex-basis: 100%;
/* Slightly larger than the values below it so the school each row belongs font-size: 0.8rem;
to is easy to read on mobile (hidden on desktop, where the column header
names the school). */
font-size: 0.9rem;
font-weight: 600; font-weight: 600;
color: var(--sc, var(--text-secondary)); color: var(--sc, var(--text-secondary));
margin-bottom: 0.2rem; margin-bottom: 0.15rem;
} }
.cellDot { .cellDot {
width: 10px; width: 9px;
height: 10px; height: 9px;
border-radius: 50%; border-radius: 50%;
background: var(--dot, var(--text-muted)); background: var(--dot, var(--text-muted));
flex: none; flex: none;