Keep the dictionary card on screen, and stop keyboard input under overlays

[min0625]

Why

The dictionary card was clamped horizontally but not vertically — top was hardcoded to anchor.bottom + 8. Anchored on the last grid row with a long definition (WordNet's GAS, 311 chars — the longest in the whole dataset; level 335 places it on the bottom row) the card ran off the bottom of the viewport. It is position: fixed, so it could be neither scrolled to nor tapped: the only way out was restarting the level. Desktop windows are tall enough that this almost never reproduces there.

What changed

• fitVertically(anchorRect, height, viewportHeight) (src/dictionary-card.js) — flips the card above the anchor when it does not fit below; when neither side fits (browser zoom, large system font) it takes the larger side and returns a maxHeight so the card scrolls itself. Split out as a pure function so the arithmetic is unit-testable (design doc §12); the measuring and style writing stay in show().
• .dict-card gets overflow-y: auto + overscroll-behavior: none, and joins the touchmove allowlist in main.js — without that a clamped card cannot be scrolled on iOS. It is now the third overflow-y: auto in the stylesheet.
• globalThis instead of window at dictionary-card.js module level, so the module imports under bun test (no DOM there — reading window would ReferenceError at import time).
• wheel.js dev-only keyboard input now bails inside .overlay. The clear and settings cards focus themselves, so Enter used to submit a word the player cannot see (scoring a hit or a miss under the mask) and letter keys mutated the covered #preview and the #status announcement. Reachable with a Bluetooth keyboard on a tablet, not just on a dev machine.
• tools/fetch-data.mjs comment: wordfreq is Apache-2.0, not MIT (design doc §14 already had it right).

Tests

Two new cases pin fitVertically: the card never leaves the viewport across a grid of anchor positions and card heights (including one taller than the screen), and it opens below the anchor when there is room, flipping up only when there is not. bun run check passes (48 tests, lint clean).

Manual verification stays on the §17 checklist — a new line covers level 335's GAS on a real device.

Docs

Design doc §4 (touchmove allowlist), §12 (testing split + the keyboard rule), §17 (acceptance line); UI doc §4-E (four-side clamping); CLAUDE.md testing split.

🤖 Generated with Claude Code

[qodo-code-review]

PR Summary by Qodo

Clamp dictionary card vertically; block wheel keyboard input under overlays

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

AI Description

• Clamp dictionary card vertically, flipping above anchors and self-scrolling when constrained.
• Allow iOS scrolling inside the dictionary card via overflow and touchmove allowlist.
• Ignore dev wheel keyboard input while overlays are open; add unit tests and docs notes.

Diagram

graph TD
  Anchor["Grid cell anchor"] --> Show["dictionary-card.show()"] --> Fit["fitVertically()"] --> Css[".dict-card scroll"]
  Touch["main.js touchmove allowlist"] --> Css
  Fit --> Tests["tests/game.test.mjs"]
  Overlay[".overlay dialogs"] --> WheelKey["wheel.js dev keyboard"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Adopt a positioning library (Floating UI/Popper)

• ➕ Battle-tested flip/shift/size middleware for viewport collision handling
• ➕ Less bespoke geometry code to maintain over time
• ➖ Introduces a new dependency/bundle weight for a small custom popover
• ➖ Harder to keep the 'pure arithmetic' unit-test split aligned with project testing philosophy

2. Use native Popover + CSS Anchor Positioning

• ➕ More declarative; browser engine handles collision/viewport constraints
• ➕ Potentially less JS and fewer reflow-sensitive measurements
• ➖ Insufficient cross-browser support (notably on iOS Safari) for a core UI affordance
• ➖ Still may need JS fallbacks, reintroducing complexity

3. CSS clamp-based layout with JS-set custom properties

• ➕ Moves most constraints to CSS (e.g., clamp() + max-height)
• ➕ Reduces branching logic in JS
• ➖ Still requires JS to compute anchor-relative variables; clamp math gets subtle
• ➖ More difficult to unit-test reliably than a pure function returning explicit numbers

Recommendation: Keep the current approach: a small, dependency-free placement policy extracted into a pure function (fitVertically) with unit tests is well aligned with the project's “test pure logic” guideline, and it directly addresses the fixed-position off-screen failure mode. The added CSS scrolling + touchmove allowlist completes the iOS usability story without introducing heavier infrastructure.

Files changed (9) +73 / -13

Bug fix (4) +42 / -8

dictionary-card.jsAdd fitVertically() and clamp dictionary card within viewport +30/-5

Add fitVertically() and clamp dictionary card within viewport

• Replaces module-level window usage with globalThis to allow importing in bun test. Extracts vertical placement into a pure fitVertically() helper that flips above the anchor when needed and returns a maxHeight for internal scrolling, then uses it in show() after clearing stale maxHeight before measurement.

src/dictionary-card.js

main.jsAllow .dict-card to receive touchmove for iOS scrolling +2/-2

Allow .dict-card to receive touchmove for iOS scrolling

• Updates the global touchmove preventDefault gate to whitelist .dict-card in addition to .level-list and .card, ensuring the dictionary card remains scrollable when constrained on iOS/WKWebView.

src/main.js

style.cssEnable vertical scrolling and overscroll containment for .dict-card +5/-0

Enable vertical scrolling and overscroll containment for .dict-card

• Adds overflow-y: auto and overscroll-behavior: none to the dictionary card, relying on JS-written max-height to keep long definitions usable without leaving the viewport.

src/style.css

wheel.jsBlock dev keyboard wheel input when overlays are open +5/-1

Block dev keyboard wheel input when overlays are open

• Extends the dev-only keydown handler early-return condition to also bail when the event target is inside an .overlay, preventing hidden submissions/mutations under modal overlays.

src/wheel.js

Tests (1) +24 / -0

game.test.mjsAdd unit tests for fitVertically viewport guarantees and flip behavior +24/-0

Add unit tests for fitVertically viewport guarantees and flip behavior

• Imports fitVertically and adds two tests: one ensures the card always remains within the viewport across a grid of anchor positions/heights (including taller-than-viewport cards), and another asserts it opens below when it fits and flips above only when necessary.

tests/game.test.mjs

Documentation (4) +7 / -5

CLAUDE.mdDocument dictionary-card placement as unit-tested pure logic +1/-1

Document dictionary-card placement as unit-tested pure logic

• Updates the testing-split guidance to explicitly include dictionary card vertical placement (fitVertically) as pure logic covered by unit tests, explaining why it must be pinned due to position: fixed off-screen failure modes.

CLAUDE.md

word-puzzle-game-design.mdUpdate design doc for dict-card scrolling and overlay keyboard rule +4/-2

Update design doc for dict-card scrolling and overlay keyboard rule

• Extends the touchmove allowlist rationale to include .dict-card as a third scrollable container. Adds guidance that dev keyboard input should not operate when overlays are open, and adds an acceptance checklist line for the level-335 long-definition edge case.

docs/word-puzzle-game-design.md

word-puzzle-ui-screens.mdSpecify four-side clamping and self-scrolling behavior for dictionary card +1/-1

Specify four-side clamping and self-scrolling behavior for dictionary card

• Expands the dictionary card spec to require vertical clamping (flip above anchor, otherwise constrain and scroll). Documents the relationship between max-height/overflow-y and the touchmove allowlist for iOS.

docs/word-puzzle-ui-screens.md

fetch-data.mjsCorrect wordfreq license note to Apache-2.0 +1/-1

Correct wordfreq license note to Apache-2.0

• Fixes the inline comment describing wordfreq’s license from MIT to Apache-2.0 to match the project’s licensing documentation.

tools/fetch-data.mjs

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (1) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 80 rules

✅ Skills: frontend-design

1. Overlay guard focus-bypass 🐞 Bug ≡ Correctness

Description

createWheel()'s dev keyboard handler only bails when the keydown event target is inside
.overlay, so if focus escapes the overlay (e.g., via Tab) the handler can still submit/mutate the
hidden game UI while the overlay mask is visible. This contradicts the intended “stop keyboard input
under overlays” behavior and can still trigger invisible submissions/preview changes.

Code

src/wheel.js[219]

+    if (ev.target.closest?.('input, textarea, .overlay')) return;

Evidence

Overlays are explicitly documented as having **no JS focus trap**, meaning focus can escape them via
Tab, but the new wheel keyboard guard only checks whether the event target is inside .overlay, not
whether an overlay is visible. Therefore, keyboard input can still reach the wheel when an overlay
is open but focus is outside it.

src/wheel.js[213-221]
index.html[141-147]
src/main.js[591-607]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`src/wheel.js` blocks keyboard input only when `ev.target.closest('.overlay')` is truthy. Overlays intentionally do **not** implement a JS focus trap, so focus can escape via Tab; once focus is outside the overlay, keyboard input can again drive the wheel while an overlay remains visible.

## Issue Context
This PR’s goal includes preventing keyboard-driven submissions under overlays. Relying on focus containment is weaker than checking whether an overlay is currently shown.

## Fix Focus Areas
- src/wheel.js[213-237]

## Suggested fix
In `onKey`, add an early return based on overlay visibility, not focus ancestry, e.g.:

```js
if (document.querySelector('.overlay:not([hidden])')) return;
```

Optionally keep the existing `input, textarea` check as well.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. CLAUDE.md includes .local.* reference 📘 Rule violation ⚙ Maintainability

Description

The modified documentation line in CLAUDE.md contains the substring .local.*, which is
explicitly disallowed in comments/docs. This can mislead readers toward local-only artifacts that
are not available in other clones/environments.

Code

CLAUDE.md[61]

+Only pure logic is auto-tested (`tests/*.test.mjs`): game rules, wheel hit/selection math, save normalization, redemption-code verification, the dictionary card's vertical placement (`fitVertically` — the card is `position: fixed`, so an unclamped `top` puts it off-screen where it can be neither scrolled to nor dismissed; the arithmetic is split out from the measuring exactly so it can be pinned here), `bridge.share`'s tri-state return (the one non-pure exception — the test swaps `navigator`/`document` on `globalThis`; its decision table is documented in UI doc §4-D and only misbehaves on a real device), plus a validator pass over `data/levels/` and consistency checks on the two machine-checkable PWA shell lists (`sw.js`'s `SHELL`, `index.html`'s `modulepreload` — see Architecture for why `main.js`'s imports are not one of them) and on `deploy.yml`'s root-file `cp` allowlist, plus a grep of the tracked tree for `.local.*` citations (see the Doc sync map for why). Unit tests use inline fixtures — never assert on `data/levels/` contents, which change on regeneration. UI, animation, and touch feel are manually tested against the §17 acceptance checklist on real devices. Desktop keyboard input in the wheel (letter keys / Backspace / Enter) exists for dev iteration, not for players.

Evidence

PR Compliance ID 2366275 forbids referencing .local.* (or any .local. substring) in newly
added/modified documentation. The updated line in CLAUDE.md explicitly contains .local.*.

Rule 2366275: Do not reference .local.* files in comments or documentation
CLAUDE.md[61-61]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A modified documentation line includes the substring `.local.*`, which violates the rule prohibiting references to `.local.*` files/patterns in comments and documentation.

## Issue Context
The compliance rule forbids mentioning any `.local.` substring in docs/comments, regardless of intent.

## Fix Focus Areas
- CLAUDE.md[61-61]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

[qodo-code-review]

1. claude.md includes .local.* reference 📘 Rule violation ⚙ Maintainability

The modified documentation line in CLAUDE.md contains the substring .local.*, which is
explicitly disallowed in comments/docs. This can mislead readers toward local-only artifacts that
are not available in other clones/environments.

Agent Prompt

## Issue description
A modified documentation line includes the substring `.local.*`, which violates the rule prohibiting references to `.local.*` files/patterns in comments and documentation.

## Issue Context
The compliance rule forbids mentioning any `.local.` substring in docs/comments, regardless of intent.

## Fix Focus Areas
- CLAUDE.md[61-61]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

[qodo-code-review]

2. Overlay guard focus-bypass 🐞 Bug ≡ Correctness

createWheel()'s dev keyboard handler only bails when the keydown event target is inside
.overlay, so if focus escapes the overlay (e.g., via Tab) the handler can still submit/mutate the
hidden game UI while the overlay mask is visible. This contradicts the intended “stop keyboard input
under overlays” behavior and can still trigger invisible submissions/preview changes.

Agent Prompt

## Issue description
`src/wheel.js` blocks keyboard input only when `ev.target.closest('.overlay')` is truthy. Overlays intentionally do **not** implement a JS focus trap, so focus can escape via Tab; once focus is outside the overlay, keyboard input can again drive the wheel while an overlay remains visible.

## Issue Context
This PR’s goal includes preventing keyboard-driven submissions under overlays. Relying on focus containment is weaker than checking whether an overlay is currently shown.

## Fix Focus Areas
- src/wheel.js[213-237]

## Suggested fix
In `onKey`, add an early return based on overlay visibility, not focus ancestry, e.g.:

```js
if (document.querySelector('.overlay:not([hidden])')) return;
```

Optionally keep the existing `input, textarea` check as well.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools