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

6.6 - Paragraphs with has-background have their automatic background overridden by global styles block padding #63164

Open
talldan opened this issue Jul 5, 2024 · 4 comments
Labels
Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Bug An existing feature does not function as intended

Comments

@talldan
Copy link
Contributor

talldan commented Jul 5, 2024

Description

This probably applies to other blocks too, and is a result of levelling css specificity.

In 6.4 & 6.5, the automatic padding applied to blocks when they have a background (.has-background) overrides global styles padding rules for blocks.

In 6.6, it's the other way around, global styles overrides the .has-background class due to a reduction in specificity.

I'm not sure what the correct behavior is, possibly what 6.6 does makes the most sense.

I'm also not sure that it's possible to go back to how it worked in 6.5 without bumping specificity, which might cause other issues.

Step-by-step reproduction instructions

  1. Open the site editor
  2. Go to global styles > Blocks > Paragraph
  3. Set a padding value of 1px for all sides and save
  4. Create a new page/post
  5. Insert a paragraph and set a background
  • In 6.4/6.5: the paragraph received a padding of 1.25em 2.375em.
  • In 6.6: the paragraph receives a padding of 1px.
  • Expected: 🤷

Screenshots, screen recording, code snippet

6.4/6.5:
Screenshot 2024-07-05 at 9 29 13 am

6.6:
Screenshot 2024-07-05 at 9 28 56 am

Environment info

Mac OS / Brave

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@talldan talldan added [Type] Bug An existing feature does not function as intended Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json labels Jul 5, 2024
@t-hamano
Copy link
Contributor

t-hamano commented Jul 5, 2024

Looking at this code, are the changes in WP6.6 intentional?

// Specificity is reduced to 0-1-0 so global styles can override this.
:root :where(p.has-background) {
padding: $block-bg-padding--v $block-bg-padding--h;
}

cc @aaronrobertshaw

@aaronrobertshaw
Copy link
Contributor

Thanks for the ping @t-hamano 👍

I discussed this problem with @talldan, @ramonjd, @tellthemachines, and @andrewserong before this issue was created. The description captures the dilemma in a nutshell. Unfortunately, at this stage, I don't have any solutions to it 😢

The general direction and thinking to date has been that global styles should override block library styles. This is the case in WP6.6 currently and has been in Gutenberg for a few releases now.

In 6.4 & 6.5, the automatic padding applied to blocks when they have a background (.has-background) overrides global styles padding rules for blocks.

In my view, this is a bug. It also requires theme authors to bump their own styles' specificity higher again to overcome it.

In 6.6, it's the other way around, global styles overrides the .has-background class due to a reduction in specificity.

IIRC the global styles referred to here are both any explicitly assigned to the block type and global padding.

If it is padding explicitly assigned to a block type in global styles, it probably should override the arbitrary default applied by .has-background. I'm a little less sure about global padding, I'd defer that one to the layout support experts.

What is a real problem here, is a possible change in padding for existing sites where their theme relies on the core .has-padding values. If they redefined their own padding values using a higher specificity, it could be less of an issue. The potential remains of course.

I'm not sure what the correct behavior is, possibly what 6.6 does makes the most sense.

The current behaviour in 6.6 makes the most sense to me also. I'm not sure how we can mitigate the downside though other than communicating it.

I'm also not sure that it's possible to go back to how it worked in 6.5 without bumping specificity, which might cause other issues.

Bumping the specificity of .has-background classes again will restore the bug preventing users from customizing padding on these blocks through global styles. That might be the direction we ultimately choose, I don't know. Personally, I don't feel there are any great options here.

Any bump in specificity may negatively impact the extended block style variations and the section styles feature which relies on uniform specificity so sections can be nested within each other e.g. creating pricing tables within a parent section.

Prior to 6.6, variation padding styles in global styles would override the .has-background classes due to style load order and matching specificity. If we restore the .has-background specificity to pre-6.6 levels, without changing block style variations too, we'll break those. Bumping the specificity of block style variations will have further ramifications that I'd need some time to unravel.

I'd love to hear others' thoughts on the best path forward from here.

@ramonjd
Copy link
Member

ramonjd commented Jul 7, 2024

If it is padding explicitly assigned to a block type in global styles, it probably should override the arbitrary default applied by .has-background.

In my opinion this is how it should work. WordPress should honor the theme's choice over its own opinionated styles. Allowing theme devs to style globally is a strong argument.

However, backwards compatibility in this case appears difficult.

I can't think of any deprecation/CSS/block parsing trickery that would circumvent the direct conflict, which means that there could be one 😄

The has-background rule has, however been around since at least 2017 - I can't find a reason for it other than it's what the old cover-text block had when it was merged with paragraph. I suspect because it made text look prettier.

ac911ae#diff-e13c67a8237605dbed9b6b676c56f4f9a7c75fe9c4117f623400ee09f5bd6a9fR17

Is there a case for deprecation? How would that look? Maybe it's worth advertising it in the Make Core blog and other channels - more ideas might surface.

Sorry, I know that's not very helpful.

@andrewserong
Copy link
Contributor

This is a tough one, good discussion, folks!

For this, I lean slightly toward the idea that global styles not overriding the paragraph block's .has-background rules was a bug all along, and the behaviour in 6.6 addresses that for consistency. Otherwise there isn't a way for themes to uniformly override paragraph padding.

Since there's a question of what should happen for WP 6.6, I'll just ping @ellatrix for visibility, as it sounds like a decision needs to be made on whether to stick with the current behaviour in trunk?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Bug An existing feature does not function as intended
5 participants