Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/web/src/components/optimized-avatar-image.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export function OptimizedAvatarImage({
) : null}
{!src || !isLoaded ? (
<AvatarFallback
seed={name}
className={cn({
"text-xs": size <= 24,
"text-lg": size >= 48,
Expand Down
6 changes: 2 additions & 4 deletions apps/web/src/components/participant.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { cn } from "@rallly/ui";
import { Avatar, AvatarFallback, getColor } from "@rallly/ui/avatar";
import { Avatar, AvatarFallback } from "@rallly/ui/avatar";
import React from "react";

export function Participant({ children }: { children: React.ReactNode }) {
Expand All @@ -13,11 +13,9 @@ export const ParticipantAvatar = ({
size?: number;
name: string;
}) => {
const color = getColor(name);

return (
<Avatar size={size}>
<AvatarFallback className="text-xs" color={color}>
<AvatarFallback className="text-xs" seed={name}>
{name[0]?.toUpperCase()}
</AvatarFallback>
</Avatar>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { Controller } from "react-hook-form";
import { OptimizedAvatarImage } from "@/components/optimized-avatar-image";
import { Participant, ParticipantName } from "@/components/participant";
import { useVotingForm } from "@/components/poll/voting-form";
import { YouAvatar } from "@/components/poll/you-avatar";
import { Trans } from "@/components/trans";

import { usePoll } from "../../poll-context";
Expand Down Expand Up @@ -60,7 +61,11 @@ const ParticipantRowForm = ({
>
<div className="flex items-center justify-between gap-x-2.5">
<Participant>
<OptimizedAvatarImage name={participantName} size={20} />
{name ? (
<OptimizedAvatarImage name={participantName} size={20} />
) : (
<YouAvatar />
)}
<ParticipantName>{participantName}</ParticipantName>
</Participant>
{!isNew ? (
Expand Down
3 changes: 2 additions & 1 deletion apps/web/src/components/poll/mobile-poll.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { OptimizedAvatarImage } from "@/components/optimized-avatar-image";
import { Participant, ParticipantName } from "@/components/participant";
import { ParticipantDropdown } from "@/components/participant-dropdown";
import { useVotingForm } from "@/components/poll/voting-form";
import { YouAvatar } from "@/components/poll/you-avatar";
import { useOptions, usePoll } from "@/components/poll-context";
import { Trans } from "@/components/trans";
import { usePermissions } from "@/contexts/permissions";
Expand Down Expand Up @@ -121,7 +122,7 @@ const MobilePoll: React.FunctionComponent = () => {
) : (
<div className="flex grow items-center px-1">
<Participant>
<OptimizedAvatarImage name={t("you")} size={20} />
<YouAvatar />
<ParticipantName>{t("you")}</ParticipantName>
</Participant>
</div>
Expand Down
8 changes: 4 additions & 4 deletions apps/web/src/components/poll/mobile-poll/poll-option.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,29 +61,29 @@ const PollOptionVoteSummary: React.FunctionComponent<{ optionId: string }> = ({
<div className="truncate text-sm">{name}</div>
</div>
))}
</div>
<div className="col-span-1 space-y-2.5">
{participantsWhoVotedIfNeedBe.map(({ name }, i) => (
<div key={i} className="flex">
<div className="relative mr-2.5 flex size-5 items-center justify-center">
<ParticipantAvatar size={20} name={name} />
<VoteIcon
type="ifNeedBe"
size="sm"
className="absolute bottom-full left-full -translate-x-1/2 translate-y-1/2 rounded-full bg-white"
className="absolute bottom-full left-full -translate-x-1.5 translate-y-2.5 rounded-full bg-white"
/>
</div>
<div className="truncate text-sm"> {name}</div>
</div>
))}
</div>
<div className="col-span-1 space-y-2.5">
{participantsWhoVotedNo.map(({ name }, i) => (
<div key={i} className="flex">
<div className="relative mr-2.5 flex size-5 items-center justify-center">
<ParticipantAvatar size={20} name={name} />
<VoteIcon
type="no"
size="sm"
className="absolute bottom-full left-full -translate-x-1/2 translate-y-1/2 rounded-full bg-white"
className="absolute bottom-full left-full -translate-x-1.5 translate-y-2.5 rounded-full bg-white"
/>
</div>
<div className="truncate text-sm">{name}</div>
Expand Down
11 changes: 11 additions & 0 deletions apps/web/src/components/poll/you-avatar.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { useTranslation } from "@/app/i18n/client";

export function YouAvatar() {
const { t } = useTranslation();

return (
<div className="inline-flex size-5 items-center justify-center rounded-full bg-gray-200 text-xs font-medium">
{t("you")[0]}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

</div>
);
}
86 changes: 39 additions & 47 deletions packages/ui/src/avatar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,9 @@

import * as React from "react";
import * as AvatarPrimitive from "@radix-ui/react-avatar";
import { cva, type VariantProps } from "class-variance-authority";

import { cn } from "@rallly/ui";

export const avatarColors = [
"indigo",
"green",
"blue",
"purple",
"emerald",
"violet",
"sky",
"cyan",
"pink",
] as const;

export type AvatarColor = (typeof avatarColors)[number];

const Avatar = React.forwardRef<
React.ElementRef<typeof AvatarPrimitive.Root>,
React.ComponentPropsWithoutRef<typeof AvatarPrimitive.Root> & {
Expand Down Expand Up @@ -50,47 +35,54 @@ const AvatarImage = React.forwardRef<
));
AvatarImage.displayName = AvatarPrimitive.Image.displayName;

const avatarFallbackVariants = cva(
"flex h-full w-full items-center justify-center rounded-full font-medium",
{
variants: {
color: {
indigo: "bg-indigo-50 text-indigo-600",
green: "bg-green-50 text-green-600",
blue: "bg-blue-50 text-blue-600",
purple: "bg-purple-50 text-purple-600",
emerald: "bg-emerald-50 text-emerald-600",
violet: "bg-violet-50 text-violet-600",
sky: "bg-sky-50 text-sky-600",
cyan: "bg-cyan-50 text-cyan-600",
pink: "bg-pink-50 text-pink-600",
},
},
defaultVariants: {
color: "indigo",
},
},
);
const colorPairs = [
{ bg: "#E6F4FF", text: "#0065BD" }, // Light blue
{ bg: "#DCFCE7", text: "#15803D" }, // Light green
{ bg: "#FFE6F4", text: "#BD007A" }, // Light pink
{ bg: "#F4E6FF", text: "#6200BD" }, // Light purple
{ bg: "#FFE6E6", text: "#BD0000" }, // Light red
{ bg: "#FFE6FF", text: "#A300A3" }, // Bright pink
{ bg: "#F0E6FF", text: "#5700BD" }, // Lavender
{ bg: "#FFE6F9", text: "#BD0066" }, // Rose
{ bg: "#E6E6FF", text: "#0000BD" }, // Periwinkle
{ bg: "#FFE6EC", text: "#BD001F" }, // Salmon pink
{ bg: "#EBE6FF", text: "#4800BD" }, // Light indigo
];

export function getColor(seed: string): AvatarColor {
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 };
Comment on lines +52 to +64
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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 };
}

}

const AvatarFallback = React.forwardRef<
React.ElementRef<typeof AvatarPrimitive.Fallback>,
React.ComponentPropsWithoutRef<typeof AvatarPrimitive.Fallback> &
VariantProps<typeof avatarFallbackVariants>
>(({ className, color, ...props }, ref) => (
<AvatarPrimitive.Fallback
ref={ref}
className={cn(avatarFallbackVariants({ color }), className)}
{...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}
/>
);
});
Comment on lines +69 to +85
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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}
/>
);
});

AvatarFallback.displayName = AvatarPrimitive.Fallback.displayName;

export { Avatar, AvatarImage, AvatarFallback };