Add srcset for cover image

[ajlende]

Description

Fixes #17463

Given that Cover Blocks are often times set to be alignfull at the top of a page, a Cover Block will often be the largest element, and will require a very large image to account for high resolution monitors. Without the ability to use srcset or another option to serve scaled images to smaller viewports, performance suffers. Our Lighthouse scores have plummeted from the mid 90s to the mid 60s fairly consistently since the inclusion of LCP, and can be traced directly to cover block images.

Additionally, this will help me with Automattic/block-experiments#143. With the image as a separate DOM Element, it'll be possible to create Block Filters that modify the appearance of the image.

• Update the background image to use a srcset
• Add old image to deprecations
• Polyfill object-fit for IE11 (not a true polyfill as the data-object-fit attribute needs to be added to the element HTML, but it was the simplest polyfill that supported object-position) (or apply fallback CSS with @supports Gallery block not "Cropping" images not working on Internet Explorer and Edge 15 #17775 (comment))
• Getting polyfill for object-fit also happened to fix an unreported bug with video positioning in IE11

How has this been tested?

• Updated e2e tests for the cover block
• Additionally tested in IE11 on Windows 7

Types of changes

New feature? Breaking change?

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• My code has proper inline documentation.
• I've included developer documentation if appropriate.
• I've updated all React Native files affected by any refactorings/renamings in this PR.

[eric-michel]

@ajlende Thanks so much for doing the work on this! I'm really excited, and hope this gets folded in for WP 5.6. In addition to the performance benefits of srcset, this will also make it possible to replace the img tag with a picture tag the directs the browser to use webp images when supported. Since cover images are usually the largest image on the page, the performance benefits end up being huge!

[aristath]

We should not be using CDNs like unpkg.
Historically WordPress bundles whatever it needs, but in this case I'm not sure what would be best to do. This is a polyfill for IE only since other browsers natively support object-fit, so perhaps this should be left out?

[ajlende]

Yeah, I wasn't 100% certain the best way to go about it.

@aduth might have some insight. Looks like he's done some things with polyfills.

The file doesn't actually get loaded from the CDN in the bundle. get-vendor-scripts.php downloads them for the bundle.

And the way this is set up, the script will only be loaded in browsers that fail the test ("objectFit" in document.documentElement.style). I confirmed that it loads in IE, but not in Firefox or Chrome.

An alternative might be to just use the transform: translate(-50%, -50%); top: 50%; left: 50% trick; although, that would break object-position in IE.

[aristath]

The file doesn't actually get loaded from the CDN in the bundle. get-vendor-scripts.php downloads them for the bundle.

I didn't know that, pretty cool! 👍

To be honest, IE users can't (and don't) expect things to work the same way they'd work on other browsers. The fact that WP still supports IE11 doesn't necessarily mean that things should be identical. As long as IE users can see and consume the content, some visual inconsistencies are to be expected. That is why global-styles use CSS variables too...

[ajlende]

@aristath Was removing the polyfill a blocker for getting this PR approved? Or are we waiting for @aduth's opinion for this?

[youknowriad]

@noisysocks was this polyfill backported to Core somehow?

[youknowriad]

Nevermind, I just found it :)

[Invader444]

Awesome work @ajlende! Looking forward to this landing, it will lead to a huge performance boost for my sites <3

[mkaz]

This looks good and tested well for me. I tried with an existing cover block and saw no problems before and after the PR applied. The srcset shows fine, and no block breakage or issues.

As far as polyfill, if it just gets bundled in build, I don't see that as a problem. I agree with @aristath that it is fine to limit the functionality for IE11 as along as nothing is completely broken, if the experience is not optimal or slightly different is ok, just not broken.

Good work on this, thanks for sticking with it 👍

[eric-michel]

@ajlende big thanks for handling this and keeping it updated! This is going to make a huge performance impact on my sites.

[stokesman]

Great work on this @ajlende! There are a couple of things I'm seeing since this was merged. One should be an easy fix and I don't know about the other.

Repeated backgrounds aren't working. That feature #26001 came along after this PR was made but got merged before it. I remember seeing it discussed #25421 (comment) that setting the Cover to have a fixed background is an exception to the img/srcset being used. So it would seem the repeated background option should trigger that same exception.

The other thing is that most of the preexisting Cover blocks I tested this with updated successfully. I found one that didn't. Maybe because it had padding applied (from the spacing/padding block supports). If that is the cause, I don't know if that can be accounted for in the upgrade logic or if it should be (is spacing support still experimental?)

