[Avatar] Fix flashing when image is already cached
Description
Fixes https://github.com/radix-ui/primitives/issues/2044 by initializing loadingStatus with loaded if the image.complete is true, based on @lauri865 suggestion in the linked issue.
Based on MDN, image.complete will be true if:
- Neither the src nor the srcset attribute is specified.
- The srcset attribute is absent and the src attribute, while specified, is the empty string ("").
- The image resource has been fully fetched and has been queued for rendering/compositing.
- The image element has previously determined that the image is fully available and ready for use.
- The image is "broken;" that is, the image failed to load due to an error or because image loading is disabled.
I added test case for all the cases, although I imagine the last case might be different from browser to browser. In Chrome, when image loading is disabled the load event will never fire and image.complete will always be false. To be safe I added another check for image.naturalWidth > 0 since image with no data will have naturalWidth = 0.
I also added test and story for the case when image src changing to make sure my change can handle the changing src.
Tests added: ✓ does not leak event listeners (7 ms) ✓ can handle changing src (627 ms) ✓ should render the image immediately after it is cached (327 ms) ✓ should not render image with no src (5 ms) ✓ should not render image with empty string as src (3 ms) ✓ should show fallback if image has no data (4 ms)
Storybook added:
- Changing image src
| Before | After |
|---|---|
Awesome, I've been waiting for this I was actually about to hack my own Avatar together myself... Will this be useable directly into Shadcn and NextUI or will we need an update from NextUI directly?
@LeakedDave the same avatar flicker issue in shadcn/ui should be fixed just by bumping the radix version if this fix is released. For nextui I think the same idea can be implemented to this hook: https://github.com/nextui-org/nextui/blob/canary/packages/hooks/use-image/src/index.ts
Hey @chaance @rkkautsar, is this PR ready to be merged? 🙏
Hello, I just want to follow if this PR is ready?
@mackermans @quachthientai need maintainer to review the change 😅
Please do not leave comments like "bump" and "+1", they are unhelpful. At the moment this project is just me, and I've got a large backlog of issues and need to prioritize accordingly.
@rkkautsar do you see an opportunity to finish this PR with the last PR comment?
@chaance sorry just got the chance to work on this again. I applied your suggestion and tested that everything still works.
Let's merge. This is a pre-2025 feature.
Hey folks 👋 any update here? Would be great to have this merged 🙏
Merge pls 🙏
Pre-2025...
Resolved conflicts and merged in https://github.com/radix-ui/primitives/commit/f46e3d6685ee67aebc9b4c574cb515b42d1f5418