ampproject / ampproject/amp-wp

Reduce cumulative layout shift (CLS) by attempting to determine heights of embeds beforehand

Open
#4,729 16 comments 3 reactions 0 assignees View on GitHub
Bento Editor Optimizer P2 WS:Perf
Dominant language
PHP
Stars
1.8k
Forks
378
Avg merge
1d 15h
Merged PRs (30d)
10

Description

## Feature description

[Cumulative Layout Shift](https://web.dev/cls/) (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-iframe`](https://amp.dev/documentation/components/amp-iframe/#iframe-resizing) and [`amp-list`](https://amp.dev/documentation/components/amp-list/#specifying-an-overflow), elements that need to resize based on their contents need to provide an `overflow` button 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 initial `height` be set to be as close as possible to where the element will be once loaded so that, in the best case, no `overflow` button needs to be presented.

Nevertheless, for pragmatic reasons, other components do not have such `overflow` logic and they will resize to fit their contents regardless of being in the current viewport or above. For example, [`amp-gist`](https://amp.dev/documentation/components/amp-gist):

```html

Super interesting information!


```

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](https://amp.dev/documentation/components/amp-gist/#attributes) for the component call this out in the description for `height`:

> ![data-gistid (required) The ID of the gist to embed. layout (required) Currently only supports fixed-height. height (required) The initial height of the gist or gist file in pixels. Note: You should obtain the height of the gist by inspecting it with your browser (e.g., Chrome Developer Tools). Once the Gist loads the contained iframe will resize to fit so that its contents will fit. data-file (optional) If specified, display only one file in a gist.](https://user-images.githubusercontent.com/134745/81976756-e1649d80-95dd-11ea-87c3-22ecff8b0d8b.png)

Similar guidance is provided for [`amp-twitter`](https://amp.dev/documentation/components/amp-twitter/):

![Twitter does not currently provide an API that yields fixed aspect ratio for embedded Tweets or Moments. Currently, AMP automatically proportionally scales the Tweet or Moment to fit the provided size, but this may yield less than ideal appearance. You might need to manually tweak the provided width and height. Also, you can use the media attribute to select the aspect ratio based on the screen width.](https://user-images.githubusercontent.com/134745/81978071-d448ae00-95df-11ea-88ed-d3b8f4fb9eb5.png)

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-facebook`](https://amp.dev/documentation/components/amp-facebook/) component lacks that same guidance.

All this being said, we should automatically provide the best approximate `height` for 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 `height` of 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

* After having added an Embed block to a post, saving and viewing the AMP page should should require any layout shifting for the block on the frontend.
* Bonus: Also set the height of embeds on non-AMP pages.

## Implementation brief

1. Check the initial post content for any embeds that lack heights, and watch for insertions or updates to Embed blocks.
2. Once the Embed block clears its loading state, measure the `height` and add as a custom block attribute.
3. Add a `render_block` filter that passes along the `height` block attribute to be inserted as a `data-amp-embed-height` HTML attribute on the `figure` wrapper element.
4. Add to`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

*

Contributor guide

Open the contributing guide

Research direction

Start with the Embed block loading and update flow, the render_block filter, and AMP_Embed_Sanitizer::sanitize(). Trace how the block height can reach the figure wrapper and then the AMP element, and verify against the acceptance criteria that saved AMP pages no longer shift layout after embeds load.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, php, wordpress
Domain
frontend, performance, web-dev
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.