fix: semantic modal routing, remove win overlay, tighten mobile header#193
Merged
Conversation
- PageShell: noKeyboard modes (semantic) emit @results instead of opening generic GameStatsModal — semantic page opens its own SemanticStatsModal. Generic StatsModal suppressed with v-if. - Remove win overlay ("Found bread" badge + green border) from semantic — inconsistent with other modes that use toasts - Remove dead CSS (win-overlay, win-badge, celebrate-fade, map-card.won) - Remove unused rankColor function from SemanticStatsModal - Mobile header: tighter horizontal padding (px-1.5 → px-3 at sm) to give more room for title text
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughCentralized game-over lifecycle into a new composable, wired semantic/page and PageShell to emit/use results differently, removed win-state UI and related styles, added mobile input focus handling, improved multi-board restoration and immediate tile rendering, many modal components switched to BaseModal, and added a reveal transition/component. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pages/[lang]/semantic.vue (1)
413-413: Remove leftover win-overlay scoped CSS that is now unreachable.After dropping the win overlay/card won state, the scoped mobile selectors at Line 742–Line 753 (
.win-overlay,.win-badge,.win-word,.win-stat) look dead and can be removed to complete the cleanup.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pages/`[lang]/semantic.vue at line 413, Remove the now-unreachable scoped mobile CSS selectors related to the removed win overlay/card by deleting the rules for .win-overlay, .win-badge, .win-word, and .win-stat from the component's scoped style block; search for these class selectors in the semantic component and remove their entire rule sets (the leftover mobile selectors previously at the bottom of the style) so the scoped CSS no longer contains dead styles.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pages/`[lang]/semantic.vue:
- Line 413: Remove the now-unreachable scoped mobile CSS selectors related to
the removed win overlay/card by deleting the rules for .win-overlay, .win-badge,
.win-word, and .win-stat from the component's scoped style block; search for
these class selectors in the semantic component and remove their entire rule
sets (the leftover mobile selectors previously at the bottom of the style) so
the scoped CSS no longer contains dead styles.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a550a86f-3111-482d-83bb-8556606ec6c8
📒 Files selected for processing (4)
components/app/AppHeader.vuecomponents/game/PageShell.vuecomponents/semantic/SemanticStatsModal.vuepages/[lang]/semantic.vue
💤 Files with no reviewable changes (1)
- components/semantic/SemanticStatsModal.vue
Critical bugs fixed: - Stats not showing on daily revisit: now calls calculateStats() with the semantic key after loading game results - Duplicate saves on restore: game-over watcher returns early when restoring from localStorage (neighbours already present) - Server sync missing: semantic bypasses game store's gameOver, so the sync plugin never fires. Now posts to /api/user/game-result directly from the semantic page - Mobile keyboard scroll: overflow-y:hidden on semantic body at ≤520px prevents the browser from scrolling behind the fixed input
loadMultiBoardFromLocalStorage() existed but was never called. Now useMultiBoardPage tries to restore daily games from localStorage before creating fresh boards — matching classic daily's pattern. Fixes: revisiting a completed dordle/quordle daily showed a fresh game instead of the completed board.
Owner
Author
|
Updated with multi-board daily restore fix + semantic state bugs. See updated test plan above. @coderabbitai review |
|
✅ Actions performedReview triggered.
|
DRY refactor: - New useGameLifecycle() composable centralizes stats save, server sync, stats recalculation, and duplicate-save guards. Used by semantic; available for any future non-tile mode. - Semantic page refactored to use lifecycle.handleGameOver() and lifecycle.initStats() instead of inline logic (~30 lines removed) Multi-board daily restore: - useMultiBoardPage now calls loadMultiBoardFromLocalStorage() for daily modes before creating fresh boards. Completed dordle/quordle dailies now show completed state on revisit. Mobile keyboard scroll (improved): - Reverted overflow-y:hidden (broke scrolling to leaderboard) - Instead: save/restore scroll position on input focus/blur to counteract browser's auto-scroll on keyboard open
- Per-mode rows now show games played + avg attempts (not just count + win%) - Speed Streak stats shown inline in overview (sessions, best score, best words, best combo) instead of a separate tab - Removed Speed tab — all stats visible in overview
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
composables/useMultiBoardPage.ts (1)
90-96:⚠️ Potential issue | 🟠 MajorRemove
wordList.lengthas a hard gate for daily fallback initialization.At Line 90, daily fallback is blocked when
wordListis empty, even thoughstartNewGame()can initialize fromdailyWordsalone. If restore fails in that state, boards won’t initialize.💡 Proposed fix
- if (!restored && wordList.length > 0) { + const hasDailyWords = dailyWords?.length === boardCount; + if (!restored && (hasDailyWords || wordList.length > 0)) { try { startNewGame(); } catch { // Initialization failed silently } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@composables/useMultiBoardPage.ts` around lines 90 - 96, The current conditional uses "if (!restored && wordList.length > 0) { startNewGame() }", which prevents daily fallback initialization when wordList is empty; remove the wordList.length check so that when restored is false you call startNewGame() regardless (i.e., change the conditional to check only restored), keeping the try/catch around startNewGame() so startNewGame can initialize from dailyWords even when wordList is empty; update references in this block (the restored variable and startNewGame function) accordingly.
🧹 Nitpick comments (3)
components/semantic/SemanticInput.vue (1)
51-67: Clarify the behavior comment and drop the no-op blur path.Line 53 says “restore on blur,” but the restore currently happens on focus (Lines 60–62), and
onBlur(Lines 65–67) is empty. This is a small readability/maintenance mismatch.Proposed cleanup
-// on mobile — it doesn't need scrolling. Lock scroll position on focus, restore on blur. +// on mobile — it doesn't need scrolling. Lock scroll position immediately on focus. let savedScrollTop = 0; function onFocus() { @@ } -function onBlur() { - // No action needed — scroll is already at the saved position -}- `@focus`="onFocus" - `@blur`="onBlur" + `@focus`="onFocus"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/semantic/SemanticInput.vue` around lines 51 - 67, Comment incorrectly says “restore on blur” while the scroll restoration happens inside onFocus; update the comment to accurately describe behavior (e.g., “prevent browser auto-scroll on mobile by saving and reapplying the scrollTop in onFocus”) and remove the no-op onBlur function; keep savedScrollTop, onFocus, inputRef, isTouch and the '.semantic-body' closest lookup as-is and ensure the requestAnimationFrame reset logic remains in onFocus.components/semantic/SemanticStatsModal.vue (1)
57-57: Prefer top alignment for this stats/end-game modal.
BaseModaldefaults to centered alignment; this modal is content-heavy and usually reads better with top alignment (percomponents/shared/BaseModal.vue:54-78API intent).Proposed tweak
- <BaseModal :visible="visible" size="lg" label-id="semantic-stats-label" `@close`="emit('close')"> + <BaseModal :visible="visible" size="lg" align="top" label-id="semantic-stats-label" `@close`="emit('close')">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/semantic/SemanticStatsModal.vue` at line 57, Update the BaseModal usage in SemanticStatsModal.vue to request top alignment instead of the default centered alignment by adding the alignment prop (e.g., align="top") to the BaseModal component invocation; modify the tag <BaseModal :visible="visible" size="lg" label-id="semantic-stats-label" `@close`="emit('close')"> to include the alignment prop so the modal renders top-aligned per BaseModal's API.composables/useGameLifecycle.ts (1)
62-64: Avoid fully silent server-sync failuresThe empty catch makes production sync issues hard to diagnose. Consider at least lightweight logging (e.g., dev-only warning).
🔎 Proposed refinement
- }).catch(() => { - /* non-fatal — will sync on next full sync */ - }); + }).catch((err) => { + if (import.meta.dev) { + console.warn('[game-lifecycle] server sync failed', err); + } + /* non-fatal — will sync on next full sync */ + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@composables/useGameLifecycle.ts` around lines 62 - 64, The empty catch in useGameLifecycle swallows server-sync failures; replace the silent handler with a lightweight log that includes the caught error (e.g., .catch((err) => { /* log err */ })), and only emit a warning in non-production builds (use NODE_ENV or the existing app logger if available) so failures are visible during development while remaining non-fatal in production.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@composables/useGameLifecycle.ts`:
- Around line 43-45: After saving and updating mode-specific stats
(stats.saveResult and stats.calculateStats) the aggregate totals remain stale
because totals are only recomputed in initStats; fix this by invoking the totals
recomputation after a successful save — call initStats() (or the function that
recalculates aggregate totals) right after stats.calculateStats(statsKey,
config.maxGuesses) inside the same try block so totals are immediately updated
for the UI.
---
Outside diff comments:
In `@composables/useMultiBoardPage.ts`:
- Around line 90-96: The current conditional uses "if (!restored &&
wordList.length > 0) { startNewGame() }", which prevents daily fallback
initialization when wordList is empty; remove the wordList.length check so that
when restored is false you call startNewGame() regardless (i.e., change the
conditional to check only restored), keeping the try/catch around startNewGame()
so startNewGame can initialize from dailyWords even when wordList is empty;
update references in this block (the restored variable and startNewGame
function) accordingly.
---
Nitpick comments:
In `@components/semantic/SemanticInput.vue`:
- Around line 51-67: Comment incorrectly says “restore on blur” while the scroll
restoration happens inside onFocus; update the comment to accurately describe
behavior (e.g., “prevent browser auto-scroll on mobile by saving and reapplying
the scrollTop in onFocus”) and remove the no-op onBlur function; keep
savedScrollTop, onFocus, inputRef, isTouch and the '.semantic-body' closest
lookup as-is and ensure the requestAnimationFrame reset logic remains in
onFocus.
In `@components/semantic/SemanticStatsModal.vue`:
- Line 57: Update the BaseModal usage in SemanticStatsModal.vue to request top
alignment instead of the default centered alignment by adding the alignment prop
(e.g., align="top") to the BaseModal component invocation; modify the tag
<BaseModal :visible="visible" size="lg" label-id="semantic-stats-label"
`@close`="emit('close')"> to include the alignment prop so the modal renders
top-aligned per BaseModal's API.
In `@composables/useGameLifecycle.ts`:
- Around line 62-64: The empty catch in useGameLifecycle swallows server-sync
failures; replace the silent handler with a lightweight log that includes the
caught error (e.g., .catch((err) => { /* log err */ })), and only emit a warning
in non-production builds (use NODE_ENV or the existing app logger if available)
so failures are visible during development while remaining non-fatal in
production.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b1b969dc-32f1-4c08-bf3f-1d9deb4ba882
📒 Files selected for processing (6)
components/semantic/SemanticInput.vuecomponents/semantic/SemanticStatsModal.vuecomposables/useGameLifecycle.tscomposables/useMultiBoardPage.tspages/[lang]/semantic.vuestores/game.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- pages/[lang]/semantic.vue
Comment on lines
+43
to
+45
| stats.saveResult(statsKey, event.won, event.attempts); | ||
| stats.calculateStats(statsKey, config.maxGuesses); | ||
| } catch (e) { |
There was a problem hiding this comment.
Recalculate total stats after a successful save
On Line 44 you update mode stats, but aggregate totals are only recomputed in initStats (Line 73). This can leave totals stale right after a game ends.
💡 Proposed fix
try {
stats.saveResult(statsKey, event.won, event.attempts);
stats.calculateStats(statsKey, config.maxGuesses);
+ stats.calculateTotalStats();
} catch (e) {
console.warn('[game-lifecycle] stats save failed', e);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| stats.saveResult(statsKey, event.won, event.attempts); | |
| stats.calculateStats(statsKey, config.maxGuesses); | |
| } catch (e) { | |
| stats.saveResult(statsKey, event.won, event.attempts); | |
| stats.calculateStats(statsKey, config.maxGuesses); | |
| stats.calculateTotalStats(); | |
| } catch (e) { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@composables/useGameLifecycle.ts` around lines 43 - 45, After saving and
updating mode-specific stats (stats.saveResult and stats.calculateStats) the
aggregate totals remain stale because totals are only recomputed in initStats;
fix this by invoking the totals recomputation after a successful save — call
initStats() (or the function that recalculates aggregate totals) right after
stats.calculateStats(statsKey, config.maxGuesses) inside the same try block so
totals are immediately updated for the UI.
All game result syncing now flows through useGameLifecycle: - syncGameResult(): handles auth, badge notifications, offline queue - syncSpeedResult(): handles speed-specific endpoint - useSync.ts refactored to call lifecycle methods instead of inline $fetch — removes ~40 lines of duplicated sync logic - Semantic page already uses lifecycle.handleGameOver() which calls syncGameResult() internally One composable, three entry points (tile gameOver watcher, speed results watcher, semantic page watcher), zero duplication.
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
- RevealTransition component: smooth fade+slide for auth-dependent content (homepage greeting, continue playing, profile sections, badges) - Drop "Shared" prefix: nuxt.config pathPrefix:false for shared/ folder, BaseModal/ToggleSwitch/StreakCalendar etc no longer need Shared prefix - Day rollover detection: useDayRollover composable reloads game pages when tab returns from background and the daily word has changed. Prevents stale word coloring bug (fixes #192)
This was referenced Apr 11, 2026
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
@resultsfornoKeyboardmodes instead of opening the genericGameStatsModal. Semantic page opens its ownSemanticStatsModal. Generic modal suppressed withv-if="!noKeyboard".map-card.wonborder removed. These were inconsistent with other modes (which use notification toasts). ~70 lines of dead CSS removed.rankColorfunction,celebrate-fadetransitions,win-overlay/win-badgestylespx-1.5on mobile (→px-3atsm) gives more room for title textTest plan
@coderabbitai review
Summary by CodeRabbit
Style
Refactor
New Features
Bug Fixes