Update overlay step function in cover block

[akasunil]

What?

Fixes https://github.com/WordPress/gutenberg/issues/51625

Why?

Currently, we can set overlay in multiple of 10 in cover block.

How?

I have updated steps to enable any number from 1 to 100 to be set as overlay value.

Testing Instructions

1. Add cover block
2. Change overlay
3. Notice opacity

Screenshots or screencast

[github-actions[bot]]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: akasunil <[email protected]>
Co-authored-by: aaronrobertshaw <[email protected]>
Co-authored-by: Mamaduka <[email protected]>
Co-authored-by: stokesman <[email protected]>
Co-authored-by: carolinan <[email protected]>
Co-authored-by: ndiego <[email protected]>
Co-authored-by: richtabor <[email protected]>
Co-authored-by: ramonjd <[email protected]>
Co-authored-by: t-hamano <[email protected]>
Co-authored-by: hanneslsm <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[akasunil]

Hi @Mamaduka, need your help with PR review here.

[Mamaduka]

Hi, @sunil25393

It's been a while since I worked on the Cover block, and I might miss some of the pre-requestions needed for this change. I would recommend contacting folks who worked on the overlay feature "recently". cc @t-hamano, @stokesman

P.S. Removing the has-background-dim-{unit} class can be considered a breaking change, though I don't have any data about their usage in real-world sites.

[akasunil]

Removing the has-background-dim-{unit} class can be considered a breaking change,

I have kept the css for backward compatibility. We can deprecate it in future version of gutenberg i guess.

[stokesman]

I have kept the css for backward compatibility.

Yes but George was referring to its corresponding class in the markup. There could be instances of CSS written using it in their selectors. Here’s what can be found in themes from wpdirectory. Most of the usage seems odd to me and I'd like to think we can not worry about breakage but I think that’d be against policy.

Otherwise, I gave this a test drive and it worked well. I only tested a basic Cover block created from trunk before switching to this branch and the block updates successfully. I'm not sure how else these types of changes need to be tested.

[akasunil]

There could be instances of CSS written using it in their selectors.

Thank you for your reply. I see it now. However, if we leave the class selectors, opacity will be applied twice. There is no significance to it. Is there a workaround for this? We can always update the changelog, though.

[stokesman]

I took a closer look at the selectors used in the themes from that search and all of them that I saw are written for older cover block versions which had the has-background-dim-{unit} class directly on the block’s root element. So those styles wouldn’t be applying to more recent cover block versions anyway. I.e. this change won’t break those styles.

Yet I didn’t examine all the results because they are many (though it appears they are all repetitions of what I did examine). I wanted to try a more exacting regex to find results that aren’t paired up with wp-cover-block classes but didn’t see a way to do it that’s supported by WP Directory’s regex interpreter.

I've also ran the search on plugins. There are some results though it appears that they are for custom blocks that adopted the use of the class and so should be unaffected by this change. For example, I tested the most installed plugin in the results (WooCommerce) and the class is used on their Featured Product block that has nothing to do with Cover.

My feeling is that this should be safe to ship as is but I don’t know the level of caution others might think is warranted.

[akasunil]

Thank you for investing @stokesman, @t-hamano can we get your view on this ?

[akasunil]

@WordPress/outreach Need this PR to be reviewed.

[carolinan]

Hi @akasunil You need to update the PR with the latest files from Gutenberg trunk, and make sure that the tests pass.

[carolinan]

Change to 6.5 compatibility files need to be moved, I assume since we are in RC already, this needs to be in a 6.7 compatibility file, not 6.6.

[akasunil]

Hi @carolinan, I have updated PR. Could use review.

[ndiego]

With the release of the https://github.com/WordPress/gutenberg/pull/57908 feature, the ability to style the Cover block's overlay via theme.json is getting more attention. I'd go so far as to say it's a must-have.

Agreed.

[richtabor]

As this PR won't address the above need and the alternative approach would also solve the may concern this PR addresses (i.e. finer control over opacity). I'd like to propose we avoid introducing the inline opacity style which would require yet another deprecation once the overlay is switched to use the background color block support.

I tend to agree. With this effort in flux yet I don't think we should push forward on too many changes with the opacity control. I think perhaps it coexists, but I'd like to push that work forward first.

[akasunil]

Thank you for the detailed feedback on this PR. I appreciate the time you're putting into reviewing.

I agree that we should minimize the deprecation as much possible, If we have better way to improve functionality, we should discuss that. Coming to the alternative approach you proposed, which involves updating the Cover block such that its overlay is configurable via theme.json and Global Styles, is definitely better solution then the inline styles this PR add into block.

The general idea is that the background color block support could be used to provide the color for a Cover block's overlay.

Just for the clarity, Does it mean we add a new setting for overlay color under block color support?

I'm open to collaborate on the alternative approach. It sounds like a more flexible solution for the Cover block's styling capabilities.

I really do appreciate the huge amount of work that has gone into this PR. FWIW it might still help inform the direction we ultimately go in.

Thank you again for your valuable insights and for recognizing the work put into this PR.

[aaronrobertshaw]

Just for the clarity, Does it mean we add a new setting for overlay color under block color support?

Ideally, we'd reuse the background color support as is, if possible. From the UI side, I think the difficulty would be labelling the "background" control as "overlay" so its purpose is still clear.

I'm open to collaborate on the alternative approach.

That sounds great, thanks 👍

As that work progresses, I'll add you to any issues and PRs as they're created.

[richtabor]

Ideally, we'd reuse the background color support as is, if possible. From the UI side, I think the difficulty would be labelling the "background" control as "overlay" so its purpose is still clear.

It's probably fine with the first iteration to just use "Background" even. There's a few ideas floating around about consolidating background color and image into one PanelBody, it may be that we don't need to change the label if that's the case.

[ramonjd]

I believe @ramonjd has also started working on this too.

Nothing recently, but it's worth highlighting the proposal to merge background controls in the UI.

https://github.com/WordPress/gutenberg/issues/63096

https://github.com/WordPress/gutenberg/pull/60100#issuecomment-2160054475

This wouldn't affect the way the background values are read and stored in theme.json, but would eventually make controls consistent across blocks that have background image and background color supports enabled, the Cover block (hopefully one day) included.