Split The code is public; the app answers only to the crew.
JavaScript off: a menu of dead buttons and not one word saying why #223
Check · Review · Land · Live done — Landed and live in production.
Pushed.
shipped merged
I couldn't submit the review for pull #223, so the pull has no review from me. I was going to approve it.
Why it didn't go through: every POST to the pull's reviews endpoint returns {"error":"bad_anchor"}. That happens even with the body ok, and with the head commit a0643e5 passed under four different field names. So the problem is on the forge's side for this pull, not in the review text. Two other things point the same way: the pull's branch no longer exists on origin (only refs/pull/223/head does), and the preview returned 404 on both tries.
What I found:
- Security: the change is only fixed markup inside a
<noscript>block, with no user input and no new way to write anything. The inline style is already allowed by the page's existing security policy. - Does what the issue asked: with JavaScript off, the added style hides every control in the menu: the settings row, Resume, Play, Fullscreen, and the Controls and Crosshair panels. With JavaScript on the style never applies, so normal visitors see no change.
- Tests: the page's test file passes 15/15 at the pull's head, and the new test would fail without the change. The full suite had one failure on my copy: a timing test on the leaderboard query that took 7.4 s. This diff doesn't touch that code.
- Terminal: has no JavaScript, so there is nothing to add there.
- Not verified live: the preview was down, so I read the page's HTML from the test instead. I also couldn't open the forge's screenshots: my file access is limited to this clone. They would have been taken with JavaScript on anyway, so they couldn't show this path.
Small points I would have noted (not blocking):
- With JavaScript off, the six paragraphs of game instructions and the "Defuse or detonate" heading still show under the new "This needs JavaScript" heading. Adding
#menu .hintto the same rule would hide the instructions and leave the message and the pitch. - The comment on the test's last check reads badly, and that check can hardly fail, because the style is written inside the
<noscript>block.
The full review text is saved in /tmp/review.json so it can be sent once the pull is fixed on the forge.
I still couldn't submit the review for pull #223, so the pull has no review from me. My verdict is unchanged: I would approve it.
Why it didn't go through: the reviews endpoint still returns {"error":"bad_anchor"} for every request. That includes the body ok with no other fields, and a comment anchored to line 309 of src/pages/game.ts, a line the change adds. The pull's own record shows the likely cause: it comes from another repository (crossRepo: true, from scout's branch scout/220-…). It looks like the forge can't anchor any review on that kind of pull. The preview also still returns 404, so I couldn't check the change live.
Verdict, from the first round:
- Security: nothing to flag; the change is fixed markup with no user input.
- Does what the issue asked: with JavaScript off, every control in the menu is hidden. Visitors with JavaScript on are unaffected.
- Tests: the new test would fail without the change, and the game page tests pass at the pull's head.
- Terminal: has no JavaScript, so there is nothing to add there.
Two small points I would have noted (not blocking):
- With JavaScript off, the six paragraphs of game instructions still show. Adding
#menu .hintto the same rule would hide them. - The comment on the new test's last check is garbled.
I didn't post the verdict as a plain comment instead, since you asked for exactly one review. Someone needs to fix reviews on this pull on the forge's side. The full review text is in /tmp/review.json, ready to send once they do.
Pull #223 still has no review from me: the forge rejected it again with {"error":"bad_anchor"}. This was the third try, with the same saved review text. The preview still returns 404.
Nothing has changed since the last round:
- The rejection: every request to the pull's reviews endpoint fails, even the simplest one. The likely cause is that the pull comes from another repository (scout's branch), and the forge can't place any review on that kind of pull.
- My verdict: I would approve. The change is small and safe. It hides every dead control when JavaScript is off, and its test would fail without it.
- The saved review: the full text is in
/tmp/review.json, ready to send once someone fixes reviews for this pull on the forge's side.
I'm stopping here rather than retrying again. I didn't post the verdict any other way, since you asked for exactly one review.
Pushed.
A
<noscript>block now sits inside#menu, right before the "Defuse or detonate" heading it explains: it says the game needs JavaScript and WebGL2 and that the leaderboard below still works, in the same tone as the existing "No WebGL2" fallback inmain.ts. Its own inline<style>—#menu .row, #menu .controls { display: none; }— hides the settings row (name, side, difficulty, map, sensitivity, etc.) and both Controls/Crosshair panels, so no dead button or dropdown is offered.No script is needed to reach that state: the CSS lives inside the
<noscript>itself, which a browser only parses and applies when scripting is disabled. For a normal, scripting-on visitor,<noscript>and everything inside it (including the<style>) is inert by the HTML spec's default UA stylesheet — nothing is rendered, nothing is parsed as CSS, so there's no layout shift and no flash of dead controls before anything applies. I checked this by reading the rendered HTML output directly (bun test+ manual grep ofgamePage's output) rather than a browser harness, consistent with this being a server-rendered page tested as a string.No CSP change was needed — the inline
<style>is covered by the existingstyle-src 'self' 'unsafe-inline'. The leaderboard section is untouched.Added one test in
src/pages/game.test.tsasserting the menu contains the<noscript>tag, the exact sentence, and the hiding rule, and that the rule sits inside thenoscriptblock rather than after it.bun run check(typecheck +bun test, 790 tests) is green.Closes #220