Skip to content

🚀 [Story performance] Remove CLS from system layer when adding buttons#37929

Merged
mszylkowski merged 19 commits intoampproject:mainfrom
mszylkowski:systemLayerCLS
Mar 22, 2022
Merged

🚀 [Story performance] Remove CLS from system layer when adding buttons#37929
mszylkowski merged 19 commits intoampproject:mainfrom
mszylkowski:systemLayerCLS

Conversation

@mszylkowski
Copy link
Copy Markdown
Contributor

@mszylkowski mszylkowski commented Mar 21, 2022

When the system layer gets created it toggles the icons inside vsync mutates. Adding the icons without user interaction will count as CLS to the system layer because it changes size, but we can fix it by giving it the initial size that fills the width so that adding icons will not generate CLS (tested on Chrome and the CLS disappears).

On desktop there is a CLS when on desktop panels because adding the desktop class happens on a vsync after adding the system layer to the shadow root. We need to initialize the desktop state (UI type classes) when the system layer is created, not after a vsync. Therefore we need to initialize the listener synchronously manually, and then use vsync when updating the UIType inside a vsync.

I found that these 2 fixes can improve CLS by 3% (0.03 CLS score) or more, which is 30% of the limit for CLS (0.1)

@mszylkowski mszylkowski requested a review from gmajoulet March 21, 2022 18:36
@amp-owners-bot
Copy link
Copy Markdown

amp-owners-bot bot commented Mar 21, 2022

Hey @gmajoulet, @newmuis! These files were changed:

extensions/amp-story/1.0/amp-story-system-layer.css
extensions/amp-story/1.0/amp-story-system-layer.js

@mszylkowski mszylkowski self-assigned this Mar 21, 2022
Co-authored-by: Gabriel Majoulet <gmajoulet@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants