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

Resize observer elements detected as non-composited animations (CLS check) #452

Closed
kinless opened this issue Jul 19, 2022 · 5 comments
Closed

Comments

@kinless
Copy link

kinless commented Jul 19, 2022

@KingSora first I want to compliment you on such a fantastic plug-in, it's been invaluable to several projects! I look forward to v2.0

Description
This is such a nit-picky bug it's probably not even worth mentioning, but as I know some devs love maximum optimization, thought I'd report it here. Likely a super-simple fix, I'm just not versed enough in this space to be messing with your CSS rules without borking something. Maybe someone here could easily adjust with a PR?

When running optimization tools to check for a website's Cumulative Layout Shift (CLS), it's flagging the "os-resize-observer" elements as non-composited animations, which has a "potential" to affect CLS. According to the description, it's due to the z-index: -1 css rule on those elements, which for some reason isn't being recognized as valid.

To Reproduce
Steps to reproduce the behavior:

  1. Run a CLS checker such as the one on gtmetrix.com (using Google LightHouse) against a site using OS
  2. Check for "Avoid non-composited animations" under the Performance tab

Screenshot Example

os-non-composited

@KingSora
Copy link
Owner

Good day @kinless

Thanks for the report! I'm using an animation animating z-index to detect when the element appears on the screen. This animation however can be whatever you want.. Back in the day I thought z-index might be a good fit, but turns out something like cursor is much better.. I'll change this in the next v1 release and already implemented it in the v2 code :)

@KingSora
Copy link
Owner

@kinless
Copy link
Author

kinless commented Jul 20, 2022

@KingSora thanks for hopping on this so quickly!

Unfortunately it looks like "cursor" is also an invalid animation method according to the checker. I can't possibly see how that would contribute to any layout shifting, but it's still being detected as unsupported.

os-non-composited-cursor

But now that I understand better what's happening with your CSS and why it's being flagged, I looked further into this, and it appears the non-composite warning triggers on any animation that is not "translate" or "opacity". So going back to what you were doing with the z-index detection, I replaced "cursor" in your keyframes rule with translateZ(0) to translateZ(-1) which still works and is also no longer detected as invalid. (Although now the CLS tool uncovered another invalid animation with the "right" property in "os-scrollbar", see screenshot below.)

Of course, translateZ support only goes back to IE10. If feasible, using translateX/Y (with -ms prefix) could stretch it back to IE9, or even using opacity to keep IE8 support, but I don't know if these would introduce glitches. It's possible that cleaning up all these non-composite flags may only be a v2.0-appropriate fix if you want to keep it simple and stick to legacy browser support for v1.x.

os-non-composited-right

@KingSora
Copy link
Owner

Oh, thats unfortunate.. I reasoned about the cursor property with the help of https://csstriggers.com/ where it has better stats than the transform or opacity properties...

If its fine for you, I'll leave it like this, because this is 100% a false positive in this case.

@kinless
Copy link
Author

kinless commented Jul 25, 2022

@KingSora no worries. Like I first mentioned, this is such a trivial issue. I don't know who had the authority to decide that only translate and opacity get a pass while everything else is automatically flagged as unsupported. https://csstriggers.com says that translate generally doesn't trigger geometry or paint so I'm OK with leaving the default animation listener overridden with translateZ in my particular scenario. Thanks again for looking into it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants