feat(leaderboard): add user avatars to table rows with DiceBear fallback#308
Conversation
✅ Deploy Preview for develop-devlovers ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR introduces a new Changes
Sequence DiagramsequenceDiagram
participant Client as Client/UI
participant UserAvatar as UserAvatar Component
participant Image as Next.js Image
participant DiceBear as DiceBear API
Client->>UserAvatar: Render with src & username
UserAvatar->>Image: Load avatar image
Image->>Image: Image load attempt
alt Image loads successfully
Image-->>UserAvatar: onLoadSuccess
UserAvatar-->>Client: Display loaded avatar
else Image fails to load
Image-->>UserAvatar: onError
UserAvatar->>DiceBear: Generate fallback SVG<br/>based on username
DiceBear-->>UserAvatar: Return SVG avatar
UserAvatar-->>Client: Display fallback avatar
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/db/queries/leaderboard.ts (1)
28-33:⚠️ Potential issue | 🟡 MinorDiceBear seed inconsistency with
UserAvatarfallback.The seed here is
${username}-${u.id}, but inUserAvatar.tsx(line 23) the client-side fallback seed is justusername. This causes two problems:
- If the DB-generated DiceBear URL somehow fails to load in the browser, the
UserAvataronErrorhandler produces a different avatar (different seed).- Two users sharing the same username will get distinct avatars from the DB layer (thanks to
u.id) but identical fallback avatars from the component layer.Align the seeds. The simplest approach is to always include the user ID in both places, or generate the fallback exclusively in one layer.
🧹 Nitpick comments (3)
frontend/components/leaderboard/UserAvatar.tsx (1)
15-37: StalehasErrorwhensrcprop changes.If the parent supplies a new
src(e.g., user updates their avatar and data revalidates),hasErrorremainstruefrom the previous render and the component continues showing the DiceBear fallback. Reset the error state whensrcchanges:Proposed fix
+ import { useState, useEffect } from 'react'; ... const [hasError, setHasError] = useState(false); + + useEffect(() => { + setHasError(false); + }, [src]);In the leaderboard context (1-hour cache), this is unlikely to manifest, but it makes the component safe for reuse elsewhere.
frontend/components/leaderboard/LeaderboardPodium.tsx (1)
88-91: Pass an appropriatesizesprop for podium avatars.The podium avatar container is
h-14 w-14 md:h-20 md:w-20(56px / 80px), butUserAvatardefaults tosizes="40px". This tells the browser to pick a ~40px-wide source from the srcset, which may appear slightly blurry at the larger rendered size. For non-SVG avatars, consider:<UserAvatar src={user.avatar} username={user.username} + sizes="(min-width: 768px) 80px, 56px" />frontend/components/leaderboard/LeaderboardTable.tsx (1)
110-116: Both branches ofleftBorderClassandrightBorderClassare identical.These ternaries evaluate to the same string regardless of
isCurrentUser. This is pre-existing, but since the surrounding avatar styling was updated in this PR, it's a good time to simplify:Suggested simplification
- const leftBorderClass = isCurrentUser - ? 'border-l-[1px] sm:border-l-[1px] border-l-transparent' - : 'border-l-[1px] sm:border-l-[1px] border-l-transparent'; + const leftBorderClass = 'border-l-[1px] sm:border-l-[1px] border-l-transparent'; - const rightBorderClass = isCurrentUser - ? 'border-r-[1px] sm:border-r-[1px] border-r-transparent' - : 'border-r-[1px] sm:border-r-[1px] border-r-transparent'; + const rightBorderClass = 'border-r-[1px] sm:border-r-[1px] border-r-transparent';
2 participants
Summary by CodeRabbit
Bug Fixes
Style