Cumulative layout shift

prathamesh-gharat commented 3 years ago

short description of the bug / issue, provide more detail below. Core Web Vitals points out that there are layout shifts occurring on a page if

the slider is above the fold

the visitor clicks a sitelink on google search result and scrolls below to content

The problem is that the slider content is rendered (items stacked vertically in most case?) in a different way before the slick slider's javascript loads and it is rendered differently after the javascript loads, mainly the height of the slider container changes.

I believe preventing a layout shift would mainly involve writing HTML & CSS of the slides in such a way that the slider container is rendered almost the same (specifically the height of the container shouldn't change).

====================================================================

Steps to reproduce the problem

Issue is re-producible here.

Open https://kenwheeler.github.io/slick/
Scroll a bit below so that a the slider is in the viewport.
Open chrome dev tools
Press Ctrl + Shift + P
Search for "web vitals"
Select "Show Core Web Vitals overlay"
Refresh the page
Check the cumulative layout shift value. (it should be around 0.28 on a desktop)

https://i.imgur.com/KXdOZAu.png

====================================================================

What is the expected behaviour?

There should be no layout shift. ...

====================================================================

What is observed behaviour?

Layout shift occurs. ...

====================================================================

More Details

Which browsers/versions does it happen on? I believe this would happen on any browser.
Which jQuery/Slick version are you using? Latest (tested on slick's demo page)
Did this work before? No

I will try to come up with an example solution/demo in a few days as time allows. Meanwhile I'm documenting the issue here. The example could be a part of the documentation showcasing an implementation of slick which doesn't cause layout shifts.

PieterDC commented 3 years ago

@prathamesh-gharat did you make any progress on this meanwhile?

Setting a max-height on the container fixed it for me on the biggest breakpoint because there I know the max-height of the images in the carousel. That approach could be documented.

But it still leaves smaller breakpoints, aka mobile view, with the CLS issue.

PieterDC commented 3 years ago

Although no other Slick issue already mentioned 'CLS', there are a bunch of related issues that talk about a flash of unstyled content (FOUC), which is what the Core Web Vital complains about as well.

We can find some inspiration here:

PieterDC commented 3 years ago

Eventually I went with something similar to https://github.com/kenwheeler/slick/issues/1741#issuecomment-154062718

That might help you as well, @prathamesh-gharat

prathamesh-gharat commented 2 years ago

@PieterDC I had completely forgotten about this topic. I remembered about this as we were trying to address this same problem again for a client today.

https://jsbin.com/cageray/edit?html,css,js,output

P.S. I checked the solution in the comment, it works but I had come up with a different approach as we didn't think setting the display property after slick has initialized would actually work. What I came up with uses a similar approach but it removes display: none from the rest of the slides after slick has initialized. + Some more control over how many items are initially visible via CSS.

Important pieces of code for reference.

<div class="slider slick-not-init">
    <div class="slide">1</div>
    <div class="slide">2</div>
    <div class="slide">3</div>
    <div class="slide">4</div>
    <div class="slide">5</div>
    <div class="slide">6</div>
</div>

.slider .slide {
  width: 100%;
  background: #ddd;
  height: 100px;
  display: flex;
  justify-content: center;
  align-items: center;
}

/*
Goal: Hide all except the 1st slide (because that is what Slick looks like after initializing; at least in this case).

Increment `+2` depends on how many items are configured to be visible in the view.

`n+2` means every `.slide` after and including the 2nd.

If you have 2 `slidesPerRow` then adjust this to `n+3`
`n+4` if you have 3 `slidesPerRow`.

Note: If you have multiple slides per view `slidesPerRow` then you would need to specifically target each slider. Maybe you would need to use media queries too if you use `responsive` for having a different number of slides at different screen sizes.
*/
.slider.slick-not-init .slide:nth-child(n+2) { 
  display: none;
}

/* Test - Set slide background to red when not initialized */
.slider.slick-not-init .slide {
  /* background: red; */
}

$(document).ready(function(){

  $('.slider').on('init', function(){
    $(this).removeClass('slick-not-init')
  })

  /* Simulate defer loading slick */
  setTimeout(function() {
    $('.slider').slick({});
  }, 2500)
});

This approach could be used to build a Prevent Slick Slider CLS - CSS Generator 🤔

Input the slider HTML.
Input the slick config JSON. (mainly for slidesPerRow & responsive)
Output could be CSS to prevent CLS + maybe modified HTML for specifically targeting this instance of the slider. This might be helpful for those who have varying slider responsive configurations.

Edit ^ That is still like providing a complicated bandaid to a problem that we know will occur. Can you see any possibility of utilizing template tags to hide content that should not be initially visible to make it simple for the user to implement? (responsive will still be hard) Ref: https://www.w3schools.com/tags/tag_template.asp

kenwheeler / slick