-
Notifications
You must be signed in to change notification settings - Fork 244
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
Don't preload hero images #1132
Conversation
We can't tell whether a hero image is responsive and is loaded depending on viewport size. The current implementation will always preload hero images, which means images might be preloaded even if they are not visible on the page. This PR reverts this behavior and disables preload generation for hero images. The only exception is when an image defines a media attribute.
preload.attribs.media = heroImage.media; | ||
if (!heroImage.media) { | ||
// We can only safely preload a hero image if there's a media attribute | ||
// as we can't detect whether it's hidden on certain viewport sizes otherwise. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? Is it an opt in sort of thing, or does the media attr provide some kind of indication of..something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is AMP's built in way to define per element media queries. The assumption is that if you use this, you won't define additional media queries in CSS. This could still lead to false positives, but I suspect that this is seldom the case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
This PR makes image optimization more flexible by making it possible to configure hero image count and blurry image placeholder generation on a per page basis. This PR also deprecates the preloadHeroImage option in favor of a new optimizeHeroImages option as preloading has been disabled in most cases (see #1132).
This PR makes image optimization more flexible by making it possible to configure hero image count and blurry image placeholder generation on a per page basis. This PR also deprecates the preloadHeroImage option in favor of a new optimizeHeroImages option as preloading has been disabled in most cases (see #1132).
This PR makes image optimization more flexible by making it possible to configure hero image count and blurry image placeholder generation on a per page basis. This PR also deprecates the preloadHeroImage option in favor of a new optimizeHeroImages option as preloading has been disabled in most cases (see #1132).
This PR makes image optimization more flexible by making it possible to configure hero image count and blurry image placeholder generation on a per page basis. This PR also deprecates the preloadHeroImage option in favor of a new optimizeHeroImages option as preloading has been disabled in most cases (see #1132).
* More flexible hero images and blurry placeholders This PR makes image optimization more flexible by making it possible to configure hero image count and blurry image placeholder generation on a per page basis. This PR also deprecates the preloadHeroImage option in favor of a new optimizeHeroImages option as preloading has been disabled in most cases (see #1132). * improve wording * log deprecated option
We can't tell whether a hero image is responsive and is loaded depending
on viewport size. The current implementation will always preload hero
images, which means images might be preloaded even if they are not
visible on the page. This PR reverts this behavior and disables
preload generation for hero images.
The only exception is when an image defines a media attribute.