search

ampproject / amphtml

The AMP web component framework.
https://amp.dev
Apache License 2.0
14.89k stars 3.89k forks source link

CLS due to amp-sidebar.css #31741

Open bobrichards opened 3 years ago

bobrichards commented 3 years ago

What's the issue?

I have found that when using amp-sidebar tag, /extensions/amp-sidebar/0.1/amp-sidebar.css is also requested. the delay requesting this CSS causes a CLS (Cumulative Layout Shift) of around 0.18 on a pc / laptop. There is no shift on mobile device (iPhone 5s) -

CLS can be seen using page speed insights and the web vitals chrome plugin (https://chrome.google.com/webstore/detail/web-vitals/ahfhijdlegdabablpippeagghigmibma?hl=en) - web vitals fails with AMP and passes with inline amp-sidebar

This causes the page to slightly jump on page load for a split second during loading causing CLS

How do we reproduce the issue?

This Layout shift occurs during page load and being an amp site loads quite fast but this can be replicated by simply refreshing the page multiple times - you will notice the page jump up for a split second - using the plugin mentioned above will help demonstrate

This can not be replicated on jsfiddle etc as its AMP that requests /extensions/amp-sidebar/0.1/amp-sidebar.css and these CSS rules does not pass onto these types of websites

I have now had the change applied to all pages but asked for one page to be left with default AMP behaviour to demonstrate the issue

URL With CLS due to delay in CSS request: https://makefreecallsonline.com/amp-issue.php

URL with /extensions/amp-sidebar/0.1/amp-sidebar.css in lined: https://makefreecallsonline.com/no-cls.php

Using chrome developer tools and/ or the web vitals plugin you can see the page loading in line amp-sidebar css has no CLS

amp-sidebar { --story-page-vh: 1vh; position: fixed!important; top: 0; max-height: 100vh!important; height: 100vh; max-width: 80vw; background-color: #efefef; min-width: 45px!important; outline: none; overflow-x: hidden!important; overflow-y: auto!important; z-index: 2147483647; -webkit-overflow-scrolling: touch; will-change: transform; }

In lining the above directly to the web page (style amp-custom) causes 0 CLS -

HOWEVER.. adding the !important inline CSS makes the AMP validator fail as this not allowed.

At present CLS can be removed by in lining above but makes AMP invalid - removing !important makes AMP valid but CLS returns

Maybe allowing !important only in amp-sidebar might be a solution

As this already is a measured metric and soon to become a ranking factor I thought It may be worth a mention on here to see if the devs may come up with a solution if suitable. No actually sure of this is bug but it certainly fixed my CLS issue and makes a visual difference

What browsers are affected?

Only tested on: Chrome Version 87.0.4280.88 / Edge Version 87.0.664.66

Which AMP version is affected?

Version 2011252111003

suneel-code commented 3 years ago

For our site, we have simply added "hidden" attribute to amp-sidebar element. CLS dropped suddenly from 1.0 to 0.05. <amp-sidebar id="" hidden> ...... </amp-sidebar> On click of Menu button toggles this "hidden" attribute.

kristoferbaxter commented 3 years ago

Hi there, this is quite an interesting issue! Leaving notes here before heading into a meeting.

A clarification that might be helpful:

Using the AMP Page Experience Tool highlights a few recommendations I would consider for this site going forward. You can see them here: https://amp.dev/page-experience/?url=https://makefreecallsonline.com/amp-issue.php. Namely:

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.