Skip to content

fix: 遊戲結束時自動捲動 modal 到最上方,讓窄版螢幕正常顯示鼓勵畫面 - #235

Merged
Aiuanyu merged 2 commits into
mainfrom
claude/game-end-modal-scroll-abfvxv
Aug 3, 2026
Merged

fix: 遊戲結束時自動捲動 modal 到最上方,讓窄版螢幕正常顯示鼓勵畫面#235
Aiuanyu merged 2 commits into
mainfrom
claude/game-end-modal-scroll-abfvxv

Conversation

@GJRobert

Copy link
Copy Markdown
Collaborator

窄螢幕(<=560px)下結果畫面改為直排堆疊,若使用者先前把 modal-body 往下捲
動過,結束遊戲時畫面不會自動回到最上方,導致阿翠妹的鼓勵圖文被裁切在
可視範圍外。比照答題畫面既有的 scrollIntoView 捲動邏輯,在 endSession()
顯示結果畫面時一併捲回最上方。

窄螢幕(<=560px)下結果畫面改為直排堆疊,若使用者先前把 modal-body 往下捲
動過,結束遊戲時畫面不會自動回到最上方,導致阿翠妹的鼓勵圖文被裁切在
可視範圍外。比照答題畫面既有的 scrollIntoView 捲動邏輯,在 endSession()
顯示結果畫面時一併捲回最上方。
@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@claude

claude Bot commented Jul 31, 2026

Copy link
Copy Markdown

Review: fix: 遊戲結束時自動捲動 modal 到最上方

Overview
Small, well-scoped fix: adds resultView.scrollIntoView({ behavior: 'smooth', block: 'start' }) in endSession() (js/game/game-ui.js:1150-1157) so the result view scrolls back to the top of the modal when a game session ends — mirroring the existing pattern already used for game-play-view (game-ui.js:658-662) and game-feedback (game-ui.js:888). The root cause (narrow-screen <=560px stacked layout pushing the encouragement image out of view if the user had scrolled the modal body) checks out against the @media (max-width: 560px) rules in style.css:4747.

Code quality

  • Consistent with the existing convention in this file (same getElementById + null-guard + scrollIntoView idiom used for game-play-view). Good reuse of an established pattern rather than inventing a new one.
  • game-result-view is a direct child of #game-modal-body (the scrollable container), same as game-play-view, so scrollIntoView targets the correct ancestor — no structural issue there.
  • Comment is short and explains the why (narrow-screen UX), consistent with repo style.

Potential issue: cache-busting convention not followed
AGENTS.md explicitly requires bumping the version query string in index.html whenever a .js/.css file is modified:

更新靜態資源版本: 當修改了專案的 .css 或 .js 檔案...必須同步至 index.html 更新對應檔案載入標籤的查詢字串(如 ?v=4.2.1 改為 ?v=4.2.2)。

index.html:36 still references js/game/game-ui.js?v=4.10.0, and this PR's diff only touches game-ui.js — the version string wasn't bumped. Per the project's own documented convention, users with a cached copy of game-ui.js may not pick up this fix. Worth bumping (e.g. ?v=4.10.1) before merge.

Minor / nitpick

  • The new scrollIntoView call fires immediately after showGameView('result'), before the score/breakdown/encouragement content is populated later in the same function (lines 1159-1179). This happens to work correctly today because game-result-view's top offset doesn't change when content below it (score text, breakdown HTML, audio playback) is filled in afterward — but it does diverge slightly from the game-play-view pattern, where the scroll call is placed after the content/options are rendered (game-ui.js:658-662). Not a bug, just a readability nit — moving the scroll call to the end of endSession() (after the breakdown/thumbup wiring) would make the ordering match the established convention and be more robust if the function is ever reordered later.
  • No functional/behavioral risk: the guard (if (resultView)) prevents errors if the element is ever missing, and behavior: 'smooth' degrades gracefully.

Performance
Negligible — a single scrollIntoView call on session end, no loops or repeated DOM queries.

Security
No concerns — no user input, no HTML injection, purely a scroll/UX fix.

Test coverage
No automated tests exist for game UI/DOM interactions in this repo (the tests/ directory only covers phonetics/sandhi logic), so the lack of a test here is consistent with existing project conventions. This is fundamentally a visual/UX fix best verified manually on a narrow viewport (<=560px) after scrolling the modal body mid-question, then finishing the session — worth a quick manual check before merge if not already done.

