dvc.org icon indicating copy to clipboard operation
dvc.org copied to clipboard

Look into ways to lazy load our images

Open julieg18 opened this issue 4 years ago • 3 comments

We were forced to take off lazy loading in #3229, since they were breaking anchor links on certain pages (#3226). But there might be a way to turn it back on and keep our scrolling behavior.

I also had the thought of changing the logic to keep lazy loading, but fixing the broken behavior takes priority and since the lazy-load fix is a one-liner it can very easily be undone should we do something more robust- I think @shcheklein is on the right track with having an empty div- I sort of thought that was how it behaved already with the blur-up placeholder images that are most of the point of the Gatsby image plugins, but I could definitely be wrong and it's worth testing. @rogermparent in #3226

julieg18 avatar Jan 27 '22 22:01 julieg18

As of now gatsby-remark-images does not support gifs. I could find two elements that are causing layout shifts:

  1. GIFs on Image as gatsby-remark-images just bypass them as the sharp didn't support gifs. (Sharp did add support for gifs very recently. But, the plugins gatsby-plugin-sharp => gatsby-remark-images are not updated accordingly.)
  2. Youtube embeds.

Another conflicting part is, scroll to hash

const scrollToHash = (hash: string, scrollOptions = {}): void => {
  if (hash) {
    scrollIntoLayout(safeQuerySelector(hash), {
      waitImages: true,
      ...scrollOptions
    })
  }
}

So, if we have lazy-loading then scrollToHash will directly conflict and will not work properly on pages with images that are not near to the viewport.

The possible solution I see we this is.

  • It makes more sense not to wait for images to load (unless there are other reasons besides this). This will solve the current images gatsby-remark-images supports.
  • And for those, it doesn't support gif specifically, I believe we can have a custom component or other way to specify the height of the wrapper maintaining the aspect ratio for responsiveness. (we can wait for gatsby-remark-images to support gifs and work by default. I believe the bump of the sharp plugin shouldn't take a long period.)
  • Similarly, we can set the height to the youtube embeds wrapper as well.

This will make sure there will not be layout shifts. So, the hash links should be working properly and we can turn on the lazy-load.

cc: @iterative/websites

yathomasi avatar Apr 11 '22 15:04 yathomasi

As of now gatsby-remark-images does not support gifs. I could find two elements that are causing layout shifts:

If I'm understanding correctly, you've tested @rogermparent's theory and while images work fine thanks to a placeholder, GIFS and YT embeds break scrollToHash?

julieg18 avatar Apr 11 '22 16:04 julieg18

With lazy-loading enabled on gatsby-remark-images, it works nicely for the images it supports with the proper layout placeholder. But, it skips GIFs/SVGs, so there will be no optimization as well as no layout placeholder causing the layout shift.

And, the issue with scrollToHash is as @julieg18 figured out, with withImages: true on the snippet above, the hash links wait for images to load properly on DOM, but if we have lazy-loading enabled, the scrollToHash breaks, so the hash links don't work.

So, the idea of scrollToHash with waiting for images and lazy-loading don't go hand-in-hand. And, I believe we have withImages: true so that we don't have the layout shift. But, if we can have layout placeholder in place, I think we can set that to false. For this, if we can implement the proposed solution, that would be great. I feel like it will be little more challenging to handle the GIFs, but I will try submitting a draft and see how it goes.

yathomasi avatar Apr 12 '22 05:04 yathomasi