GoogleChrome / web-vitals

Essential metrics for a healthy site.
https://web.dev/vitals
Apache License 2.0
7.47k stars 410 forks source link

CLS being reported due to filter CSS transition #470

Closed Magellol closed 4 months ago

Magellol commented 4 months ago

Hello!

I was investigating various CLS reports on our app when I came across this one which seems to be triggered by a CSS transition with a filter property on a :after pseudo element. I'm not sure I understand why a brightness filter could cause a layout shift.

I was able to create a small reproducible example: https://codesandbox.io/p/sandbox/cls-after-el-filter-fxj5jn

Here's also a recording

https://github.com/GoogleChrome/web-vitals/assets/3819093/c9990e59-d668-4faf-8bd7-fd201915165c

Note: Replacing the :after pseudo element by a span removes the CLS entirely, i've got a second example here: https://codesandbox.io/p/sandbox/no-cls-after-el-filter-df2s9s

Backstory

We often create cards with this HTML/CSS structure to be able to make a whole card clickable while also providing other links inside it because we can't nest anchor tags. We use an :after pseudo element that we stretch over the whole card and z-index position other links above it. This keeps every links accessible (tab navigation and whatnot).

The CLS is quite small here but in our case the magnitude is much higher (without actual visible shift) but I think we get numbers like 0.05 which is already half of the "good" threshold so we don't have much room left if other CLS are found.

I'm not sure if this is a bug on this library or a bug on our end so I'm looking for guidance. What do you think? I'm also curious if this is accounted for our overall CWV score or is strictly a potential false positive from this library and doesn't impact our score.

Let me know if you need more information. Thank you.

tunetheweb commented 4 months ago

This is a very interesting one! And I had to pull out some favours with some CSS colleagues to figure it out.

Apparently applying filter is one of the properties that creates a new stacking context so, as that's added and removed as you hover over it, that's causing a slight recalc in the CSS which technically causes a change in the pseudo elements position (even though there's no visible change to the user).

The easiest way to prevent this is to create that new stacking context in advance, by uncommenting one of the two properties below:

      .overlay-container {
        transition: filter 0.2s ease-in-out;
        /* filter: brightness(1); */
        /* will-change: filter; */

The first (filter: brightness(1);) sets the filter property even in unhovered state to the default of 1 so it has a filter value and knows therefore that it needs it's own stacking context for when you change this default later.

The second (will-change: filter;) explicitly tells the browser that it needs it's own stacking context as a filter is going to be changed later. It's probably the cleaner of these two options as doesn't require you to explicitly set a default value (that might interfere with other CSS you have).

Should Chrome count this as CSS when there is no user-visible change? No, it probably shouldn't. So feel free to raise a bug at crbug.com/new, however it's quite a minor one in this case (do let us know if you have a bigger example!) and it'll actually be ever so slightly more performant to do the solution above, so I imagine it won't be the biggest priority to look at this.

Oh and btw, on that note, Chrome-specific issues should be raised to crbug.com or to our web-vitals-feedback group, rather than here. This repo is specifically for the web-vitals.js library and this issue could have been repeated with a layout-shift performance observer even outside the library (though you may not have realised that!).

Magellol commented 4 months ago

UPDATE: using the will-change property fixes the issue on our end as well 💯


Thanks for looking into this and for the suggestions. I guess on our end, I still need to figure out why the numbers we're seeing are larger than the reproducible example I sent above but in the meantime, I'll apply your suggestion and move on.

Oh and btw, on that note, Chrome-specific issues should be raised to crbug.com rather than here, or to our web-vitals-feedback group. This repo is specifically for the web-vitals.js library and this issue could have been repeated with a layout-shift performance observer even outside the library (though you may not have realised that!).

That makes sense. I wasn't sure where the problem was, good to know for next time.

tunetheweb commented 4 months ago

UPDATE: using the will-change property fixes the issue on our end as well 💯

Excellent! Great to hear.