Repository navigation
fix(gh-384): keep city HUD panels inside the window - #403
Open
db-tycoon-stephen wants to merge 1 commit into
Open
db-tycoon-stephen wants to merge 1 commit into
db-tycoon-stephen wants to merge 1 commit into
Conversation
PR title checkmode:
Result: ✅ title matches the convention. |
PR size checkmode: Files changed: 6 counted / 8 allowed
Result: ✅ within limits. |
db-tycoon-stephen
changed the base branch from
v0.2.3
to
fix/gh-383-detail-panel-wrap
October 13, 2026 10:50
A trackpad pinch reaches the page as ctrl+wheel. Over the canvas the camera controls consume it, but over a HUD panel or the header the browser zoomed the whole page, so the panels and the first header chip ran past both window edges (gh-384). That matches the report, which only showed up on a laptop with a trackpad, and it reproduces with a synthesized pinch in headless Chrome. The page now swallows ctrl+wheel outside the canvas and disables touch pinch-zoom; keyboard zoom is a layout zoom and still works. Panels also cap their width at the window so a very narrow window keeps them on screen. Rebuilds the shipped web bundle so CI's freshness check stays green. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
db-tycoon-stephen
force-pushed
the
fix/gh-383-detail-panel-wrap
branch
from
October 27, 2026 20:07
a84734c to
ab227aa
Compare
db-tycoon-stephen
force-pushed
the
fix/gh-384-hud-viewport
branch
from
October 27, 2026 20:07
d5b840f to
e19b951
Compare
Contributor
Author
There was a problem hiding this comment.
Independent review at e19b951. This replaces an earlier automated comment that was not based on the diff.
Verdict: approve, with one note.
- Panels stay inside the window after a resize from 1280 to 400 px and at a device pixel ratio of 2.
- The new pinch test is falsifiable: with the wheel listener stripped, the page zooms to 1.6, and with the fix it stays at 1. Ctrl+wheel over the canvas still zooms the camera.
- Note: at 360 px the footer overflows by 26 px and the keys button is clipped. It fits at 420 px and above.
web/e2e/hud_layout.spec.tschecks three panels but not the footer, so either add the footer to the assertion or narrow the test's claim.
4 of 6 tasks
JesuFemi-O
approved these changes
Nov 4, 2026
JesuFemi-O
left a comment
Collaborator
There was a problem hiding this comment.
Verdict: approve with fixes. The pinch fix is real and tested, and the width caps are harmless, but the width caps alone change nothing at laptop widths, the new tests leave two of the changes unguarded, and one remaining path to the reported symptom is untouched.
What it does
- Cancels
ctrl+wheelonwindow(web/src/boot/input.ts), so a trackpad pinch over a HUD panel or the header no longer zooms the page. Addstouch-action: pan-x pan-yonhtml, body. - Adds
max-width: calc(100% - 24px)to#inspector,#problems,#run-paneland#legend, plusmax-heightandoverflow-y: autoon#legend. - Two Playwright tests, a CHANGELOG entry, and a rebuilt
web_dist.
Checked (at e19b951, with #401 and #402 below it)
- CONFIRMED:
npx tsc --noEmitclean.uv run pytest -q: 1575 passed, 10 skipped (MOTHERDUCK_TOKEN invalid, fresh DLT_DATA_DIR).ruff checkandruff format --checkclean.npx playwright test: 134 passed, 1 failed once in the full run (framing.spec.ts"clicking resolves every unoccluded lot at three poses") and passed when rerun alone; the failure is in camera framing, not in anything this PR touches, so it looks like load flakiness, not a regression. - CONFIRMED:
scripts/sync_web_bundle.pyrun on the branch produces no diff. Diffing the old and new JS bundles shows exactly one change: thewindow.addEventListener("wheel", ...)listener. No hidden source change in the bundle. - CONFIRMED: 6 files, under the cap of 8. No em dashes in added authored lines.
- Mutation checks on
hud_layout.spec.ts:- Removing the
max-widthrules: the narrow-window test fails (#run-panelright edge 372 > 360). - Removing the
ctrlKeypreventDefault: the pinch test fails. - Removing the legend
max-width: both tests still pass. - Removing
touch-action: both tests still pass.
- Removing the
- Screenshots and bounding-box metrics, before (the CSS caps neutralised in page) and after, on the rich fixture, at 1440x900, 1024x560, 800x600, 600x500, 360x640 and 800x260, with inspector, library, stats, problems, run panel, legend, keys and notes popovers, and all three panels together.
- Before the change, no HUD panel overflowed horizontally at 1024, 800 or 600 wide. The only horizontal overflow was
#run-panelat 360 wide (right edge 372, its close button cut off). After: it fits. So the CSS caps fix only windows narrower than about 372px; at laptop widths the pinch listener is the fix, and the PR body says so. - After the change, panels shorter than their content scroll internally (
overflow-y: auto; for example the inspector is 399px tall with 730px of content at 1024x560) so content stays reachable. At 800x260 panels shrink to about 99px tall. That is usable only by scrolling, and no scroll affordance is visible. This was already so before; the PR only adds the same behaviour to#legend. - The 600x500 screenshot with problems, run panel and inspector open shows them stacked over each other. The caps keep them inside the window, not apart.
- Before the change, no HUD panel overflowed horizontally at 1024, 800 or 600 wide. The only horizontal overflow was
Should fix
web/e2e/hud_layout.spec.ts:113(narrow-window test) only guards#run-panel. At 360 wide#problems(352 right edge) and#inspectoralready fit without the new CSS, and#legendis never measured, so deleting the#legendrule or any of the three othermax-widthrules (other than the run panel's) does not fail the suite (CONFIRMED by mutation). Use a viewport narrower than the widest panel (for example 300 wide), and assert#legendand the inspector with content open.web/index.html:14(touch-action: pan-x pan-y) has no test. Removing it leaves the suite green (CONFIRMED). Either assertgetComputedStyle(document.body).touchAction, or drop the line if the wheel listener is enough; the PR body says only the ctrl+wheel path was reproduced.CHANGELOG.mdentry says "A pinch now only zooms the camera over the canvas", but the listener atweb/src/boot/input.ts:33cancels everyctrl+wheel, including a mouse wheel with Ctrl held, which users with low vision may use for browser zoom. The PR body notes this trade-off; the user-facing changelog should too (CONFIRMED by code, not by a mouse test).
Nits and questions
- Question, PLAUSIBLE as a second cause of the report: below about 503px the footer is wider than the window (
scrollWidth503 vs 360 at 360 wide). Focusing the notes button then scrolls theoverflow: hiddenbody sideways by 143px (CONFIRMED withfocus()at 360 wide), which shifts the header and chips off the left edge. That matches "first header chip cut off on the left". The PR does not touch it. Out of scope for laptop widths, but "Closes #384" with an unconfirmed cause could be worth a follow-up issue. #keys-popat 800x260 starts at y = -142 (above the window); also present before the PR. The#notes-popand#keys-poppopovers were not given caps. Not caused here.- The listener is
{ passive: false }onwindow, so every wheel event waits on the main thread. Limiting it toevent.ctrlKeyevents is already done; this is only a note. - The PR cannot show the pinch cause is what the reporter hit (the PR body says so). Worth confirming with the reporter before merging, as the PR suggests.
Not checked
- Safari
gesturestartpath; a real trackpad on macOS. The pinch was synthesized through CDP only. - Live window resize with a panel open,
Rrefresh with panels open, and pointer events through the clamped legend area were not run (only static viewport sizes). - Whether the CI "Tests pass locally ... csv-import" template line is accurate (CI is green on it).
- Upgrade-safety against a v0.2.3 project: this change is front-end only and was not tested that way.
This was referenced Nov 4, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
A trackpad pinch over a HUD panel zoomed the whole page, which cuts the panels and header chips off at both edges. That is the likeliest cause of the report, and it reproduces with a synthesized pinch in headless Chrome. The page now cancels ctrl+wheel outside the camera's own handling and disables touch pinch-zoom, and panels cap their width at the window width. Also rebuilds
web_dist.Mental model
Pinch zooms the city, never the page. Panels are sized in px but capped at the window width.
Linked issue
Closes #384
Type of change
Checklist
pre-commit run --all-filespassescsv-importpipeline still runs end to end (this is the CI gate — if it breaks, the release is blocked)CHANGELOG.mdif this is user-facingNotes for reviewers
The cause is inferred, not confirmed: window widths from 700 to 1440 all fit, and only a pinch reproduced the reported picture. Trade-off: Ctrl+mouse-wheel page zoom is now blocked; keyboard zoom still works. Safari's
gesturestartpath is untested. Worth confirming with the reporter before merging.Depends on
The shipped
web_distbundle is rebuilt here, so whichever city PR merges later needsscripts/sync_web_bundle.pyrerun.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.