If it helps, this was the invalidation errror:

Block validation: Block validation failed for `core/cover` ({name: "core/cover", icon: {…}, keywords: Array(0), attributes: {…}, providesContext: {…}, …}apiVersion: 2attributes: {url: {…}, id: {…}, hasParallax: {…}, isRepeated: {…}, dimRatio: {…}, …}category: "media"deprecated: (6) [{…}, {…}, {…}, {…}, {…}, {…}]description: "Add an image or video with a text overlay — great for headers."edit: ƒ (r)editorStyle: "wp-block-cover-editor"example: {attributes: {…}, innerBlocks: Array(1)}getEditWrapperProps: ƒ (n)icon: {src: {…}}keywords: []name: "core/cover"providesContext: {}save: ƒ (e)style: "wp-block-cover"styles: []supports: {anchor: true, align: true, html: false, spacing: {…}}title: "Cover"transforms: {from: Array(2), to: Array(2)}usesContext: []__proto__: Object).

Content generated by `save` function:

<div class="wp-block-cover has-background-dim-90 has-background-dim" style="padding-top:69px;padding-right:69px;padding-bottom:69px;padding-left:69px;background-color:#c58ea4;min-height:374px"><img class="wp-block-cover__image-background wp-image-2628" alt="" src="http://wip-wp.test/wp-content/uploads/2020/11/solen-feyissa-r5kuTIVvzlI-unsplash-scaled.jpg" data-object-fit="cover"/><div class="wp-block-cover__inner-container"></div></div>

Content retrieved from post body:

<div class="wp-block-cover has-background-dim-90 has-background-dim" style="padding-bottom:69px;padding-left:69px;padding-right:69px;padding-top:69px;background-image:url(http://wip-wp.test/wp-content/uploads/2020/11/solen-feyissa-r5kuTIVvzlI-unsplash-scaled.jpg);background-color:#c58ea4;min-height:374px"><div class="wp-block-cover__inner-container"></div></div>

[ajlende]

@stokesman Where are you getting the spacing/padding block supports, a plugin? I didn't see any settings for changing that when I was testing on the master branch. If it's a plugin, I don't think there's much that I can do, but you can still send me the link and I can take a look just in case.

[stokesman]

In the theme I was testing it is enabled via experimental-theme.json.

The content from my theme's file pared-down to the relevant bits

{
  "global": {
    "settings": {
      "spacing": {
        "customPadding": true,
        "units": [
          "%",
          "px"
        ]
      }
    }
  }
}

I guess it also can be enabled with a bit of PHP add_theme_support('custom-spacing');.

Apparently, it's stable since #25788.

[stokesman]

I've made more tests of upgrades on various Cover blocks. I'm finding that when a custom color has been used for the overlay, after the upgrade, the custom color shows in the settings sidebar but not in the block. Here's what I see:

cover-srcset-custom-color.mp4

Works fine with colors from the palette or gradients (custom or from palette).

[stokesman]

Regarding my last comment, it actually applies to newly created cover blocks too (not just upgraded ones). Using a custom color is apparently being saved in the block attributes and does show up in the frontend. In the editor, it does as shown in the recording.

[ajlende]

I'm also seeing a similar bug with the image not reappearing on refresh.

img-bug.mp4

EDIT: Not sure if that or the color are related to this PR, but I'm investigating.

[noisysocks]

Hi @ajlende. Unfortunately we can't use https://github.com/constancecchen/object-fit-polyfill as it doesn't have a license. Would you able able to reach out to the author and ask them if they can add a GPL2 compatible license?

edit: Never mind! It's dual ISC / MIT. Just didn't have a LICENSE file so I missed it 🙂

https://github.com/constancecchen/object-fit-polyfill/blob/0a59e20c327c8b7984df3eaf852537656e3ada1c/package.json#L20
https://github.com/constancecchen/object-fit-polyfill/blob/0a59e20c327c8b7984df3eaf852537656e3ada1c/src/objectFitPolyfill.js#L5

[ajlende]

@noisysocks Their package.json and bower.json suggest an ISC license which I believe is compatible. Is that enough or do you specifically need the LICENSE file in the repo?

edit: Glad that's okay!

[ajlende]

@noisysocks here's the PR for uploading the unminified version if you wanted to keep an eye on it: cee-chen/object-fit-polyfill#59

[joshm33333]

uugh this is why i hate using wordpress
i have piles of css commited to the idea the covers use background images
wordpress update, html changed, now i have to redo all my work
why cant you make things optional :'(