fix: enforce authentication before loading leaderboard - #12
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR enforces authentication before allowing users to access the leaderboard screen by redirecting unauthenticated users to the player name entry flow.
- Adds authentication pre-check to the leaderboard screen that routes unauthenticated users through name entry
- Updates player name flow to support returning to the leaderboard after successful authentication
- Implements proper component lifecycle management with mount checking to prevent state updates on unmounted components
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| frontend/src/types/components.ts | Adds 'Leaderboard' as a valid navigation target for PlayerNameScreen |
| frontend/src/components/onboarding/PlayerNameScreen/PlayerNameScreen.tsx | Adds navigation logic to route to Leaderboard screen after name entry |
| frontend/src/components/leaderboard/LeaderboardScreen/LeaderboardScreen.tsx | Implements authentication check and proper lifecycle management for leaderboard loading |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| const { playerName } = usePlayer(); | ||
| const trimmedName = playerName.trim(); | ||
| const hasPlayerName = trimmedName.length > 0; |
There was a problem hiding this comment.
The playerName.trim() operation is executed on every render. Consider memoizing this computation using useMemo to avoid unnecessary string operations when playerName hasn't changed.
| } | ||
| }, [ensureAuthenticated, hasPlayerName, loadLeaderboard, navigation]); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
[nitpick] Using 'void' to ignore the promise return value in useEffect is acceptable, but consider adding error handling or at least a comment explaining why the promise rejection is intentionally not handled here.
| useEffect(() => { | |
| useEffect(() => { | |
| // All errors are handled inside verifyAndLoad; promise rejection is intentionally ignored here. |
Summary
Testing
https://chatgpt.com/codex/tasks/task_b_68de73c72c1083228b7b6892ac4a1723