Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove arbitrary padding when there is a background colour #49230

Closed
phil-sola opened this issue Mar 21, 2023 · 9 comments
Closed

Remove arbitrary padding when there is a background colour #49230

phil-sola opened this issue Mar 21, 2023 · 9 comments
Labels
[Feature] Colors Color management [Status] Needs More Info Follow-up required in order to be actionable. [Type] Enhancement A suggestion for improvement.

Comments

@phil-sola
Copy link

What problem does this address?

I am currently trying to move more and more away from ACF blocks in favour of core blocks, and today while setting up a header template part for my navigation, I wrapped it in a group block to enable the sticky functionality, added a white background colour so the navigation is still visible on scroll, and for some random reason there is padding now added to the group block on the basis that it has a background colour.

The CSS being applied:

:where(.wp-block-group.has-background) {
padding: 1.25em 2.375em;
}

There is already a padding UI setting for the group block so it makes no sense really for this opinionated CSS to be applied.

I can think of some use cases when this may be helpful but it is so easy to add in the UI, it doesn't seem necessary to have a blanket CSS rule on background colours.

What is your proposed solution?

Remove the unneccesary CSS targeting blocks when they have a background colour to apply padding.

It isn't needed and only gets in the way. I could very easily add padding, if that is what I needed or wanted. The opposite isn't true.

@cbirdsong
Copy link

I think this is a good default, since in most cases you would want to avoid text running right up against the edges of a container and having to manually assign padding every time you add a background color would add a lot of clicks and leave space for someone to forget to do that.

For your case, couldn't you remove it using the padding controls specifically for your sticky header?

@fclaussen
Copy link

@cbirdsong If this were the case, the padding would be there from the start without the colors.
Why are the colors related to paddings?

@cbirdsong
Copy link

If padding was added universally, group blocks would have a bunch of extra whitespace around them even if you were only using them to adjust non-layout properties like font size and text color:
CleanShot 2023-03-21 at 10 22 48

If padding was not added with a background, group blocks with a background color would require the user to add padding in most cases, since generally you wouldn't want text to run directly against the edges of the container:
CleanShot 2023-03-21 at 10 19 27

@phil-sola
Copy link
Author

For your case, couldn't you remove it using the padding controls specifically for your sticky header?

Yes I can and have done, but I still don't think this is a good default personally. I don't think the editor should be making decisions for users and theme developers, especially where the UI exists for that specific setting.

I also don't think padding should be tied to selecting a colour. I understand fully why you wouldn't want text running up against the edges of a container, but there are so many other ways to add padding.

I'm currently forced to override this CSS with custom CSS or assign padding 0 with a load of unneccesary inline styles.

I appreciate this is probably personal preference but I still think we need to be very careful about adding opinionated styles to core blocks, espeically where the UI exists literally for this purpose.

@fclaussen
Copy link

The inconsistency is also bothersome.
Here's an example:

I've added a paragraph and added a background color to it. Now I have padding that I did not have before but the padding control isn't marked with anything. This padding is coming from nowhere visible.
Screenshot 2023-03-21 at 12 04 33 PM

However, if I just click the padding control it now says 0 and the padding goes away.
Screenshot 2023-03-21 at 12 05 47 PM

But if I move it to 1 it goes back to the same magical padding as before.
Screenshot 2023-03-21 at 12 06 11 PM

I just realized looking at both images that the padding is slightly different which is even worse than I thought.
Screenshot 2023-03-21 at 12 08 48 PM

So why not set padding control to 1 instead of putting an obscure padding decision there?

@cbirdsong
Copy link

cbirdsong commented Mar 21, 2023

Yeah, it's really unintuitive. It's much more clear when the slider is a <select>-style box, which happens when you add more than 6 or 7 spacing presets1:
CleanShot 2023-03-21 at 12 17 04

Otherwise, the only way to reset to the default is by clicking on the three dots menu and then clicking the padding item:
CleanShot 2023-03-21 at 12 18 41

6.2 changes it to say "reset", but it's still buried in the menu:
CleanShot 2023-03-21 at 12 20 56

In my opinion, there needs to be a much clearer "revert to default" button on all control types. There is further discussion of this problem in #43082 and #47976.

Footnotes

  1. This also happens for font sizes.

@kathrynwp
Copy link

Heya @phil-sola this looks like it may be a duplicate of #30725 - would you mind having a look and confirm? Thanks!

@kathrynwp kathrynwp added [Feature] Colors Color management [Type] Enhancement A suggestion for improvement. [Status] Needs More Info Follow-up required in order to be actionable. labels Mar 21, 2023
@phil-sola
Copy link
Author

Hi @kathrynwp, you’re absolutely right, sorry I did look as I always do beforehand but couldn’t find that one. Happy for you to close this in favour of the existing issue, sorry for duplicating!

@kathrynwp
Copy link

kathrynwp commented Mar 21, 2023

@phil-sola Absolutely no problem, there are a ton of issues here and it can be tricky to find dupes! Thank you for adding your thoughts to the other issue.

@kathrynwp kathrynwp closed this as not planned Won't fix, can't repro, duplicate, stale Mar 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Colors Color management [Status] Needs More Info Follow-up required in order to be actionable. [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

4 participants