Size Change: +736 B (0%)

Total Size: 1.09 MB

_{compressed-size-action}

This works as advertised for me:

✅ When setting background color via the block placeholder, its default opacity is set to 100%
✅ When setting image/video via the block placeholder, the default overlay color opacity is set to 50%
✅ When transforming from the Image/video block, the default overlay opacity should be 50%

Here's how it looks on 11.7.1

And on trunk:

And the same post running this branch:

It looks like the inserting-blocks.test.js.snap snapshot file needs to be regenerated maybe?

Hi, @WunderBart

I see block validation errors when testing this branch with the TT2 theme.

Screenshot

I see block validation errors when testing this branch with the TT2 theme.

I think this is due to this change

@WunderBart I pushed a change that adds a check for url so doesn't require the deprecation caused by removing backgroundType default. It seems to work with all the use cases - but there are a couple of failing fixtures due to a dimRatio of Nan being returned somewhere, but have run out of time to get to the bottom if it - will sort first thing tomorrow - but feel free to revert my commit if you see other issues with it.

@WunderBart I noticed when trying to sort the fixtures that the removal of the default for dimRatio also meant the need for a deprecation to be added in order to prevent block validation issues for some existing content. I took another look at it and have pushed a change that simplifies things a bit by putting back the attribute defaults, and instead sets the initial dimRatio in the transformations. This seems to work across all the options for adding a background, including transforming an image or video block.

Just retesting this after @glendaviesnz's change in 65b5290

The cover block works as expected. Fixture tests are passing for me, and also pasting in deprecated Cover Block code migrates correctly.

Transforming from an Image > Cover Block adds a default dimRatio of 50.

Creating a Cover Block with media, i.e., an image or video, sets the dimRatio to 50

With a color background, it sets the dimRatio to 100

@WunderBart I noticed when trying to sort the fixtures that the removal of the default for dimRatio also meant the need for a deprecation to be added in order to prevent block validation issues for some existing content. I took another look at it and have pushed a change that simplifies things a bit by putting back the attribute defaults, and instead sets the initial dimRatio in the transformations. This seems to work across all the options for adding a background, including transforming an image or video block.

Oh, this looks much better now, thanks! It does seem to cover all the testing scenarios and we don't need to regenerate the fixtures 💪

@ramonjd thanks for testing! It seems we need another approval to unlock this PR though 🙏

@glendaviesnz one small issue that I've noticed - the title color is not properly set initially when setting a video as a Cover background. Once the opacity slider is moved, it seems to be detecting and seting the correct value though:

Should we leave this PR as is and address this issue in a follow-up PR? Let me know what you think!

@glendaviesnz one small issue that I've noticed - the title color is not properly set initially when setting a video as a Cover background. Once the opacity slider is moved, it seems to be detecting and seting the correct value though:
Should we leave this PR as is and address this issue in a follow-up PR? Let me know what you think!

@WunderBart lets make this a separate issue - #36549 - it seems related to the third party fast-average-color library that is being used to get the average background color - it doesn't seem to return a result when a video background is chosen, so lets not hold up this fix waiting for that to be resolved.

I will get someone to give us a final review and sign off on this today.

This PR was cherry picked into the GB 11.9.1 point release in c51a153

Just noting for posterity that I had to cherry-pick #36312 and #36321 into wp/trunk in order to cherry-pick a683c28 without conflicts.