💄 Update avatar colors #1351
💄 Update avatar colors #1351
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes introduce a new 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Outside diff range and nitpick comments (3)
apps/web/src/components/optimized-avatar-image.tsx (1)
43-45: Consider handling size values between 25 and 47In the
classNameassignment forAvatarFallback, only sizes<= 24and>= 48have text size classes applied. For sizes between 25 and 47, no class is applied, which may lead to inconsistent styling or unexpected text sizes. Consider adding a default text size class or adjusting the conditions to ensure consistent appearance across all sizes.apps/web/src/components/poll/mobile-poll.tsx (2)
125-125: Ensure Consistent Rendering of Participant AvatarsIn the current user's participant view, you have used the
YouAvatarcomponent:<Participant> <YouAvatar /> <ParticipantName>{t("you")}</ParticipantName> </Participant>Elsewhere, for other participants, you use
OptimizedAvatarImagewith the participant's name:<Participant> <OptimizedAvatarImage name={participant.name} size={20} /> <ParticipantName>{participant.name}</ParticipantName> {session.ownsObject(participant) && ( <Badge> <Trans i18nKey="you" /> </Badge> )} </Participant>Consider whether the
YouAvatarcomponent should accept props likenameandsizefor consistency, or if the current differentiation is intentional. Ensuring consistent avatar rendering enhances the user experience.
Line range hint
125-129: Refactor Common Participant Rendering LogicThere is duplicated code in how participants are rendered, with slight variations for the current user and others. To improve maintainability and reduce duplication, consider refactoring the common elements into a shared component or function.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- apps/web/src/components/optimized-avatar-image.tsx (1 hunks)
- apps/web/src/components/participant.tsx (2 hunks)
- apps/web/src/components/poll/desktop-poll/participant-row-form.tsx (2 hunks)
- apps/web/src/components/poll/mobile-poll.tsx (2 hunks)
- apps/web/src/components/poll/mobile-poll/poll-option.tsx (1 hunks)
- apps/web/src/components/poll/you-avatar.tsx (1 hunks)
- packages/ui/src/avatar.tsx (1 hunks)
Additional comments not posted (8)
apps/web/src/components/poll/you-avatar.tsx (1)
3-5: Component implementation looks goodThe
YouAvatarcomponent is well-implemented, and the use of theuseTranslationhook is appropriate for internationalization.apps/web/src/components/optimized-avatar-image.tsx (1)
42-42: Change looks goodThe addition of
seed={name}toAvatarFallbackenhances the component by allowing for more personalized fallback avatars based on the user's name.apps/web/src/components/participant.tsx (1)
18-20: Change toAvatarFallbackis appropriateThe switch from using the
colorprop withgetColor(name)to using theseedprop withnameinAvatarFallbackappears correct. This should ensure consistent avatar representations based on the participant's name.packages/ui/src/avatar.tsx (1)
38-50: LGTM! The color pairs are well-defined and enhance visual appealThe
colorPairsarray provides a diverse set of background and text color combinations that will improve the visual aesthetics of the avatar component.apps/web/src/components/poll/desktop-poll/participant-row-form.tsx (2)
64-68: Conditional rendering logic is correctThe conditional rendering based on the presence of
nameis appropriate. It enhances the user interface by displaying a personalized avatar for participants and a distinct avatar for the current user.
18-18: Verify the import of 'YouAvatar' componentEnsure that the
YouAvatarcomponent exists at the specified path and is correctly exported from@/components/poll/you-avatar.You can run the following script to confirm the existence and export of
YouAvatar:Verification successful
To further investigate the issue and locate the correct file, I'll run additional scripts to search for the 'YouAvatar' component.
To complete our verification, we need to examine the contents of the 'you-avatar.tsx' file to understand how 'YouAvatar' is exported.
Based on the analysis, we can now provide a final response regarding the verification of the 'YouAvatar' import.
Import of 'YouAvatar' component is correct
The 'YouAvatar' component is correctly defined and exported from '@/components/poll/you-avatar.tsx'. The import statement in 'participant-row-form.tsx' matches the export, using the correct named import syntax. No issues were found with the import of the 'YouAvatar' component.
- The component is exported as a named function:
export function YouAvatar() { ... }- The import statement
import { YouAvatar } from "@/components/poll/you-avatar";correctly references this exportScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that 'YouAvatar' is exported from '@/components/poll/you-avatar'. # Test: Search for the export statement of 'YouAvatar'. Expect: At least one match. ast-grep --lang tsx --pattern 'export const YouAvatar = $_' src/components/poll/you-avatar.tsxLength of output: 178
Script:
#!/bin/bash # Search for 'you-avatar.tsx' file in the repository fd -e tsx -e ts 'you-avatar' # If the above doesn't find anything, search for files containing 'YouAvatar' rg -l 'YouAvatar' # If a file is found, check for the export of 'YouAvatar' # Note: This command will run for all matching files, which might not be ideal if there are many matches fd -e tsx -e ts 'you-avatar' -x ast-grep --lang tsx --pattern 'export const YouAvatar = $_' {}Length of output: 343
Script:
#!/bin/bash # Display the contents of the you-avatar.tsx file cat apps/web/src/components/poll/you-avatar.tsx # Search for any export statements related to YouAvatar rg 'export.*YouAvatar' apps/web/src/components/poll/you-avatar.tsxLength of output: 429
apps/web/src/components/poll/mobile-poll/poll-option.tsx (1)
77-77: Adjustment of closing</div>tag is appropriateThe closing
</div>tag at line 77 correctly aligns with the modified JSX structure, ensuring proper rendering of the component.apps/web/src/components/poll/mobile-poll.tsx (1)
23-23: Import ofYouAvatarComponent Is CorrectThe import statement for the
YouAvatarcomponent is appropriately added and follows the project's conventions.
|
|
||
| return ( | ||
| <div className="inline-flex size-5 items-center justify-center rounded-full bg-gray-200 text-xs font-medium"> | ||
| {t("you")[0]} |
There was a problem hiding this comment.
Potential localization issue when extracting the first character
Using t("you")[0] extracts the first character of the translated word for "you". This approach may not work correctly in all languages, especially those with non-Latin scripts or where the word for "you" is more than one character. This could lead to incorrect or unintelligible displays for users in different locales.
Consider the following alternatives:
-
Use an Icon: Replace the text with a user icon that universally represents the current user.
- {t("you")[0]} + <UserIcon />
-
Display the Full Word: Show the entire translated word to ensure clarity across all languages.
- {t("you")[0]} + {t("you")}
-
Use Initials with Caution: If initials are important, implement a function that handles locale-specific considerations, possibly using libraries that support transliteration.
Ensure that the chosen solution provides a consistent and understandable experience for all users regardless of their language settings.
Committable suggestion was skipped due to low confidence.
| React.ComponentPropsWithoutRef<typeof AvatarPrimitive.Fallback> & { | ||
| seed: string; | ||
| } | ||
| >(({ className, seed, ...props }, ref) => { | ||
| const { bgColor, textColor } = getColor(seed); | ||
| return ( | ||
| <AvatarPrimitive.Fallback | ||
| ref={ref} | ||
| className={cn( | ||
| "flex h-full w-full items-center justify-center rounded-full font-medium", | ||
| className, | ||
| )} | ||
| style={{ backgroundColor: bgColor, color: textColor }} | ||
| {...props} | ||
| /> | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Make the seed prop optional in AvatarFallback for consistency
Since the getColor function handles an undefined seed by providing default colors, making the seed prop optional in AvatarFallback would align the component's behavior with the function. This enhances flexibility and avoids potential errors if seed isn't provided.
Apply this diff to update the prop type:
React.ComponentPropsWithoutRef<typeof AvatarPrimitive.Fallback> & {
- seed: string;
+ seed?: string;
}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.
| React.ComponentPropsWithoutRef<typeof AvatarPrimitive.Fallback> & { | |
| seed: string; | |
| } | |
| >(({ className, seed, ...props }, ref) => { | |
| const { bgColor, textColor } = getColor(seed); | |
| return ( | |
| <AvatarPrimitive.Fallback | |
| ref={ref} | |
| className={cn( | |
| "flex h-full w-full items-center justify-center rounded-full font-medium", | |
| className, | |
| )} | |
| style={{ backgroundColor: bgColor, color: textColor }} | |
| {...props} | |
| /> | |
| ); | |
| }); | |
| React.ComponentPropsWithoutRef<typeof AvatarPrimitive.Fallback> & { | |
| seed?: string; | |
| } | |
| >(({ className, seed, ...props }, ref) => { | |
| const { bgColor, textColor } = getColor(seed); | |
| return ( | |
| <AvatarPrimitive.Fallback | |
| ref={ref} | |
| className={cn( | |
| "flex h-full w-full items-center justify-center rounded-full font-medium", | |
| className, | |
| )} | |
| style={{ backgroundColor: bgColor, color: textColor }} | |
| {...props} | |
| /> | |
| ); | |
| }); |
| export function getColor(seed?: string): { | ||
| bgColor: string; | ||
| textColor: string; | ||
| } { | ||
| if (!seed) { | ||
| return { bgColor: "#E6F4FF", textColor: "#0065BD" }; | ||
| } | ||
| let hash = 0; | ||
| for (let i = 0; i < seed.length; i++) { | ||
| hash = seed.charCodeAt(i) + ((hash << 5) - hash); | ||
| } | ||
| return avatarColors[Math.abs(hash) % avatarColors.length]; | ||
| const colorPair = colorPairs[Math.abs(hash) % colorPairs.length]; | ||
| return { bgColor: colorPair.bg, textColor: colorPair.text }; |
There was a problem hiding this comment.
Consider using a more robust hash function for better color distribution
The current hash function may lead to uneven color distribution for similar seed strings, resulting in less variation than expected. Utilizing a more robust hashing algorithm could improve the randomness and ensure a more uniform assignment of colors.
Apply this diff to update the hash function:
+import crypto from 'crypto';
export function getColor(seed?: string): {
bgColor: string;
textColor: string;
} {
if (!seed) {
return { bgColor: "#E6F4FF", textColor: "#0065BD" };
}
- let hash = 0;
- for (let i = 0; i < seed.length; i++) {
- hash = seed.charCodeAt(i) + ((hash << 5) - hash);
- }
- const colorPair = colorPairs[Math.abs(hash) % colorPairs.length];
+ const hash = crypto.createHash('sha256').update(seed).digest('hex');
+ const hashInt = parseInt(hash.substring(0, 8), 16);
+ const colorPair = colorPairs[hashInt % colorPairs.length];
return { bgColor: colorPair.bg, textColor: colorPair.text };
}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.
| export function getColor(seed?: string): { | |
| bgColor: string; | |
| textColor: string; | |
| } { | |
| if (!seed) { | |
| return { bgColor: "#E6F4FF", textColor: "#0065BD" }; | |
| } | |
| let hash = 0; | |
| for (let i = 0; i < seed.length; i++) { | |
| hash = seed.charCodeAt(i) + ((hash << 5) - hash); | |
| } | |
| return avatarColors[Math.abs(hash) % avatarColors.length]; | |
| const colorPair = colorPairs[Math.abs(hash) % colorPairs.length]; | |
| return { bgColor: colorPair.bg, textColor: colorPair.text }; | |
| import crypto from 'crypto'; | |
| export function getColor(seed?: string): { | |
| bgColor: string; | |
| textColor: string; | |
| } { | |
| if (!seed) { | |
| return { bgColor: "#E6F4FF", textColor: "#0065BD" }; | |
| } | |
| const hash = crypto.createHash('sha256').update(seed).digest('hex'); | |
| const hashInt = parseInt(hash.substring(0, 8), 16); | |
| const colorPair = colorPairs[hashInt % colorPairs.length]; | |
| return { bgColor: colorPair.bg, textColor: colorPair.text }; | |
| } |
Summary by CodeRabbit
Release Notes
New Features
seedproperty based on the user's name.YouAvatarcomponent for better user representation in polls.Improvements
Bug Fixes