primitives icon indicating copy to clipboard operation
primitives copied to clipboard

Adding support for framework image component in Radix Avatar

Open rubenskj opened this issue 1 year ago • 1 comments

Description

Hello there, how you guys doing?

As discussed in issue #2230, we would love to enhance image optimization by integrating the Next.js Image component. I've implemented a solution that allows us to utilize the Next.js Image component while maintaining the existing functionality.

In this update, I've removed the image load logic from useLayoutEffect and delegated it directly to the respective components. This means that Primitive.img will handle image loading as usual, while the Next.js Image component will load images with its optimizations.

I would greatly appreciate your feedback on this solution. Let's collaborate to refine this feature and ensure it meets our needs.

rubenskj avatar Jul 04 '24 17:07 rubenskj

Thanks for the suggestion! I'm not quite sure the implementation here is what we're looking for. A few things:

First: we shouldn't need the component prop since we already have asChild. This is our preferred API for polymorphic components, and you can do this today:

<Avatar.Image asChild src="https://whatever.com/img.jpg" alt="cute kitten">
  <NextImage width={500} height={300} />
</Avatar.Image>

While you might get type errors since Next requires src and alt, you should be able to suppress them since we merge props internally when we clone the child element.

while the Next.js Image component will load images with its optimizations

Not everyone uses Next JS, and we shouldn't assume as much. I think I'd rather have a boolean prop that disables our internal optimizations rather than removing them completely.

chaance avatar Aug 16 '24 19:08 chaance

New to Next, but I found that supplying src to Avatar.Image also downloads the original file, which defeats the purpose of the optimizations provided by NextImage. I got it working by supplying AvatarImage with a small 1x1 transparent image and NextImage with the actual image url:

<AvatarImage src="/images/blank.png" asChild>
  <Image alt={user.username} width={48} height={48} src={user.image}  />
</AvatarImage>

dragospaulpop avatar Sep 06 '24 17:09 dragospaulpop

I've improved this code and raised PR in @RubensKj's fork. Let's wait for a week, if he isn't active I will rebase the changes from this repository and raise PR here.

Changes: https://github.com/RubensKj/primitives/pull/1

Udhayarajan avatar Sep 13 '24 14:09 Udhayarajan

@Udhayarajan thanks for the PR! I've merged and also fixed the merge problems with main branch.

Thanks for contributing together 🚀 . Now let's wait for the review ❤️

rubenskj avatar Sep 13 '24 20:09 rubenskj