-
-
Notifications
You must be signed in to change notification settings - Fork 3.8k
feat(web): don't animate cached thumbnails #20773
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Deploying preview environment to https://pr-20773.preview.internal.immich.cloud/ |
a44bce8
to
dd4a4cb
Compare
846f192
to
88b3ff6
Compare
88b3ff6
to
747a480
Compare
></canvas> | ||
{#if asset.thumbhash} | ||
{#await isCached(new Request(thumbnailURL))} | ||
<canvas |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a second canvas which doesn't fade out, it's used whilst it's waiting to check if the thumbnail is already in the cache. If the thumbnail is cached, then it is removed immediately, whereas if the thumbnail is not cached then the other canvas is used and that will fade out when the actual thumbnail is ready to use.
747a480
to
04abdec
Compare
Note - |
style:height="{height}px" | ||
class:rounded-xl={selected} | ||
draggable="false" | ||
out:fade={{ duration: THUMBHASH_FADE_DURATION }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how the two canvases behave - is it decoding the thumbhash and allocating a canvas twice, or does it know they're the same thing? It might be better to use the same canvas and just change the fade value here with reactive state. So the state starts with null/undefined and switches to this value once we know it's a cache miss.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking the same thing, I'll do that.
04abdec
to
26f8995
Compare
Thumbnails for assets always are displayed with a thumbhash which fades out over 100ms, even if the thumbnail is cached and ready immediately. This can be a bit distracting and make Immich feel 'slow', or inefficient as it feels like the thumbnails are always being reloaded. Skipping the thumbhash and animation for cached thumbnails makes it feel much more responsive.
26f8995
to
2bba33f
Compare
Description
Thumbnails for assets always are displayed with a thumbhash which fades out over 100ms, even if the thumbnail is cached and ready immediately. This can be a bit distracting and make Immich feel 'slow', or inefficient as it feels like the thumbnails are always being reloaded. Skipping the thumbhash and animation for cached thumbnails makes it feel much more responsive.
How Has This Been Tested?
Locally.
I can attach a couple of videos to compare, but also we can see how it feels side-by-side in the preview env.
Checklist:
src/services/
uses repositories implementations for database calls, filesystem operations, etc.src/repositories/
is pretty basic/simple and does not have any immich specific logic (that belongs insrc/services/
)