Repository navigation
Fix repeat-status silence, all-clear gate label, and wheel keyboard leak - #91
Merged
Merged
Conversation
…eaking under overlays
- showPreview clears #status itself before the 900ms timer would, so a
second identical message (e.g. two invalid words in a row) still gets
announced by VoiceOver instead of being swallowed by unchanged textContent.
- boot() rewrites the gate's level line to the all-clear string when the
save's currentLevel is out of range, so a player who cleared level 500
doesn't see "Level 501" before landing on the all-clear screen.
- Wheel keydown handling now checks whether any overlay/gate is open or the
game screen is hidden, not just focus location — closing a clear-card
overlay or the opening gate never focuses the card, so the old
`.closest('.overlay')` check let keystrokes leak into an invisible wheel
(submitting unseen words, mutating hidden #preview/#status).
- .card:focus outline removed (card is a programmatic focus target, not a
tabbable control) and CJK line-height fixed on .chip so the player-id pill
doesn't grow taller when its label switches to Chinese "已複製" text.
- shareCopied wording reworded for clarity.
- Docs and tests updated to match.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
PR Summary by QodoFix repeated status announcements, gate labels, and wheel key leaks
AI Description
Diagram
High-Level Assessment
Files changed (8)
|
Code Review by Qodo
1. Added comments use Chinese
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+277
to
+279
| // 取消的那個 timer 同時是 #status 的清空者，所以這裡要自己補清一次：下一次拖曳在 900ms 內 | ||
| // 開始（拼錯後馬上重試就是這樣）會走到這裡，不清的話 #status 就永久停在上一句， | ||
| // 而連撞兩次同樣的訊息時 textContent 沒變 → 輔助技術不會播報第二次，正是下面那段註解要避免的事。 |
There was a problem hiding this comment.
1. Added comments use chinese 📘 Rule violation ⚙ Maintainability
New comments in multiple source files contain Chinese natural-language text. The checklist requires all source-code comments, including mixed-language comments, to be written in English.
Agent Prompt
## Issue description
New source-code comments are written wholly or partly in Chinese, contrary to the English-only comment requirement.
## Issue Context
Preserve identifiers and technical meaning while translating all newly added human-readable comments into English.
## Fix Focus Areas
- src/main.js[277-279]
- src/main.js[865-867]
- src/strings.js[46-46]
- src/style.css[168-170]
- src/style.css[660-662]
- src/wheel.js[7-18]
- tests/game.test.mjs[260-264]
- tests/game.test.mjs[266-266]
- tests/game.test.mjs[270-270]
- tests/game.test.mjs[282-282]
- tests/game.test.mjs[287-289]
- tests/game.test.mjs[296-296]
- tests/game.test.mjs[301-301]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+268
to
+270
| test('uid：合法編號驗得過，打錯一碼被校驗碼擋下', () => { | ||
| assert.ok(isUid(UID)); | ||
| assert.equal(isUid(`0${UID.slice(1)}`), false); // 首碼 4 → 0 |
There was a problem hiding this comment.
2. Checksum vectors remain untested 📘 Rule violation ≡ Correctness
The new UID test changes a body character but only verifies that the unchanged checksum is rejected; it never asserts the predictable checksum characters for the modified body. This leaves the checksum test requirement incomplete and can miss regressions outside the single fixed UID.
Agent Prompt
## Issue description
The UID tests do not assert the exact checksum produced for changed 10-character bodies.
## Issue Context
PR Compliance ID 2241551 requires predictable checksum coverage, not only rejection of a body paired with its old checksum. Add deterministic vectors that exercise the checksum calculation and assert the expected final two characters; minimally expose the checksum helper for testing if necessary.
## Fix Focus Areas
- tests/game.test.mjs[268-270]
- src/storage.js[24-27]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| // 全破的存檔 currentLevel 是 levelCount+1（一個不存在的關），上面那行閘門文案先寫的就是它—— | ||
| // 破完 500 關的人會看到「第 501 關」，點下去卻是全破畫面。要到這裡才知道超界，所以改在這裡補寫： | ||
| // 此時閘門幾乎必然還蓋著（玩家還沒點），而且不必為了正確的第一印象多等 index.json 那趟 RTT。 | ||
| if (save.currentLevel > levelCount) $('gate-level').textContent = strings.allClear; |
There was a problem hiding this comment.
3. All-clear label still flashes 🐞 Bug ≡ Correctness
boot() renders the out-of-range level immediately and applies the all-clear correction only after index.json finishes loading. On a slow request, the gate therefore shows the nonexistent level and can be dismissed before the correction runs, defeating this fix's stated behavior.
Agent Prompt
## Issue description
The all-clear gate initially displays the out-of-range saved level and remains dismissible while the level index is loading. Ensure the gate cannot present or be dismissed with the incorrect label.
## Issue Context
`boot()` writes `strings.levelTitle(save.currentLevel)` before awaiting `index.json`, while the existing click handler can hide the gate at any time. The added correction therefore arrives too late under network latency.
## Fix Focus Areas
- src/main.js[844-868]
- index.html[75-79]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+663
to
+664
| .card:focus { | ||
| outline: none; |
There was a problem hiding this comment.
4. Focused dialogs lose indicator 🐞 Bug ≡ Correctness
The new rule removes the outline from cards even though opening either dialog programmatically moves focus to that card. Sighted keyboard users consequently receive no visible indication of the initial focus location until they Tab to a child control.
Agent Prompt
## Issue description
Dialog cards receive programmatic focus but now suppress its only explicit visual indication. Preserve a focus indicator when the dialog was opened through keyboard interaction while avoiding an unwanted pointer-focus ring.
## Issue Context
Both cards use `tabindex="-1"`, and `main.js` focuses them when they open so assistive technology announces their headings. An unconditional `:focus` outline removal also affects keyboard-triggered openings; use an appropriate `:focus-visible` treatment or a custom indicator instead.
## Fix Focus Areas
- src/style.css[660-665]
- src/main.js[479-483]
- src/main.js[687-694]
- index.html[129-153]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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
1 participant
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.
Summary
showPreviewnow clears#statusitself before the 900ms timer would, so two identical VoiceOver announcements in a row (e.g. two invalid words back to back) aren't swallowed by unchangedtextContent.boot()rewrites the opening gate's level line to the all-clear string when the save'scurrentLevelis out of range, so a player who cleared level 500 doesn't briefly see "Level 501" before landing on the all-clear screen.keydownhandling now checks whether any overlay/gate is open or#screen-gameis hidden, instead of relying onev.target.closest('.overlay')— that check only worked when focus happened to be inside the card, which isn't true when the clear-card overlay is dismissed (focus returns tobody) or when the opening gate is showing (never focused at all). Previously keystrokes could leak into an invisible wheel and submit an unseen word..card:focusoutline removed (the card is a programmatic focus target for VoiceOver, not a tabbable control) and.chipline-height pinned so the player-id pill doesn't grow taller when its label switches to the Chinese "已複製" text.shareCopiedcopy reworded for clarity.docs/word-puzzle-game-design.md,docs/word-puzzle-ui-screens.md) and tests updated to match, plus new unit tests for the player-id round trip (newUid/isUid/normalizeUid/formatUid).Test plan
mise run check(biome lint +bun test, 52 tests pass)🤖 Generated with Claude Code