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

Optimize hero images #5055

Closed
sebastianbenz opened this issue Jul 16, 2020 · 5 comments · Fixed by #5350
Closed

Optimize hero images #5055

sebastianbenz opened this issue Jul 16, 2020 · 5 comments · Fixed by #5350
Assignees
Labels
Groomed Needs sizing Optimizer P0 High priority WS:Perf Work stream for Metrics, Performance and Optimizer
Milestone

Comments

@sebastianbenz
Copy link

Feature description

The validator support for server-side rendering amp-img as img has been merged (cl/321463348). Would be great to also support this in the AMP WP plugin. As a reference on how to add it, you can find the AMP Optimizer integration here.

A few notes on the AMP Optimizer integration:

  1. It will either auto-detect a single hero image on the page, or look for hero images explicitly being marked via the data-hero attribute.
  2. For each detected hero image (up to a max of 2) it will generate a preload (if no srcset is defined) and inject an img into the amp-img element (this also works for amp-iframe placeholders).
  3. It does not specify loading=lazy/eager (see Validator: Allow SSR for amp-img's img amphtml#29025 (comment)).

//cc @westonruter @schlessera


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation brief

QA testing instructions

Demo

Changelog entry

@amedina amedina added this to the v1.7 milestone Jul 16, 2020
@westonruter westonruter added the P0 High priority label Jul 16, 2020
@westonruter
Copy link
Member

westonruter commented Jul 16, 2020

This is great.

For added heuristics in the WordPress context, we may be able to use the same logic that is being used for lazy-loading images that landed in 5.5-beta: the loading=lazy is omitted from images that are likely to be used in the first viewport. We could use the same logic to add data-hero to the page if no amp-img elements exist on the page with that attribute. See https://core.trac.wordpress.org/ticket/50425

@westonruter
Copy link
Member

Landed in AMP core: ampproject/amphtml#28672

AMP Toolbox Optimizer PR: ampproject/amp-toolbox#880

@kmyram kmyram added the WS:Perf Work stream for Metrics, Performance and Optimizer label Aug 5, 2020
@westonruter
Copy link
Member

westonruter commented Sep 23, 2020

As discussed just now, let's also include the hero preloading logic detection for CSS background images that are defined with inline style attributes. For example, the Cover Image block in core is rendered on an AMP page as follows:

image

In the same way that a transformer is looking for amp-img@src, amp-video@poster, amp-iframe > amp-img (and amp-youtube per https://github.com/ampproject/amp-wp/pull/5350/files#r485283525), it can also look for any element @style (or @data-amp-original-style) to extract the background-image URL, as another candidate for the hero image.

One complication here is that an element with an inline CSS background could be tiny, and we'd have no way of using isTinyElement to detect whether it is too small to be a candidate. Perhaps this interplays with whether the element has data-hero as referenced in #5350 (comment). Otherwise, we could have WP-specific support, so we'd automatically consider elements that have wp-block-* as a class name to be considered to be not tiny.

@schlessera
Copy link
Collaborator

I am planning on adding a WP-specific transformer that is responsible for tagging the best images with the data-hero attribute. This will be in the plugin, not the Optimizer library.

@pierlon
Copy link
Contributor

pierlon commented Apr 28, 2021

QA Passed

Images within the initial viewport are being optimized and receive the data-hero attribute:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Groomed Needs sizing Optimizer P0 High priority WS:Perf Work stream for Metrics, Performance and Optimizer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants