fix: make result screens scrollable and raise mobile touch targets - #125
Conversation
Closes #120. Closes #121. The post-dive and game-over screens are painted onto the canvas, and `html, body { overflow: hidden }` means the page never scrolls. Both draw functions accumulate `y` in fixed pixels with no clamp, so on a phone the content simply ran off the bottom and was unreachable: the entire tissue compartment chart started below the fold at 320x568, and all seven game-over reasons overflowed — narcosis by 438 px. The setup and help overlays solve this by being HTML with overflow-y:auto. These two are canvas, so they carry their own scroll offset instead. The renderer translates by -resultScrollY and reports the height it just laid out, which becomes the bound; input comes from wheel, touch drag and arrow/PageUp/PageDown/Home/End, all inert when the content already fits. Four layout bugs found alongside it, each of which needed its own fix: - The stats card divides min(560, W-80) into three cells, so at 320 px each cell is 80 px while the values render at 30 px — "1840:00" overlapped its neighbour. Now passed through fillText's maxWidth so it condenses instead. - "DIVE COMPLETE" (38px) and "GAME OVER" (46px) were stepped 30 px below their kickers, less than their own ascent, so both drew through the line above. - S('safetyExpl') is authored as fixed-length lines wrapped for desktop; they measure 383-416 px and bled off both edges of a 320 px screen. Re-wrapped to the available width instead of trusting the authored breaks. - The four in-dive HUD chips are shown/hidden inside updateDiving(), which stops being called the instant the dive ends, so whatever was visible then stayed visible. Ending a wreck or cave dive left the rule-of-thirds chip latched over the post-dive header. Cleared on exit from 'diving'. Scroll reset and chip clearing both hook the state transition rather than the 13 individual places that end a dive, so a 14th cannot miss them. For #121, every gas-setup control was under the 44 px WCAG 2.5.5 / iOS HIG minimum — 71 undersized targets across the measured screens, now zero. The direct cause was a `max-width: 480px` rule setting `min-height: 38px`, which overrode the 44 px base specifically on the phones where hitting a target is hardest. Spacing was tightened up too: 3 px between two mode pills left almost no dead zone between them. Verified with the instrumentation that found both issues: canvas text captured in screen space across the whole scroll range, and getBoundingClientRect over every visible control. Unreachable text 48 -> 0, horizontal clipping 5 -> 0, text-over-text 2 -> 0, undersized targets 71 -> 0, no horizontal overflow introduced at 320 px. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
N1k4G
added
bug
…den mode gap Three review findings on #125, all reproduced before changing anything. [P1] Scrolling recovered height but not width, and several single-line strings were still drawn past both edges at 320px: three game-over reasons (worst "PULMONARY BAROTRAUMA — PNEUMOTHORAX" at x=-61.7…381.7), the German post-dive title, and the CCR cylinder lines in both languages. These are headings and stat lines that cannot wrap without breaking the layout around them, so drawFittedText() shrinks the font to fit and hands anything still too wide to the canvas maxWidth squeeze. Applied to every centred dynamic string on both result screens rather than only the three that clip today, since the next translation would surface the rest. Replacing the CCR lines also fixed a hardcoded ' bar left' that stayed English in German output; it is now S('barLeft'). [P2] The wheel/touch handlers live on `window`, so events bubbling out of the HTML help overlay were preventDefault()ed on the canvas's behalf: the overlay (scrollHeight 2268 in a 568px viewport) stayed pinned at scrollTop 0 while the result screen hidden behind it scrolled instead. resultScreenScrollable() now returns false while showHelp or showGasInfo is set, which also covers the keyboard path, and the handlers ignore events originating inside an HTML overlay so a future one is covered without a new state flag. [P3] Rec/Tec/CCR reached 44px tall but stayed 6px apart, under the 8px target in #121 — and flex rounding took one gap to 5px. Now 10px, measuring 10/10. The earlier PR description was wrong to attribute the remaining tight pairs solely to the D-pad: the setup screen now reports 19 controls, 0 under 44px and 0 pairs under 8px apart. Adds tests/result-screen.spec.js, which is what was missing when these three shipped. It wraps fillText/strokeText and asserts on real geometry, because none of this is visible to the DOM. Each test fails on its own defect and no other: reverting the fitted draws reports the exact spans above, dropping the overlay guards fails "overlay must scroll", and restoring the 6px gap reports both mode-button pairs. New gameAPI hooks for those assertions: gameOverReason and showHelp setters, resultScrollY/resultScrollMaxY, and S. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
All three confirmed and fixed —
|
| Reverted | Result |
|---|---|
| the fitted draws | fails, reporting the exact spans above |
| the overlay guards | fails overlay must scroll |
| gap back to 6px | fails, reporting both mode-button pairs |
New gameAPI hooks so it can assert without reaching into globals: gameOverReason and showHelp setters, resultScrollY / resultScrollMaxY, and S.
typecheck, lint, build, sites:check, 133 unit, 28 parity, 27 e2e (was 22) all green locally.
Both weaknesses were confirmed by reintroducing the defect and watching the suite stay green. The scroll test asserted only that scrolling reached `resultScrollMaxY`, which is the renderer's own declared range — so it was checking that value against itself. Pinning `resultScrollMaxY` to 1 left every line below the fold unreachable and all five tests still passed. It now measures the content: the declared range must cover the overflow the drawn text actually has, and after scrolling to the bottom the last line must land inside the viewport, computed from the offset the renderer really applied. The clipping test walked both languages but never left Rec mode, so the CCR cylinder lines — the longest strings the screen can draw, and two of the five that were clipping — were never rendered. Un-fitting them left the test green. The matrix is now Rec/Tec/CCR x en/de, and it asserts that the CCR iterations actually drew the cylinder lines, so a silent failure to switch mode fails rather than quietly proving nothing. All five defect classes now trip their own test and no other: un-fitting the headings reports the exact spans, dropping the overlay guards fails "overlay must scroll", restoring the 6px gap reports both mode pairs, pinning the scroll range fails reachability, and un-fitting the CCR lines fails the matrix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An editing slip put a NUL where a space belonged in
`runs.map(r => r.text).join(' ')`. The test still worked — joining with NUL
still concatenates — but git classified the file as binary, so the previous
commit landed as "Bin 10247 -> 12660 bytes" with no reviewable diff.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both test weaknesses confirmed and fixed —
|
| Reintroduced | Fails |
|---|---|
| un-fit the headings | exact spans, e.g. -61.7…381.7 in 320px |
| drop the overlay guards | overlay must scroll |
| gap back to 6px | Rec <-> Tec = 6.0px |
pin resultScrollMaxY to 1 |
reachability |
| un-fit the CCR lines | the mode matrix |
Two things worth flagging
A pre-existing flake in the e2e suite, not from this PR. A full run failed once on wreck-slice.spec.js:69 — saved depth 27 m vs restored 28.1 m, a timing-sensitive persistence check. I reproduced it on 9cd4977 with today's test changes stashed (1 failure in 2 full runs), so it predates this work. wreck-slice.spec.js alone passed 3/3. Not addressed here — happy to file it or dig in if you want.
24d5e97 landed as Bin 10247 -> 12660 bytes with no reviewable diff. An editing slip had put a NUL byte where a space belonged in join(' '). The test still worked, but git classified the file as binary. Fixed in e4b778a; the PR diff against main now shows 287 text lines.
typecheck, lint, build, sites:check, 133 unit, 28 parity green locally; e2e 27 passed on a clean run.
Closes the last item on #124. The art direction was settled and both halves implemented (#135 cave, #136 wreck); this is the guard that keeps them, and it is the piece the issue asked be done LAST, deliberately, so the thresholds describe the intended look rather than the shortfall. npm run interior:check verify (CI-gated in pr.yml and deploy.yml) npm run interior:update re-record the reference FRAMES, deliberately Nine scenes: four wreck interiors, three cave interiors, two open-water controls. Each has a band on per-pixel chroma and one on mean luminance. WHY THRESHOLDS ARE CODE AND FRAMES ARE DATA --update re-records the PNGs and does not touch the thresholds. That split is the whole point of #124's sequencing warning: thresholds derived from what is currently on screen describe the current state, so recording them while the current state is the shortfall locks the shortfall in. The bands therefore live in the script with their derivation attached, and moving one is a deliberate edit visible in review rather than a side effect of a red run. HOW THE FLOORS WERE PLACED Each floor sits midway between the scene's measured flat state and its measured fixed state — the placement with the most headroom either side that still fails the regression it exists to catch: scene flat -> fixed floor catches headroom wreck-vehicle-deck 26.4 -> 35.5 31.0 +4.6 -4.5 wreck-crew-deck 22.6 -> 31.7 27.0 +4.4 -4.7 wreck-cargo-hold 19.1 -> 27.8 23.5 +4.4 -4.3 wreck-engine-room 14.3 -> 21.1 17.5 +3.2 -3.6 cave-upper-tunnel 10.1 -> 17.8 14.0 +3.9 -3.8 cave-restriction 12.2 -> 19.4 15.5 +3.3 -3.9 cave-cathedral 16.6 -> 23.6 20.0 +3.4 -3.6 Both columns were measured by this script by reverting the two gloom colours and re-running it, so every floor is calibrated against a real state of this codebase rather than an estimate. Verified three ways: clean passes all nine; reverting #136 fails exactly the four wreck interiors; reverting #135 fails exactly the three cave ones. Controls hold in both. Run-to-run noise is under 0.1 across three consecutive full runs, so the whole 3.2-4.7 of headroom is there for cross-platform drift, which could not be measured — CI is linux and there is no linux node here. If linux sits further out than that, the first run says so with the number, and the fix is to widen the one band that moved. TWO CONTAMINANTS THE CROP HAD TO BE MOVED OFF #124's baseline came with a caveat that its crop still caught fixed chrome, and a note to tighten it before deriving thresholds. Doing that turned up two, and both were found by measurement rather than by eye, which is the part worth keeping: The toasts are painted onto the CANVAS, not the DOM, at y fractions 0.18, 0.30 and 0.36 — amber text, the highest-chroma pixels in frame, with the hint line refilling from a queue on a timer. They were worth 0.6-0.9 chroma and 6 SD. Suppressed at source, and the run now asserts they are silent; clearing them from one evaluate and measuring from the next left room for a frame in between that re-armed one, which is why settle, silence and read are now a single evaluation. The dive computer looks like DOM and is not: it is on the canvas at x >= 880, and a window running to 0.80 had its corner inside, worth 1.79% of the measured pixels. The window is 0.48 wide instead. Rather than trust a crop that looked right once, assertNoChrome now proves it every run: two scenes sharing no scenery (wreck-engine-room, reef-open-water) cannot legitimately agree pixel for pixel, so anything byte-identical across both is chrome. Measured 0.03%, against a 0.5% budget and the 1.79% the gauge corner cost. That is what stops this crop rotting the next time the HUD moves — #125 alone reflowed the result screens and grew the touch targets. Moving the window below the toasts instead was tried and rejected: at that offset it slides off the interior onto the seabed, taking the engine room's contrast from SD 17.1 to 7.9. WHY THE FRAMES ARE NOT BYTE-COMPARED #133 established Playwright frames are deterministic per platform but not across them. There is no linux set here, so the frames are committed as review artefacts — cut from the same canvas pixels the statistics read, so the picture and the numbers beside it cannot disagree, and sized to the sample window, which also keeps nine frames from outweighing the rest of the repo's history. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 participant
Closes #120. Closes #121.
Both are mobile-viewport defects with no overlap in the files they touch, so they land together as one pass over phone layout.
#120 — result screens ran off the bottom with no way to scroll
The post-dive and game-over screens are painted onto the canvas, and
html, body { overflow: hidden }(style.css:27) means the page itself never scrolls.drawPostDive()anddrawGameOver()accumulateyin fixed pixels with no clamp, scale or scroll surface, so content past the fold was simply unreachable.The entire tissue-compartment chart began at y ≈ 756 on a 568 px screen. All seven
gameOverReasonvalues overflowed at 320×568.Approach. The setup and help overlays already solve this by being HTML with
overflow-y: auto. These two are canvas, so they carry their own scroll offset instead: the renderer translates by-resultScrollYand reports the height it just laid out, which becomes the bound for the next frame. Input is wheel, touch drag, and Arrow/PageUp/PageDown/Home/End — all inert when the content already fits, so no key changes meaning anywhere else.Measuring the frame just drawn, rather than adding a separate measure pass, keeps one source of layout truth. The bound is correct from the second frame; the first is drawn at offset 0 regardless.
Four layout bugs found alongside it
Each needed its own fix — scrolling alone would have left all four.
Stat row collided. The card is
min(560, W - 80)split three ways, so at 320 px each cell is 80 px while values render at 30 px."1840:00"is wider than its cell and ran into"38.4m". Now passed throughfillText'smaxWidthso it condenses.Headers drew through their kickers.
"DIVE COMPLETE"(38px) and"GAME OVER"(46px) were stepped 30 px below the line above — less than their own ascent. 125×9 px of overlap on the game-over header.Advisory copy bled off both edges.
S('safetyExpl')is authored as fixed-length lines wrapped for desktop; they measure 383–416 px on a 320 px screen. Re-wrapped to the available width rather than trusting the authored breaks.An in-dive HUD chip stayed latched on. The four
#hud-*chips are shown/hidden insideupdateDiving(), which stops being called the instant the dive ends — so whatever was on screen at that moment stayed there. Ending a wreck or cave dive left the rule-of-thirds chip sitting over the post-dive header, which is why "DIVE LOG" was half-covered.Both the scroll reset and the chip clearing hook the
gameStatetransition rather than the 13 individual places that end a dive, so a 14th cannot miss them.#121 — every gas-setup control was under the 44 px minimum
71 undersized targets across the measured screens. The direct cause was a
@media (max-width: 480px)rule settingmin-height: 38px, overriding the 44 px base rule specifically on the phones where hitting a target is hardest.Spacing went with it: 3 px between two mode pills left almost no dead zone, so a slightly-off tap landed on the neighbour. Tight pairs (<8 px) dropped from 18 to 2 on the densest screen.
The remaining tight pairs are the in-dive D-pad at 6 px, which is deliberate — the CSS says the four should "read as one cluster", and they are 56 px targets.
Verification
Re-ran the instrumentation that found both issues. Canvas text is captured in screen space (reading the live transform, since the renderer now translates) and sampled across the whole scroll range — capturing only the two ends misses anything visible solely at an intermediate offset.
lint,typecheck,build,sites:check, 133 unit, 28 parity, 22 e2e all pass — including the in-browser suite viagame.spec.js.Notes for review
wheelandtouchmovelisteners callpreventDefault. Both are guarded onresultScreenScrollable(), so they only capture on a result screen that actually overflows, and touch drags starting on a.t-btnare ignored.diving-simulator-tests.html. The change is presentational and the harness covers physics/gas/deco/gameAPI; I did not want to add a canvas-layout assertion there without a pattern to follow. Say the word if you would rather it were covered.