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

Transformer: Preload images #11

Closed
5 tasks
schlessera opened this issue Mar 27, 2020 · 8 comments
Closed
5 tasks

Transformer: Preload images #11

schlessera opened this issue Mar 27, 2020 · 8 comments
Assignees
Labels

Comments

@schlessera
Copy link
Collaborator

Feature description

Add a new transformer BrowserHints that adds browser hints as meta tags to the <head> node. The initial implementation should add the following hint:

  • Add preload hint for the first X images in the page

The number X should be configurable by the user and default to 5.

Related ampproject/amp-wp#4213


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

Acceptance criteria

  • Generated HTML output from the Optimizer contains one or more resource hints that tell the browser to preload images
  • Images selected for preloading should be the first X in the main post, as well as anything needed for the site header (X defaults to 5)
  • Images smaller then Y pixels are ignored (Y defaults to 150)
  • Images with an aspect ratio larger than Z are ignored (Z defaults to 16)
  • The number of images X, the minimum size Y and the maximum aspect ratio Z are configurable by the user

Implementation brief

QA testing instructions

  1. Verify: AMP optimization is enabled
  2. Go to the AMP version of a page that contains multiple images
  3. Use "View Source..." on the page to see the generated HTML markup
  4. Verify: The <head> element contains <link rel=preload ...> elements pointing to the first few images

Demo

Changelog entry

@westonruter
Copy link
Member

westonruter commented Mar 27, 2020

The number X should be configurable by the user and default to 5.

In the WordPress plugin, I think this should include:

  1. Custom logo
  2. Post featured image
  3. Theme header image
  4. Theme background image

However, it gets complicated for images that have srcset. In order to successfully preload images, we need to depend on browsers supporting imagesrcset and imagesizes. Otherwise, the browser may preload the wrong image for the viewport.

More resources:

This is only supported by Chromium-based browsers as far as I know.

@sebastianbenz
Copy link

The tricky part here is not to take away bandwidth from more critical resources (e.g. v0.js). The AMP Cache team is currently working on a (single) hero image identification heuristic. Once this has been done, we should port it to AMP Optimizer and WP.

@westonruter
Copy link
Member

Identifying the hero image should be relatively straightforward in WordPress as it is normally the featured image that has been selected for a given post.

@sebastianbenz
Copy link

That's perfect of course, is this always going to be an image in the first viewport?

@westonruter
Copy link
Member

Not necessarily, as a theme may also select a header image and a custom logo. But the featured image would be the most important image in the first viewport.

Here's a fairly typical scenario for images on a theme:

image

My face is the custom logo. The plants on the table are the header image. And the cloudy skyline is the post's featured image. Themes will differ in how they arrange these in a template, but they are often all visible in the first viewport.

@westonruter
Copy link
Member

Closely related to #10 which is for adding preconnect links for components used on the page. These links should only be done for components that are in the first viewport.

@schlessera
Copy link
Collaborator Author

I'm not sure this should still be included, given the work that the PreloadHeroImage transformer is already doing.

@schlessera schlessera transferred this issue from ampproject/amp-wp Nov 6, 2020
@schlessera
Copy link
Collaborator Author

Given the PreloadHeroImage transformer not even does preloads anymore now (see #127), I'm closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants