Skip to content
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

Galleria: Load first image earlier #307

Merged
merged 1 commit into from
Mar 18, 2018

Conversation

edwinsteele
Copy link
Contributor

Galleria triggers loading of the main gallery iamge via javascript
which has two problems. It only triggers the load when javascript
has been processed and loads it at the same time as the second and
third image which means it's competing with background images for
bandwidth. We know that the first image will always be seen, so
trigger a load in the document rather than waiting for js to do it.

Comparison (this patch is the only difference between the two runs): https://webpagetest.org/video/compare.php?tests=180303_KS_87bf0854a5a346d90d71fad871ae423c,180302_9A_ff950fed21454ed20fd5e9750d3157ca

@codecov
Copy link

codecov bot commented Mar 3, 2018

Codecov Report

Merging #307 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master    #307   +/-   ##
======================================
  Coverage    87.9%   87.9%           
======================================
  Files          20      20           
  Lines        1463    1463           
======================================
  Hits         1286    1286           
  Misses        177     177

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 55399b2...d81476f. Read the comment docs.

Galleria triggers loading of the main gallery iamge via javascript
which has two problems. It only triggers the load when javascript
has been processed and loads it at the same time as the second and
third image which means it's competing with background images for
bandwidth. We know that the first image will always be seen, so
trigger a load in the document rather than waiting for js to do it.
@edwinsteele edwinsteele force-pushed the feature/faster-first-image-load branch from 52163ce to d81476f Compare March 4, 2018 03:18
@saimn
Copy link
Owner

saimn commented Mar 4, 2018

I agree it would be useful to optimize the page loading for albums with many images, but your solution sounds too hacky and I think we could get better results with galleria loading JSON data:
https://docs.galleria.io/article/18-quick-start
It should not be too difficult to modify the current template to produce the list of images in JSON format, and then I think galleria can also load the images lazily which would be great.

@edwinsteele
Copy link
Contributor Author

Loading an asset in a hidden div, or a suppressed element is an established method for prioritised loading without javascript. It’s inelegant, to be sure, but until preload gets wider support, I don’t think there’s a nicer way.

I’ve done a quick prototype of json loading with lazy loading of thumbnails: edwinsteele@552db1e

From that I can see that json+lazyload does delay the thumbnail loading, so does reduce contention however the image loading only begins after js has been loaded and even then it triggers a download of the first 3 images in parallel.

I think the json work will give a win, and I think this will give us an improvement too. Is it possible to get this in? I’ll followup with the json work.

@saimn
Copy link
Owner

saimn commented Mar 5, 2018

Yes, the json approach seems actually much cleaner, it would be great to get this if you are ready to finalize your patch (which seems already quite good).

@edwinsteele
Copy link
Contributor Author

edwinsteele commented Mar 7, 2018

I’ve opened #312 for the switch to json. Are you happy to merge this too? I would prefer to use this solution for loading the first image earlier, but I can rebase with the more elegant but less supported preload tag if you feel strongly about the “hackiness"

@saimn
Copy link
Owner

saimn commented Mar 10, 2018

preload seems to have poor support currently, sadly, so I guess we could integrate this for some time. Does it make a big difference even with the json method ?

@edwinsteele
Copy link
Contributor Author

Yes, it still makes quite a difference. On my test I saw the json branch have the first image loaded after 1.3s compared to 0.7s with json + this patch. I would also like to move to preload once there’s more support. I’ll make a note to look at preload support in 6 months for review.

https://webpagetest.org/video/compare.php?tests=180311_D6_5fd54c4fc9e1b198f6c89000963fa9c9,180311_3H_8c3a4271a573410657849a86e517b7b4

@saimn
Copy link
Owner

saimn commented Mar 18, 2018

Ok, let's go with this for now, thanks.

@saimn saimn merged commit bd9e547 into saimn:master Mar 18, 2018
@saimn saimn added this to the 2.0 milestone Mar 19, 2018
@edwinsteele edwinsteele deleted the feature/faster-first-image-load branch October 1, 2018 00:44
kontza pushed a commit to kontza/sigal that referenced this pull request Aug 28, 2020
…age-load

Galleria: Load first image earlier
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants