Fix #156 Add feat: Added gamification to celebrate new badges and top 10 appearance #166
Conversation
📝 WalkthroughWalkthroughThe PR refactors gamification UI by extracting badge-display logic from the Leaderboard page into a new global GamificationOverlay component. The overlay listens to WebSocket events for badge awards and score updates, displaying badge notifications globally. A Top10 badge with crown icon is added to recognize users entering the top 10 leaderboard. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App (Root)
participant Overlay as GamificationOverlay
participant WS as WebSocket
participant User as Backend/User
participant Badge as BadgeUnlocked
App->>Overlay: Mount component
Note over Overlay: Fetch leaderboard<br/>(init previousRank)
Overlay->>WS: Connect with token
User->>WS: Emit badge_awarded event
WS->>Overlay: Receive gamification event
Overlay->>Overlay: Update badge state (badgeName, isOpen)
Overlay->>Badge: Render with state
Badge-->>App: Display badge dialog
User->>WS: Emit score_updated event
WS->>Overlay: Receive score event
Overlay->>Overlay: Fetch updated leaderboard
Overlay->>Overlay: Check rank transition
alt User entered Top 10
Overlay->>Overlay: Emit Top10 badge unlock
Overlay->>Badge: Render Top10 badge
Badge-->>App: Display Top10 dialog
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
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 |
DeveloperAmrit
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/src/components/GamificationOverlay.tsx (1)
86-86: Simplify state update in onClose handler.The spread operator is unnecessary since only
isOpenis changing. ThebadgeNamecan remain unchanged without spreading the entire state object.🔎 Proposed simplification
<BadgeUnlocked badgeName={badgeUnlocked.badgeName} isOpen={badgeUnlocked.isOpen} - onClose={() => setBadgeUnlocked({ ...badgeUnlocked, isOpen: false })} + onClose={() => setBadgeUnlocked({ badgeName: badgeUnlocked.badgeName, isOpen: false })} />Or use a functional update:
<BadgeUnlocked badgeName={badgeUnlocked.badgeName} isOpen={badgeUnlocked.isOpen} - onClose={() => setBadgeUnlocked({ ...badgeUnlocked, isOpen: false })} + onClose={() => setBadgeUnlocked(prev => ({ ...prev, isOpen: false }))} />
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
frontend/src/App.tsxfrontend/src/Pages/Leaderboard.tsxfrontend/src/components/BadgeUnlocked.tsxfrontend/src/components/GamificationOverlay.tsx
💤 Files with no reviewable changes (1)
- frontend/src/Pages/Leaderboard.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/src/components/GamificationOverlay.tsx (2)
frontend/src/hooks/useUser.ts (1)
useUser(12-112)frontend/src/services/gamificationService.ts (2)
fetchGamificationLeaderboard(41-55)createGamificationWebSocket(93-129)
frontend/src/App.tsx (1)
frontend/src/components/ui/toaster.tsx (1)
Toaster(11-33)
🔇 Additional comments (4)
frontend/src/components/BadgeUnlocked.tsx (1)
3-3: LGTM! Top10 badge addition is well-integrated.The new Top10 badge follows the existing pattern consistently. The crown icon and yellow-400 color appropriately convey elite status, and the description clearly celebrates the achievement.
Also applies to: 19-19, 28-28
frontend/src/App.tsx (1)
31-32: LGTM! Global overlays are properly positioned.The GamificationOverlay and Toaster are correctly placed after AppRoutes within the provider hierarchy, ensuring they have access to authentication and theme context while remaining globally available across all routes.
Also applies to: 115-116
frontend/src/components/GamificationOverlay.tsx (2)
38-71: LGTM! WebSocket event handling logic is sound.The event filtering by
userIdprevents cross-user events, and the rank-change detection logic correctly identifies when a user enters the top 10 from outside. The null check onoldRankappropriately prevents false positives on the first rank update.
75-79: LGTM! WebSocket cleanup is properly implemented.The cleanup function correctly closes the WebSocket connection when the component unmounts or when dependencies change, preventing memory leaks.
| } else if (event.type === "score_updated") { | ||
| // Check for rank change | ||
| try { | ||
| const data = await fetchGamificationLeaderboard(token); | ||
| const myEntry = data.debaters.find(d => d.id === user.id); | ||
| if (myEntry) { | ||
| const newRank = myEntry.rank; | ||
| const oldRank = previousRankRef.current; | ||
|
|
||
| // Celebrate if entering top 10 | ||
| if (newRank <= 10 && oldRank !== null && oldRank > 10) { | ||
| setBadgeUnlocked({ | ||
| badgeName: "Top10", | ||
| isOpen: true, | ||
| }); | ||
| } | ||
| previousRankRef.current = newRank; | ||
| } | ||
| } catch (e) { | ||
| console.error("Failed to check rank", e); | ||
| } | ||
| } |
There was a problem hiding this comment.
Guard against state updates after unmount.
The async fetchGamificationLeaderboard call inside the score_updated handler could complete after the component unmounts, causing the setBadgeUnlocked call (line 59) to trigger a React warning about state updates on unmounted components.
🔎 Recommended fix: Add mounted flag to prevent state updates after unmount
const GamificationOverlay: React.FC = () => {
const { user } = useUser();
const [badgeUnlocked, setBadgeUnlocked] = useState<{
badgeName: string;
isOpen: boolean;
}>({
badgeName: "",
isOpen: false,
});
const wsRef = useRef<WebSocket | null>(null);
const previousRankRef = useRef<number | null>(null);
+ const isMountedRef = useRef(true);
useEffect(() => {
+ isMountedRef.current = true;
const token = localStorage.getItem("token");
if (!token || !user) return;
// Initial rank check
fetchGamificationLeaderboard(token).then((data) => {
+ if (!isMountedRef.current) return;
const myEntry = data.debaters.find(d => d.id === user.id);
if (myEntry) {
previousRankRef.current = myEntry.rank;
}
}).catch(console.error);
if (wsRef.current) {
wsRef.current.close();
}
const ws = createGamificationWebSocket(
token,
async (event: GamificationEvent) => {
if (event.userId !== user.id) return;
if (event.type === "badge_awarded" && event.badgeName) {
+ if (!isMountedRef.current) return;
setBadgeUnlocked({
badgeName: event.badgeName,
isOpen: true,
});
} else if (event.type === "score_updated") {
// Check for rank change
try {
const data = await fetchGamificationLeaderboard(token);
+ if (!isMountedRef.current) return;
const myEntry = data.debaters.find(d => d.id === user.id);
if (myEntry) {
const newRank = myEntry.rank;
const oldRank = previousRankRef.current;
// Celebrate if entering top 10
if (newRank <= 10 && oldRank !== null && oldRank > 10) {
setBadgeUnlocked({
badgeName: "Top10",
isOpen: true,
});
}
previousRankRef.current = newRank;
}
} catch (e) {
console.error("Failed to check rank", e);
}
}
}
);
wsRef.current = ws;
return () => {
+ isMountedRef.current = false;
if (wsRef.current) {
wsRef.current.close();
}
};
}, [user]);📝 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.
| } else if (event.type === "score_updated") { | |
| // Check for rank change | |
| try { | |
| const data = await fetchGamificationLeaderboard(token); | |
| const myEntry = data.debaters.find(d => d.id === user.id); | |
| if (myEntry) { | |
| const newRank = myEntry.rank; | |
| const oldRank = previousRankRef.current; | |
| // Celebrate if entering top 10 | |
| if (newRank <= 10 && oldRank !== null && oldRank > 10) { | |
| setBadgeUnlocked({ | |
| badgeName: "Top10", | |
| isOpen: true, | |
| }); | |
| } | |
| previousRankRef.current = newRank; | |
| } | |
| } catch (e) { | |
| console.error("Failed to check rank", e); | |
| } | |
| } | |
| } else if (event.type === "score_updated") { | |
| // Check for rank change | |
| try { | |
| const data = await fetchGamificationLeaderboard(token); | |
| if (!isMountedRef.current) return; | |
| const myEntry = data.debaters.find(d => d.id === user.id); | |
| if (myEntry) { | |
| const newRank = myEntry.rank; | |
| const oldRank = previousRankRef.current; | |
| // Celebrate if entering top 10 | |
| if (newRank <= 10 && oldRank !== null && oldRank > 10) { | |
| setBadgeUnlocked({ | |
| badgeName: "Top10", | |
| isOpen: true, | |
| }); | |
| } | |
| previousRankRef.current = newRank; | |
| } | |
| } catch (e) { | |
| console.error("Failed to check rank", e); | |
| } | |
| } |
🤖 Prompt for AI Agents
In frontend/src/components/GamificationOverlay.tsx around lines 48-69, the async
fetchGamificationLeaderboard call may resolve after the component unmounts
causing setBadgeUnlocked to update state on an unmounted component; add an
isMounted flag (useRef) or an AbortController: set isMounted.current = true on
mount and false in a cleanup return, or abort the fetch in cleanup, then before
calling setBadgeUnlocked or updating previousRankRef.current check that
isMounted.current (or that the fetch was not aborted); this prevents state
updates after unmount and avoids React warnings.
1 participant
Fixes #156 Added gamification to celebrate new badges and top 10 appearance