schlessera
commented
4 years ago For some of these, we might still be able to calculate a close approximate height on the serverside based on the content from the embed, but it would mean supporting adding per-embed logic to do so.
For example, for a gist, you'll probably get very close by counting the number of lines and then calculating something like $top_chrome + ( $lines * $line_height ) + $horizontal_scrollbar? + $bottom_chrome. In most cases, this can yield a pixel perfect height estimate, so you'd be able to block the correct height right from the start.
To get the number of lines and the line height, you'd fetch the embed content from the server and parse the return HTML.
Given that there is direct support in AMP for something lie a gist via its specific component, I don't think it's necessarily bad to mirror that custom support in the Optimizer, and have a few lines of custom logic per component to calculate its height on the server-side.
westonruter
thelovekesh
Feature description
Cumulative Layout Shift (CLS) is a huge detriment to user experience on the web. It is also a huge challenge to solve in the case of 3rd-party embeds. For some AMP components, in particular
amp-iframeandamp-list, elements that need to resize based on their contents need to provide anoverflowbutton that needs to be clicked by the user to trigger the element to grow in height if the element is currently in or above the current viewport. The presence of this overflow button can be annoying for users and so it's important the initialheightbe set to be as close as possible to where the element will be once loaded so that, in the best case, nooverflowbutton needs to be presented.Nevertheless, for pragmatic reasons, other components do not have such
overflowlogic and they will resize to fit their contents regardless of being in the current viewport or above. For example,amp-gist:When the page loads at first, the user will see “Super interesting information!” momentarily until the Gist loads. This is a gross instance of CLS. The docs for the component call this out in the description for
height:Similar guidance is provided for
amp-twitter:This issue, however, is not unique to Gists and Tweets, but it's also an issue for other embeds. For example, embedding Facebook posts also have this issue and the
amp-facebookcomponent lacks that same guidance.All this being said, we should automatically provide the best approximate
heightfor these components as much as possible.One way this could be done, specifically in the block editor, is to render the component and then scrape the resulting height and then inject that as the
heightof the embed. This couldn't be done currently with the PHP Optimizer since we need a browser context to load the page, but perhaps the Node Optimizer could add Puppeteer as a dependency to obtain initial heights.This would essentially be an augmentation of SSR.
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
Implementation brief
heightand add as a custom block attribute.render_blockfilter that passes along theheightblock attribute to be inserted as adata-amp-embed-heightHTML attribute on thefigurewrapper element.AMP_Embed_Sanitizer::sanitize()logic which finds all//figure[ @data-amp-embed-height ]and then copies the value to the./div[ contains( @class, 'wp-block-embed__wrapper' ) ]/*[starts-with(name(), 'amp-')].QA testing instructions
Demo
Changelog entry