Repository navigation
fix(ui): fit the Lantern Duel board above the fold on phone landscape - #586
Conversation
Refs #580. In short landscape (600-1000px wide, 500px tall or less) the Duel page hides the sidebar, theatre rail and page heading and lays the board out beside its controls, sized from the viewport height. Scoped with :has(#duel-0) so Tic-Tac-Toe, which reuses .duel-grid, is unchanged. Adds a geometry check for 667x375 and 844x390 plus a portrait/desktop unchanged check, and raises the shell and stylesheet-gzip ceilings by the measured excess.
Review findings: above 800px there is no bottom nav, so the Games room button stays; the title stays in the accessibility tree; the main padding-bottom that clears the fixed nav is no longer overridden. Ceilings re-measured.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Chris0Jeky
left a comment
Chris0Jeky
left a comment
There was a problem hiding this comment.
Grok 4.7 gate review (fresh context)
Read-only lens (no shell, no edits), run from a Claude estate-hub sweep session on head def0282; blocking findings checked by Claude Sonnet 5.5.
Verdict: No blocking issues
Blocking
- None.
Non-blocking
- None.
Summary
No blocking defect. The new rules sit in (min-width: 600px) and (max-width: 1000px) and (max-height: 500px) and (orientation: landscape), and every selector requires :has(#duel-0). Tic-Tac-Toe cells are tictactoe-N, and render replaces #app, so the layout cannot apply on other pages, in portrait, or at desktop sizes. The 801–1000px query only restores the Games room row and shrinks the board. Both budget raises match the comments: shell +1,168 (27,504 → 28,672) covers a 1,134-byte excess and leaves 34 bytes; stylesheet gzip +288 (384 → 672) covers a 251-byte excess and leaves 37 bytes. No other ceiling changes, and the only shipped bytes are this CSS. The Playwright helper checks 36 cells inside the viewport and above a visible bottom nav, size ≥ 28, no horizontal overflow, scrollY 0, one h1, Games room when width > 800, and a move plus undo via the duel log. Portrait 332px and desktop 480px with a visible sidebar match the existing grid math and sit outside the media query.
Checked / not checked
Checked: Agents.md (no CLAUDE.md in this checkout); .grokrev/pr.json, checks.txt, and diff.patch; src/club.css landscape rules (2933-2994) plus .duel-grid/.club-playlayout and the 600/900/1150px rules they override; src/app.css shell, sidebar, topbar, .mobile-nav (hidden above 800px), and border-box; src/after-hours.css short-viewport query (play chrome only, not the duel); src/club.js duelPage vs ticTacToePage (ids duel-N vs tictactoe-N), heading/go/toolbar, full #app replace in app.js render, undo-until-player-turn, AlibiClub.diagnostics; src/app.js mobileNav (Games room tab at width <= 800); tests/duel_landscape_cases.py and the two new methods in tests/browser_mobile_qa.py; tests/budget.test.cjs shell and alibi.css ceilings, including the prior +27504 and +384 baselines; check.yml verify job runs npm run verify and tests/browser_mobile_qa.py
Not checked: No shell in this review, so the suite, the build, and the claimed 1,411,538 → 1,412,758 and 34,157 → 34,427 byte measurements were not re-run; CI logs were not opened; checks.txt shows one verify job still pending and does not prove which SHA the passing verify job built; No browser or physical-phone pass, and no viewport other than the ones pinned in the tests
grok exit 0, 20 turns, 1,674,004 tokens, $0.420, session 01a1141c-59bb-77c3-a01e-da7fb219698a. CI at review time: browser-controls x2, controls x3, storage x2 and the short verify job pass; 'Verify puzzle cabinet' (the full npm run verify incl. budget test) still in progress; local npm run verify per PR body: 1,267/1,271 pass, 1 known symlink EPERM fail
1 participant
What and why
Refs #580 (does not close it: physical-phone confirmation is separate). On phone landscape (short viewport, 600-1000px wide, 500px tall or less) the Lantern Duel page stacked everything in one column, so the 6x6 board started at y=546 (667x375) or y=940 (844x390) and almost no cell was usable without scrolling. This adds a Duel-only layout in that same media-query band that Bridges already uses: the sidebar and "Room atmosphere" rail are hidden, the board sits on the left sized from the viewport height, and the mode buttons, strength picker, scores, status and Undo/Redo/Start again sit beside it (scrolling if they do not fit). Scoped with
:has(#duel-0), so Tic-Tac-Toe (which reuses.duel-grid) and every other page are untouched; portrait and desktop geometry is pinned unchanged by a test.After the independent review below: the page title stays in the accessibility tree (visually hidden), above 800px (no bottom nav) the Zen and Games room buttons stay as a compact row so a standalone-PWA user has a way back, and the main padding-bottom that clears the fixed bottom nav is no longer overridden.
Byte ceilings (reviewer: please judge)
Main had 86 bytes of shell headroom and 19 gzip bytes of stylesheet headroom, so no real layout rule fits.
tests/budget.test.cjsis raised by the measured excess, the repo's usual pattern: shell1.32 MiB + 27,504 -> + 28,672(measured 1,411,538 -> 1,412,758, leaving 34 bytes) and stylesheet gzip33 KiB + 384 -> + 672(34,157 -> 34,427, leaving 37). These are the only budget changes; JS and initial-payload ceilings are untouched. If you prefer a trim elsewhere first, say so.Evidence
PYTHONUTF8=1 py -3 tests/browser_mobile_qa.pyagainst a fresh build: 18/18 on two consecutive runs. One earlier full run (before the review fixes, same machine under load) reported one failure that I did not identify and could not reproduce in the next two runs.check_duel_landscapeat 667x375 and 844x390 (all 36 cells fully inside the viewport and above the bottom nav, at least 28px, no horizontal overflow, scroll 0, a legal move registers and Undo restores it, oneh1, a way back); portrait 390x844 (grid 332px) and desktop 1280x900 (grid 480px, sidebar visible) unchanged. Screenshots inspected for both landscape sizes.npm run verifyon this head (Windows, Node 24): prettier clean, Android build OK, 1,271 tests, 1,267 pass, 3 intentional skips, 1 fail. The failure isfilesystem loader refuses symlink sources without following them(tests/workshop-catalogue.test.cjs:330):EPERMfromfs.symlinkSyncon this Windows machine; file untouched, Linux CI creates symlinks.revieweragent, a lens and not the merge gate; the Muse wrapper was blocked by a stale host-worker marker): HIGH no way back at 801-1000px, MEDIUM footer covered by the bottom nav, MEDIUM title and atmosphere rail removed in landscape, LOW board small at the narrow end. First two fixed in the last commit. Non-blocking, left as is: the room-atmosphere toggles (sound, motion, edition) are not shown in short landscape (the same toggles remain on other pages and in Settings); at 600x300 the cells shrink to about 23px, below the test's 28px floor, which only covers 667x375 and 844x390; the controls beside the board may need a scroll.NOT verified
Hosted CI for this head; a physical phone or TalkBack (stays with the owner, HUMAN_TODO.md); viewports other than 667x375, 844x390, 390x844, 1280x900; the Muse lens (unavailable).
Worker cost
Grok 4.7 medium wrote the first CSS and tests: two runs (30 turns each, both cut off at the turn cap), 4,094,692 + 2,424,561 tokens, $0.773 + $0.499. I finished the measurement, review fixes, budgets and verification myself.