-
-
Notifications
You must be signed in to change notification settings - Fork 488
✨ Use avatar on login page #1336
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe pull request introduces a new component, Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- apps/web/src/app/[locale]/auth/login/login-page.tsx (3 hunks)
- apps/web/src/components/current-user-avatar.tsx (2 hunks)
- apps/web/src/components/optimized-avatar-image.tsx (1 hunks)
- apps/web/src/trpc/routers/user.ts (1 hunks)
- packages/ui/src/avatar.tsx (1 hunks)
Additional comments not posted (6)
apps/web/src/components/current-user-avatar.tsx (1)
Line range hint
3-20: Simplified and optimized avatar rendering.The changes to
CurrentUserAvatarsimplify the component by usingOptimizedAvatarImagedirectly. This approach enhances maintainability and readability by reducing complexity and delegating responsibilities.Ensure that
OptimizedAvatarImageadequately handles all edge cases previously covered bygetAvatarUrl, particularly with different avatar sources.packages/ui/src/avatar.tsx (1)
10-20: Enhanced flexibility withsizeprop inAvatar.The addition of the
sizeprop to theAvatarcomponent enhances its flexibility and usability without affecting existing functionality. The default value of 48 ensures backward compatibility.apps/web/src/app/[locale]/auth/login/login-page.tsx (3)
8-8: Approved import statement forOptimizedAvatarImage.The import statement is correctly added and follows the project's conventions.
79-79: Approved CSS change with a suggestion to verify visual impact.The change in margin from
mt-4tomt-6is noted. It's important to verify this change visually to ensure it aligns with the intended design.Run the following script to verify the visual impact:
56-61: Approved usage ofOptimizedAvatarImagewith suggestions.The
OptimizedAvatarImagecomponent is used correctly with appropriate props. Ensure that thedataobject structure supports these properties as expected.Run the following script to verify the structure of the
dataobject:apps/web/src/trpc/routers/user.ts (1)
46-46: Approved addition ofimagefield in theselectobject.The addition of the
imagefield is correctly implemented. Ensure that client applications handle this new field appropriately.Run the following script to verify the impact on client applications:
| "use client"; | ||
| import { Avatar, AvatarFallback, AvatarImage } from "@rallly/ui/avatar"; | ||
| import Image from "next/image"; | ||
|
|
||
| function getAvatarUrl(imageKey: string) { | ||
| // Some users have avatars that come from external providers (e.g. Google). | ||
| if (imageKey.startsWith("https://")) { | ||
| return imageKey; | ||
| } | ||
|
|
||
| return `/api/storage/${imageKey}`; | ||
| } | ||
|
|
||
| export const OptimizedAvatarImage = ({ | ||
| size, | ||
| className, | ||
| src, | ||
| name, | ||
| }: { | ||
| size: number; | ||
| src?: string; | ||
| name: string; | ||
| className?: string; | ||
| }) => { | ||
| return ( | ||
| <Avatar className={className} style={{ width: size, height: size }}> | ||
| {src ? ( | ||
| src.startsWith("http") ? ( | ||
| <AvatarImage src={src} alt={name} /> | ||
| ) : ( | ||
| <Image | ||
| src={getAvatarUrl(src)} | ||
| width={128} | ||
| height={128} | ||
| alt={name} | ||
| style={{ objectFit: "cover" }} | ||
| objectFit="cover" | ||
| /> | ||
| ) | ||
| ) : ( | ||
| <AvatarFallback>{name[0]}</AvatarFallback> | ||
| )} | ||
| </Avatar> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Robust and flexible avatar handling in OptimizedAvatarImage.
The implementation of OptimizedAvatarImage is comprehensive, handling different avatar sources effectively. The use of next/image for local images is a good choice for performance optimization.
Consider adding unit tests to verify that all conditional paths in OptimizedAvatarImage function as expected. Would you like assistance in setting up these tests?
Summary by CodeRabbit
New Features
OptimizedAvatarImagecomponent for improved user avatar rendering.Improvements
Avatarcomponent with a new optionalsizeproperty.Bug Fixes