responsive featured image

[LittleBigThing]

Related to #50 and to #181.

First take on implementing the featured image as an <img> tag to take advantage of the responsive images srcset attribute (using object-fit). It does not work right now on IE11, but I think we could make it work using @supports and some additional CSS (see Twenty Seventeen).

Definitely an enhancement. The benefit would be that smaller images would be loaded on smaller screens instead of the same large image as with a background image.

[kjellr]

This is a fantastic PR — thanks @LittleBigThing!

What problem(s) should we be looking out for in IE? I expected it to be obvious, but it's actually looking okay for me...

---

IE11:

---

Edge

---

I also tested in FF, Safari, and Chrome, and on iOS. It's looking great in all those places.

In any case, if we can get any IE bugs sorted out, I'd love to get this in for RC1.

[LittleBigThing]

Try to resize the browser for IE, at some point it is stretching the image. Also, it behaves differently in IE using a portrait image.

I’ll look at the implementation in Twenty Seventeen in more detail when I have some time tomorrow...

[LittleBigThing]

I just updated the PR with a fix for IE.
It is borrowed from the solution in Twenty Seventeen and I think it works fine now for all major browsers supported by the theme.

All browsers supporting object-fit and @supports will use object-fit now to fit the featured image just a background image would be fitted (cover). IE11 is using a kind of manual fitting using transform: translate(...). Anyway, IE11 and lower do not support srcset to take advantage of responsive images.

Benefit
I tested the benefits just for a page with a featured image in Chrome:

• it is loading the post-thumbnail sized image at _1920px and 1280_px screen width: 176kb
• it is loading a smaller image (1024 x 683px) for a screen width of 920px: 72.6kb
• an even smaller image(768 x 512px) is loaded at a screen width of 768px and below: 40.7kb.

We should probably look at the semantics of the html added. The <img> is not wrapped in any additional div or figure element at this moment... Also, the .wp-post-image class of the image is targeted, which may not be ideal (?).

[kjellr]

Awesome, thanks again @LittleBigThing. I'll give this a review later today. 👍

[mor10]

I think the CSS here could be cleaned up significantly by using grid instead of flex + other complexities, but as it stands the code looks good on first pass.

[joyously]

Did you try it with a small featured image?

[kjellr]

I tested in Safari, FF, Chrome, Edge, and IE11 (with featured images using variety of sizes/orientations) and it's looking great in every case so far. 👍

We should probably look at the semantics of the html added. The is not wrapped in any additional div or figure element at this moment... Also, the .wp-post-image class of the image is targeted, which may not be ideal (?).

I'll cc @allancole for thoughts on that, since he may have some thoughts.

I think the CSS here could be cleaned up significantly by using grid instead of flex + other complexities, but as it stands the code looks good on first pass.

Agreed. That could be handled separately though, since this PR doesn't really modify the flexbox styles.

Barring those semantic questions, I think this is likely good to go as a first pass. It's a clear improvement to what we've got. 👍

[kjellr]

Did you try it with a small featured image?

I did test that, and they appear near-identically to the way they appear in master right now.

[allancole]

We should probably look at the semantics of the html added. The is not wrapped in any additional div or figure element at this moment... Also, the .wp-post-image class of the image is targeted, which may not be ideal (?).

Yup, I think we should add a <figure> element on this as long as it doesn’t break things 👍

[LittleBigThing]

Just updated the PR to use twentynineteen_post_thumbnail() to output the featured image. This seemed more consistent. This means that <img> is now wrapped in <figure class="post-thumbnail">.

I checked on Chrome, IE and Edge, haven't noticed any changes.

One drawback is maybe that there is now a double check for is_singular() && twentynineteen_can_show_post_thumbnail().

[kjellr]

I went ahead and re-tested —  this looks great to me too.

One drawback is maybe that there is now a double check for is_singular() && twentynineteen_can_show_post_thumbnail().

Agreed, but in the absence of a straightforward way to solve for that, I think we can keep it as is for this first pass at the very least. I'm going to go ahead and merge this. Thanks again all your work on this! Think of all the bytes of data you're saving people with this commit! 👏 🎉

[mrwweb]

I think we should add a <figure> element on this as long as it doesn’t break things

I'm not sure I agree.

Is this image really content or is it decorative? It feels more decorative to me, but I could see people disagreeing. If it's decorative (which might be why it was implemented as a background image in the first place), then applying aria-hidden="true" to the markup might actually be a better solution.

From MDN's <figure> docs:

The HTML <figure> element represents self-contained content, frequently with a caption (<figcaption>), and is typically referenced as a single unit.

Usually a <figure> is an image, illustration, diagram, code snippet, etc., that is referenced in the main flow of a document, but that can be moved to another part of the document or to an appendix without affecting the main flow.

[LittleBigThing]

@mrwweb I agree that it’s a difficult call. In the post list, the same featured image is not a background image. It will probably depend on the use case. The color filter over the image can be disabled, that also makes a difference in visibility.