Summary
Solid, minimal, convention-following fix for a real UX bug. Main actionable item: bump the ?v= query string for game-ui.js in index.html per AGENTS.md's cache-busting convention. The content-population-order nitpick is optional polish, not a blocker.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 31, 2026

Copy link
Copy Markdown

Deploying hakspring with  Cloudflare Pages  Cloudflare Pages

Latest commit: ae1a8b0
Status: ✅  Deploy successful!
Preview URL: https://27233a84.hakspring.pages.dev
Branch Preview URL: https://claude-game-end-modal-scroll.hakspring.pages.dev

View logs

- scrollIntoView 移到 endSession() 最尾端,待戰績/鼓勵內容都渲染完再捲動,
  與 game-play-view 既有慣例一致
- 依 AGENTS.md 慣例同步將 game-ui.js 的版本查詢字串調到 4.10.1

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014Ly8xEFniyiQSXdaky969m

GJRobert commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

已依 review 建議調整(ae1a8b0):

  1. scrollIntoView 移到 endSession() 最尾端,等戰績/鼓勵內容都渲染完再捲動,跟 game-play-view 既有慣例一致。
  2. 依 AGENTS.md 慣例把 game-ui.js 的版本查詢字串同步調到 ?v=4.10.1

Cloudflare Pages 的 build failure 這邊沒有 dashboard 存取權限看不到詳細 log,這次 diff 只動到純前端的 game-ui.jsindex.html,研判與此改動無關,麻煩再確認一下。


Generated by Claude Code

@claude

claude Bot commented Aug 3, 2026

Copy link
Copy Markdown

Code Review

Overview

Small, well-scoped fix: when the game session ends and the result view renders, the modal body now auto-scrolls back to the top via scrollIntoView. This addresses the reported bug where, on narrow screens (≤560px) with the result layout stacked vertically, a modal body that was previously scrolled down stayed scrolled down when the result appeared — clipping the celebratory "阿翠妹" graphic out of view.

Code Quality & Conventions

  • Consistent with existing patterns: The fix mirrors the exact style already used elsewhere in game-ui.js for the same purpose — e.g. playView.scrollIntoView({ behavior: 'smooth', block: 'start' }) in the question-render flow (line ~661) and feedback.scrollIntoView(...) in the answer-feedback flow (line ~888). Same options object, same defensive null-check pattern, same inline Chinese comment style. This is a good sign of the PR following established conventions rather than inventing a new approach.
  • Correct placement: scrollIntoView is called at the very end of endSession(), after showGameView('result') has already made #game-result-view visible (display: block) and after all the DOM content (score, breakdown, thumbup handler) has been populated. This avoids scrolling against a hidden or not-yet-laid-out element, which could otherwise produce an incorrect/no-op scroll.
  • Cache-busting version bump: index.html's game-ui.js script tag version was bumped from 4.10.04.10.1, consistent with this repo's apparent convention of bumping the query-string version on any JS change.

Potential Issues / Considerations

  • Minor: behavior: 'smooth' combined with the audio auto-play (playAwesomeAudio) happening just before the scroll — both fire around the same time. This mirrors existing code elsewhere in the file, so it's not a regression, but worth a quick manual check that the smooth-scroll doesn't feel janky when it coincides with the audio kicking off.
  • Scroll target vs. focus: Since #game-result-view sits inside #game-modal-body (a scrollable container per the modal structure in index.html), scrollIntoView should correctly scroll that ancestor container rather than the whole page — this matches the PR description's intent and looks correct.
  • No functional/logic issues spotted; the null-check (if (resultView)) guards against the element being absent, consistent with the other two call sites.

Test Coverage

  • No automated tests are included, but this appears consistent with the rest of the codebase (no existing test suite was found for game-ui.js's UI/scroll behavior). Given the change is a small, visually-verifiable UI tweak mirroring existing patterns, manual verification on a narrow viewport (as described in the PR body) is likely the appropriate validation method here.

Security / Performance

  • No security concerns — purely a client-side DOM scroll adjustment.
  • No performance concerns — scrollIntoView is a cheap, one-time call at session end.

Overall this is a clean, minimal, low-risk fix that follows the codebase's existing conventions closely. Nice work.

@Aiuanyu
Aiuanyu merged commit 3e6cea8 into main Aug 3, 2026
2 checks passed
@Aiuanyu
Aiuanyu deleted the claude/game-end-modal-scroll-abfvxv branch August 3, 2026 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants