Reported: "Nearby schools" missing from the mobile "Jump to section" menu.
It was there — underneath the fixed bottom tab bar, painted over and
untappable.
Diagnosis, on staging
The sticky section nav sets position: sticky with z-index: 10, which makes
it a stacking context. The sheet's own z-index: 1600 therefore only orders
it inside that context. Against the tab bar (Navigation.module.css, z-index: 1000) the competing value is the nav's 10, so the bar wins.
Asking elementFromPoint at the centre of each menu item on staging, at 390px:
Item
Topmost element at its centre
Ofsted
the menu item
Results
the menu item
Admissions
the menu item
Pupils
the menu item
History
the menu item
Nearby schools
the bottom tab bar
Injecting z-index: 1100 onto the nav on that same live page makes all six
reachable, which is what this change does properly.
Latent, not new
With five sections the list stopped just above the bar. "Nearby schools" made
six, and the sixth is the first to reach it — so any section added later would
have hit this. Worth knowing when the next one lands.
The fix
The nav is lifted only while the sheet is open, and only to 1100: above the
bar, below the comparison toast (2000), the fullscreen map (5000) and the info
popover (9999). The backdrop rises with it, so tapping over the bar now
dismisses the sheet rather than navigating away.
The journey
It asks what a thumb asks — for each item, whether it is the topmost element at
its own centre. A bounding-box or visibility check cannot catch this: the item
is in the viewport, the right size, and toBeVisible() passes. It is simply
underneath something.
Confirmed to fail against current staging before the fix, naming ["Nearby schools"], so it is testing the real thing rather than passing
vacuously.
Verification
the mechanism and the fix both verified against live staging by injection
the new journey fails on staging as it stands today, for the right reason
frontend: 475 passed, tsc --noEmit clean
The fix itself cannot be proven end-to-end until it is deployed; the post-merge
staging run is the evidence.
Reported: "Nearby schools" missing from the mobile "Jump to section" menu.
It was there — underneath the fixed bottom tab bar, painted over and
untappable.
## Diagnosis, on staging
The sticky section nav sets `position: sticky` with `z-index: 10`, which makes
it **a stacking context**. The sheet's own `z-index: 1600` therefore only orders
it *inside* that context. Against the tab bar (`Navigation.module.css`,
`z-index: 1000`) the competing value is the nav's `10`, so the bar wins.
Asking `elementFromPoint` at the centre of each menu item on staging, at 390px:
| Item | Topmost element at its centre |
|---|---|
| Ofsted | the menu item |
| Results | the menu item |
| Admissions | the menu item |
| Pupils | the menu item |
| History | the menu item |
| **Nearby schools** | **the bottom tab bar** |
Injecting `z-index: 1100` onto the nav on that same live page makes all six
reachable, which is what this change does properly.
## Latent, not new
With five sections the list stopped just above the bar. "Nearby schools" made
six, and the sixth is the first to reach it — so any section added later would
have hit this. Worth knowing when the next one lands.
## The fix
The nav is lifted only while the sheet is open, and only to `1100`: above the
bar, below the comparison toast (2000), the fullscreen map (5000) and the info
popover (9999). The backdrop rises with it, so tapping over the bar now
dismisses the sheet rather than navigating away.
## The journey
It asks what a thumb asks — for each item, whether it is the topmost element at
its own centre. A bounding-box or visibility check cannot catch this: the item
is in the viewport, the right size, and `toBeVisible()` passes. It is simply
underneath something.
**Confirmed to fail against current staging before the fix**, naming
`["Nearby schools"]`, so it is testing the real thing rather than passing
vacuously.
## Verification
- the mechanism and the fix both verified against live staging by injection
- the new journey fails on staging as it stands today, for the right reason
- frontend: 475 passed, `tsc --noEmit` clean
The fix itself cannot be proven end-to-end until it is deployed; the post-merge
staging run is the evidence.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
On mobile the last item in "Jump to section" was painted over by the fixed
bottom tab bar and could not be tapped. Reported as nearby schools missing
from the menu; it was there, underneath the bar.
The sticky nav sets `position: sticky` with `z-index: 10`, which makes it a
stacking context. The sheet's own `z-index: 1600` therefore orders it only
inside that context — against the tab bar (z-index 1000) the nav's 10 is what
counts, so the bar wins. Verified on staging: every menu item returns itself
from elementFromPoint except the last, which returns the tab bar.
Latent rather than new. With five sections the list stopped just above the bar;
"Nearby schools" made six, and the sixth is the first to reach it. Any section
added later would have done the same.
Lifted only while the sheet is open, and only to 1100 — above the bar, below
the comparison toast (2000), the fullscreen map (5000) and the info popover
(9999). The backdrop rises with it, so tapping over the bar now dismisses the
sheet instead of navigating away.
The journey asks what a thumb asks: for each item, whether it is the topmost
element at its own centre. A bounding-box check cannot see this — the item is
in the viewport and the right size, just underneath something. Confirmed to
fail against current staging, naming "Nearby schools", before the fix.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The PR adds an e2e test to check that jump-sheet menu items aren't covered by the bottom tab bar, and attempts to fix the underlying stacking-context bug by raising the sticky nav's z-index while the sheet is open. However, the CSS edit accidentally moved layout-critical declarations from the base .sectionNav rule into the new conditional .sectionNavSheetOpen class instead of copying them, breaking the nav's normal appearance.
🔴 Severe (blocks merge)
nextjs-app/components/school/SchoolDetailShell.module.css: The refactor closes .sectionNav right after padding and moves margin-bottom, box-shadow, display: flex, align-items, and gap into the new .sectionNavSheetOpen rule instead of duplicating them. Since sectionNavSheetOpen is only applied to the <nav> when the mobile jump sheet is open (SchoolDetailShell.tsx), the sticky section nav loses its flex layout, spacing, and shadow in its normal (closed) state on every school detail page — this is the default state for virtually all page views, not just when the sheet is open, so it's a site-wide layout regression, not a mobile-only fix.
## 🤖 AI Code Review (Claude Code)
The PR adds an e2e test to check that jump-sheet menu items aren't covered by the bottom tab bar, and attempts to fix the underlying stacking-context bug by raising the sticky nav's z-index while the sheet is open. However, the CSS edit accidentally moved layout-critical declarations from the base `.sectionNav` rule into the new conditional `.sectionNavSheetOpen` class instead of copying them, breaking the nav's normal appearance.
### 🔴 Severe (blocks merge)
- **nextjs-app/components/school/SchoolDetailShell.module.css**: The refactor closes `.sectionNav` right after `padding` and moves `margin-bottom`, `box-shadow`, `display: flex`, `align-items`, and `gap` into the new `.sectionNavSheetOpen` rule instead of duplicating them. Since `sectionNavSheetOpen` is only applied to the `<nav>` when the mobile jump sheet is open (SchoolDetailShell.tsx), the sticky section nav loses its flex layout, spacing, and shadow in its normal (closed) state on every school detail page — this is the default state for virtually all page views, not just when the sheet is open, so it's a site-wide layout regression, not a mobile-only fix.
Review catch, and a bad one: the previous commit anchored its insertion on
`padding: 0.5rem 0.75rem;` and closed .sectionNav there. Everything that
followed in the rule — margin-bottom, box-shadow, display: flex, align-items,
gap — was orphaned into .sectionNavSheetOpen, which is only applied while the
mobile jump sheet is open.
So the sticky nav lost its flex layout, spacing and shadow in the closed
state, which is virtually every page view on every school detail page. A
site-wide regression introduced by a fix for one mobile menu.
Redone by anchoring on the complete rule, closing brace included, so nothing
can be orphaned. .sectionNav is now byte-identical to main and the diff is
purely additive; .sectionNavSheetOpen carries the z-index and nothing else.
The staging experiment that validated this fix set nav.style.zIndex = '1100'
with every other declaration intact, so it was always testing this version
rather than the broken one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This PR fixes a mobile bottom-sheet z-index bug: the sticky .sectionNav establishes a stacking context (via position: sticky + z-index: 10), so its position: fixed jump-sheet descendant was painted under the fixed bottom tab bar (z-index: 1000) despite its own higher z-index. The fix conditionally lifts only .sectionNav's z-index to 1100 while the sheet is open, and adds an e2e test that hit-tests each sheet item's center point via elementFromPoint to catch exactly this class of covered-element bug. The CSS cascade order and specificity were verified (no later same-specificity rule overrides the new z-index), and the fix is scoped correctly; no correctness, security, or deploy issues found.
✅ No issues found.
## 🤖 AI Code Review (Claude Code)
This PR fixes a mobile bottom-sheet z-index bug: the sticky `.sectionNav` establishes a stacking context (via `position: sticky` + `z-index: 10`), so its `position: fixed` jump-sheet descendant was painted under the fixed bottom tab bar (`z-index: 1000`) despite its own higher z-index. The fix conditionally lifts only `.sectionNav`'s z-index to 1100 while the sheet is open, and adds an e2e test that hit-tests each sheet item's center point via `elementFromPoint` to catch exactly this class of covered-element bug. The CSS cascade order and specificity were verified (no later same-specificity rule overrides the new z-index), and the fix is scoped correctly; no correctness, security, or deploy issues found.
✅ No issues found.
tudor
merged commit 271ffe92d4 into main2026-09-22 19:55:00 +00:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Reported: "Nearby schools" missing from the mobile "Jump to section" menu.
It was there — underneath the fixed bottom tab bar, painted over and
untappable.
Diagnosis, on staging
The sticky section nav sets
position: stickywithz-index: 10, which makesit a stacking context. The sheet's own
z-index: 1600therefore only ordersit inside that context. Against the tab bar (
Navigation.module.css,z-index: 1000) the competing value is the nav's10, so the bar wins.Asking
elementFromPointat the centre of each menu item on staging, at 390px:Injecting
z-index: 1100onto the nav on that same live page makes all sixreachable, which is what this change does properly.
Latent, not new
With five sections the list stopped just above the bar. "Nearby schools" made
six, and the sixth is the first to reach it — so any section added later would
have hit this. Worth knowing when the next one lands.
The fix
The nav is lifted only while the sheet is open, and only to
1100: above thebar, below the comparison toast (2000), the fullscreen map (5000) and the info
popover (9999). The backdrop rises with it, so tapping over the bar now
dismisses the sheet rather than navigating away.
The journey
It asks what a thumb asks — for each item, whether it is the topmost element at
its own centre. A bounding-box or visibility check cannot catch this: the item
is in the viewport, the right size, and
toBeVisible()passes. It is simplyunderneath something.
Confirmed to fail against current staging before the fix, naming
["Nearby schools"], so it is testing the real thing rather than passingvacuously.
Verification
tsc --noEmitcleanThe fix itself cannot be proven end-to-end until it is deployed; the post-merge
staging run is the evidence.
🤖 Generated with Claude Code
🤖 AI Code Review (Claude Code)
The PR adds an e2e test to check that jump-sheet menu items aren't covered by the bottom tab bar, and attempts to fix the underlying stacking-context bug by raising the sticky nav's z-index while the sheet is open. However, the CSS edit accidentally moved layout-critical declarations from the base
.sectionNavrule into the new conditional.sectionNavSheetOpenclass instead of copying them, breaking the nav's normal appearance.🔴 Severe (blocks merge)
.sectionNavright afterpaddingand movesmargin-bottom,box-shadow,display: flex,align-items, andgapinto the new.sectionNavSheetOpenrule instead of duplicating them. SincesectionNavSheetOpenis only applied to the<nav>when the mobile jump sheet is open (SchoolDetailShell.tsx), the sticky section nav loses its flex layout, spacing, and shadow in its normal (closed) state on every school detail page — this is the default state for virtually all page views, not just when the sheet is open, so it's a site-wide layout regression, not a mobile-only fix.🤖 AI Code Review (Claude Code)
This PR fixes a mobile bottom-sheet z-index bug: the sticky
.sectionNavestablishes a stacking context (viaposition: sticky+z-index: 10), so itsposition: fixedjump-sheet descendant was painted under the fixed bottom tab bar (z-index: 1000) despite its own higher z-index. The fix conditionally lifts only.sectionNav's z-index to 1100 while the sheet is open, and adds an e2e test that hit-tests each sheet item's center point viaelementFromPointto catch exactly this class of covered-element bug. The CSS cascade order and specificity were verified (no later same-specificity rule overrides the new z-index), and the fix is scoped correctly; no correctness, security, or deploy issues found.✅ No issues found.