-
-
Notifications
You must be signed in to change notification settings - Fork 419
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
Add loading state to ImageGallery #3462
Comments
Hi, I haven't checked the code but what I've used in the past is the (somewhat) new See https://developer.mozilla.org/en-US/docs/Web/Performance/Lazy_loading#loading_attribute for more info |
FYI this is also quite bad for the user expereince in user-profiles. |
Looked into underlying issue and adding the lazy load itself is good for initial page load, which includes the smaller thumbnails and the larger resolution active image. That won't address larger issue of needing to tell browser to pre-load all of the larger resolution images for the non-active images, that way when swapping to them there won't be such a long delay. Alternative could be some sort of spinner/indicator when swapping images while the larger resolution image is loading. |
Thanks for looking into this @CubeLuke! I think a spinner is fine. Can I assign you the issue? |
Me and Tony looked into this during a Wednesday Mob programming session. We learned some things: We think there are no resized images (aka thumbnails) stored in Firebase. The small images that show up at the bottom are the full sized images but scaled down on the browser side. We think this because we uploaded five very big images to an example Research Article (https://precious-plastics-v4-dev.firebaseapp.com/research/this-is-a-test) And if you open it, you can see them slowly load (small images would not do this.) So the fact that the image is "lazy loaded" probably isn't related to the problem. Once the images come onto screen, even the small ones, the full image is loaded. The problem really is: (we think) clicking on a thumbnail causes it to request the the full image again, even though we already have the full image as the thumbnail. In the image above, in the inspector, I have hovered a thumbnail, you can see it is actually big. The URL has some parameters at the end, maybe those prevent caching. I am not sure how those parameters get there. Alternative solution could be: prevent re-requests for the data we already have by storing the image data for each image in a variable, and then on click replacing the image data. |
Actually, regarding my first comment. The thumbnails are served via a CDN. And the CDN does seem to offer thumbnails (smaller versions of the images.) But it is true, that the example I posted above, the with the big images, the thumbnails are the original images. For an image, the URL for the thumbnail and the URL for the full image are actually different (assuming a CDN is configured.) This is controlled by https://github.com/ONEARMY/community-platform/blob/0deaf9260fbdca79fda88432224e62d1dfc320fb/src/utils/cdnImageUrl.ts The thumbnails are served via a CDN but the active image is not. Their URLs are different. So the approach I proposed earlier would not work. So in the end, it is probably best to do what was originally suggested, adding a loading indicator. |
here is some WIP code Tony and I played around with: https://github.com/ONEARMY/community-platform/compare/master...pizzaisdavid:community-platform:wip-image-loading-state?expand=1 Who ever continues, feel free to use it. |
Sorry, I've just realised, I've forgotten to mention, that I've been working on this. Thank you to everyone, who helped. @pizzaisdavid My code is based on your work :) |
🎉 This issue has been resolved in version 2.29.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Due to lazy loading (which we want to keep), there's a noticeable lag when selected a different image to show on the ImageGallery. Sometimes that gets picked up by our feature tests.
image-gall.mov
Please add some kind of loading state to the component - it's in the component library. This can then mean the step 'Step image is updated on thumbnail click' in
src/integration/howto/read.spec.ts
can be un-commented out.The text was updated successfully, but these errors were encountered: