Compare commits

..
Author SHA1 Message Date
TudorandClaude Fable 5 2fd997bfe6 fix(compare): keep the add-school sheet above the on-screen keyboard
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m6s
PR Checks / Backend Smoke (pull_request) Successful in 7s
PR Checks / Build Backend (no push) (pull_request) Successful in 11s
PR Checks / Build Frontend (no push) (pull_request) Successful in 47s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 10s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 1m31s
The bottom-sheet overlay was position:fixed inset:0, sized to the LAYOUT
viewport, so when the mobile keyboard opened it kept full height: the
sheet (anchored to the overlay's bottom) and the lower half of the dim
backdrop sat behind the keyboard, hiding the search input and results.

Track window.visualViewport (height + offsetTop) while the modal is open
and pin the overlay to the visible region, so backdrop and sheet stay
above the keyboard. Size the sheet against the overlay (max-height:100%,
min-height:min(55vh,100%)) instead of vh, and drop the results list's own
inner scroll on mobile so the sheet is a single scroll area.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0146VHeLAWjDVE2B5uU67jCB
2026-07-18 08:00:31 +01:00
tudor bdd9bef349 Merge pull request 'fix(compare): repair mobile add-school modal + re-apply stranded chart-height fix' (#56) from fix/mobile-compare-polish into main
Stage (build -> staging -> E2E gate) / Build Backend (FastAPI) (push) Successful in 13s
Stage (build -> staging -> E2E gate) / Build Frontend (Next.js) (push) Successful in 51s
Stage (build -> staging -> E2E gate) / Build Pipeline (Meltano + dbt + Airflow) (push) Successful in 13s
Stage (build -> staging -> E2E gate) / Deploy to Staging (push) Successful in 1s
Stage (build -> staging -> E2E gate) / E2E Journeys against Staging (push) Successful in 41s
Reviewed-on: #56
2026-07-17 22:11:45 +00:00
TudorandClaude Fable 5 e5f7f4c959 fix(compare): repair the mobile add-school modal
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m6s
PR Checks / Backend Smoke (pull_request) Successful in 6s
PR Checks / Build Backend (no push) (pull_request) Successful in 10s
PR Checks / Build Frontend (no push) (pull_request) Successful in 48s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 10s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 52s
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0146VHeLAWjDVE2B5uU67jCB
2026-07-17 23:02:12 +01:00
TudorandClaude Fable 5 60918da483 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:58:35 +01:00
tudor 284215fbce Merge pull request 'fix(compare): expert should-fixes S1-S4, S6 (S5/S7 closed by verification)' (#55) from fix/expert-signoff-shouldfixes into main
Stage (build -> staging -> E2E gate) / Build Backend (FastAPI) (push) Successful in 13s
Stage (build -> staging -> E2E gate) / Build Frontend (Next.js) (push) Successful in 53s
Stage (build -> staging -> E2E gate) / Build Pipeline (Meltano + dbt + Airflow) (push) Successful in 14s
Stage (build -> staging -> E2E gate) / Deploy to Staging (push) Successful in 1s
Stage (build -> staging -> E2E gate) / E2E Journeys against Staging (push) Failing after 45s
Reviewed-on: #55
2026-07-17 17:53:16 +00:00
4 changed files with 59 additions and 22 deletions
+7 -1
View File
@@ -123,7 +123,13 @@
.modal {
width: 100%;
max-width: 100%;
max-height: 95vh;
/* Bottom sheet sized against the overlay (which tracks the visual
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-right-radius: 0;
animation: slideUp 0.3s ease;
+30 -2
View File
@@ -5,7 +5,7 @@
'use client';
import { useEffect, useCallback } from 'react';
import { useEffect, useCallback, useRef } from 'react';
import { createPortal } from 'react-dom';
import styles from './Modal.module.css';
@@ -18,6 +18,8 @@ interface ModalProps {
}
export function Modal({ isOpen, onClose, children, title, size = 'medium' }: ModalProps) {
const overlayRef = useRef<HTMLDivElement>(null);
const handleEscape = useCallback((e: KeyboardEvent) => {
if (e.key === 'Escape') {
onClose();
@@ -39,6 +41,32 @@ export function Modal({ isOpen, onClose, children, title, size = 'medium' }: Mod
};
}, [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;
const handleOverlayClick = (e: React.MouseEvent) => {
@@ -48,7 +76,7 @@ export function Modal({ isOpen, onClose, children, title, size = 'medium' }: Mod
};
return createPortal(
<div className={styles.overlay} onClick={handleOverlayClick}>
<div ref={overlayRef} className={styles.overlay} onClick={handleOverlayClick}>
<div className={`${styles.modal} ${styles[size]}`}>
<div className={styles.header}>
{title && <h2 className={styles.title}>{title}</h2>}
@@ -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,30 @@
padding: 1rem;
}
.title {
font-size: 1.25rem;
/* One scroll container on mobile: the modal content itself scrolls, so the
results list must not add its own inner scroll (double scrollbars, and
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 {
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;
}
}
+3 -5
View File
@@ -81,10 +81,8 @@ export function SchoolSearchModal({ isOpen, onClose }: SchoolSearchModalProps) {
};
return (
<Modal isOpen={isOpen} onClose={handleClose}>
<Modal isOpen={isOpen} onClose={handleClose} title="Add School to Comparison">
<div className={styles.modalContent}>
<h2 className={styles.title}>Add School to Comparison</h2>
{!canAddMore && (
<div className={styles.warning}>
Maximum 5 schools can be compared. Remove a school to add another.
@@ -129,9 +127,9 @@ export function SchoolSearchModal({ isOpen, onClose }: SchoolSearchModalProps) {
<button
onClick={() => handleAddSchool(school)}
disabled={alreadySelected || !canAddMore}
className={
className={`${styles.resultButton} ${
alreadySelected ? "btn btn-active" : "btn btn-secondary"
}
}`}
>
{alreadySelected ? "✓ Comparing" : "+ Compare"}
</button>