-
Notifications
You must be signed in to change notification settings - Fork 101
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
Occluded initial-viewport images should get fetchpriority=low #1309
Comments
@westonruter |
@devansh016 Actually I don't think it's going to be as simple as what you've proposed in #1320. Namely if I understand correctly, your PR would add |
@westonruter So currently, Optimization Detective will also consider any of those "hidden carousel images" for example as above the fold right? So they won't have On a related note, what is even the right way to mark such images? Should they have |
@felixarntz Good question. I just tested, and actually the hidden carousel images do get <div class="ub_image_slider swiper-container wp-block-ub-image-slider" id="ub_image_slider_aaa72125-78c1-40f4-88c9-f0f5068f5f8b" data-swiper-data='{"speed":300,"spaceBetween":20,"slidesPerView":1,"loop":true,"pagination":{"el": ".swiper-pagination" , "type": "bullets", "clickable":true} ,"navigation": {"nextEl": ".swiper-button-next", "prevEl": ".swiper-button-prev"}, "keyboard": { "enabled": true }, "effect": "slide","simulateTouch":false}'><div class="swiper-wrapper">
<div class="swiper-wrapper">
<figure class="swiper-slide">
<img
data-od-added-fetchpriority
fetchpriority="high"
decoding="async"
src="http://localhost:8888/wp-content/uploads/2024/06/bison3-scaled.jpg"
alt=""
/>
<figcaption class="ub_image_slider_image_caption"></figcaption>
</figure>
<figure class="swiper-slide">
<img
data-od-added-loading
loading="lazy"
decoding="async"
src="http://localhost:8888/wp-content/uploads/2024/06/bison2-scaled.jpg"
alt=""
/>
<figcaption class="ub_image_slider_image_caption"></figcaption>
</figure>
<figure class="swiper-slide">
<img
data-od-added-loading
loading="lazy"
decoding="async"
src="http://localhost:8888/wp-content/uploads/2024/06/bison1-scaled.jpg"
alt=""
/>
<figcaption class="ub_image_slider_image_caption"></figcaption>
</figure>
</div>
... This is not ideal because it means that as soon as the next carousel slide is displayed, the image may have to download from the beginning, as there are no heuristics for the browser to detect when the image is nearing the viewport. According to the aforementioned Web.dev article, the best markup for this is: <ul class="carousel">
<img src="img/carousel-1.jpg" fetchpriority="high">
<img src="img/carousel-2.jpg" fetchpriority="low">
<img src="img/carousel-3.jpg" fetchpriority="low">
<img src="img/carousel-4.jpg" fetchpriority="low">
</ul> I believe this is especially good for when only one image can be displayed at a time. If, however, there is a horizontally-scrolled container in which the other images come into the viewport gradually, this would be a case where I haven't had a breakthrough yet on how to detect when an image should get But how to handle the case of detecting occluded |
Oh, I just had another idea for a carousel which is much less hacky. I think we actually may be collecting the information we need in the end, and it's not in the Screen.recording.2024-08-07.18.04.51.webmHere are the three [
{
"isLCP": true,
"isLCPCandidate": true,
"xpath": "/*[1][self::HTML]/*[2][self::BODY]/*[2][self::DIV]/*[2][self::MAIN]/*[2][self::DIV]/*[1][self::DIV]/*[1][self::DIV]/*[1][self::FIGURE]/*[1][self::IMG]",
"intersectionRatio": 1,
"intersectionRect": {
"x": 1275.4166259765625,
"y": 343.1510314941406,
"width": 650,
"height": 250,
"top": 343.1510314941406,
"right": 1925.4166259765625,
"bottom": 593.1510314941406,
"left": 1275.4166259765625
},
"boundingClientRect": {
"x": 1275.4166259765625,
"y": 343.1510314941406,
"width": 650,
"height": 250,
"top": 343.1510314941406,
"right": 1925.4166259765625,
"bottom": 593.1510314941406,
"left": 1275.4166259765625
}
},
{
"isLCP": false,
"isLCPCandidate": false,
"xpath": "/*[1][self::HTML]/*[2][self::BODY]/*[2][self::DIV]/*[2][self::MAIN]/*[2][self::DIV]/*[1][self::DIV]/*[1][self::DIV]/*[2][self::FIGURE]/*[1][self::IMG]",
"intersectionRatio": 0,
"intersectionRect": {
"x": 0,
"y": 0,
"width": 0,
"height": 0,
"top": 0,
"right": 0,
"bottom": 0,
"left": 0
},
"boundingClientRect": {
"x": 1945.4166259765625,
"y": 343.1510314941406,
"width": 650,
"height": 250,
"top": 343.1510314941406,
"right": 2595.4166259765625,
"bottom": 593.1510314941406,
"left": 1945.4166259765625
}
},
{
"isLCP": false,
"isLCPCandidate": false,
"xpath": "/*[1][self::HTML]/*[2][self::BODY]/*[2][self::DIV]/*[2][self::MAIN]/*[2][self::DIV]/*[1][self::DIV]/*[1][self::DIV]/*[3][self::FIGURE]/*[1][self::IMG]",
"intersectionRatio": 0,
"intersectionRect": {
"x": 0,
"y": 0,
"width": 0,
"height": 0,
"top": 0,
"right": 0,
"bottom": 0,
"left": 0
},
"boundingClientRect": {
"x": 2615.41650390625,
"y": 343.1510314941406,
"width": 650,
"height": 250,
"top": 343.1510314941406,
"right": 3265.41650390625,
"bottom": 593.1510314941406,
"left": 2615.41650390625
}
}
] Note how the first element has |
@westonruter I like your last idea: For images that aren't visible, but they're on the same horizontal line as a visible image, use A related but separate question: What happens to images that are completely hidden (either by them or a containing element having |
Oh yes, oops. I've corrected my comments to say I also just realized that it's probably even simpler than I suggested. Instead of detecting whether an This also could be relevant for images that are placed outside the viewport in a negative direction to the top, left, or right. I recall this is a more accessible way to style dropdown menus since the submenu items remain focusable with the keyboard even when the submenu is not displayed. But maybe this is old news as I see with the current dropdown menus in Gutenberg the submenus are styled with To summarize:
I was thinking about this when trying to think of an approach to images appearing in a mega menu above, but now that I see that core's built-in submenus get But let's say that's not always the case, that sometimes images in a mega menu get <details
class="wp-block-details is-layout-flow wp-block-details-is-layout-flow"
open=""
>
<summary>Check out this image</summary>
<figure class="wp-block-image size-large">
<img
fetchpriority="high"
decoding="async"
width="1024"
height="668"
src="http://localhost:10023/wp-content/uploads/2024/08/bison1-1024x668.jpg"
alt=""
class="wp-image-8"
srcset="
http://localhost:10023/wp-content/uploads/2024/08/bison1-1024x668.jpg 1024w,
http://localhost:10023/wp-content/uploads/2024/08/bison1-300x196.jpg 300w,
http://localhost:10023/wp-content/uploads/2024/08/bison1-768x501.jpg 768w,
http://localhost:10023/wp-content/uploads/2024/08/bison1-1536x1002.jpg 1536w,
http://localhost:10023/wp-content/uploads/2024/08/bison1-2048x1336.jpg 2048w
"
sizes="(max-width: 1024px) 100vw, 1024px"
/>
</figure>
</details> Image Prioritizer fixes this by removing <details
class="wp-block-details is-layout-flow wp-block-details-is-layout-flow"
>
<summary>Check out this image</summary>
<figure class="wp-block-image size-large">
<img
data-od-added-loading
data-od-removed-fetchpriority="high"
loading="lazy"
decoding="async"
width="1024"
height="668"
src="http://localhost:10023/wp-content/uploads/2024/08/bison1-1024x668.jpg"
alt=""
class="wp-image-8"
srcset="
http://localhost:10023/wp-content/uploads/2024/08/bison1-1024x668.jpg 1024w,
http://localhost:10023/wp-content/uploads/2024/08/bison1-300x196.jpg 300w,
http://localhost:10023/wp-content/uploads/2024/08/bison1-768x501.jpg 768w,
http://localhost:10023/wp-content/uploads/2024/08/bison1-1536x1002.jpg 1536w,
http://localhost:10023/wp-content/uploads/2024/08/bison1-2048x1336.jpg 2048w
"
sizes="(max-width: 1024px) 100vw, 1024px"
/>
</figure>
</details> I'd say this is definitely better than the status quo, but would it be even better if the image inside the Even so, let's say we do add this special case for the Details block. Does adding |
Sounds great.
Shouldn't this say "less than the viewport height" instead, like you say earlier? Just want to make sure the summary is accurate. Regarding the |
Oh yes, you're right. Updating my comment to reflect this. Good catch.
Sounds good! |
Aside: I was confused why WordPress stopped adding |
Now implemented in #1482:
|
I've split this off into #1587:
|
Feature Description
When an image is in the initial viewport but is hidden, it should get
fetchpriority=low
. For example, this would apply to images which appear inside of a mega menu or images which are part of hidden carousel slides. Such images should not getloading=lazy
since they should still be loaded and ready to go the moment the user interacts with the page to reveal it, but they should not get the same priority as images which are visible when first loading the page.See https://web.dev/articles/fetch-priority#lower-carousel-priority
The text was updated successfully, but these errors were encountered: