-
Notifications
You must be signed in to change notification settings - Fork 615
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
Remove lazysizes and use browser-default lazyloading #2147
base: master
Are you sure you want to change the base?
Conversation
Autotagging @bigcommerce/themes-team |
This can probably be fixed by either using a |
Not sure if it is related, but I found that Chromes mobile emulation had issues with picking the right srcset image. I raised a bug: https://bugs.chromium.org/p/chromium/issues/detail?id=1246772#c3 My solution was to avoid the mobile mode and just manually adjust the window/viewport size. |
It looks like the lack of the sizes attribute is the problem. I hacked in a hand coded sizes value and it then chose the right image. Maybe the lazysizes can still work for us. |
I've also written an article on a way to use the native lazyload while still being able to use a placeholder image. either a spinner for all images or LQIPs for each. On a side note, lazy sizing triggers on the onload event which is delayed by all sorts of things, including tracking code. If the primary image of a page is set to lazyload using it, the LCP for the page can get really bad. Native lazyload is quite smart and will instantly load the image if it is in the viewport, no waiting for the load event to do it. I've also noticed on a lot of stores that the lazysizes method often means you see the spinner as you scroll, while native lazyload seems to handle this a lot better. |
8c4dc76
to
6719827
Compare
6719827
to
ce8da84
Compare
ce8da84
to
cfb75d2
Compare
@Tiggerito I've made further updates, now I set the You can see the updates here: https://test-store686.mybigcommerce.com/shop-all/ Interested in your thoughts. |
I am seeing consistently better performance on the Lighthouse mobile test. https://cornerstone-light-demo.mybigcommerce.com/shop-all/ is 63-83 (wide variation based on luck-of-the-draw with JS execution, I bet) I'll remove the WIP label and request for review from @bigcommerce/themes-team - FYI @bc-as |
I couldn't get it to trigger lazy loading. I think the native system has quite a big window to start loading images off screen. Even on a narrow display they all loaded at once. The main thing is that the visible images started loading as soon as the CSS file was loaded. It was nice to see that the browser put the visible product images on high importance and the rest on low. On that page the sizes attribute did it's job and all images are 320px. The mobile emulation bug I mentioned is still affecting Lighthouse. When fixed the "Properly size images" issue should go away and the score should go up. LCP can still be improved, but maybe that can be later (the other issue): Removing lazy load of the main images and adding a preload have those images load even sooner. I suspect that we don't need to remove the native lazy load as it's quite smart. And a preload would ensure it loads early anyhow. Removing it may be to just gain some points and remove a warning. |
The Google dev team provide an example of using the inbuilt loading attribute if available and doing a fall back to lazysizes if not. Something like that may work well and add lazy loading support for Safari. A slight alteration could be done to detect images with the lazyloading class but not the loading attribute and add the attribute so lazyloading still works on browsers that support the attribute. At some point the lazysizes part could be dropped. This hack does mean that image src values won't be set until around the DCL event, which means initially visible images would be delayed. It would be nice if the server could detect if lazyloading is supported and directly set the src (User Agent?). The main image should not be lazyloaded anyhow. Maybe directly set the src for images that are likely to show in the viewport so only native lazyloading is supported for them. |
I've put together a possible solution: https://bigcommerce.websiteadvantage.com.au/tag-rocket/articles/improving-image-loading-without-javascript/#lazysizes |
Over the break I did a lot of work and testing related to this. And came up with a variant of this solution. Some notes: I think we need to support lazysizes until at least Safari supports native lazy-loading and so that manually added images don't break. I did an extensive rewrite of responsive-img with that in mind, plus: I implemented LQIP using CSS and a background image. This means it still works with native lazy-loading and it has some technical advantages over the src swapping technique (like I can preload the LQIP image). The down side is it requires a little bit of CSS to make the background image line up with the main image, and its more at risk of display issues. I re-structured the properties you could set for responsive-img to make it more flexible, while keeping backwards compatibility with the lazyload parameter. The 'loading' parameter lets you decide the lazy-loading mode. 'auto' (default), 'lazy', 'eager' I added an 'importance' parameter ('auto' (default), 'high', 'low'). This is a future attribute (closed Chrome beta at the moment) that can be used to get an image to load earlier. Kind of the opposite of lazy-load and very similar to adding a preload for the image. LQIP is enabled via the 'placeholder' parameter: 'auto' (default), 'lqip'. I added a 'lazysizes' parameter ('auto' (default), 'disabled') to force the use if native lazy-loading which would be faster. I also implemented a 'sizes' parameter with a slight twist. If the size specified was a single pixel with it would optimise the generated html to provide just that size of image. No need to provide a whole list of images when you've already said which one to use. This will reduce a lot of code bloat. On the functionality, I added some more classes to identify things. Like 'default-image' if it had to fall back to the default image. Here's the code I am currently testing.
|
As my responsive-img solution still uses lazysizes I developed some code to dynamically convert img tags using lazysizes to use native lazy loading if it is available. It also only loaded the lazysizes script if required (the hard coded script tag should be removed). e.g. Lazysizes will be loaded and used on Safari, but not on most other browsers. The script needs to be inline and in the head section. This is so that it can monitor img tags as they get added to the DOM and change them instantly. This means they get the benefits of native lazy loading instantly. e.g. if the image is in the viewport it starts loading asap. The script also applies classes to mimic what lazysizes does, just in case people use them. And for fun I created a set of classes to track image loading when lazy loading is not enabled: normalloading, normalloaded One improvement would be to host lazysizes on the BC CDN to remove a connection delay. I guess that was already done by the other script. Here's my current code:
|
And here is the CSS to make LQIP images work:
The second bit is to remove the background image once the main image was loaded. This was after seeing some issues with the product gallery feature that would swap images around and the background could show up. I already have a use for my new normalloaded class :-) |
Oh Lort, this is a lot... I'm working on a version of the code that will test: vanilla lazyload, loading-attribute-polyfill vs lazysizes, and also swapping CDNs & using Cloudflare for image transformations (you'd get domain authority, etc). It stinks that the only way to really test is to push things to a BC server and compare load times (and those seem to fluctuate based on time of day, etc) |
Testing out whether removing JS-based lazyloading via lazysizes improves page speed metrics.
Our use of lazysizes was very useful when originally implemented, when browser lazyloading did not have significant adoption. With browser adoption now at 75%, we can consider moving to the native solution and reduce our use of JS.