fix(use-image): cached image flashing #3444
Conversation
🦋 Changeset detectedLatest commit: 431a5e0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@rkkautsar is attempting to deploy a commit to the NextUI Inc Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes address an issue with cached images flashing in the Changes
Assessment against linked issues
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 as PR comments)
Additionally, you can add 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: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
Files selected for processing (5)
- .changeset/pink-beans-sit.md (1 hunks)
- packages/hooks/use-image/tests/use-image.test.tsx (1 hunks)
- packages/hooks/use-image/package.json (2 hunks)
- packages/hooks/use-image/src/index.ts (3 hunks)
- packages/utilities/test-utils/src/mocks/image.ts (1 hunks)
Files skipped from review due to trivial changes (2)
- .changeset/pink-beans-sit.md
- packages/hooks/use-image/package.json
Additional comments not posted (15)
packages/utilities/test-utils/src/mocks/image.ts (1)
17-20: EnhancemockImageto simulate image properties.The addition of
naturalWidthand thecompletegetter improves the simulation of image loading behavior. This change aligns with the PR objectives to handle cached images more effectively.packages/hooks/use-image/__tests__/use-image.test.tsx (8)
1-2: Add necessary imports for the tests.The added imports ensure the testing utilities and the
useImagehook are available for the test cases.
6-7: Initialize mockImage for each test case.The
mockImageinitialization ensures that each test case starts with a clean state.
10-11: Set up mockImage before each test case.This ensures that the mock image functionality is correctly set up before each test case runs.
12-14: Restore mockImage after each test case.Restoring the mock image ensures that the tests do not interfere with each other by leaving residual state.
16-20: Test handling of missing src.This test case ensures that the
useImagehook correctly handles scenarios where the image source is missing.
22-28: Test handling of loading image.This test case ensures that the
useImagehook correctly handles scenarios where the image is in the process of loading.
30-36: Test handling of error image.This test case ensures that the
useImagehook correctly handles scenarios where the image fails to load.
38-43: Test handling of cached image.This test case ensures that the
useImagehook correctly handles scenarios where the image is already cached.packages/hooks/use-image/src/index.ts (6)
4-4: Add necessary import for MutableRefObject.The addition of
MutableRefObjectis necessary for the new function handling image initialization and status determination.
6-6: Ensure correct imports.The imports ensure that the required hooks and utilities are available for the
useImagehook.
70-71: Initialize state with image status.The state initialization now checks the image loading status upon creation, addressing the cached image flashing issue.
73-85: Update useSafeLayoutEffect dependencies.The updated dependencies ensure that the image status is re-evaluated when relevant properties change.
87-99: Handle image load and error events.The event handlers ensure that the image status is correctly updated when the image loads or fails to load.
116-139: Add function to set image and get initial status.The new function
setImageAndGetInitialStatusdetermines the initial status of the image based on its properties, improving the handling of cached images.
|
@wingkwong yes, I think it had to do with the actual image caching behaviour of each browser. I used chrome, and from what I can see the caching seems to be size limited, if I were to guess it's because there is a few level of caching, the one that is instant is memory-cached, while the other one that still flashes is disk-cached. If I show a big image (like in Conversely, for small image like in avatars (and avatar groups), it seems that most of the small images is cached in memory so it always never flash (unless you load a big image and come back, the big image will replace the memory cache I think) |
|
will revisit this PR after the one in Radix is merged. May check if those comments can be applied here or not. |
|
Hey @rkkautsar thanks for you great contribution, we had to create the PR internally to move things a bit faster #3987 but do not worry we will add you to the contributors list, thanks 🙏🏻 |
Closes #3437
📝 Description
As mentioned in #3437, avatars may flicker even if its already cached. The fix was to check the image loading status immediately after creating the image to see if it's instantly loaded (when the image is cached), and use it to initialize the state. This will affect Avatar, AvatarGroup, and Image components.
Also added test for use-image too since it doesn't exist before.
⛳️ Current behavior (updates)
Image flickers due to use-image initializes with
pendingand only updated the status inuseLayoutEffect.🚀 New behavior
Initialize image loading status with loaded if possible.
💣 Is this a breaking change (Yes/No): No
📝 Additional Information
Summary by CodeRabbit
Bug Fixes
Tests
useImagehook covering scenarios like missing source, loading, errors, and cached images.Chores
use-imagepackage devDependencies to include@nextui-org/test-utils.Refactor
useImagefunction to improve the image initialization and status determination process.