Fixed the issue of extra padding of the heading

[aezazs-multidots]

What?

Fixed the extra padding that was added in the heading block H1 tag.

Why?

When we used the H1 tag in the heading block and added the background color of this H1 tag, the design was getting spoiled due to too much extra padding. We have solved that issue in this PR.

Testing Instructions

1. Go to the editor.
2. Add a heading block.
3. Select the H1 tag for the heading.
4. Click on accordion color from the setting sidebar.
5. Choose any background color.

Screenshots or screencast

After creating PR the issue is solved.

Environment info

WordPress Version:- 5.9.3
WordPress Theme :- Twenty Twenty-Two / Twenty Twenty-One
Browser: Google Chrome 100.0.4896.75 (Official Build) (arm64)
OS Version: Mac 12.2.1

[github-actions]

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @aezazs-multidots! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

[ndiego]

@aezazs-multidots while the default em styles do provide too much padding in most cases, I do like how they scale based on the size of the text. Converting to rem breaks this functionality. Perhaps we just make the values smaller. Maybe padding: 0.5em 1em?

[aezazs-multidots]

@aezazs-multidots while the default em styles do provide too much padding in most cases, I do like how they scale based on the size of the text. Converting to rem breaks this functionality. Perhaps we just make the values smaller. Maybe padding: 0.5em 1em?

[aezazs-multidots]

@ndiego
Yes, You are right I use the em parameter again and decrease the value of padding.

[ntsekouras]

Thanks for the PR @aezazs-multidots!

These css variables are used in other places too and it's going to be a breaking change, because it will affect the styles of existing content.

@jasmussen do you know if we have changed in the past similar values of css variables and the steps taken?

[jasmussen]

Indeed, this padding is also used for Paragraphs. Nick has a good point about ems scaling with the text, but since this padding is present both for headings and paragraphsk, if you use the background feature on both, the difference between them will be stark due to the differing font sizes.

In that light, moving to use rems instead might be the least disruptive change we can make at this point, as I believe it honors the original intent of those variables: making the text not be flush with the box when a background is applied. The thing is, when these paddings were added, a padding control did not exist, so it was all or nothing. In that light, the placement of the variables in the global stylesheet does suggest that these should line up across blocks.

Alternatively: we should remove those variables entirely, and no longer apply a padding at all, just because a block has a background. We've already stopped doing this with new blocks such as Quote and List, and generally these preset styles should be absorbed by design tools and global styles. That's a larger undertaking, though, but in terms of picking between breaking changes, it might be the right one?

[aezazs-multidots]

Hello @jasmussen @ntsekouras

When we set the background color of the text of the paragraph and heading block, then we should remove the padding?

[ntsekouras]

Indeed, this padding is also used for Paragraphs.

Just noting that the variables are used in more blocks(Group, Template Part, etc..). Just search the code for $block-bg-padding--v.

I'm not sure yet, but maybe we could handle some changes by removing or updating the specific styles from some blocks at first and not change the variable values. Also we should see if the affected blocks have padding support and even consider a migration for those 🤔 .

[aezazs-multidots]

Hello @jasmussen @ntsekouras

When we set the background color to headings and paragraphs, then only we should remove the padding. Because of this, it will not have any effect anywhere else.

Please share your thoughts on this.
Thanks.

[jasmussen]

My suggestion is that changing the values to rem is the smallest change we can make, which is still breaking but less breaking, and arguably just respects the original intent of the variable. This is what it looks like in trunk:

This is what it would look like if we changed the property to rems:

[aezazs-multidots]

Hello @jasmussen
Ok. Thanks for your suggestion.

So I replace em with rem.

Thanks

[ndiego]

In retrospect, I wish padding was never added. Removing the padding would be a pretty drastic breaking change for some users, but it reduces the opinionation of core styles. At this point, you all know how much I love dimension controls, so I would love to see padding support added to these blocks and the default padding CSS removed. 😂

But that said, I agree that switching to rem while maintaining the original value is likely the easiest, most lightweight change we can make to fix the immediate problem.

[carolinan]

@aezazs-multidots @jasmussen @ndiego Is this still a change that should be merged?
Related: #30725

[github-actions]

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: aezazs-multidots <aezazshekh@git.wordpress.org>
Co-authored-by: ntsekouras <ntsekouras@git.wordpress.org>
Co-authored-by: ndiego <ndiego@git.wordpress.org>
Co-authored-by: jasmussen <joen@git.wordpress.org>
Co-authored-by: carolinan <poena@git.wordpress.org>
Co-authored-by: sabernhardt <sabernhardt@git.wordpress.org>
Co-authored-by: kmadhak <kmadhak@git.wordpress.org>

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

[jasmussen]

Issue: headings and paragraphs get an intrinsic padding when a background is applied, and this padding is set in ems so it looks like this:

Coming back to this one. We have three options:

1. Remove the paddings.
2. Change the paddings from using ems to using rems, so they at least scale according to the same metric and are uniform.
3. Do nothing, keep this as is.

Unfortunately I don't think we can do either 1 or 2, simply because this has been out there for years now. It would in most ways, be a breaking change.

I also like to think that with the addition of better padding tools, so you can explicitly zero out paddings if you want, this is less of a headache.

That, and you might probably be grouping elements together, then applying the background on the group itself, rather than the element directly.

For all those reasons, I'd think we can close this one.