Skip to content

fix(gh-384): keep city HUD panels inside the window - #403

Open
db-tycoon-stephen wants to merge 1 commit into
fix/gh-383-detail-panel-wrapfrom
fix/gh-384-hud-viewport
Open

db-tycoon-stephen wants to merge 1 commit into
fix/gh-383-detail-panel-wrapfrom
fix/gh-384-hud-viewport

Conversation

@db-tycoon-stephen

@db-tycoon-stephen db-tycoon-stephen commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

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

  • Bug fix

Checklist

  • pre-commit run --all-files passes
  • Tests pass locally, and the offline csv-import pipeline still runs end to end (this is the CI gate — if it breaks, the release is blocked)
  • Added or updated tests where it made sense
  • Updated CHANGELOG.md if this is user-facing
  • Updated docs / README if behavior or commands changed

Notes 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 gesturestart path is untested. Worth confirming with the reporter before merging.

Depends on

The shipped web_dist bundle is rebuilt here, so whichever city PR merges later needs scripts/sync_web_bundle.py rerun.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

@github-actions

Copy link
Copy Markdown

PR title check

mode: warn

fix(gh-384): keep city HUD panels inside the window

Result: ✅ title matches the convention.

@github-actions

github-actions Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

PR size check

mode: warn | file limit: 8

Files changed: 6 counted / 8 allowed

  • CHANGELOG.md
  • src/tycoon_city/web_dist/assets/index-CmYH7S6A.js
  • src/tycoon_city/web_dist/index.html
  • web/e2e/hud_layout.spec.ts
  • web/index.html
  • web/src/boot/input.ts

Result: ✅ within limits.

@db-tycoon-stephen
db-tycoon-stephen changed the base branch from v0.2.3 to fix/gh-383-detail-panel-wrap October 13, 2026 10:50
@db-tycoon-stephen db-tycoon-stephen added this to the v0.2.3 milestone Oct 13, 2026
@db-tycoon-stephen db-tycoon-stephen self-assigned this Oct 13, 2026
@db-tycoon-stephen db-tycoon-stephen modified the milestones: v0.2.3, v0.2.4 Oct 21, 2026
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
db-tycoon-stephen force-pushed the fix/gh-383-detail-panel-wrap branch from a84734c to ab227aa Compare October 27, 2026 20:07

@db-tycoon-stephen db-tycoon-stephen left a comment •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.ts checks three panels but not the footer, so either add the footer to the assertion or narrow the test's claim.

@JesuFemi-O JesuFemi-O left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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+wheel on window (web/src/boot/input.ts), so a trackpad pinch over a HUD panel or the header no longer zooms the page. Adds touch-action: pan-x pan-y on html, body.
  • Adds max-width: calc(100% - 24px) to #inspector, #problems, #run-panel and #legend, plus max-height and overflow-y: auto on #legend.
  • Two Playwright tests, a CHANGELOG entry, and a rebuilt web_dist.

Checked (at e19b951, with #401 and #402 below it)

  • CONFIRMED: npx tsc --noEmit clean. uv run pytest -q: 1575 passed, 10 skipped (MOTHERDUCK_TOKEN invalid, fresh DLT_DATA_DIR). ruff check and ruff format --check clean. 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.py run on the branch produces no diff. Diffing the old and new JS bundles shows exactly one change: the window.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-width rules: the narrow-window test fails (#run-panel right edge 372 > 360).
    • Removing the ctrlKey preventDefault: the pinch test fails.
    • Removing the legend max-width: both tests still pass.
    • Removing touch-action: both tests still pass.
  • 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-panel at 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.

Should fix

  1. web/e2e/hud_layout.spec.ts:113 (narrow-window test) only guards #run-panel. At 360 wide #problems (352 right edge) and #inspector already fit without the new CSS, and #legend is never measured, so deleting the #legend rule or any of the three other max-width rules (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 #legend and the inspector with content open.
  2. web/index.html:14 (touch-action: pan-x pan-y) has no test. Removing it leaves the suite green (CONFIRMED). Either assert getComputedStyle(document.body).touchAction, or drop the line if the wheel listener is enough; the PR body says only the ctrl+wheel path was reproduced.
  3. CHANGELOG.md entry says "A pinch now only zooms the camera over the canvas", but the listener at web/src/boot/input.ts:33 cancels every ctrl+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 (scrollWidth 503 vs 360 at 360 wide). Focusing the notes button then scrolls the overflow: hidden body sideways by 143px (CONFIRMED with focus() 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-pop at 800x260 starts at y = -142 (above the window); also present before the PR. The #notes-pop and #keys-pop popovers were not given caps. Not caused here.
  • The listener is { passive: false } on window, so every wheel event waits on the main thread. Limiting it to event.ctrlKey events 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 gesturestart path; a real trackpad on macOS. The pinch was synthesized through CDP only.
  • Live window resize with a panel open, R refresh 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 branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

2 participants