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

Add Lazyload attribute to images #384

Merged
merged 2 commits into from
Mar 24, 2021
Merged

Conversation

rebeccawong7
Copy link
Contributor

@rebeccawong7 rebeccawong7 commented Mar 23, 2021

This pull request addresses issue #355, I have added lazy load attributes to images not in the initial viewport to help increase site performance.

I had some acceptance tests fail but I think it was due to issues with pages not loading quickly enough on my PC.

This is my first pull request so let me know if it's all OK.

@jimaek jimaek temporarily deployed to jsdelivr-com-master-9sytussw9x March 23, 2021 16:40 Inactive
@jimaek jimaek linked an issue Mar 23, 2021 that may be closed by this pull request
@@ -1,5 +1,5 @@
<div class="c-infographic-banner">
<img src="{{@shared.assetsHost}}/img/landing/infographic-bg.png" srcset="{{@shared.assetsHost}}/img/landing/infographic-bg@2x.png 2x">
<img src="{{@shared.assetsHost}}/img/landing/infographic-bg.png" srcset="{{@shared.assetsHost}}/img/landing/infographic-bg@2x.png 2x" loading="lazy">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For anything above the fold I dont think there is a point to do lazy loading. Here the whole point of the page is the image, so lazyloading would make a bad user experience. Please remove it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, let me know if there are any other images you want me to revert :)

@jimaek jimaek temporarily deployed to jsdelivr-com-master-9sytussw9x March 24, 2021 13:59 Inactive
@jimaek jimaek merged commit d6676c4 into jsdelivr:master Mar 24, 2021
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.

Lazyload images
2 participants